Merge branch 'encrypted-credentials' into 'master'

Add new function for reading/writing encrypted config files

See merge request gitlab-org/gitlab!43220

config/gitlab.yml.example

@@ -1042,6 +1042,10 @@ production: &base
   shared:
     # path: /mnt/gitlab # Default: shared
 
+  # Encrypted Settings configuration
+  encrypted_settings:
+    # path: /mnt/gitlab/encrypted_settings  # Default: shared/encrypted_settings
+
   # Gitaly settings
   gitaly:
     # Path to the directory containing Gitaly client executables.

config/initializers/01_secret_token.rb

@@ -34,6 +34,9 @@ def create_tokens
     openid_connect_signing_key: generate_new_rsa_private_key
   }
 
+  # encrypted_settings_key_base is optional for now
+  defaults[:encrypted_settings_key_base] = generate_new_secure_token if ENV['GITLAB_GENERATE_ENCRYPTED_SETTINGS_KEY_BASE']
+
   missing_secrets = set_missing_keys(defaults)
   write_secrets_yml(missing_secrets) unless missing_secrets.empty?
 

config/initializers/1_settings.rb

@@ -3,6 +3,13 @@ require_relative '../object_store_settings'
 require_relative '../smime_signature_settings'
 
 # Default settings
+Settings['shared'] ||= Settingslogic.new({})
+Settings.shared['path'] = Settings.absolute(Settings.shared['path'] || "shared")
+
+Settings['encrypted_settings'] ||= Settingslogic.new({})
+Settings.encrypted_settings['path'] ||= File.join(Settings.shared['path'], "encrypted_settings")
+Settings.encrypted_settings['path'] = Settings.absolute(Settings.encrypted_settings['path'])
+
 Settings['ldap'] ||= Settingslogic.new({})
 Settings.ldap['enabled'] = false if Settings.ldap['enabled'].nil?
 Settings.ldap['prevent_ldap_sign_in'] = false if Settings.ldap['prevent_ldap_sign_in'].blank?
@@ -140,9 +147,6 @@ if Gitlab.ee? && Rails.env.test? && !saml_provider_enabled
   Settings.omniauth.providers << Settingslogic.new({ 'name' => 'group_saml' })
 end
 
-Settings['shared'] ||= Settingslogic.new({})
-Settings.shared['path'] = Settings.absolute(Settings.shared['path'] || "shared")
-
 Settings['issues_tracker'] ||= {}
 
 #

config/settings.rb

@@ -152,6 +152,14 @@ class Settings < Settingslogic
       Gitlab::Application.secrets.db_key_base
     end
 
+    def encrypted(path)
+      Gitlab::EncryptedConfiguration.new(
+        content_path: path,
+        base_key: Gitlab::Application.secrets.encrypted_settings_key_base,
+        previous_keys: Gitlab::Application.secrets.rotated_encrypted_settings_key_base || []
+      )
+    end
+
     def load_dynamic_cron_schedules!
       cron_jobs['gitlab_usage_ping_worker']['cron'] ||= cron_for_usage_ping
     end

doc/development/application_secrets.md

