Project

General

Profile

Bug #9775

CR encryption key not loaded before it's checked, encryption is disabled

Added by Dominic Cleal over 6 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
High
Assignee:
Category:
Security
Target version:
Difficulty:
Triaged:
Bugzilla link:
Fixed in Releases:
Found in Releases:

Description

In Foreman 1.8/nightlies, since #4478, the compute resource password encryption key isn't being used and so CR passwords are stored and loaded only in plain text.

The key is stored in an initialiser (config/initializers/encryption_key.rb, locally generated during package installation) which should be loaded before the Encryptable concern is loaded. The Encryptable concern is a no-op if the key isn't initialised already.

#4478 added config/initializers/apipie.rb which is calling ComputeResource.providers, leading to earlier loading of Encryptable (used in ComputeResource), before the encryption key initialiser is reached (as 'apipie' < 'encryption_key').

Thanks to Daniel Lobato Garcia for reporting this to .


Related issues

Related to Foreman - Feature #4478: API docs need to be localizedClosed2014-02-27
Related to Foreman - Feature #2424: encrypt compute resource passwordClosed2013-04-24
Has duplicate Foreman - Bug #9771: undefined method `encryptable_fields' during db migrateClosed2015-03-15

Associated revisions

Revision 1fcea0e9 (diff)
Added by Dominic Cleal over 6 years ago

fixes #9775 - always load Encryptable when key's missing, log runtime warning

a59972c3 causes Encryptable to be loaded before the encryption_key.rb
initialiser and the majority of the class was skipped as the key was undefined.

Now Encryptable always loads, but logs at runtime if the key is unavailable,
allowing it to be defined a bit later.

Revision c0429ee2 (diff)
Added by Dominic Cleal over 6 years ago

fixes #9775 - always load Encryptable when key's missing, log runtime warning

a59972c3 causes Encryptable to be loaded before the encryption_key.rb
initialiser and the majority of the class was skipped as the key was undefined.

Now Encryptable always loads, but logs at runtime if the key is unavailable,
allowing it to be defined a bit later.

(cherry picked from commit 1fcea0e919384f9f0f384d450ecac571d5953c82)

History

#1 Updated by Dominic Cleal over 6 years ago

Daniel adds:

  • `foreman-rake security:generate_encryption_key` doesn't run by
    default because of the permissions set by the installer. `Permission
    denied - /usr/share/foreman/config/initializers/encryption_key.rb`

This works correctly during package installation, it's just a post-install issue that prevents you re-running it. I'll file this separately as it's a low priority and impact bug.

Before 1.8, I think we should address this. I've naively renamed the
initializer to 0_encrypted_key.rb and it fixes the issue. Before 1.8:

  • We should document Compute Resource encryption through
    EncryptionKey in the manual.
  • There should be tests for the tasks that deal with this.
  • Tests for should ensure the initializer runs before the concern is
    loaded.

Renaming the initialiser certainly works, though as it's a locally created file then we'll need to handle this in packaging somehow - a bit messy. Renaming the apipie initialiser might be easier!

#2 Updated by Dominic Cleal over 6 years ago

#9771 is caused by the same issue I believe. The Encryptable concern isn't being loaded due to the initialiser reordering, so the encrypt rake task is failing as the concern methods aren't present.

#4 Updated by Dominic Cleal over 6 years ago

  • Description updated (diff)
  • Private changed from Yes to No

#5 Updated by Dominic Cleal over 6 years ago

#6 Updated by Dominic Cleal over 6 years ago

  • Has duplicate Bug #9771: undefined method `encryptable_fields' during db migrate added

#7 Updated by Dominic Cleal over 6 years ago

  • Related to Feature #2424: encrypt compute resource password added

#8 Updated by Daniel Lobato Garcia over 6 years ago

Can confirm #9771 is completely related, as when I make the initializer load earlier it does work.

#9 Updated by Dominic Cleal over 6 years ago

  • Status changed from New to Assigned
  • Assignee set to Dominic Cleal

#10 Updated by The Foreman Bot over 6 years ago

  • Status changed from Assigned to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/2248 added
  • Pull request deleted ()

#11 Updated by Og Maciel over 6 years ago

  • Bugzilla link set to 1204914

#12 Updated by Dominic Cleal over 6 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100

Also available in: Atom PDF