Raise error for S3 region redirects, check that HTTP response is OK.

lib/citadel/error.rb

@@ -24,10 +24,20 @@
 
 
 class Citadel
-  # Base class for Citadell errors.
+  # Base class for Citadel errors.
   #
   # @since 1.0.0
   # @api private
-  class CitadelError < Exception
+  class CitadelError < StandardError
+
+    # If a CitadelError is raised from a rescue block, the wrapped_exception will
+    # by default be the original exception (pulled from $!).
+    #
+    attr_reader :wrapped_exception
+
+    def initialize(message=nil, wrapped_exception: $!)
+      super(message)
+      @wrapped_exception = wrapped_exception
+    end
   end
 end

lib/citadel/s3.rb

@@ -51,34 +51,119 @@ module S3
     def get(bucket:, path:, access_key_id:, secret_access_key:, token: nil, region: nil)
       region ||= 'us-east-1' # Most buckets.
       path = path[1..-1] if path[0] == '/'
-      now = Time.now().utc.strftime('%a, %d %b %Y %H:%M:%S GMT')
 
-      string_to_sign = "GET\n\n\n#{now}\n"
-      string_to_sign << "x-amz-security-token:#{token}\n" if token
-      string_to_sign << "/#{bucket}/#{path}"
+      verb = 'GET'
+      uri_path = "/#{bucket}/#{path}"
+      body = ''
 
-      signed = OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), secret_access_key, string_to_sign)
-      signed_base64 = Base64.encode64(signed)
+      datetime = Time.now.utc.strftime('%Y%m%dT%H%M%SZ')
+      date = datetime[0,8]
+
+      c_scope = _credential_scope(date, region)
+      credential = "#{access_key_id}/#{c_scope}"
+
+      algorithm = 'AWS4-HMAC-SHA256'
+
+      if region == 'us-east-1'
+        hostname = 's3.amazonaws.com'
+      else
+        hostname = "s3-#{region}.amazonaws.com"
+      end
 
       headers = {
-        'date' => now,
-        'authorization' => "AWS #{access_key_id}:#{signed_base64}",
+        'host' => hostname,
+        'x-amz-content-sha256' => hexdigest(body),
+        'x-amz-date' => datetime,
+        'x-amz-expires' => '900', # 15 minutes
       }
       headers['x-amz-security-token'] = token if token
 
-      hostname = case region
-      when 'us-east-1'
-        's3.amazonaws.com'
-      else
-        "s3-#{region}.amazonaws.com"
-      end
+      canonical_request = _canonical_request(verb: verb, path: uri_path,
+                                             querystring: '', headers: headers,
+                                             content_hash: hexdigest(body))
+      signed_headers = headers.keys.sort.join(';')
 
+      to_sign = _string_to_sign(datetime, c_scope, canonical_request)
+      signed = _signature(secret_access_key, date, region, 's3', to_sign)
+
+      headers['authorization'] = "#{algorithm} Credential=#{credential}, SignedHeaders=#{signed_headers}, Signature=#{signed}"
+
+      # Debug information useful if the signature is wrong
+      Chef::Log.debug { "CanonicalRequest: " + canonical_request.inspect }
+      Chef::Log.debug { "StringToSign: " + to_sign.inspect }
+      Chef::Log.debug { "headers: " + headers.inspect }
+
+      client = Chef::HTTP.new("https://#{hostname}")
       begin
-        Chef::HTTP.new("https://#{hostname}").get("#{bucket}/#{path}", headers)
+        content = client.get(uri_path[1..-1], headers)
       rescue Net::HTTPServerException => e
         raise CitadelError.new("Unable to download #{path}: #{e}")
       end
+
+      response = client.last_response
+
+      case response
+      when Net::HTTPOK
+        return content
+      when Net::HTTPRedirection
+        # When you request a bucket at the wrong region endpoint, S3 returns an
+        # HTTP 301, but it doesn't set a Location header, so chef doesn't
+        # follow it and returns a nil response.
+        true_region = response.header['x-amz-bucket-region']
+        raise CitadelError.new(
+          "Bucket #{bucket} is actually in #{true_region}, not #{region}")
+      else
+        Chef::Log.warn("Unexpected HTTP response: #{response.inspect}")
+        Chef::Log.warn("Response body: #{response.body.inspect}")
+        raise CitadelError.new("Unexpected HTTP response: #{response.inspect}")
+      end
     end
 
