SigV4 as Default for All Amazon S3 Calls

aws-sdk-core/features/s3/step_definitions.rb

@@ -14,7 +14,9 @@
       break if objects.empty?
       @client.delete_objects(bucket: bucket, delete: { objects: objects })
     end
+    puts "MADE IT TO DELETING BUCKET"
     @client.delete_bucket(bucket: bucket)
+    puts "DELETED BUCKET"
   end
 end
 

aws-sdk-core/lib/aws-sdk-core/plugins/s3_request_signer.rb

@@ -6,28 +6,15 @@ module Plugins
     # @api private
     class S3RequestSigner < Seahorse::Client::Plugin
 
-      class SigningHandler < RequestSigner::Handler
+      option :signature_version, 'v4'
 
-        # List of regions that support older S3 signature versions.
-        # All new regions only support signature version 4.
-        V2_REGIONS = Set.new(%w(
-          us-east-1
-          us-west-1
-          us-west-2
-          ap-northeast-1
-          ap-southeast-1
-          ap-southeast-2
-          sa-east-1
-          eu-west-1
-          us-gov-west-1
-        ))
+      class SigningHandler < RequestSigner::Handler
 
         def call(context)
           require_credentials(context)
-          version = signature_version(context)
-          case version
-          when /v4/ then apply_v4_signature(context)
-          when /s3/ then apply_v2_signature(context)
+          case context.config.signature_version
+          when 'v4' then apply_v4_signature(context)
+          when 's3' then apply_s3_legacy_signature(context)
           else raise "unsupported signature version #{version.inspect}"
           end
           @handler.call(context)
@@ -42,59 +29,10 @@ def apply_v4_signature(context)
           ).sign(context.http_request)
         end
 
-        def apply_v2_signature(context)
+        def apply_s3_legacy_signature(context)
           Signers::S3.sign(context)
         end
 
-        def signature_version(context)
-          context[:cached_signature_version] ||
-          context.config.signature_version ||
-          version_by_region(context)
-        end
-
-        def version_by_region(context)
-          if classic_endpoint?(context)
-            classic_sigv(context)
-          else
-            regional_sigv(context)
-          end
-        end
-
-        def classic_endpoint?(context)
-          context.config.region == 'us-east-1'
-        end
-
-        # When accessing the classic endpoint, s3.amazonaws.com, we don't know
-        # the region name. This makes creating a version 4 signature difficult.
-        # Choose v4 only if using KMS encryptions, which requires v4.
-        def classic_sigv(context)
-          if kms_encrypted?(context)
-            :v4
-          else
-            :s3
-          end
-        end
-
-        def regional_sigv(context)
-          # Drop back to older S3 signature version when uploading objects for
-          # better performance. This optimization may be removed at some point
-          # in favor of always using signature version 4.
-          if V2_REGIONS.include?(context.config.region)
-            uploading_file?(context) && !kms_encrypted?(context) ? :s3 : :v4
-          else
-            :v4
-          end
-        end
-
-        def kms_encrypted?(context)
-          context.params[:server_side_encryption] == 'aws:kms'
-        end
-
-        def uploading_file?(context)
-          [:put_object, :upload_part].include?(context.operation_name) &&
-            context.http_request.body.size > 0
-        end
-
       end
 
       # Abstract base class for the other two handlers
@@ -143,8 +81,6 @@ def use_regional_endpoint_when_known(context, bucket)
       # request against the regional endpoint.
       class BucketSigningErrorHandler < Handler
 
-        SIGV4_MSG = /(Please use AWS4-HMAC-SHA256|AWS Signature Version 4)/
-
         def call(context)
           response = @handler.call(context)
           handle_region_errors(response)
@@ -153,52 +89,34 @@ def call(context)
         private
 
         def handle_region_errors(response)
-          if sigv4_required_error?(response)
-            detect_region_and_retry(response)
-          elsif wrong_sigv4_region?(response)
-            extract_body_region_and_retry(response.context)
+          if wrong_sigv4_region?(response)
+            get_region_and_retry(response.context)
           else
             response
           end
         end
 
-        def sigv4_required_error?(resp)
-          resp.context.http_response.status_code == 400 &&
-          resp.context.http_response.body_contents.match(SIGV4_MSG) &&
-          resp.context.http_response.body.respond_to?(:truncate)
-        end
-
-        def wrong_sigv4_region?(resp)
-          resp.context.http_response.status_code == 400 &&
-          resp.context.http_response.body_contents.match(/<Region>.+?<\/Region>/)
-        end
-
-        def extract_body_region_and_retry(context)
+        def get_region_and_retry(context)
           actual_region = region_from_body(context)
