From f0b321947c39ab9be6b63286a5a002399a75f3f3 Mon Sep 17 00:00:00 2001 From: "Balasankar \"Balu\" C" Date: Sun, 5 Nov 2023 18:05:07 +0530 Subject: [PATCH 1/2] Enhance EncryptedCommandBase to receive args from rake tasks Signed-off-by: Balasankar "Balu" C --- lib/gitlab/encrypted_command_base.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/gitlab/encrypted_command_base.rb b/lib/gitlab/encrypted_command_base.rb index b35c28b85cda1a..679d9d8e31ac4e 100644 --- a/lib/gitlab/encrypted_command_base.rb +++ b/lib/gitlab/encrypted_command_base.rb @@ -7,12 +7,12 @@ class EncryptedCommandBase EDIT_COMMAND_NAME = "base" class << self - def encrypted_secrets + def encrypted_secrets(**args) raise NotImplementedError end - def write(contents) - encrypted = encrypted_secrets + def write(contents, args: {}) + encrypted = encrypted_secrets(**args) return unless validate_config(encrypted) validate_contents(contents) @@ -25,8 +25,8 @@ def write(contents) warn "Couldn't decrypt #{encrypted.content_path}. Perhaps you passed the wrong key?" end - def edit - encrypted = encrypted_secrets + def edit(args: {}) + encrypted = encrypted_secrets(**args) return unless validate_config(encrypted) if ENV["EDITOR"].blank? @@ -58,8 +58,8 @@ def edit temp_file&.unlink end - def show - encrypted = encrypted_secrets + def show(args: {}) + encrypted = encrypted_secrets(**args) return unless validate_config(encrypted) puts encrypted.read.presence || "File '#{encrypted.content_path}' does not exist. Use `gitlab-rake #{self::EDIT_COMMAND_NAME}` to change that." -- GitLab From 5eb16d8f9eb4c20e8467ab7f473ffc7562b644b3 Mon Sep 17 00:00:00 2001 From: "Balasankar \"Balu\" C" Date: Sun, 5 Nov 2023 14:52:17 +0530 Subject: [PATCH 2/2] Extend encrypted secrets support to Redis Add the ability to use an encrypted file with password for GitLab Rails to connect to Redis. Changelog: added Signed-off-by: Balasankar "Balu" C --- .rubocop_todo/gitlab/namespaced_class.yml | 1 + .../lint/redundant_cop_disable_directive.yml | 1 + .../style/inline_disable_annotation.yml | 1 + lib/gitlab/encrypted_ldap_command.rb | 2 - lib/gitlab/encrypted_redis_command.rb | 56 ++++++ lib/gitlab/redis/wrapper.rb | 40 +++- lib/tasks/gitlab/redis.rake | 23 +++ .../redis/redis_shared_examples.rb | 15 ++ spec/tasks/gitlab/redis_rake_spec.rb | 188 ++++++++++++++++++ 9 files changed, 320 insertions(+), 7 deletions(-) create mode 100644 lib/gitlab/encrypted_redis_command.rb create mode 100644 lib/tasks/gitlab/redis.rake create mode 100644 spec/tasks/gitlab/redis_rake_spec.rb diff --git a/.rubocop_todo/gitlab/namespaced_class.yml b/.rubocop_todo/gitlab/namespaced_class.yml index de89c1dc576622..8b59571249c215 100644 --- a/.rubocop_todo/gitlab/namespaced_class.yml +++ b/.rubocop_todo/gitlab/namespaced_class.yml @@ -1109,6 +1109,7 @@ Gitlab/NamespacedClass: - 'lib/gitlab/encrypted_configuration.rb' - 'lib/gitlab/encrypted_incoming_email_command.rb' - 'lib/gitlab/encrypted_ldap_command.rb' + - 'lib/gitlab/encrypted_redis_command.rb' - 'lib/gitlab/encrypted_service_desk_email_command.rb' - 'lib/gitlab/encrypted_smtp_command.rb' - 'lib/gitlab/environment_logger.rb' diff --git a/.rubocop_todo/lint/redundant_cop_disable_directive.yml b/.rubocop_todo/lint/redundant_cop_disable_directive.yml index bbc9aecf6378dc..6a9b68a832a5fe 100644 --- a/.rubocop_todo/lint/redundant_cop_disable_directive.yml +++ b/.rubocop_todo/lint/redundant_cop_disable_directive.yml @@ -202,6 +202,7 @@ Lint/RedundantCopDisableDirective: - 'lib/gitlab/diff/parser.rb' - 'lib/gitlab/encrypted_incoming_email_command.rb' - 'lib/gitlab/encrypted_ldap_command.rb' + - 'lib/gitlab/encrypted_redis_command.rb' - 'lib/gitlab/encrypted_service_desk_email_command.rb' - 'lib/gitlab/encrypted_smtp_command.rb' - 'lib/gitlab/git/commit.rb' diff --git a/.rubocop_todo/style/inline_disable_annotation.yml b/.rubocop_todo/style/inline_disable_annotation.yml index 87c4ca68ad02f7..136c5fe9a0c8b1 100644 --- a/.rubocop_todo/style/inline_disable_annotation.yml +++ b/.rubocop_todo/style/inline_disable_annotation.yml @@ -2572,6 +2572,7 @@ Style/InlineDisableAnnotation: - 'lib/gitlab/encrypted_command_base.rb' - 'lib/gitlab/encrypted_incoming_email_command.rb' - 'lib/gitlab/encrypted_ldap_command.rb' + - 'lib/gitlab/encrypted_redis_command.rb' - 'lib/gitlab/encrypted_service_desk_email_command.rb' - 'lib/gitlab/encrypted_smtp_command.rb' - 'lib/gitlab/error_tracking/processor/context_payload_processor.rb' diff --git a/lib/gitlab/encrypted_ldap_command.rb b/lib/gitlab/encrypted_ldap_command.rb index 5e1eabe7ec6a56..442c675f19e9c0 100644 --- a/lib/gitlab/encrypted_ldap_command.rb +++ b/lib/gitlab/encrypted_ldap_command.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# rubocop:disable Rails/Output module Gitlab class EncryptedLdapCommand < EncryptedCommandBase DISPLAY_NAME = "LDAP" @@ -21,4 +20,3 @@ def encrypted_file_template end end end -# rubocop:enable Rails/Output diff --git a/lib/gitlab/encrypted_redis_command.rb b/lib/gitlab/encrypted_redis_command.rb new file mode 100644 index 00000000000000..608edcdb9500a9 --- /dev/null +++ b/lib/gitlab/encrypted_redis_command.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +# rubocop:disable Rails/Output +module Gitlab + class EncryptedRedisCommand < EncryptedCommandBase + DISPLAY_NAME = "Redis" + EDIT_COMMAND_NAME = "gitlab:redis:secret:edit" + + class << self + def all_redis_instance_class_names + Gitlab::Redis::ALL_CLASSES.map do |c| + normalized_instance_name(c) + end + end + + def normalized_instance_name(instance) + if instance.is_a?(Class) + # Gitlab::Redis::SharedState => sharedstate + instance.name.demodulize.to_s.downcase + else + # Drop all hyphens, underscores, and spaces from the name + # eg.: shared_state => sharedstate + instance.gsub(/[-_ ]/, '').downcase + end + end + + def encrypted_secrets(**args) + if args[:instance_name] + instance_class = Gitlab::Redis::ALL_CLASSES.find do |instance| + normalized_instance_name(instance) == normalized_instance_name(args[:instance_name]) + end + + unless instance_class + error_message = <<~MSG + Specified instance name #{args[:instance_name]} does not exist. + The available instances are #{all_redis_instance_class_names.join(', ')}." + MSG + + raise error_message + end + else + instance_class = Gitlab::Redis::Cache + end + + instance_class.encrypted_secrets + end + + def encrypted_file_template + <<~YAML + # password: '123' + YAML + end + end + end +end +# rubocop:enable Rails/Output diff --git a/lib/gitlab/redis/wrapper.rb b/lib/gitlab/redis/wrapper.rb index 2bcf4769b5a746..d5470bc0016ff6 100644 --- a/lib/gitlab/redis/wrapper.rb +++ b/lib/gitlab/redis/wrapper.rb @@ -19,7 +19,7 @@ class Wrapper InvalidPathError = Class.new(StandardError) class << self - delegate :params, :url, :store, to: :new + delegate :params, :url, :store, :encrypted_secrets, to: :new def with pool.with { |redis| yield redis } @@ -110,6 +110,14 @@ def sentinels raw_config_hash[:sentinels] end + def secret_file + if raw_config_hash[:secret_file].blank? + File.join(Settings.encrypted_settings['path'], 'redis.yaml.enc') + else + Settings.absolute(raw_config_hash[:secret_file]) + end + end + def sentinels? sentinels && !sentinels.empty? end @@ -118,22 +126,44 @@ def store(extras = {}) ::Redis::Store::Factory.create(redis_store_options.merge(extras)) end + def encrypted_secrets + # In rake tasks, we have to populate the encrypted_secrets even if the + # file does not exist, as it is the job of one of those tasks to create + # the file. In other cases, like when being loaded as part of spinning + # up test environment via `scripts/setup-test-env`, we should gate on + # the presence of the specified secret file so that + # `Settings.encrypted`, which might not be loadable does not gets + # called. + Settings.encrypted(secret_file) if File.exist?(secret_file) || ::Gitlab::Runtime.rake? + end + private def redis_store_options config = raw_config_hash config[:instrumentation_class] ||= self.class.instrumentation_class - result = if config[:cluster].present? - config[:db] = 0 # Redis Cluster only supports db 0 - config + decrypted_config = parse_encrypted_config(config) + + result = if decrypted_config[:cluster].present? + decrypted_config[:db] = 0 # Redis Cluster only supports db 0 + decrypted_config else - parse_redis_url(config) + parse_redis_url(decrypted_config) end parse_client_tls_options(result) end + def parse_encrypted_config(encrypted_config) + encrypted_config.delete(:secret_file) + + decrypted_secrets = encrypted_secrets&.config + encrypted_config.merge!(decrypted_secrets) if decrypted_secrets + + encrypted_config + end + def parse_redis_url(config) redis_url = config.delete(:url) redis_uri = URI.parse(redis_url) diff --git a/lib/tasks/gitlab/redis.rake b/lib/tasks/gitlab/redis.rake new file mode 100644 index 00000000000000..6983c5fc3182c9 --- /dev/null +++ b/lib/tasks/gitlab/redis.rake @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +namespace :gitlab do + namespace :redis do + namespace :secret do + desc "GitLab | Redis | Secret | Show Redis secret" + task :show, [:instance_name] => [:environment] do |_t, args| + Gitlab::EncryptedRedisCommand.show(args: args) + end + + desc "GitLab | Redis | Secret | Edit Redis secret" + task :edit, [:instance_name] => [:environment] do |_t, args| + Gitlab::EncryptedRedisCommand.edit(args: args) + end + + desc "GitLab | Redis | Secret | Write Redis secret" + task :write, [:instance_name] => [:environment] do |_t, args| + content = $stdin.tty? ? $stdin.gets : $stdin.read + Gitlab::EncryptedRedisCommand.write(content, args: args) + end + end + end +end diff --git a/spec/support/shared_examples/redis/redis_shared_examples.rb b/spec/support/shared_examples/redis/redis_shared_examples.rb index 1270efd4701426..f184f6782837b2 100644 --- a/spec/support/shared_examples/redis/redis_shared_examples.rb +++ b/spec/support/shared_examples/redis/redis_shared_examples.rb @@ -365,6 +365,21 @@ end end + describe '#secret_file' do + context 'when explicitly specified in config file' do + it 'returns the absolute path of specified file inside Rails root' do + allow(subject).to receive(:raw_config_hash).and_return({ secret_file: '/etc/gitlab/redis_secret.enc' }) + expect(subject.send(:secret_file)).to eq('/etc/gitlab/redis_secret.enc') + end + end + + context 'when not explicitly specified' do + it 'returns the default path in the encrypted settings shared directory' do + expect(subject.send(:secret_file)).to eq(Rails.root.join("shared/encrypted_settings/redis.yaml.enc").to_s) + end + end + end + describe "#parse_client_tls_options" do let(:dummy_certificate) { OpenSSL::X509::Certificate.new } let(:dummy_key) { OpenSSL::PKey::RSA.new } diff --git a/spec/tasks/gitlab/redis_rake_spec.rb b/spec/tasks/gitlab/redis_rake_spec.rb new file mode 100644 index 00000000000000..bfad25be4fd96e --- /dev/null +++ b/spec/tasks/gitlab/redis_rake_spec.rb @@ -0,0 +1,188 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'gitlab:redis:secret rake tasks', :silence_stdout, feature_category: :build do + let(:redis_secret_file) { 'tmp/tests/redisenc/redis_secret.yaml.enc' } + + before do + Rake.application.rake_require 'tasks/gitlab/redis' + stub_env('EDITOR', 'cat') + stub_warn_user_is_not_gitlab + FileUtils.mkdir_p('tmp/tests/redisenc/') + allow(::Gitlab::Runtime).to receive(:rake?).and_return(true) + allow_next_instance_of(Gitlab::Redis::Cache) do |instance| + allow(instance).to receive(:secret_file).and_return(redis_secret_file) + end + allow(Gitlab::Application.secrets).to receive(:encrypted_settings_key_base).and_return(SecureRandom.hex(64)) + end + + after do + FileUtils.rm_rf(Rails.root.join('tmp/tests/redisenc')) + end + + describe ':show' do + it 'displays error when file does not exist' do + expect do + run_rake_task('gitlab:redis:secret:show') + end.to output(/File .* does not exist. Use `gitlab-rake gitlab:redis:secret:edit` to change that./).to_stdout + end + + it 'displays error when key does not exist' do + Settings.encrypted(redis_secret_file).write('somevalue') + allow(Gitlab::Application.secrets).to receive(:encrypted_settings_key_base).and_return(nil) + expect do + run_rake_task('gitlab:redis:secret:show') + end.to output(/Missing encryption key encrypted_settings_key_base./).to_stderr + end + + it 'displays error when key is changed' do + Settings.encrypted(redis_secret_file).write('somevalue') + allow(Gitlab::Application.secrets).to receive(:encrypted_settings_key_base).and_return(SecureRandom.hex(64)) + expect do + run_rake_task('gitlab:redis:secret:show') + end.to output(/Couldn't decrypt .* Perhaps you passed the wrong key?/).to_stderr + end + + it 'outputs the unencrypted content when present' do + encrypted = Settings.encrypted(redis_secret_file) + encrypted.write('somevalue') + expect { run_rake_task('gitlab:redis:secret:show') }.to output(/somevalue/).to_stdout + end + end + + describe 'edit' do + it 'creates encrypted file' do + expect { run_rake_task('gitlab:redis:secret:edit') }.to output(/File encrypted and saved./).to_stdout + expect(File.exist?(redis_secret_file)).to be true + value = Settings.encrypted(redis_secret_file) + expect(value.read).to match(/password: '123'/) + end + + it 'displays error when key does not exist' do + allow(Gitlab::Application.secrets).to receive(:encrypted_settings_key_base).and_return(nil) + expect do + run_rake_task('gitlab:redis:secret:edit') + end.to output(/Missing encryption key encrypted_settings_key_base./).to_stderr + end + + it 'displays error when key is changed' do + Settings.encrypted(redis_secret_file).write('somevalue') + allow(Gitlab::Application.secrets).to receive(:encrypted_settings_key_base).and_return(SecureRandom.hex(64)) + expect do + run_rake_task('gitlab:redis:secret:edit') + end.to output(/Couldn't decrypt .* Perhaps you passed the wrong key?/).to_stderr + end + + it 'displays error when write directory does not exist' do + FileUtils.rm_rf(Rails.root.join('tmp/tests/redisenc')) + expect { run_rake_task('gitlab:redis:secret:edit') }.to output(/Directory .* does not exist./).to_stderr + end + + it 'shows a warning when content is invalid' do + Settings.encrypted(redis_secret_file).write('somevalue') + expect do + run_rake_task('gitlab:redis:secret:edit') + end.to output(/WARNING: Content was not a valid Redis secret yml file/).to_stdout + value = Settings.encrypted(redis_secret_file) + expect(value.read).to match(/somevalue/) + end + + it 'displays error when $EDITOR is not set' do + stub_env('EDITOR', nil) + expect do + run_rake_task('gitlab:redis:secret:edit') + end.to output(/No \$EDITOR specified to open file. Please provide one when running the command/).to_stderr + end + end + + describe 'write' do + before do + allow($stdin).to receive(:tty?).and_return(false) + allow($stdin).to receive(:read).and_return('testvalue') + end + + it 'creates encrypted file from stdin' do + expect { run_rake_task('gitlab:redis:secret:write') }.to output(/File encrypted and saved./).to_stdout + expect(File.exist?(redis_secret_file)).to be true + value = Settings.encrypted(redis_secret_file) + expect(value.read).to match(/testvalue/) + end + + it 'displays error when key does not exist' do + allow(Gitlab::Application.secrets).to receive(:encrypted_settings_key_base).and_return(nil) + expect do + run_rake_task('gitlab:redis:secret:write') + end.to output(/Missing encryption key encrypted_settings_key_base./).to_stderr + end + + it 'displays error when write directory does not exist' do + FileUtils.rm_rf('tmp/tests/redisenc/') + expect { run_rake_task('gitlab:redis:secret:write') }.to output(/Directory .* does not exist./).to_stderr + end + + it 'shows a warning when content is invalid' do + Settings.encrypted(redis_secret_file).write('somevalue') + expect do + run_rake_task('gitlab:redis:secret:edit') + end.to output(/WARNING: Content was not a valid Redis secret yml file/).to_stdout + expect(Settings.encrypted(redis_secret_file).read).to match(/somevalue/) + end + end + + context 'when an instance class is specified' do + before do + allow_next_instance_of(Gitlab::Redis::SharedState) do |instance| + allow(instance).to receive(:secret_file).and_return(redis_secret_file) + end + end + + context 'when actual name is used' do + it 'uses the correct Redis class' do + expect(Gitlab::Redis::SharedState).to receive(:encrypted_secrets).and_call_original + + run_rake_task('gitlab:redis:secret:edit', 'SharedState') + end + end + + context 'when name in lowercase is used' do + it 'uses the correct Redis class' do + expect(Gitlab::Redis::SharedState).to receive(:encrypted_secrets).and_call_original + + run_rake_task('gitlab:redis:secret:edit', 'sharedstate') + end + end + + context 'when name with underscores is used' do + it 'uses the correct Redis class' do + expect(Gitlab::Redis::SharedState).to receive(:encrypted_secrets).and_call_original + + run_rake_task('gitlab:redis:secret:edit', 'shared_state') + end + end + + context 'when name with hyphens is used' do + it 'uses the correct Redis class' do + expect(Gitlab::Redis::SharedState).to receive(:encrypted_secrets).and_call_original + + run_rake_task('gitlab:redis:secret:edit', 'shared-state') + end + end + + context 'when name with spaces is used' do + it 'uses the correct Redis class' do + expect(Gitlab::Redis::SharedState).to receive(:encrypted_secrets).and_call_original + + run_rake_task('gitlab:redis:secret:edit', 'shared state') + end + end + + context 'when an invalid name is used' do + it 'raises error' do + expect do + run_rake_task('gitlab:redis:secret:edit', 'foobar') + end.to raise_error(/Specified instance name foobar does not exist./) + end + end + end +end -- GitLab