+    def _canonical_request(verb:, path:, querystring:, headers:, content_hash:)
+      # This isn't a super robust way to calculate the canonical request, since
+      # we don't really deal properly with URIs that need escaping or quoted
+      # header values.
+      [
+        verb,
+        path,
+        querystring,
+        headers.sort_by(&:first).map {|k, v| "#{k}:#{v}" }.join("\n") + "\n", # sign all headers
+        headers.keys.sort.join(';'),
+        content_hash,
+      ].join("\n")
+    end
+
+    def _string_to_sign(datetime_string, credential_scope, canonical_request)
+      [
+        'AWS4-HMAC-SHA256',
+        datetime_string,
+        credential_scope,
+        hexdigest(canonical_request),
+      ].join("\n")
+    end
+
+    def _credential_scope(date_string, region, service='s3')
+      [date_string, region, 's3', 'aws4_request'].join('/')
+    end
+
+    def _signature(secret_access_key, date, region, service, string_to_sign)
+      k_date = hmac('AWS4' + secret_access_key, date)
+      k_region = hmac(k_date, region)
+      k_service = hmac(k_region, service)
+      k_credentials = hmac(k_service, 'aws4_request')
+      hmac_hex(k_credentials, string_to_sign)
+    end
+
+    def hexdigest(data)
+      OpenSSL::Digest::SHA256.hexdigest(data)
+    end
+
+    def hmac(key, value)
+      OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha256'), key, value)
+    end
+
+    def hmac_hex(key, value)
+      OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha256'), key, value)
+    end
   end
 end

test/spec/s3_spec.rb

@@ -20,6 +20,7 @@
   let(:fake_http) { double('Chef::HTTP') }
   let(:fake_response) { double('Net::HTTPResponse') }
   let(:s3_hostname) { 's3.amazonaws.com' }