-          updgrade_to_v4(context, actual_region)
+          if actual_region.nil? || actual_region == ""
+            raise "Couldn't get region from body: #{context.body}"
+          end
+          update_bucket_cache(context, actual_region)
           log_warning(context, actual_region)
+          update_region_header(context, actual_region)
           @handler.call(context)
         end
 
-        def region_from_body(context)
-          context.http_response.body_contents.match(/<Region>(.+?)<\/Region>/)[1]
+        def update_bucket_cache(context, actual_region)
+          S3::BUCKET_REGIONS[context.params[:bucket]] = actual_region
         end
 
-        def detect_region_and_retry(resp)
-          context = resp.context
-          updgrade_to_v4(context, 'us-east-1')
-          resp = @handler.call(context)
-          if resp.successful?
-            resp
-          else
-            actual_region = region_from_location_header(context)
-            updgrade_to_v4(context, actual_region)
-            log_warning(context, actual_region)
-            @handler.call(context)
-          end
+        def wrong_sigv4_region?(resp)
+          resp.context.http_response.status_code == 400 &&
+            resp.context.http_response.body_contents.match(/<Region>.+?<\/Region>/)
         end
 
-        def updgrade_to_v4(context, region)
+        def update_region_header(context, region)
           context.http_response.body.truncate(0)
           context.http_request.headers.delete('authorization')
           context.http_request.headers.delete('x-amz-security-token')
@@ -207,13 +125,11 @@ def updgrade_to_v4(context, region)
           signer.sign(context.http_request)
         end
 
-        def region_from_location_header(context)
-          location = context.http_response.headers['location']
-          location.match(/s3[.-](.+?)\.amazonaws\.com/)[1]
+        def region_from_body(context)
+          context.http_response.body_contents.match(/<Region>(.+?)<\/Region>/)[1]
         end
 
         def log_warning(context, actual_region)
-          S3::BUCKET_REGIONS[context.params[:bucket]] = actual_region
           msg = "S3 client configured for #{context.config.region.inspect} " +
             "but the bucket #{context.params[:bucket].inspect} is in " +
             "#{actual_region.inspect}; Please configure the proper region " +

aws-sdk-core/spec/aws/s3/client/region_detection_spec.rb

@@ -17,72 +17,6 @@ module S3
         S3::BUCKET_REGIONS.clear
       end
 
-      describe 'accessing eu-central-1 via classic endpoint and sigv2' do
-
-        before(:each) do
-
-          stub_request(:put, 'https://bucket.s3.amazonaws.com/key').
-            to_return(status: [400, 'Bad Request'], body: "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<Error><Code>InvalidRequest</Code><Message>The authorization mechanism you have provided is not supported. Please use AWS4-HMAC-SHA256.</Message><RequestId>164473A33C2BACDF</RequestId><HostId>I8hhmfGvN74WPyu6kduBoVhnHPgKATFF4nWlro8kRdpuFxXVgcRgLZ2oTxJFhZMgAn75GqyhUHY=</HostId></Error>")
-
-          stub_request(:put, 'https://bucket.s3-external-1.amazonaws.com/key').
-            to_return(status: [307, 'Temporary Redirect'], headers: {
-              'Location' => 'https://bucket.s3.eu-central-1.amazonaws.com/key'
-            })
-
-          stub_request(:put, 'https://bucket.s3.eu-central-1.amazonaws.com/key').
-            to_return(status: [200, 'Ok'])
-
-        end
-
-        it 'detects the sigv4 error, determines actual region and redirects' do
-          resp = client.put_object(bucket:'bucket', key:'key', body:'body')
-          host = resp.context.http_request.endpoint.host
-          expect(host).to eq('bucket.s3.eu-central-1.amazonaws.com')
-        end
-
-        it 'uses cached regions to determine the actual region' do
-          client.put_object(bucket:'bucket', key:'key', body:'body')
-          client.put_object(bucket:'bucket', key:'key', body:'body')
-          client.put_object(bucket:'bucket', key:'key', body:'body')
-        end
-
-        it 'sends a warning to stderr' do
-          expect($stderr).to receive(:write).with(<<-WARNING.strip + "\n")
-S3 client configured for "us-east-1" but the bucket "bucket" is in "eu-central-1"; Please configure the proper region to avoid multiple unecessary redirects and signing attempts
-          WARNING
-          client.put_object(bucket:'bucket', key:'key', body:'body')
-        end
-
-        it 'sends the warning to the logger if configured' do
-          logger = double('logger').as_null_object
-          expect(logger).to receive(:warn).with(<<-WARNING.strip + "\n")
-S3 client configured for "us-east-1" but the bucket "bucket" is in "eu-central-1"; Please configure the proper region to avoid multiple unecessary redirects and signing attempts
-          WARNING
-          client = S3::Client.new(client_opts.merge(logger: logger))
-          client.put_object(bucket:'bucket', key:'key', body:'body')
-        end
-
-        it 'triggers a callback where you can listen for new bucket regions' do
-          yielded = nil
-          S3::BUCKET_REGIONS.bucket_added {|*args| yielded = args }
-          client.put_object(bucket:'bucket', key:'key', body:'body')
-          expect(yielded).to eq(['bucket', 'eu-central-1'])
-        end
-
-        it 'triggers a callback where you can listen for new bucket regions' do
-          yielded = nil
-          S3::BUCKET_REGIONS.bucket_added {|*args| yielded = args }
-          client.put_object(bucket:'bucket', key:'key', body:'body')
-          expect(yielded).to eq(['bucket', 'eu-central-1'])
-          expect(S3::BUCKET_REGIONS.to_h).to eq('bucket' => 'eu-central-1')
-
-          expect {
-            S3::BUCKET_REGIONS.bucket_added
-          }.to raise_error(ArgumentError, 'missing required block')
-        end
-
-      end
-
       describe 'accessing a bucket in eu-central-1 with wrong region' do
 
         before(:each) do
