From e8528a957b8352ee743dce443f9a65704581ded0 Mon Sep 17 00:00:00 2001 From: amirfefer Date: Wed, 24 Feb 2016 16:21:16 +0200 Subject: [PATCH] Fixes #13870 - encrypt specific settings values in db --- app/helpers/audits_helper.rb | 2 +- app/models/concerns/audit_extensions.rb | 12 +++ app/models/concerns/encrypt_value.rb | 75 ++++++++++++++++++ app/models/concerns/encryptable.rb | 77 +------------------ app/models/setting.rb | 18 ++++- app/models/setting/auth.rb | 6 +- ...0160315161936_add_encrypted_to_settings.rb | 5 ++ test/fixtures/settings.yml | 6 ++ test/models/concerns/audit_extensions_test.rb | 11 +++ test/models/setting_test.rb | 21 +++++ 10 files changed, 150 insertions(+), 83 deletions(-) create mode 100644 app/models/concerns/encrypt_value.rb create mode 100644 db/migrate/20160315161936_add_encrypted_to_settings.rb diff --git a/app/helpers/audits_helper.rb b/app/helpers/audits_helper.rb index ef89931dc64..d534c7a0ea0 100644 --- a/app/helpers/audits_helper.rb +++ b/app/helpers/audits_helper.rb @@ -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") diff --git a/app/models/concerns/audit_extensions.rb b/app/models/concerns/audit_extensions.rb index c6131537ad8..fa31293b272 100644 --- a/app/models/concerns/audit_extensions.rb +++ b/app/models/concerns/audit_extensions.rb @@ -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 @@ -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) + end + end + end + def ensure_username self.user_as_model = User.current self.username = User.current.try(:to_label) diff --git a/app/models/concerns/encrypt_value.rb b/app/models/concerns/encrypt_value.rb new file mode 100644 index 00000000000..687fefbe53a --- /dev/null +++ b/app/models/concerns/encrypt_value.rb @@ -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 diff --git a/app/models/concerns/encryptable.rb b/app/models/concerns/encryptable.rb index 1ba21d7c778..7da52419b3c 100644 --- a/app/models/concerns/encryptable.rb +++ b/app/models/concerns/encryptable.rb @@ -1,8 +1,6 @@ module Encryptable extend ActiveSupport::Concern - - ENCRYPTION_PREFIX = "encrypted-" - + include EncryptValue included do before_save :encrypt_setters end @@ -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 diff --git a/app/models/setting.rb b/app/models/setting.rb index 45893914409..09be39908c4 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -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 } @@ -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 @@ -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 @@ -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]) 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 @@ -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 diff --git a/app/models/setting/auth.rb b/app/models/setting/auth.rb index bd1eb9999c6..01430a3b288 100644 --- a/app/models/setting/auth.rb +++ b/app/models/setting/auth.rb @@ -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')), @@ -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')), diff --git a/db/migrate/20160315161936_add_encrypted_to_settings.rb b/db/migrate/20160315161936_add_encrypted_to_settings.rb new file mode 100644 index 00000000000..b4d80b8a028 --- /dev/null +++ b/db/migrate/20160315161936_add_encrypted_to_settings.rb @@ -0,0 +1,5 @@ +class AddEncryptedToSettings < ActiveRecord::Migration + def change + add_column :settings, :encrypted, :boolean, :null => false, :default => false + end +end diff --git a/test/fixtures/settings.yml b/test/fixtures/settings.yml index 60ceca76488..0a4059cffe3 100644 --- a/test/fixtures/settings.yml +++ b/test/fixtures/settings.yml @@ -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 diff --git a/test/models/concerns/audit_extensions_test.rb b/test/models/concerns/audit_extensions_test.rb index 72a55e925c6..1995901be45 100644 --- a/test/models/concerns/audit_extensions_test.rb +++ b/test/models/concerns/audit_extensions_test.rb @@ -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 diff --git a/test/models/setting_test.rb b/test/models/setting_test.rb index 64a4f530a3d..44506eb2739 100644 --- a/test/models/setting_test.rb +++ b/test/models/setting_test.rb @@ -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"]