diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index 63b6197a04d45f07daccb9f70cef1b6c75edce1a..ac1f022c63fd9800bc6b0427329bea2af238de3c 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -169,10 +169,6 @@ def object_store_credentials object_store_options.connection.to_hash.deep_symbolize_keys end - def consolidated_settings? - object_store_options.fetch('consolidated_settings', false) - end - def remote_store_path object_store_options.remote_directory end @@ -193,14 +189,18 @@ def workhorse_local_upload_path File.join(self.root, TMP_UPLOAD_PATH) end + def object_store_config + ObjectStorage::Config.new(object_store_options) + end + def workhorse_remote_upload_options(has_length:, maximum_size: nil) return unless self.object_store_enabled? return unless self.direct_upload_enabled? id = [CarrierWave.generate_cache_id, SecureRandom.hex].join('-') upload_path = File.join(TMP_UPLOAD_PATH, id) - direct_upload = ObjectStorage::DirectUpload.new(self.object_store_credentials, remote_store_path, upload_path, - has_length: has_length, maximum_size: maximum_size, consolidated_settings: consolidated_settings?) + direct_upload = ObjectStorage::DirectUpload.new(self.object_store_config, upload_path, + has_length: has_length, maximum_size: maximum_size) direct_upload.to_hash.merge(ID: id) end @@ -283,6 +283,10 @@ def fog_credentials self.class.object_store_credentials end + def fog_attributes + @fog_attributes ||= self.class.object_store_config.fog_attributes + end + # Set ACL of uploaded objects to not-public (fog-aws)[1] or no ACL at all # (fog-google). Value is ignored by other supported backends (fog-aliyun, # fog-openstack, fog-rackspace) diff --git a/changelogs/unreleased/sh-s3-encryption.yml b/changelogs/unreleased/sh-s3-encryption.yml new file mode 100644 index 0000000000000000000000000000000000000000..3a92728820dd8f8d94ddcfb906fe952a07dd3ad1 --- /dev/null +++ b/changelogs/unreleased/sh-s3-encryption.yml @@ -0,0 +1,5 @@ +--- +title: Add support for specifying AWS S3 Server Side Encryption (AWS-KMS) +merge_request: 38240 +author: +type: added diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 7ba256b39cd0af7373ce14b6c0c2f4248860aa0e..e16a254e256bee08dc3d7094edfb38e91869e9a3 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -218,6 +218,9 @@ production: &base # region: us-east-1 # aws_signature_version: 4 # For creation of signed URLs. Set to 2 if provider does not support v4. # endpoint: 'https://s3.amazonaws.com' # default: nil - Useful for S3 compliant services such as DigitalOcean Spaces + # storage_options: + # server_side_encryption: AES256 # AES256, aws:kms + # server_side_encryption_kms_key_id: # Amazon Resource Name. See https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingKMSEncryption.html # objects: # artifacts: # bucket: artifacts diff --git a/config/initializers/carrierwave_patch.rb b/config/initializers/carrierwave_patch.rb new file mode 100644 index 0000000000000000000000000000000000000000..94a79e5990d9d0e69faf872a8fe036923f08a22e --- /dev/null +++ b/config/initializers/carrierwave_patch.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require "carrierwave/storage/fog" + +# This pulls in https://github.com/carrierwaveuploader/carrierwave/pull/2504 to support +# sending AWS S3 encryption headers when copying objects. +module CarrierWave + module Storage + class Fog < Abstract + class File + def copy_to(new_path) + connection.copy_object(@uploader.fog_directory, file.key, @uploader.fog_directory, new_path, copy_to_options) + CarrierWave::Storage::Fog::File.new(@uploader, @base, new_path) + end + + def copy_to_options + acl_header.merge(@uploader.fog_attributes) + end + end + end + end +end diff --git a/config/object_store_settings.rb b/config/object_store_settings.rb index d8e1939a346c0cfe4037adaa655834e646834d41..0d3461354633ab4d73b9f21d5f8ce0a54a1847e5 100644 --- a/config/object_store_settings.rb +++ b/config/object_store_settings.rb @@ -13,6 +13,7 @@ def self.legacy_parse(object_store) object_store['direct_upload'] = false if object_store['direct_upload'].nil? object_store['background_upload'] = true if object_store['background_upload'].nil? object_store['proxy_download'] = false if object_store['proxy_download'].nil? + object_store['storage_options'] ||= {} # Convert upload connection settings to use string keys, to make Fog happy object_store['connection']&.deep_stringify_keys! @@ -37,6 +38,8 @@ def initialize(settings) # region: gdk # endpoint: 'http://127.0.0.1:9000' # path_style: true + # storage_options: + # server_side_encryption: AES256 # proxy_download: true # objects: # artifacts: @@ -49,7 +52,7 @@ def initialize(settings) # # Settings.artifacts['object_store'] = { # "enabled" => true, - # "connection"=> { + # "connection" => { # "provider" => "AWS", # "aws_access_key_id" => "minio", # "aws_secret_access_key" => "gdk-minio", @@ -57,6 +60,9 @@ def initialize(settings) # "endpoint" => "http://127.0.0.1:9000", # "path_style" => true # }, + # "storage_options" => { + # "server_side_encryption" => "AES256" + # }, # "direct_upload" => true, # "background_upload" => false, # "proxy_download" => false, @@ -73,6 +79,9 @@ def initialize(settings) # "endpoint" => "http://127.0.0.1:9000", # "path_style" => true # }, + # "storage_options" => { + # "server_side_encryption" => "AES256" + # }, # "direct_upload" => true, # "background_upload" => false, # "proxy_download" => true, @@ -91,12 +100,13 @@ def parse! return unless use_consolidated_settings? main_config = settings['object_store'] - common_config = main_config.slice('enabled', 'connection', 'proxy_download') + common_config = main_config.slice('enabled', 'connection', 'proxy_download', 'storage_options') # Convert connection settings to use string keys, to make Fog happy common_config['connection']&.deep_stringify_keys! # These are no longer configurable if common config is used common_config['direct_upload'] = true common_config['background_upload'] = false + common_config['storage_options'] ||= {} SUPPORTED_TYPES.each do |store_type| overrides = main_config.dig('objects', store_type) || {} diff --git a/lib/object_storage/config.rb b/lib/object_storage/config.rb new file mode 100644 index 0000000000000000000000000000000000000000..489b85104d6c6a2db5e3d868da5e7830c641e38d --- /dev/null +++ b/lib/object_storage/config.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +module ObjectStorage + class Config + attr_reader :options + + def initialize(options) + @options = options.to_hash.deep_symbolize_keys + end + + def credentials + @credentials ||= options[:connection] || {} + end + + def storage_options + @storage_options ||= options[:storage_options] || {} + end + + def enabled? + options[:enabled] + end + + def bucket + options[:remote_directory] + end + + def consolidated_settings? + options.fetch(:consolidated_settings, false) + end + + # AWS-specific options + def aws? + provider == 'AWS' + end + + def use_iam_profile? + credentials.fetch(:use_iam_profile, false) + end + + def use_path_style? + credentials.fetch(:path_style, false) + end + + def server_side_encryption + storage_options[:server_side_encryption] + end + + def server_side_encryption_kms_key_id + storage_options[:server_side_encryption_kms_key_id] + end + + def provider + credentials[:provider].to_s + end + # End AWS-specific options + + def google? + provider == 'Google' + end + + def fog_attributes + @fog_attributes ||= begin + return {} unless enabled? && aws? + return {} unless server_side_encryption.present? + + aws_server_side_encryption_headers.compact + end + end + + private + + def aws_server_side_encryption_headers + { + 'x-amz-server-side-encryption' => server_side_encryption, + 'x-amz-server-side-encryption-aws-kms-key-id' => server_side_encryption_kms_key_id + } + end + end +end diff --git a/lib/object_storage/direct_upload.rb b/lib/object_storage/direct_upload.rb index 76f92f62e9c1da84399b3d440387af7124ed7261..5784a089bbac432db4f8691151931a8f688ecd05 100644 --- a/lib/object_storage/direct_upload.rb +++ b/lib/object_storage/direct_upload.rb @@ -22,20 +22,20 @@ class DirectUpload MAXIMUM_MULTIPART_PARTS = 100 MINIMUM_MULTIPART_SIZE = 5.megabytes - attr_reader :credentials, :bucket_name, :object_name - attr_reader :has_length, :maximum_size, :consolidated_settings + attr_reader :config, :credentials, :bucket_name, :object_name + attr_reader :has_length, :maximum_size - def initialize(credentials, bucket_name, object_name, has_length:, maximum_size: nil, consolidated_settings: false) + def initialize(config, object_name, has_length:, maximum_size: nil) unless has_length raise ArgumentError, 'maximum_size has to be specified if length is unknown' unless maximum_size end - @credentials = credentials - @bucket_name = bucket_name + @config = config + @credentials = config.credentials + @bucket_name = config.bucket @object_name = object_name @has_length = has_length @maximum_size = maximum_size - @consolidated_settings = consolidated_settings end def to_hash @@ -62,7 +62,7 @@ def multipart_upload_hash end def workhorse_client_hash - return {} unless aws? + return {} unless config.aws? { UseWorkhorseClient: use_workhorse_s3_client?, @@ -73,16 +73,18 @@ def workhorse_client_hash Bucket: bucket_name, Region: credentials[:region], Endpoint: credentials[:endpoint], - PathStyle: credentials.fetch(:path_style, false), - UseIamProfile: credentials.fetch(:use_iam_profile, false) - } + PathStyle: config.use_path_style?, + UseIamProfile: config.use_iam_profile?, + ServerSideEncryption: config.server_side_encryption, + SSEKMSKeyID: config.server_side_encryption_kms_key_id + }.compact } } end def use_workhorse_s3_client? return false unless Feature.enabled?(:use_workhorse_s3_client, default_enabled: true) - return false unless credentials.fetch(:use_iam_profile, false) || consolidated_settings + return false unless config.use_iam_profile? || config.consolidated_settings? # The Golang AWS SDK does not support V2 signatures return false unless credentials.fetch(:aws_signature_version, 4).to_i >= 4 @@ -95,7 +97,7 @@ def provider # Implements https://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectGET.html def get_url - if google? + if config.google? connection.get_object_https_url(bucket_name, object_name, expire_at) else connection.get_object_url(bucket_name, object_name, expire_at) @@ -169,23 +171,15 @@ def number_of_multipart_parts ].min end - def aws? - provider == 'AWS' - end - - def google? - provider == 'Google' - end - def requires_multipart_upload? - aws? && !has_length + config.aws? && !has_length end def upload_id return unless requires_multipart_upload? strong_memoize(:upload_id) do - new_upload = connection.initiate_multipart_upload(bucket_name, object_name) + new_upload = connection.initiate_multipart_upload(bucket_name, object_name, config.fog_attributes) new_upload.body["UploadId"] end end diff --git a/spec/lib/object_storage/config_spec.rb b/spec/lib/object_storage/config_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..6cbd96608dd260a0e5c2499230d29bd644707f95 --- /dev/null +++ b/spec/lib/object_storage/config_spec.rb @@ -0,0 +1,150 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe ObjectStorage::Config do + let(:region) { 'us-east-1' } + let(:bucket_name) { 'test-bucket' } + let(:path_style) { false } + let(:use_iam_profile) { false } + let(:credentials) do + { + provider: 'AWS', + aws_access_key_id: 'AWS_ACCESS_KEY_ID', + aws_secret_access_key: 'AWS_SECRET_ACCESS_KEY', + region: region, + path_style: path_style, + use_iam_profile: use_iam_profile + } + end + + let(:storage_options) do + { + server_side_encryption: 'AES256', + server_side_encryption_kms_key_id: 'arn:aws:12345' + } + end + + let(:raw_config) do + { + enabled: true, + connection: credentials, + remote_directory: bucket_name, + storage_options: storage_options + } + end + + subject { described_class.new(raw_config.as_json) } + + describe '#credentials' do + it { expect(subject.credentials).to eq(credentials) } + end + + describe '#storage_options' do + it { expect(subject.storage_options).to eq(storage_options) } + end + + describe '#enabled?' do + it { expect(subject.enabled?).to eq(true) } + end + + describe '#bucket' do + it { expect(subject.bucket).to eq(bucket_name) } + end + + context 'with unconsolidated settings' do + describe 'consolidated_settings? returns false' do + it { expect(subject.consolidated_settings?).to be false } + end + end + + context 'with consolidated settings' do + before do + raw_config[:consolidated_settings] = true + end + + describe 'consolidated_settings? returns true' do + it { expect(subject.consolidated_settings?).to be true } + end + end + + context 'with IAM profile in use' do + let(:use_iam_profile) { true } + + it '#use_iam_profile? returns true' do + expect(subject.use_iam_profile?).to be true + end + end + + context 'with IAM profile not in use' do + it '#use_iam_profile? returns false' do + expect(subject.use_iam_profile?).to be false + end + end + + context 'with path style' do + let(:path_style) { true } + + it '#use_path_style? returns true' do + expect(subject.use_path_style?).to be true + end + end + + context 'with hostname style access' do + it '#use_path_style? returns false' do + expect(subject.use_path_style?).to be false + end + end + + context 'with AWS credentials' do + it { expect(subject.provider).to eq('AWS') } + it { expect(subject.aws?).to be true } + it { expect(subject.google?).to be false } + end + + context 'with Google credentials' do + let(:credentials) do + { + provider: 'Google', + google_client_email: 'foo@gcp-project.example.com', + google_json_key_location: '/path/to/gcp.json' + } + end + + it { expect(subject.provider).to eq('Google') } + it { expect(subject.aws?).to be false } + it { expect(subject.google?).to be true } + it { expect(subject.fog_attributes).to eq({}) } + end + + context 'with SSE-KMS enabled' do + it { expect(subject.server_side_encryption).to eq('AES256') } + it { expect(subject.server_side_encryption_kms_key_id).to eq('arn:aws:12345') } + it { expect(subject.fog_attributes.keys).to match_array(%w(x-amz-server-side-encryption x-amz-server-side-encryption-aws-kms-key-id)) } + end + + context 'with only server side encryption enabled' do + let(:storage_options) { { server_side_encryption: 'AES256' } } + + it { expect(subject.server_side_encryption).to eq('AES256') } + it { expect(subject.server_side_encryption_kms_key_id).to be_nil } + it { expect(subject.fog_attributes).to eq({ 'x-amz-server-side-encryption' => 'AES256' }) } + end + + context 'without encryption enabled' do + let(:storage_options) { {} } + + it { expect(subject.server_side_encryption).to be_nil } + it { expect(subject.server_side_encryption_kms_key_id).to be_nil } + it { expect(subject.fog_attributes).to eq({}) } + end + + context 'with object storage disabled' do + before do + raw_config['enabled'] = false + end + + it { expect(subject.enabled?).to be false } + it { expect(subject.fog_attributes).to eq({}) } + end +end diff --git a/spec/lib/object_storage/direct_upload_spec.rb b/spec/lib/object_storage/direct_upload_spec.rb index 1c1455e2456042910869a0e2500c47619c2e7fc3..7e0f31cbd23ebeef5117e1ab66c0d1c22ab08d1e 100644 --- a/spec/lib/object_storage/direct_upload_spec.rb +++ b/spec/lib/object_storage/direct_upload_spec.rb @@ -18,13 +18,25 @@ } end + let(:storage_options) { {} } + let(:raw_config) do + { + enabled: true, + connection: credentials, + remote_directory: bucket_name, + storage_options: storage_options, + consolidated_settings: consolidated_settings + } + end + + let(:config) { ObjectStorage::Config.new(raw_config) } let(:storage_url) { 'https://uploads.s3.amazonaws.com/' } let(:bucket_name) { 'uploads' } let(:object_name) { 'tmp/uploads/my-file' } let(:maximum_size) { 1.gigabyte } - let(:direct_upload) { described_class.new(credentials, bucket_name, object_name, has_length: has_length, maximum_size: maximum_size, consolidated_settings: consolidated_settings) } + let(:direct_upload) { described_class.new(config, object_name, has_length: has_length, maximum_size: maximum_size) } before do Fog.unmock! @@ -62,7 +74,7 @@ end describe '#get_url' do - subject { described_class.new(credentials, bucket_name, object_name, has_length: true) } + subject { described_class.new(config, object_name, has_length: true) } context 'when AWS is used' do it 'calls the proper method' do @@ -111,6 +123,7 @@ expect(s3_config[:Region]).to eq(region) expect(s3_config[:PathStyle]).to eq(path_style) expect(s3_config[:UseIamProfile]).to eq(use_iam_profile) + expect(s3_config.keys).not_to include(%i(ServerSideEncryption SSEKMSKeyID)) end context 'when feature flag is disabled' do @@ -150,6 +163,33 @@ expect(subject[:UseWorkhorseClient]).to be true end end + + context 'when only server side encryption is used' do + let(:storage_options) { { server_side_encryption: 'AES256' } } + + it 'sends server side encryption settings' do + s3_config = subject[:ObjectStorage][:S3Config] + + expect(s3_config[:ServerSideEncryption]).to eq('AES256') + expect(s3_config.keys).not_to include(:SSEKMSKeyID) + end + end + + context 'when SSE-KMS is used' do + let(:storage_options) do + { + server_side_encryption: 'AES256', + server_side_encryption_kms_key_id: 'arn:aws:12345' + } + end + + it 'sends server side encryption settings' do + s3_config = subject[:ObjectStorage][:S3Config] + + expect(s3_config[:ServerSideEncryption]).to eq('AES256') + expect(s3_config[:SSEKMSKeyID]).to eq('arn:aws:12345') + end + end end shared_examples 'a valid Google upload' do diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index 694aafe5ed5a10eb96a5a185db84c281e459d409..12c936e154bd2647fd06538aee1200dc1614b7fe 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -382,6 +382,32 @@ def when_file_is_in_use it { is_expected.to eq(nil) } end + describe '#fog_attributes' do + subject { uploader.fog_attributes } + + it { is_expected.to eq({}) } + + context 'with encryption configured' do + let(:raw_options) do + { + "enabled" => true, + "connection" => { "provider" => 'AWS' }, + "storage_options" => { "server_side_encryption" => "AES256" } + } + end + + let(:options) { Settingslogic.new(raw_options) } + + before do + allow(uploader_class).to receive(:options) do + double(object_store: options) + end + end + + it { is_expected.to eq({ "x-amz-server-side-encryption" => "AES256" }) } + end + end + describe '.workhorse_authorize' do let(:has_length) { true } let(:maximum_size) { nil } @@ -459,13 +485,18 @@ def when_file_is_in_use context 'uses AWS' do let(:storage_url) { "https://uploads.s3-eu-central-1.amazonaws.com/" } + let(:credentials) do + { + provider: "AWS", + aws_access_key_id: "AWS_ACCESS_KEY_ID", + aws_secret_access_key: "AWS_SECRET_ACCESS_KEY", + region: "eu-central-1" + } + end before do - expect(uploader_class).to receive(:object_store_credentials) do - { provider: "AWS", - aws_access_key_id: "AWS_ACCESS_KEY_ID", - aws_secret_access_key: "AWS_SECRET_ACCESS_KEY", - region: "eu-central-1" } + expect_next_instance_of(ObjectStorage::Config) do |instance| + allow(instance).to receive(:credentials).and_return(credentials) end end @@ -502,12 +533,17 @@ def when_file_is_in_use context 'uses Google' do let(:storage_url) { "https://storage.googleapis.com/uploads/" } + let(:credentials) do + { + provider: "Google", + google_storage_access_key_id: 'ACCESS_KEY_ID', + google_storage_secret_access_key: 'SECRET_ACCESS_KEY' + } + end before do - expect(uploader_class).to receive(:object_store_credentials) do - { provider: "Google", - google_storage_access_key_id: 'ACCESS_KEY_ID', - google_storage_secret_access_key: 'SECRET_ACCESS_KEY' } + expect_next_instance_of(ObjectStorage::Config) do |instance| + allow(instance).to receive(:credentials).and_return(credentials) end end @@ -537,15 +573,18 @@ def when_file_is_in_use context 'uses GDK/minio' do let(:storage_url) { "http://minio:9000/uploads/" } + let(:credentials) do + { provider: "AWS", + aws_access_key_id: "AWS_ACCESS_KEY_ID", + aws_secret_access_key: "AWS_SECRET_ACCESS_KEY", + endpoint: 'http://minio:9000', + path_style: true, + region: "gdk" } + end before do - expect(uploader_class).to receive(:object_store_credentials) do - { provider: "AWS", - aws_access_key_id: "AWS_ACCESS_KEY_ID", - aws_secret_access_key: "AWS_SECRET_ACCESS_KEY", - endpoint: 'http://minio:9000', - path_style: true, - region: "gdk" } + expect_next_instance_of(ObjectStorage::Config) do |instance| + allow(instance).to receive(:credentials).and_return(credentials) end end