From 39e21fb2661693fed914012a39fb3a53b2b687c2 Mon Sep 17 00:00:00 2001 From: Vladimir Shushlin Date: Fri, 31 May 2019 05:22:55 +0000 Subject: Generate lets_encrypt_private_key on the fly Remove migration generating lets encrypt key Don't generate private_key if database is readonly For reference: This reverts commit 988a7f70489b99383b95e9f271a2caf6bb5b3a44. This reverts commit 21acbe531592d55caf0e5b8716a3b551dafd6233. --- ...0524062810_generate_lets_encrypt_private_key.rb | 17 +------- lib/gitlab/lets_encrypt/client.rb | 23 ++++++++++- spec/lib/gitlab/lets_encrypt/client_spec.rb | 46 ++++++++++++++++++++-- .../generate_lets_encrypt_private_key_spec.rb | 12 +----- spec/support/matchers/eq_pem.rb | 11 ++++++ 5 files changed, 79 insertions(+), 30 deletions(-) create mode 100644 spec/support/matchers/eq_pem.rb diff --git a/db/migrate/20190524062810_generate_lets_encrypt_private_key.rb b/db/migrate/20190524062810_generate_lets_encrypt_private_key.rb index 21d7049b998..ae93a76575a 100644 --- a/db/migrate/20190524062810_generate_lets_encrypt_private_key.rb +++ b/db/migrate/20190524062810_generate_lets_encrypt_private_key.rb @@ -9,23 +9,8 @@ class GenerateLetsEncryptPrivateKey < ActiveRecord::Migration[5.1] # Set this constant to true if this migration requires downtime. DOWNTIME = false - class ApplicationSetting < ActiveRecord::Base - self.table_name = 'application_settings' - - attr_encrypted :lets_encrypt_private_key, - mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base_truncated, - algorithm: 'aes-256-gcm', - encode: true - end - + # we now generate this key on the fly, but since this migration was merged to master, we don't remove it def up - ApplicationSetting.reset_column_information - - private_key = OpenSSL::PKey::RSA.new(4096).to_pem - ApplicationSetting.find_each do |setting| - setting.update!(lets_encrypt_private_key: private_key) - end end def down diff --git a/lib/gitlab/lets_encrypt/client.rb b/lib/gitlab/lets_encrypt/client.rb index 5501f7981ec..66aea137012 100644 --- a/lib/gitlab/lets_encrypt/client.rb +++ b/lib/gitlab/lets_encrypt/client.rb @@ -3,6 +3,8 @@ module Gitlab module LetsEncrypt class Client + include Gitlab::Utils::StrongMemoize + PRODUCTION_DIRECTORY_URL = 'https://acme-v02.api.letsencrypt.org/directory' STAGING_DIRECTORY_URL = 'https://acme-staging-v02.api.letsencrypt.org/directory' @@ -35,6 +37,8 @@ module Gitlab def enabled? return false unless Feature.enabled?(:pages_auto_ssl) + return false unless private_key + Gitlab::CurrentSettings.lets_encrypt_terms_of_service_accepted end @@ -45,7 +49,11 @@ module Gitlab end def private_key - @private_key ||= OpenSSL::PKey.read(Gitlab::CurrentSettings.lets_encrypt_private_key) + strong_memoize(:private_key) do + private_key_string = Gitlab::CurrentSettings.lets_encrypt_private_key + private_key_string ||= generate_private_key + OpenSSL::PKey.read(private_key_string) if private_key_string + end end def admin_email @@ -69,6 +77,19 @@ module Gitlab STAGING_DIRECTORY_URL end end + + def generate_private_key + return if Gitlab::Database.read_only? + + application_settings = Gitlab::CurrentSettings.current_application_settings + application_settings.with_lock do + unless application_settings.lets_encrypt_private_key + application_settings.update(lets_encrypt_private_key: OpenSSL::PKey::RSA.new(4096).to_pem) + end + + application_settings.lets_encrypt_private_key + end + end end end end diff --git a/spec/lib/gitlab/lets_encrypt/client_spec.rb b/spec/lib/gitlab/lets_encrypt/client_spec.rb index d63a2fbee04..5454d9c1af4 100644 --- a/spec/lib/gitlab/lets_encrypt/client_spec.rb +++ b/spec/lib/gitlab/lets_encrypt/client_spec.rb @@ -5,14 +5,12 @@ require 'spec_helper' describe ::Gitlab::LetsEncrypt::Client do include LetsEncryptHelpers - set(:private_key) { OpenSSL::PKey::RSA.new(4096).to_pem } let(:client) { described_class.new } before do stub_application_setting( lets_encrypt_notification_email: 'myemail@test.example.com', - lets_encrypt_terms_of_service_accepted: true, - lets_encrypt_private_key: private_key + lets_encrypt_terms_of_service_accepted: true ) end @@ -28,6 +26,36 @@ describe ::Gitlab::LetsEncrypt::Client do ) end + it 'generates and stores private key and initialize acme client with it' do + expect(Gitlab::CurrentSettings.lets_encrypt_private_key).to eq(nil) + + subject + + saved_private_key = Gitlab::CurrentSettings.lets_encrypt_private_key + + expect(saved_private_key).to be + expect(Acme::Client).to have_received(:new).with( + hash_including(private_key: eq_pem(saved_private_key)) + ) + end + + context 'when private key is saved in settings' do + let!(:saved_private_key) do + key = OpenSSL::PKey::RSA.new(4096).to_pem + Gitlab::CurrentSettings.current_application_settings.update(lets_encrypt_private_key: key) + key + end + + it 'uses current value of private key' do + subject + + expect(Acme::Client).to have_received(:new).with( + hash_including(private_key: eq_pem(saved_private_key)) + ) + expect(Gitlab::CurrentSettings.lets_encrypt_private_key).to eq(saved_private_key) + end + end + context 'when acme integration is disabled' do before do stub_application_setting(lets_encrypt_terms_of_service_accepted: false) @@ -94,6 +122,18 @@ describe ::Gitlab::LetsEncrypt::Client do context 'when terms of service are accepted' do it { is_expected.to eq(true) } + context "when private_key isn't present and database is read only" do + before do + allow(::Gitlab::Database).to receive(:read_only?).and_return(true) + end + + it 'returns false' do + expect(::Gitlab::CurrentSettings.lets_encrypt_private_key).to eq(nil) + + is_expected.to eq(false) + end + end + context 'when feature flag is disabled' do before do stub_feature_flags(pages_auto_ssl: false) diff --git a/spec/migrations/generate_lets_encrypt_private_key_spec.rb b/spec/migrations/generate_lets_encrypt_private_key_spec.rb index f47cc0c36ef..773bf5222f0 100644 --- a/spec/migrations/generate_lets_encrypt_private_key_spec.rb +++ b/spec/migrations/generate_lets_encrypt_private_key_spec.rb @@ -3,17 +3,9 @@ require Rails.root.join('db', 'migrate', '20190524062810_generate_lets_encrypt_p describe GenerateLetsEncryptPrivateKey, :migration do describe '#up' do - let(:applications_settings) { table(:applications_settings) } - - it 'generates RSA private key and saves it in application settings' do - application_setting = described_class::ApplicationSetting.create! - - described_class.new.up - application_setting.reload - - expect(application_setting.lets_encrypt_private_key).to be_present + it 'does not fail' do expect do - OpenSSL::PKey::RSA.new(application_setting.lets_encrypt_private_key) + described_class.new.up end.not_to raise_error end end diff --git a/spec/support/matchers/eq_pem.rb b/spec/support/matchers/eq_pem.rb new file mode 100644 index 00000000000..158281e4a19 --- /dev/null +++ b/spec/support/matchers/eq_pem.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +RSpec::Matchers.define :eq_pem do |expected_pem_string| + match do |actual| + actual.to_pem == expected_pem_string + end + + description do + "contain pem #{expected_pem_string}" + end +end -- cgit v1.2.1