@@ -102,7 +36,7 @@ module S3
 
       end
 
-      describe 'accessing eu-central-1 bucket using classic endpoitn and wrong sigv4 region' do
+      describe 'accessing eu-central-1 bucket using classic endpoint and wrong sigv4 region' do
 
         before(:each) do
 
@@ -116,7 +50,7 @@ module S3
 
         it 'detects the moved permanently and redirects' do
           client = S3::Client.new(client_opts.merge(
-            region: 'us-west-2', signature_version:'v4')
+            region: 'us-west-2')
           )
           resp = client.put_object(bucket:'bucket', key:'key', body:'body')
           host = resp.context.http_request.endpoint.host

aws-sdk-core/spec/aws/s3/client_spec.rb

@@ -102,37 +102,6 @@ module S3
           "sa-east-1",
           "eu-west-1",
           "us-gov-west-1",
-        ].each do |region|
-
-          it "defaults signature version 4 for #{region}" do
-            client = Client.new(stub_responses: true, region: region)
-            resp = client.head_object(bucket:'name', key:'key')
-            expect(resp.context.http_request.headers['authorization']).to match(
-              'AWS4-HMAC-SHA256')
-          end
-
-          it "falls back on classic S3 signing for #put_object in #{region}" do
-            client = Client.new(stub_responses: true, region: region)
-            resp = client.put_object(bucket:'name', key:'key', body:'data')
-            expect(resp.context.http_request.headers['authorization']).to match(
-              'AWS akid:')
-          end
-
-          it "forces v4 signing when aws:kms used for server side encryption" do
-            client = Client.new(stub_responses: true, region: region)
-            resp = client.put_object(
-              bucket: 'name',
-              key: 'key',
-              server_side_encryption: 'aws:kms',
-              body: 'data'
-            )
-            expect(resp.context.http_request.headers['authorization']).to match(
-              'AWS4-HMAC-SHA256')
-          end
-
-        end
-
-        [
           "cn-north-1",
           "eu-central-1",
           "unknown-region",
@@ -165,14 +134,7 @@ module S3
           end
         end
 
-        it "defaults classic s3 signature us-east-1" do
-          client = Client.new(stub_responses: true, region: 'us-east-1')
-          resp = client.head_object(bucket:'name', key:'key')
-          expect(resp.context.http_request.headers['authorization']).to match(
-            'AWS akid:')
-        end
-
-        it "upgrades to signature version 4 when aws:kms used for sse" do
+        it "uses signature version 4 when aws:kms used for sse" do
           client = Client.new(stub_responses: true, region: 'us-east-1')
           resp = client.put_object(
             bucket: 'name',