+  let(:empty_sha256) { 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' }
   before do
     # Stub out the HTTP object.
     expect(Chef::HTTP).to receive(:new).with("https://#{s3_hostname}").and_return(fake_http)
@@ -30,7 +31,14 @@
   context 'with mybucket/mysecret' do
     subject { described_class.get(bucket: 'mybucket', path: 'mysecret', access_key_id: 'AKIAJMKSMHNNCQX4ILAH', secret_access_key: '0ljyHQrk1AGsc2bgx/8fbNghZNYSdckHADR4vNcL') }
     before do
-      expect(fake_http).to receive(:get).with('mybucket/mysecret', 'date' => 'Thu, 01 Jan 1970 00:00:00 GMT', 'authorization' => "AWS AKIAJMKSMHNNCQX4ILAH:TQPmJfb3Mx2MblMnRHJS1EG6jus=\n").and_return(fake_response)
+      expect(fake_http).to receive(:get).with('mybucket/mysecret',
+        'host' => 's3.amazonaws.com',
+        'x-amz-content-sha256' => empty_sha256,
+        'x-amz-date' => '19700101T000000Z',
+        'x-amz-expires' => '900',
+        'authorization' => 'AWS4-HMAC-SHA256 Credential=AKIAJMKSMHNNCQX4ILAH/19700101/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date;x-amz-expires, Signature=fcfb9ddef446db9d31c9f08a4beb05116930b9861947cb49e0c8e8f46f176938'
+      ).and_return(fake_response)
+      expect(fake_http).to receive(:last_response).and_return(Net::HTTPOK.new(nil, nil, nil))
     end
 
     it { is_expected.to be fake_response }
@@ -39,7 +47,15 @@
   context 'with token' do
     subject { described_class.get(bucket: 'mybucket', path: 'mysecret', access_key_id: 'AKIAJMKSMHNNCQX4ILAH', secret_access_key: '0ljyHQrk1AGsc2bgx/8fbNghZNYSdckHADR4vNcL', token: 'EIZvol3NYAGhIYo3mxmF8Bw3GjRFQq6xmjrlXNQs') }
     before do
-      expect(fake_http).to receive(:get).with('mybucket/mysecret', 'date' => 'Thu, 01 Jan 1970 00:00:00 GMT', 'authorization' => "AWS AKIAJMKSMHNNCQX4ILAH:ZapoW/urO8FlRSEf+y5iWYeNsrs=\n", 'x-amz-security-token' => 'EIZvol3NYAGhIYo3mxmF8Bw3GjRFQq6xmjrlXNQs').and_return(fake_response)
+      expect(fake_http).to receive(:get).with('mybucket/mysecret',
+        'host' => 's3.amazonaws.com',
+        'x-amz-content-sha256' => empty_sha256,
+        'x-amz-date' => '19700101T000000Z',
+        'x-amz-expires' => '900',
+        'x-amz-security-token' => 'EIZvol3NYAGhIYo3mxmF8Bw3GjRFQq6xmjrlXNQs',
+        'authorization' => 'AWS4-HMAC-SHA256 Credential=AKIAJMKSMHNNCQX4ILAH/19700101/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date;x-amz-expires;x-amz-security-token, Signature=e5ea717df9b8c93e15b30c86900c016a0d81b46c4b007644ffc80795ff8da15f'
+      ).and_return(fake_response)
+      expect(fake_http).to receive(:last_response).and_return(Net::HTTPOK.new(nil, nil, nil))
     end
 
     it { is_expected.to be fake_response }
@@ -49,7 +65,14 @@
     let(:s3_hostname) { 's3-us-west-2.amazonaws.com' }
     subject { described_class.get(bucket: 'mybucket', path: 'mysecret', access_key_id: 'AKIAJMKSMHNNCQX4ILAH', secret_access_key: '0ljyHQrk1AGsc2bgx/8fbNghZNYSdckHADR4vNcL', region: 'us-west-2') }
     before do
-      expect(fake_http).to receive(:get).with('mybucket/mysecret', 'date' => 'Thu, 01 Jan 1970 00:00:00 GMT', 'authorization' => "AWS AKIAJMKSMHNNCQX4ILAH:TQPmJfb3Mx2MblMnRHJS1EG6jus=\n").and_return(fake_response)
+      expect(fake_http).to receive(:get).with('mybucket/mysecret',
+        'host' => 's3-us-west-2.amazonaws.com',
+        'x-amz-content-sha256' => empty_sha256,
+        'x-amz-date' => '19700101T000000Z',
+        'x-amz-expires' => '900',
+        'authorization' => 'AWS4-HMAC-SHA256 Credential=AKIAJMKSMHNNCQX4ILAH/19700101/us-west-2/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date;x-amz-expires, Signature=be57f3bb860d84dae7e8637a75741efdad9796c281c5d3d34ec2c8a5d76663d5'
+      ).and_return(fake_response)
+      expect(fake_http).to receive(:last_response).and_return(Net::HTTPOK.new(nil, nil, nil))
     end
 
     it { is_expected.to be fake_response }
@@ -58,10 +81,19 @@
   context 'with an exception' do
     subject { described_class.get(bucket: 'mybucket', path: 'mysecret', access_key_id: 'AKIAJMKSMHNNCQX4ILAH', secret_access_key: '0ljyHQrk1AGsc2bgx/8fbNghZNYSdckHADR4vNcL') }
     before do
-      expect(fake_http).to receive(:get).with('mybucket/mysecret', 'date' => 'Thu, 01 Jan 1970 00:00:00 GMT', 'authorization' => "AWS AKIAJMKSMHNNCQX4ILAH:TQPmJfb3Mx2MblMnRHJS1EG6jus=\n").and_raise(Net::HTTPServerException.new(nil, nil))
+      expect(fake_http).to receive(:get).with('mybucket/mysecret',
+        'host' => 's3.amazonaws.com',
+        'x-amz-content-sha256' => empty_sha256,
+        'x-amz-date' => '19700101T000000Z',
+        'x-amz-expires' => '900',
+        'authorization' => 'AWS4-HMAC-SHA256 Credential=AKIAJMKSMHNNCQX4ILAH/19700101/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date;x-amz-expires, Signature=fcfb9ddef446db9d31c9f08a4beb05116930b9861947cb49e0c8e8f46f176938'
+      ).and_raise(Net::HTTPServerException.new(nil, nil))
     end
 
-    it { expect { subject }.to raise_error Citadel::CitadelError }
+    it { expect { subject }.to raise_error {|error|
+      expect(error).to be_a(Citadel::CitadelError)
+      expect(error.wrapped_exception).to be_a(Net::HTTPServerException)
+    } }
   end # /context with an exception
 end