Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #13870 - encrypts specific settings values in db #3230

Merged
merged 1 commit into from
Oct 7, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/helpers/audits_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def id_to_label(name, change)
when /.*_id$/
name.classify.gsub('Id','').constantize.find(change).to_label
else
change.to_s
change.to_s == "[encrypted]" ? _(change.to_s) : change.to_s
end.truncate(50)
rescue
_("N/A")
Expand Down
12 changes: 12 additions & 0 deletions app/models/concerns/audit_extensions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ module AuditExtensions
scoped_search :in => :search_users, :on => :login, :complete_value => true, :rename => :user, :only_explicit => true

before_save :ensure_username, :ensure_auditable_and_associated_name
before_save :filter_encrypted, :if => Proc.new {|audit| audit.audited_changes.present?}
after_validation :fix_auditable_type

include Authorizable
Expand All @@ -43,6 +44,17 @@ def self.humanize_class_name

private

def filter_encrypted
self.audited_changes.each do |name,change|
next if change.nil? || change.to_s.empty?
if change.is_a? Array
change.map! {|c| c.to_s.start_with?(EncryptValue::ENCRYPTION_PREFIX) ? N_("[encrypted]") : c}
else
audited_changes[name] = N_("[encrypted]") if change.to_s.start_with?(EncryptValue::ENCRYPTION_PREFIX)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does [encrypted] get translated in the UI? My last comment here suggested doing it in a UI helper, but I can't see that in this PR. (See #3230 (comment))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just forgot to commit the changes in audits_helper.

end
end
end

def ensure_username
self.user_as_model = User.current
self.username = User.current.try(:to_label)
Expand Down
75 changes: 75 additions & 0 deletions app/models/concerns/encrypt_value.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
module EncryptValue
ENCRYPTION_PREFIX = "encrypted-"
def matches_prefix?(str)
ENCRYPTION_PREFIX == str.to_s[0..(ENCRYPTION_PREFIX.length - 1)]
end

def encryption_key
return ENV['ENCRYPTION_KEY'] if ENV['ENCRYPTION_KEY'].present?
return EncryptionKey::ENCRYPTION_KEY if defined? EncryptionKey::ENCRYPTION_KEY
nil
end

def is_encryptable?(str)
if !encryption_key.present?
puts_and_logs "Missing ENCRYPTION_KEY configuration, so #{self.class.name} #{name} could not be encrypted", Logger::WARN
false
elsif str.blank?
puts_and_logs "String is blank', so #{self.class.name} #{name} was not encrypted", Logger::DEBUG
false
elsif matches_prefix?(str)
puts_and_logs "String starts with the prefix '#{ENCRYPTION_PREFIX}', so #{self.class.name} #{name} was not encrypted again", Logger::DEBUG
false
else
true
end
end

def is_decryptable?(str)
if !matches_prefix?(str)
puts_and_logs "String does not start with the prefix '#{ENCRYPTION_PREFIX}', so #{self.class.name} #{name} was not decrypted", Logger::DEBUG
false
elsif !encryption_key.present?
puts_and_logs "Missing ENCRYPTION_KEY configuration, so #{self.class.name} #{name} could not be decrypted", Logger::WARN
false
else
true
end
end

def encrypt_field(str)
return str unless is_encryptable?(str)
encryptor = ActiveSupport::MessageEncryptor.new(encryption_key)
begin
# add prefix to encrypted string
str_encrypted = "#{ENCRYPTION_PREFIX}#{encryptor.encrypt_and_sign(str)}"
puts_and_logs "Successfully encrypted field for #{self.class.name} #{name}"
str = str_encrypted
rescue
puts_and_logs "WARNING: Encryption failed for string. Please check that the ENCRYPTION_KEY has not changed.", Logger::WARN
end
str
end