@@ -16,6 +16,7 @@ This page is a development guide for application secrets.
 | `otp_key_base`                   | The base key for One Time Passwords, described in [User management](../raketasks/user_management.md#rotate-two-factor-authentication-encryption-key)              |
 |`db_key_base`                     | The base key to encrypt the data for `attr_encrypted` columns     |
 |`openid_connect_signing_key`      | The singing key for OpenID Connect                                |
+| `encrypted_settings_key_base`    | The base key to encrypt settings files with                       |
 
 ## Where the secrets are stored
 

lib/gitlab/encrypted_configuration.rb

+# frozen_string_literal: true
+
+module Gitlab
+  class EncryptedConfiguration
+    delegate :[], :fetch, to: :config
+    delegate_missing_to :options
+    attr_reader :content_path, :key, :previous_keys
+
+    CIPHER = "aes-256-gcm"
+    SALT = "GitLabEncryptedConfigSalt"
+
+    class MissingKeyError < RuntimeError
+      def initialize(msg = "Missing encryption key to encrypt/decrypt file with.")
+        super
+      end
+    end
+
+    class InvalidConfigError < RuntimeError
+      def initialize(msg = "Content was not a valid yml config file")
+        super
+      end
+    end
+
+    def self.generate_key(base_key)
+      # Because the salt is static, we want uniqueness to be coming from the base_key
+      # Error if the base_key is empty or suspiciously short
+      raise 'Base key too small' if base_key.blank? || base_key.length < 16
+
+      ActiveSupport::KeyGenerator.new(base_key).generate_key(SALT, ActiveSupport::MessageEncryptor.key_len(CIPHER))
+    end
+
+    def initialize(content_path: nil, base_key: nil, previous_keys: [])
+      @content_path = Pathname.new(content_path).yield_self { |path| path.symlink? ? path.realpath : path } if content_path
+      @key = self.class.generate_key(base_key) if base_key
+      @previous_keys = previous_keys
+    end
+
+    def active?
+      content_path&.exist?
+    end
+
+    def read
+      if active?
+        decrypt(content_path.binread)
+      else
+        ""
+      end
+    end
+
+    def write(contents)
+      # ensure contents are valid to deserialize before write
+      deserialize(contents)
+
+      temp_file = Tempfile.new(File.basename(content_path), File.dirname(content_path))
+      File.open(temp_file.path, 'wb') do |file|
+        file.write(encrypt(contents))
+      end
+      FileUtils.mv(temp_file.path, content_path)
+    ensure
+      temp_file&.unlink
+    end
+
+    def config
+      return @config if @config
+
+      contents = deserialize(read)
+
+      raise InvalidConfigError.new unless contents.is_a?(Hash)
+
+      @config = contents.deep_symbolize_keys
+    end
+
+    def change(&block)
+      writing(read, &block)
+    end
+
+    private
+
+    def writing(contents)
+      updated_contents = yield contents
+
+      write(updated_contents) if updated_contents != contents
+    end
+
+    def encrypt(contents)
+      handle_missing_key!
+      encryptor.encrypt_and_sign(contents)
+    end
+
+    def decrypt(contents)
+      handle_missing_key!
+      encryptor.decrypt_and_verify(contents)
+    end
+
+    def encryptor
+      return @encryptor if @encryptor
+
+      @encryptor = ActiveSupport::MessageEncryptor.new(key, cipher: CIPHER)
+
+      # Allow fallback to previous keys
+      @previous_keys.each do |key|
+        @encryptor.rotate(self.class.generate_key(key))
+      end
+
+      @encryptor
+    end
+
+    def options
+      # Allows top level keys to be referenced using dot syntax
+      @options ||= ActiveSupport::InheritableOptions.new(config)
+    end
+
+    def deserialize(contents)
+      YAML.safe_load(contents, permitted_classes: [Symbol]).presence || {}
+    end
+
+    def handle_missing_key!
+      raise MissingKeyError.new if @key.nil?
+    end
+  end
+end

spec/config/settings_spec.rb

@@ -134,4 +134,20 @@ RSpec.describe Settings do
       end
     end
   end
+
+  describe '.encrypted' do
+    before do
+      allow(Gitlab::Application.secrets).to receive(:encryped_settings_key_base).and_return(SecureRandom.hex(64))
+    end
+
+    it 'defaults to using the encrypted_settings_key_base for the key' do
+      expect(Gitlab::EncryptedConfiguration).to receive(:new).with(hash_including(base_key: Gitlab::Application.secrets.encrypted_settings_key_base))
+      Settings.encrypted('tmp/tests/test.enc')
+    end
+
+    it 'returns empty encrypted config when a key has not been set' do
+      allow(Gitlab::Application.secrets).to receive(:encrypted_settings_key_base).and_return(nil)
+      expect(Settings.encrypted('tmp/tests/test.enc').read).to be_empty
+    end
+  end
 end

spec/initializers/secret_token_spec.rb

@@ -24,7 +24,7 @@ RSpec.describe 'create_tokens' do
 
   describe 'ensure acknowledged secrets in any installations' do
     let(:acknowledged_secrets) do
-      %w[secret_key_base otp_key_base db_key_base openid_connect_signing_key]
+      %w[secret_key_base otp_key_base db_key_base openid_connect_signing_key encrypted_settings_key_base rotated_encrypted_settings_key_base]
     end
 
     it 'does not allow to add a new secret without a proper handling' do
@@ -90,6 +90,7 @@ RSpec.describe 'create_tokens' do
           expect(new_secrets['otp_key_base']).to eq(secrets.otp_key_base)
           expect(new_secrets['db_key_base']).to eq(secrets.db_key_base)
           expect(new_secrets['openid_connect_signing_key']).to eq(secrets.openid_connect_signing_key)
+          expect(new_secrets['encrypted_settings_key_base']).to eq(secrets.encrypted_settings_key_base)
         end
 
         create_tokens
@@ -106,6 +107,7 @@ RSpec.describe 'create_tokens' do
       before do
         secrets.db_key_base = 'db_key_base'
         secrets.openid_connect_signing_key = 'openid_connect_signing_key'
+        secrets.encrypted_settings_key_base = 'encrypted_settings_key_base'
 
         allow(File).to receive(:exist?).with('.secret').and_return(true)
         stub_file_read('.secret', content: 'file_key')
@@ -158,6 +160,7 @@ RSpec.describe 'create_tokens' do
           expect(secrets.otp_key_base).to eq('otp_key_base')
           expect(secrets.db_key_base).to eq('db_key_base')
           expect(secrets.openid_connect_signing_key).to eq('openid_connect_signing_key')
+          expect(secrets.encrypted_settings_key_base).to eq('encrypted_settings_key_base')
         end
 
         it 'deletes the .secret file' do
@@ -208,12 +211,34 @@ RSpec.describe 'create_tokens' do
           create_tokens
         end
       end
+
+      context 'when rotated_encrypted_settings_key_base does not exist' do
+        before do
+          secrets.secret_key_base = 'secret_key_base'
+          secrets.otp_key_base = 'otp_key_base'
+          secrets.openid_connect_signing_key = 'openid_connect_signing_key'
+          secrets.encrypted_settings_key_base = 'encrypted_settings_key_base'
+        end
+
+        it 'does not warn about the missing secrets' do
+          expect(self).not_to receive(:warn_missing_secret).with('rotated_encrypted_settings_key_base')
+
+          create_tokens
+        end
+
+        it 'does not update secrets.yml' do
+          expect(File).not_to receive(:write)
+
+          create_tokens
+        end
+      end
     end
 
     context 'when db_key_base is blank but exists in secrets.yml' do
       before do
         secrets.otp_key_base = 'otp_key_base'
         secrets.secret_key_base = 'secret_key_base'
+        secrets.encrypted_settings_key_base = 'encrypted_settings_key_base'
         yaml_secrets = secrets.to_h.stringify_keys.merge('db_key_base' => '<%= an_erb_expression %>')
 
         allow(File).to receive(:exist?).with('.secret').and_return(false)

spec/lib/gitlab/encrypted_configuration_spec.rb

+# frozen_string_literal: true
+
+require "spec_helper"
+
+RSpec.describe Gitlab::EncryptedConfiguration do
+  subject(:configuration) { described_class.new }
+
+  let!(:config_tmp_dir) { Dir.mktmpdir('config-') }
+
+  after do
+    FileUtils.rm_f(config_tmp_dir)
+  end
+
+  describe '#initialize' do
+    it 'accepts all args as optional fields' do
+      expect { configuration }.not_to raise_exception
+
+      expect(configuration.key).to be_nil
+      expect(configuration.previous_keys).to be_empty
+    end
+
+    it 'generates 32 byte key when provided a larger base key' do
+      configuration = described_class.new(base_key: 'A' * 64)
+
+      expect(configuration.key.bytesize).to eq 32
+    end
+
+    it 'generates 32 byte key when provided a smaller base key' do
+      configuration = described_class.new(base_key: 'A' * 16)
+
+      expect(configuration.key.bytesize).to eq 32
+    end
+
+    it 'throws an error when the base key is too small' do
+      expect { described_class.new(base_key: 'A' * 12) }.to raise_error 'Base key too small'
+    end
+  end
+
+  context 'when provided a config file but no key' do
+    let(:config_path) { File.join(config_tmp_dir, 'credentials.yml.enc') }
+
+    it 'throws an error when writing without a key' do
+      expect { described_class.new(content_path: config_path).write('test') }.to raise_error Gitlab::EncryptedConfiguration::MissingKeyError
+    end
+
+    it 'throws an error when reading without a key' do
+      config = described_class.new(content_path: config_path)
+      File.write(config_path, 'test')
+      expect { config.read }.to raise_error Gitlab::EncryptedConfiguration::MissingKeyError
+    end
+  end
+
+  context 'when provided key and config file' do
+    let(:credentials_config_path) { File.join(config_tmp_dir, 'credentials.yml.enc') }
+    let(:credentials_key) { SecureRandom.hex(64) }
+
+    describe '#write' do
+      it 'encrypts the file using the provided key' do
+        encryptor = ActiveSupport::MessageEncryptor.new(Gitlab::EncryptedConfiguration.generate_key(credentials_key), cipher: 'aes-256-gcm')
+        config = described_class.new(content_path: credentials_config_path, base_key: credentials_key)
+
+        config.write('sample-content')
+        expect(encryptor.decrypt_and_verify(File.read(credentials_config_path))).to eq('sample-content')
+      end
+    end
+
+    describe '#read' do
+      it 'reads yaml configuration' do
+        config = described_class.new(content_path: credentials_config_path, base_key: credentials_key)
+
+        config.write({ foo: { bar: true } }.to_yaml)
+        expect(config[:foo][:bar]).to be true
+      end
+
+      it 'allows referencing top level keys via dot syntax' do
+        config = described_class.new(content_path: credentials_config_path, base_key: credentials_key)
+
+        config.write({ foo: { bar: true } }.to_yaml)
+        expect(config.foo[:bar]).to be true
+      end
+
+      it 'throws a custom error when referencing an invalid key map config' do
+        config = described_class.new(content_path: credentials_config_path, base_key: credentials_key)
+
+        config.write("stringcontent")
+        expect { config[:foo] }.to raise_error Gitlab::EncryptedConfiguration::InvalidConfigError
+      end
+    end
+
+    describe '#change' do
+      it 'changes yaml configuration' do
+        config = described_class.new(content_path: credentials_config_path, base_key: credentials_key)
+
+        config.write({ foo: { bar: true } }.to_yaml)
+        config.change do |unencrypted_contents|
+          contents = YAML.safe_load(unencrypted_contents, permitted_classes: [Symbol])
+          contents.merge(beef: "stew").to_yaml
+        end
+        expect(config.foo[:bar]).to be true
+        expect(config.beef).to eq('stew')
+      end
+    end
+
+    context 'when provided previous_keys for rotation' do
+      let(:credential_key_original) { SecureRandom.hex(64) }
+      let(:credential_key_latest) { SecureRandom.hex(64) }
+      let(:config_path_original) { File.join(config_tmp_dir, 'credentials-orig.yml.enc') }
+      let(:config_path_latest) { File.join(config_tmp_dir, 'credentials-latest.yml.enc') }
+
+      def encryptor(key)
+        ActiveSupport::MessageEncryptor.new(Gitlab::EncryptedConfiguration.generate_key(key), cipher: 'aes-256-gcm')
+      end
+
+      describe '#write' do
+        it 'rotates the key when provided a new key' do
+          config1 = described_class.new(content_path: config_path_original, base_key: credential_key_original)
+          config1.write('sample-content1')
+
+          config2 = described_class.new(content_path: config_path_latest, base_key: credential_key_latest, previous_keys: [credential_key_original])
+          config2.write('sample-content2')
+
+          original_key_encryptor = encryptor(credential_key_original) # can read with the initial key
+          latest_key_encryptor = encryptor(credential_key_latest) # can read with the new key
+          both_key_encryptor = encryptor(credential_key_latest) # can read with either key
+          both_key_encryptor.rotate(Gitlab::EncryptedConfiguration.generate_key(credential_key_original))
+
+          expect(original_key_encryptor.decrypt_and_verify(File.read(config_path_original))).to eq('sample-content1')
+          expect(both_key_encryptor.decrypt_and_verify(File.read(config_path_original))).to eq('sample-content1')
+          expect(latest_key_encryptor.decrypt_and_verify(File.read(config_path_latest))).to eq('sample-content2')
+          expect(both_key_encryptor.decrypt_and_verify(File.read(config_path_latest))).to eq('sample-content2')
+          expect { original_key_encryptor.decrypt_and_verify(File.read(config_path_latest)) }.to raise_error(ActiveSupport::MessageEncryptor::InvalidMessage)
+        end
+      end
+
+      describe '#read' do
+        it 'supports reading using rotated config' do
+          described_class.new(content_path: config_path_original, base_key: credential_key_original).write({ foo: { bar: true } }.to_yaml)
+
+          config = described_class.new(content_path: config_path_original, base_key: credential_key_latest,  previous_keys: [credential_key_original])
+          expect(config[:foo][:bar]).to be true
+        end
+      end
+    end
+  end
+end