From 3c8ad1fc642f42b1125990f04ec49a9ca9849d9d Mon Sep 17 00:00:00 2001 From: matt-domsch-sp Date: Mon, 18 Nov 2024 04:00:33 +0000 Subject: [PATCH 1/7] The CarrierWave::Storage::File#public_url method returns the standard S3 endpoints even when ENV['AWS_USE_FIPS_ENDPOINT']=='true'. When FIPS is called for, and we are in a region where FIPS endpoints are available, this method should return the FIPS endpoint. Furthermore, when S3 Transfer Acceleration (S3TA) is requested by configuration, the above endpoint gets overridden to select the S3TA endpoint. However, S3TA is not avaialble in GovCloud, and has no FIPS endpoint equivalents. In this instance, if the region is a GovCloud region, or if FIPS mode is called for, do not override the endpoint to use S3TA. This is functionally equivalent to an issue submitted to the fog-aws project. https://github.com/fog/fog-aws/issues/729 --- lib/carrierwave/storage/fog.rb | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/carrierwave/storage/fog.rb b/lib/carrierwave/storage/fog.rb index 1d9fe1f12..82a945c39 100644 --- a/lib/carrierwave/storage/fog.rb +++ b/lib/carrierwave/storage/fog.rb @@ -163,6 +163,8 @@ def connection class File DEFAULT_S3_REGION = 'us-east-1'.freeze + AWS_FIPS_REGIONS = %w(us-east-1 us-east-2 us-west-1 us-west-2 us-gov-east-1 us-gov-west-1 ca-central-1 ca-west-1).freeze + AWS_GOVCLOUD_REGIONS = %w(us-gov-east-1 us-gov-west-1).freeze include CarrierWave::Utilities::Uri include CarrierWave::Utilities::FileName @@ -383,15 +385,17 @@ def public_url use_virtual_hosted_style = @uploader.fog_directory.to_s =~ subdomain_regex && !(protocol == 'https' && @uploader.fog_directory =~ /\./) region = @uploader.fog_credentials[:region].to_s - regional_host = case region - when DEFAULT_S3_REGION, '' - 's3.amazonaws.com' - else - "s3.#{region}.amazonaws.com" - end + regional_host = 's3.amazonaws.com' # used for DEFAULT_S3_REGION or no region set + if ENV['AWS_USE_FIPS_ENDPOINT'] == 'true' && AWS_FIPS_REGIONS.include?(region) + regional_host = "s3-fips.#{region}.amazonaws.com" # https://aws.amazon.com/compliance/fips/ + elsif ![DEFAULT_S3_REGION, ''].include?(region) + regional_host = "s3.#{region}.amazonaws.com" + end if use_virtual_hosted_style - regional_host = 's3-accelerate.amazonaws.com' if @uploader.fog_aws_accelerate + # GovCloud doesn't support S3 Transfer Acceleration https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/govcloud-s3.html + # S3 Transfer Acceleration doesn't support FIPS endpoints. When both fog_aws_accelerate=true and AWS_USE_FIPS_ENDPOINT=true, don't use Accelerate. + regional_host = 's3-accelerate.amazonaws.com' if @uploader.fog_aws_accelerate && !AWS_GOVCLOUD_REGIONS.include?(region) && ENV['AWS_USE_FIPS_ENDPOINT'] != 'true' "#{protocol}://#{@uploader.fog_directory}.#{regional_host}/#{encoded_path}" else # directory is not a valid subdomain, so use path style for access "#{protocol}://#{regional_host}/#{@uploader.fog_directory}/#{encoded_path}" From ae2a27791748d06466082cda9d859589123fb213 Mon Sep 17 00:00:00 2001 From: matt-domsch-sp Date: Wed, 20 Nov 2024 03:23:32 +0000 Subject: [PATCH 2/7] Move test for GovCloud or FIPS higher The code can be simplified if we move the test for GovCloud or FIPS outside of the if use_virtual_hosted_style block. This makes it explicit what it's trying to do - which is disable the use of acceleration. We won't emit a warning here. The corresponding code going into the fog-aws gem will emit a warning once that code is merged. We don't need two warnings for the same thing. --- lib/carrierwave/storage/fog.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/carrierwave/storage/fog.rb b/lib/carrierwave/storage/fog.rb index 82a945c39..8d43435ae 100644 --- a/lib/carrierwave/storage/fog.rb +++ b/lib/carrierwave/storage/fog.rb @@ -392,10 +392,14 @@ def public_url regional_host = "s3.#{region}.amazonaws.com" end + # GovCloud doesn't support S3 Transfer Acceleration https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/govcloud-s3.html + # S3 Transfer Acceleration doesn't support FIPS endpoints. When both fog_aws_accelerate=true and AWS_USE_FIPS_ENDPOINT=true, don't use Accelerate. + if @uploader.fog_aws_accelerate && (AWS_GOVCLOUD_REGIONS.include?(region) || ENV['AWS_USE_FIPS_ENDPOINT'] == 'true') + @uploader.fog_aws_accelerate = false + end + if use_virtual_hosted_style - # GovCloud doesn't support S3 Transfer Acceleration https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/govcloud-s3.html - # S3 Transfer Acceleration doesn't support FIPS endpoints. When both fog_aws_accelerate=true and AWS_USE_FIPS_ENDPOINT=true, don't use Accelerate. - regional_host = 's3-accelerate.amazonaws.com' if @uploader.fog_aws_accelerate && !AWS_GOVCLOUD_REGIONS.include?(region) && ENV['AWS_USE_FIPS_ENDPOINT'] != 'true' + regional_host = 's3-accelerate.amazonaws.com' if @uploader.fog_aws_accelerate "#{protocol}://#{@uploader.fog_directory}.#{regional_host}/#{encoded_path}" else # directory is not a valid subdomain, so use path style for access "#{protocol}://#{regional_host}/#{@uploader.fog_directory}/#{encoded_path}" From 738a5ce2b400f79e66d246de27a2b2ff42aedc13 Mon Sep 17 00:00:00 2001 From: matt-domsch-sp Date: Wed, 20 Nov 2024 03:30:52 +0000 Subject: [PATCH 3/7] Add warning for S3TA disabling The fog-aws gem won't emit a warning if we've disabled S3TA ourselves. It would only emit the warning if _it_ disabled S3TA using the same test. Therefore, we want the same warning emitted in Carrierwave as is in fog-aws after all. --- lib/carrierwave/storage/fog.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/carrierwave/storage/fog.rb b/lib/carrierwave/storage/fog.rb index 8d43435ae..a714a046c 100644 --- a/lib/carrierwave/storage/fog.rb +++ b/lib/carrierwave/storage/fog.rb @@ -395,6 +395,7 @@ def public_url # GovCloud doesn't support S3 Transfer Acceleration https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/govcloud-s3.html # S3 Transfer Acceleration doesn't support FIPS endpoints. When both fog_aws_accelerate=true and AWS_USE_FIPS_ENDPOINT=true, don't use Accelerate. if @uploader.fog_aws_accelerate && (AWS_GOVCLOUD_REGIONS.include?(region) || ENV['AWS_USE_FIPS_ENDPOINT'] == 'true') + warn "S3 Transfer Acceleration is not available in GovCloud regions or when AWS_USE_FIPS_ENDPOINT=true. Disabling accelearation." @uploader.fog_aws_accelerate = false end From 504bf0e6ab8b72bdca1c9dcf1dd648b88352e540 Mon Sep 17 00:00:00 2001 From: matt-domsch-sp Date: Thu, 21 Nov 2024 23:47:03 +0000 Subject: [PATCH 4/7] fix typo in warning message --- lib/carrierwave/storage/fog.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/carrierwave/storage/fog.rb b/lib/carrierwave/storage/fog.rb index a714a046c..3b9f3f520 100644 --- a/lib/carrierwave/storage/fog.rb +++ b/lib/carrierwave/storage/fog.rb @@ -395,7 +395,7 @@ def public_url # GovCloud doesn't support S3 Transfer Acceleration https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/govcloud-s3.html # S3 Transfer Acceleration doesn't support FIPS endpoints. When both fog_aws_accelerate=true and AWS_USE_FIPS_ENDPOINT=true, don't use Accelerate. if @uploader.fog_aws_accelerate && (AWS_GOVCLOUD_REGIONS.include?(region) || ENV['AWS_USE_FIPS_ENDPOINT'] == 'true') - warn "S3 Transfer Acceleration is not available in GovCloud regions or when AWS_USE_FIPS_ENDPOINT=true. Disabling accelearation." + warn "S3 Transfer Acceleration is not available in GovCloud regions or when AWS_USE_FIPS_ENDPOINT=true. Disabling acceleration." @uploader.fog_aws_accelerate = false end From c35e37b6ae77244cc8b393ae0a41664003a8c501 Mon Sep 17 00:00:00 2001 From: matt-domsch-sp Date: Sat, 23 Nov 2024 15:25:23 +0000 Subject: [PATCH 5/7] remove disabling S3TA --- lib/carrierwave/storage/fog.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/carrierwave/storage/fog.rb b/lib/carrierwave/storage/fog.rb index 3b9f3f520..8a1d290cd 100644 --- a/lib/carrierwave/storage/fog.rb +++ b/lib/carrierwave/storage/fog.rb @@ -164,7 +164,6 @@ def connection class File DEFAULT_S3_REGION = 'us-east-1'.freeze AWS_FIPS_REGIONS = %w(us-east-1 us-east-2 us-west-1 us-west-2 us-gov-east-1 us-gov-west-1 ca-central-1 ca-west-1).freeze - AWS_GOVCLOUD_REGIONS = %w(us-gov-east-1 us-gov-west-1).freeze include CarrierWave::Utilities::Uri include CarrierWave::Utilities::FileName @@ -392,13 +391,6 @@ def public_url regional_host = "s3.#{region}.amazonaws.com" end - # GovCloud doesn't support S3 Transfer Acceleration https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/govcloud-s3.html - # S3 Transfer Acceleration doesn't support FIPS endpoints. When both fog_aws_accelerate=true and AWS_USE_FIPS_ENDPOINT=true, don't use Accelerate. - if @uploader.fog_aws_accelerate && (AWS_GOVCLOUD_REGIONS.include?(region) || ENV['AWS_USE_FIPS_ENDPOINT'] == 'true') - warn "S3 Transfer Acceleration is not available in GovCloud regions or when AWS_USE_FIPS_ENDPOINT=true. Disabling acceleration." - @uploader.fog_aws_accelerate = false - end - if use_virtual_hosted_style regional_host = 's3-accelerate.amazonaws.com' if @uploader.fog_aws_accelerate "#{protocol}://#{@uploader.fog_directory}.#{regional_host}/#{encoded_path}" From ccd66036a00ca11687c6d668a707d16ddfc2786e Mon Sep 17 00:00:00 2001 From: matt-domsch-sp Date: Mon, 25 Nov 2024 17:34:16 +0000 Subject: [PATCH 6/7] Add fog_aws_fips config option, tests This adds the [:fog_aws_fips] option, default false. When true, it causes the default endpoint hostname to be changed from `s3` to `s3-fips`. --- lib/carrierwave/storage/fog.rb | 7 +++-- lib/carrierwave/uploader/configuration.rb | 2 ++ spec/storage/fog_helper.rb | 38 ++++++++++++++++++++++- 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/lib/carrierwave/storage/fog.rb b/lib/carrierwave/storage/fog.rb index 8a1d290cd..94a1bd8a1 100644 --- a/lib/carrierwave/storage/fog.rb +++ b/lib/carrierwave/storage/fog.rb @@ -18,6 +18,8 @@ module Storage # [:fog_use_ssl_for_aws] (optional) #public_url will use https for the AWS generated URL] # [:fog_aws_accelerate] (optional) #public_url will use s3-accelerate subdomain # instead of s3, defaults to false + # [:fog_aws_fips] (optional) #public_url will use s3-fips subdomain + # instead of s3, defaults to false # # # AWS credentials contain the following keys: @@ -163,7 +165,6 @@ def connection class File DEFAULT_S3_REGION = 'us-east-1'.freeze - AWS_FIPS_REGIONS = %w(us-east-1 us-east-2 us-west-1 us-west-2 us-gov-east-1 us-gov-west-1 ca-central-1 ca-west-1).freeze include CarrierWave::Utilities::Uri include CarrierWave::Utilities::FileName @@ -383,9 +384,11 @@ def public_url # To use the virtual-hosted style, the bucket name needs to be representable as a subdomain use_virtual_hosted_style = @uploader.fog_directory.to_s =~ subdomain_regex && !(protocol == 'https' && @uploader.fog_directory =~ /\./) + return nil if !use_virtual_hosted_style && @uploader.fog_aws_fips # FIPS Endpoints can only be used with Virtual Hosted-Style addressing. + region = @uploader.fog_credentials[:region].to_s regional_host = 's3.amazonaws.com' # used for DEFAULT_S3_REGION or no region set - if ENV['AWS_USE_FIPS_ENDPOINT'] == 'true' && AWS_FIPS_REGIONS.include?(region) + if @uploader.fog_aws_fips regional_host = "s3-fips.#{region}.amazonaws.com" # https://aws.amazon.com/compliance/fips/ elsif ![DEFAULT_S3_REGION, ''].include?(region) regional_host = "s3.#{region}.amazonaws.com" diff --git a/lib/carrierwave/uploader/configuration.rb b/lib/carrierwave/uploader/configuration.rb index dce36306e..216c5cf15 100644 --- a/lib/carrierwave/uploader/configuration.rb +++ b/lib/carrierwave/uploader/configuration.rb @@ -35,6 +35,7 @@ module Configuration add_config :fog_authenticated_url_expiration add_config :fog_use_ssl_for_aws add_config :fog_aws_accelerate + add_config :fog_aws_fips # Mounting add_config :ignore_integrity_errors @@ -197,6 +198,7 @@ def reset_config config.fog_authenticated_url_expiration = 600 config.fog_use_ssl_for_aws = true config.fog_aws_accelerate = false + config.fog_aws_fips = false config.store_dir = 'uploads' config.cache_dir = 'uploads/tmp' config.delete_tmp_file_after_storage = true diff --git a/spec/storage/fog_helper.rb b/spec/storage/fog_helper.rb index 48cf0b20d..f41a79c3a 100644 --- a/spec/storage/fog_helper.rb +++ b/spec/storage/fog_helper.rb @@ -526,7 +526,8 @@ def check_file nil => 's3.amazonaws.com', 'us-east-1' => 's3.amazonaws.com', 'us-east-2' => 's3.us-east-2.amazonaws.com', - 'eu-central-1' => 's3.eu-central-1.amazonaws.com' + 'eu-central-1' => 's3.eu-central-1.amazonaws.com', + 'us-gov-west-1' => 's3.us-gov-west-1.amazonaws.com' }.each do |region, expected_host| it "should use a #{expected_host} hostname when using path style for access #{region} region" do allow(@uploader).to receive(:fog_use_ssl_for_aws).and_return(true) @@ -539,6 +540,23 @@ def check_file end end + context 'when the directory is not a valid subdomain and :fog_aws_fips' do + [ + 'us-east-1', + 'us-east-2', + 'us-gov-west-1' + ].each do |region| + it "public_url should be nil" do + allow(@uploader).to receive(:fog_use_ssl_for_aws).and_return(true) + allow(@uploader).to receive(:fog_directory).and_return('foo.bar') + allow(@uploader).to receive(:fog_aws_fips).and_return(true) + allow(@uploader).to receive(:fog_credentials).and_return(@uploader.fog_credentials.merge(region: region)) + + expect(@fog_file.public_url).to be_nil + end + end + end + context 'when the directory is a valid subdomain' do { nil => 'foobar.s3.amazonaws.com', @@ -557,6 +575,24 @@ def check_file end end + context 'when the directory is a valid subdomain and :fog_aws_fips' do + { + 'us-east-1' => 'foobar.s3-fips.us-east-1.amazonaws.com', + 'us-east-2' => 'foobar.s3-fips.us-east-2.amazonaws.com', + 'eu-central-1' => 'foobar.s3-fips.eu-central-1.amazonaws.com', # Carrierwave shouldn't know which regions are FIPS-capable + 'us-gov-west-1' => 'foobar.s3-fips.us-gov-west-1.amazonaws.com' + }.each do |region, expected_host| + it "should use a #{expected_host} hostname when using path style for access #{region} region" do + allow(@uploader).to receive(:fog_use_ssl_for_aws).and_return(true) + allow(@uploader).to receive(:fog_directory).and_return('foobar') + allow(@uploader).to receive(:fog_aws_fips).and_return(true) + allow(@uploader).to receive(:fog_credentials).and_return(@uploader.fog_credentials.merge(region: region)) + + expect(@fog_file.public_url).to include("https://#{expected_host}/") + end + end + end + it "should use https as a default protocol" do expect(@uploader.fog_use_ssl_for_aws).to be true expect(@fog_file.public_url).to start_with 'https://' From 0d5a66192a039cb3542bda2a383c000e570d5ffa Mon Sep 17 00:00:00 2001 From: matt-domsch-sp Date: Thu, 28 Nov 2024 04:09:00 +0000 Subject: [PATCH 7/7] Return nil if both :endpoint given and :fog_aws_fips=true public_url returns a path-based URL when :endpoint is given. AWS FIPS endpoints only work with virtual host-style URLs per https://aws.amazon.com/compliance/fips/ Add a warning and return nil if both :endpoint is given and :fog_aws_fips=true. Add tests for :endpoint for both :fog_aws_fips=false (default) and true. --- lib/carrierwave/storage/fog.rb | 7 ++++++- spec/storage/fog_helper.rb | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/carrierwave/storage/fog.rb b/lib/carrierwave/storage/fog.rb index 94a1bd8a1..7e97c0bb3 100644 --- a/lib/carrierwave/storage/fog.rb +++ b/lib/carrierwave/storage/fog.rb @@ -376,7 +376,12 @@ def public_url when 'AWS' # check if some endpoint is set in fog_credentials if @uploader.fog_credentials.has_key?(:endpoint) - "#{@uploader.fog_credentials[:endpoint]}/#{@uploader.fog_directory}/#{encoded_path}" + if !@uploader.fog_aws_fips + "#{@uploader.fog_credentials[:endpoint]}/#{@uploader.fog_directory}/#{encoded_path}" + else + warn 'Use of options :endpoint and :fog_aws_fips=true together will fail, as FIPS endpoints do not support path-style URLs.' + nil + end else protocol = @uploader.fog_use_ssl_for_aws ? "https" : "http" diff --git a/spec/storage/fog_helper.rb b/spec/storage/fog_helper.rb index f41a79c3a..43c554439 100644 --- a/spec/storage/fog_helper.rb +++ b/spec/storage/fog_helper.rb @@ -362,9 +362,11 @@ def check_file describe "CarrierWave::Storage::Fog::File" do let(:store_path) { 'uploads/test.jpg' } let(:fog_public) { true } + let(:endpoint) { nil } before do allow(@uploader).to receive(:store_path).and_return(store_path) allow(@uploader).to receive(:fog_public).and_return(fog_public) + allow(@uploader).to receive(:endpoint).and_return(endpoint) @fog_file = @storage.store!(CarrierWave::SanitizedFile.new(stub_file('test.jpg', 'image/jpeg'))) end @@ -504,6 +506,20 @@ def check_file expect(@fog_file.public_url).to include("https://#{CARRIERWAVE_DIRECTORY}.s3-accelerate.amazonaws.com") end + it 'returns nil when both :endpoint and :fog_aws_fips=true' do + allow(@uploader).to receive(:fog_credentials).and_return(@uploader.fog_credentials.merge(endpoint: 'https://custom-endpoint.example.com')) + allow(@uploader).to receive(:fog_directory).and_return('SiteAssets') + allow(@uploader).to receive(:fog_aws_fips).and_return(true) + expect(@fog_file.url).to be nil + end + + it 'returns endpoint+bucket when :endpoint and !:fog_aws_fips' do + allow(@uploader).to receive(:fog_credentials).and_return(@uploader.fog_credentials.merge(endpoint: 'https://custom-endpoint.example.com')) + allow(@uploader).to receive(:fog_directory).and_return('SiteAssets') + allow(@uploader).to receive(:fog_aws_fips).and_return(false) + expect(@fog_file.url).to include('https://custom-endpoint.example.com/SiteAssets') + end + context 'when the directory is not a valid subdomain' do it "should not use a subdomain URL for AWS" do allow(@uploader).to receive(:fog_directory).and_return('SiteAssets')