def decrypt_field(str)
return str unless is_decryptable?(str)
encryptor = ActiveSupport::MessageEncryptor.new(encryption_key)
begin
# remove prefix before decrypting string
str_no_prefix = str.gsub(/^#{ENCRYPTION_PREFIX}/, "")
str_decrypted = encryptor.decrypt_and_verify(str_no_prefix)
puts_and_logs "Successfully decrypted field for #{self.class.name} #{name}"
str = str_decrypted
rescue ActiveSupport::MessageVerifier::InvalidSignature
puts_and_logs "WARNING: Decryption failed for string. Please check that the ENCRYPTION_KEY has not changed.", Logger::WARN
end
str
end

private

def puts_and_logs(msg, level = Logger::INFO)
logger.add level, msg
puts msg if Foreman.in_rake? && !Rails.env.test? && level >= Logger::INFO
end
end
77 changes: 1 addition & 76 deletions app/models/concerns/encryptable.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
module Encryptable
extend ActiveSupport::Concern

ENCRYPTION_PREFIX = "encrypted-"

include EncryptValue
included do
before_save :encrypt_setters
end
Expand Down Expand Up @@ -45,77 +43,4 @@ def encrypt_setters
end
end
end

def matches_prefix?(str)
ENCRYPTION_PREFIX == str.to_s[0..(ENCRYPTION_PREFIX.length - 1)]
end

def encryption_key
return ENV['ENCRYPTION_KEY'] if ENV['ENCRYPTION_KEY'].present?
return EncryptionKey::ENCRYPTION_KEY if defined? EncryptionKey::ENCRYPTION_KEY
nil
end

def is_encryptable?(str)
if !encryption_key.present?
puts_and_logs "Missing ENCRYPTION_KEY configuration, so #{self.class.name} #{name} could not be encrypted", Logger::WARN
false
elsif str.blank?
puts_and_logs "String is blank', so #{self.class.name} #{name} was not encrypted", Logger::DEBUG
false
elsif matches_prefix?(str)
puts_and_logs "String starts with the prefix '#{ENCRYPTION_PREFIX}', so #{self.class.name} #{name} was not encrypted again", Logger::DEBUG
false
else
true
end
end

def is_decryptable?(str)
if !matches_prefix?(str)
puts_and_logs "String does not start with the prefix '#{ENCRYPTION_PREFIX}', so #{self.class.name} #{name} was not decrypted", Logger::DEBUG
false
elsif !encryption_key.present?
puts_and_logs "Missing ENCRYPTION_KEY configuration, so #{self.class.name} #{name} could not be decrypted", Logger::WARN
false
else
true
end
end

def encrypt_field(str)
return str unless is_encryptable?(str)
encryptor = ActiveSupport::MessageEncryptor.new(encryption_key)
begin
# add prefix to encrypted string
str_encrypted = "#{ENCRYPTION_PREFIX}#{encryptor.encrypt_and_sign(str)}"
puts_and_logs "Successfully encrypted field for #{self.class.name} #{name}"
str = str_encrypted
rescue
puts_and_logs "WARNING: Encryption failed for string. Please check that the ENCRYPTION_KEY has not changed.", Logger::WARN
end
str
end

def decrypt_field(str)
return str unless is_decryptable?(str)
encryptor = ActiveSupport::MessageEncryptor.new(encryption_key)
begin
# remove prefix before decrypting string
str_no_prefix = str.gsub(/^#{ENCRYPTION_PREFIX}/, "")
str_decrypted = encryptor.decrypt_and_verify(str_no_prefix)
puts_and_logs "Successfully decrypted field for #{self.class.name} #{name}"
str = str_decrypted
rescue ActiveSupport::MessageVerifier::InvalidSignature
puts_and_logs "WARNING: Decryption failed for string. Please check that the ENCRYPTION_KEY has not changed.", Logger::WARN
end
str
end

private

def puts_and_logs(msg, level = Logger::INFO)
logger.add level, msg
puts msg if Foreman.in_rake? && !Rails.env.test? && level >= Logger::INFO
end
end
18 changes: 15 additions & 3 deletions app/models/setting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ class Setting < ActiveRecord::Base
extend FriendlyId
friendly_id :name
include ActiveModel::Validations
include EncryptValue
self.inheritance_column = 'category'

TYPES= %w{ integer boolean hash array string }
Expand All @@ -25,7 +26,7 @@ def validate(record)

validates_lengths_from_database
# audit the changes to this model
audited :except => [:name, :description, :category, :settings_type, :full_name], :on => [:update]
audited :except => [:name, :description, :category, :settings_type, :full_name, :encrypted], :on => [:update]

validates :name, :presence => true, :uniqueness => true
validates :description, :presence => true
Expand Down Expand Up @@ -107,11 +108,14 @@ def self.method_missing(method, *args)

def value=(v)
v = v.to_yaml unless v.nil?
# the has_attribute is for enabling DB migrations on older versions
v = encrypt_field(v) if has_attribute?(:encrypted) && encrypted
write_attribute :value, v
end

def value
v = read_attribute(:value)
v = decrypt_field(v)
v.nil? ? default : YAML.load(v)
end
alias_method :value_before_type_cast, :value
Expand Down Expand Up @@ -201,12 +205,19 @@ def self.convert_array_to_regexp(array)

def self.create_existing(s, opts)
bypass_readonly(s) do
attrs = column_check([:default, :description, :full_name])
attrs = column_check([:default, :description, :full_name, :encrypted])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're not mass-assigning :encrypted on line 215 then it perhaps shouldn't be listed here.

to_update = Hash[opts.select { |k,v| attrs.include? k }]
to_update.merge!(:value => SETTINGS[opts[:name].to_sym]) if SETTINGS.key?(opts[:name].to_sym)
s.update_attributes(to_update)
s.update_column :category, opts[:category] if s.category != opts[:category]
s.update_column :full_name, opts[:full_name] if !column_check([:full_name]).empty?
raw_value = s.read_attribute(:value)
if s.is_encryptable?(raw_value) && attrs.include?(:encrypted) && opts[:encrypted]
s.update_column :value, s.encrypt_field(raw_value)
end
if s.is_decryptable?(raw_value) && attrs.include?(:encrypted) && !opts[:encrypted]
s.update_column :value, s.decrypt_field(raw_value)
end
end
s
end
Expand Down Expand Up @@ -237,7 +248,8 @@ def self.set(name, description, default, full_name = nil, value = nil, options =
define_method("#{name}_collection".to_sym){ options[:collection].call }
end
end
{:name => name, :value => value, :description => description, :default => default, :full_name => full_name}
options[:encrypted] ||= false
{:name => name, :value => value, :description => description, :default => default, :full_name => full_name, :encrypted => options[:encrypted]}
end

def self.model_name
Expand Down
6 changes: 3 additions & 3 deletions app/models/setting/auth.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ def self.load_defaults
self.transaction do
[
self.set('oauth_active', N_("Foreman will use OAuth for API authorization"), false, N_('OAuth active')),
self.set('oauth_consumer_key', N_("OAuth consumer key"), '', N_('OAuth consumer key')),
self.set('oauth_consumer_secret', N_("OAuth consumer secret"), '', N_("OAuth consumer secret")),
self.set('oauth_consumer_key', N_("OAuth consumer key"), '', N_('OAuth consumer key'), nil, {:encrypted => true}),
self.set('oauth_consumer_secret', N_("OAuth consumer secret"), '', N_("OAuth consumer secret"), nil, {:encrypted => true}),
self.set('oauth_map_users', N_("Foreman will map users by username in request-header. If this is set to false, OAuth requests will have admin rights."), true, N_('OAuth map users')),
self.set('restrict_registered_smart_proxies', N_('Only known Smart Proxies may access features that use Smart Proxy authentication'), true, N_('Restrict registered smart proxies')),
self.set('require_ssl_smart_proxies', N_('Client SSL certificates are used to identify Smart Proxies (:require_ssl should also be enabled)'), true, N_('Require SSL for smart proxies')),
Expand All @@ -25,7 +25,7 @@ def self.load_defaults
self.set('ssl_client_dn_env', N_('Environment variable containing the subject DN from a client SSL certificate'), 'SSL_CLIENT_S_DN', N_('SSL client DN env')),
self.set('ssl_client_verify_env', N_('Environment variable containing the verification status of a client SSL certificate'), 'SSL_CLIENT_VERIFY', N_('SSL client verify env')),
self.set('ssl_client_cert_env', N_("Environment variable containing a client's SSL certificate"), 'SSL_CLIENT_CERT', N_('SSL client cert env')),
self.set('websockets_ssl_key', N_("Private key that Foreman will use to encrypt websockets "), nil, N_('Websockets SSL key')),
self.set('websockets_ssl_key', N_("Private key file that Foreman will use to encrypt websockets "), nil, N_('Websockets SSL key')),
self.set('websockets_ssl_cert', N_("Certificate that Foreman will use to encrypt websockets "), nil, N_('Websockets SSL certificate')),
# websockets_encrypt depends on key/cert when true, so initialize it last
self.set('websockets_encrypt', N_("VNC/SPICE websocket proxy console access encryption (websockets_ssl_key/cert setting required)"), !!SETTINGS[:require_ssl], N_('Websockets encryption')),
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20160315161936_add_encrypted_to_settings.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddEncryptedToSettings < ActiveRecord::Migration
def change
add_column :settings, :encrypted, :boolean, :null => false, :default => false
end
end
6 changes: 6 additions & 0 deletions test/fixtures/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -302,3 +302,9 @@ attributes62:
category: Setting::Auth
default: "true"
description: 'Permits access to BMC interface passwords through ENC YAML output and in templates'
attributes63:
name: password
category: Setting::Puppet
default: nil
description: 'Encrypted password'
encrypted: true
11 changes: 11 additions & 0 deletions test/models/concerns/audit_extensions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,15 @@ def setup
hosts = Audit.complete_for("host = ", {:controller => 'audits'})
assert hosts.count > 0
end

test "audit's change is filtered when data is encrypted" do
setting = settings(:attributes63)
setting.expects(:encryption_key).at_least_once.returns('25d224dd383e92a7e0c82b8bf7c985e815f34cf5')
setting.value = '654321'
as_admin do
assert setting.save
end
a = Audit.where(auditable_type: 'Setting')
assert_equal "[encrypted]", a.last.audited_changes["value"][1]
end
end
21 changes: 21 additions & 0 deletions test/models/setting_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,27 @@ def test_should_not_find_a_value_if_doesnt_exists
assert_nil Setting["no_such_thing"]
end

test "encrypted value is saved encrypted when created" do
setting = Setting.create(:name => "foo", :value => 5, :default => 5, :description => "test foo", :encrypted => true)
setting.expects(:encryption_key).at_least_once.returns('25d224dd383e92a7e0c82b8bf7c985e815f34cf5')
setting.value = "123456"
as_admin do
assert setting.save
end
assert setting.read_attribute(:value).include? EncryptValue::ENCRYPTION_PREFIX
end

test "update an encrypted value should saved encrypted in db, and decrypted while reading" do
setting = settings(:attributes63)
setting.expects(:encryption_key).at_least_once.returns('25d224dd383e92a7e0c82b8bf7c985e815f34cf5')
setting.value = '123456'
as_admin do
assert setting.save
end
assert setting.read_attribute(:value).include? EncryptValue::ENCRYPTION_PREFIX
assert_equal '123456', setting.value
end

def test_should_provide_default_if_no_value_defined
assert Setting.create(:name => "foo", :default => 5, :description => "test foo")
assert_equal 5, Setting["foo"]
Expand Down