URL encoding causes 403 SignatureDoesNotMatch on direct folder uploads

[damienstanton]

Versions:

AWS SDK: 1.8.44

Go: 1.9

Issue:

When uploading to a folder within a S3 bucket, the bucket name cannot be passed directly to the s3manager UploadInput type without a trailing slash.

Given the bucket damienstanton-test/childpath as the destination, one expects the S3 path to be parsed as exactly such. But this does not occur.

With debug logging on the session, we examine the canonical string and observe that instead of the usual POST and host

# expected
---[ CANONICAL STRING  ]-----------------------------
POST
/test.txt
uploads=
host:damienstanton-test/childpath.s3.amazonaws.com

the SDK seems to incorrectly parse the bucket and filename. We get this instead:

# actual
---[ CANONICAL STRING  ]-----------------------------
POST
/damienstanton-test%2Fchildpath/test.txt
uploads=
host:s3.amazonaws.com

This throws a key-signing error:

2017/10/10 13:15:55 DEBUG: Response s3/CreateMultipartUpload Details:
---[ RESPONSE ]--------------------------------------
HTTP/1.1 403 Forbidden
Transfer-Encoding: chunked
Content-Type: application/xml
Date: Tue, 10 Oct 2017 17:15:55 GMT
Server: AmazonS3
X-Amz-Id-2: elided...
X-Amz-Request-Id: elided...
-----------------------------------------------------
2017/10/10 13:15:55 <?xml version="1.0" encoding="UTF-8"?>
<Error><Code>SignatureDoesNotMatch</Code><Message>The request signature we calculated does not match the signature you provided. Check your key and signing method.</Message> ...

Steps to reproduce:

A sanity check can be performed by comparing permissions on a bucket versus a folder within that bucket using the official AWS cli tool.

$ aws s3 ls s3://damienstanton-test/childpath

An error occurred (AccessDenied) when calling the ListObjects operation: Access Denied

while

$ aws s3 ls s3://damienstanton-test/childpath/

2017-10-09 18:24:08   12345 test.txt

It seems we can assume that as far as S3 permissions are concerned, <s3://bucketname/path>/ refers to a folder within the bucket, but the CLI tool can run cp to the same path minus the slash.

We have not tested this against buckets with multiple folders, nor could we find any relevant documentation on why the SDK parses the bucket string differently than the CLI. A fix may be to just ensure that the Go SDK parses the bucket URL in the same way the boto library does.

This can be reproduced in full via the following unit tests:

package main

import (
	"fmt"
	"os"
	"testing"

	"github.com/aws/aws-sdk-go/aws"
	"github.com/aws/aws-sdk-go/aws/credentials"
	"github.com/aws/aws-sdk-go/aws/session"
	"github.com/aws/aws-sdk-go/service/s3/s3manager"
)

func TestFails(t *testing.T) {
	sess, _ := newSession("<KEY>", "<SECRET>")
	file, _ := os.Open("test.txt")
	defer file.Close()

	uploader := s3manager.NewUploader(sess)
	_, err := uploader.Upload(&s3manager.UploadInput{
		Bucket: aws.String("damienstanton-test/childpath"), // <- no trailing slash
		Key:    aws.String("test.txt"),
		Body:   file,
	})
	if err != nil {
		t.Fatal("💥 ")
	}
}

func TestWorks(t *testing.T) {
	sess, _ := newSession("<KEY>", "<SECRET>")
	file, _ := os.Open("test.txt")
	defer file.Close()

	uploader := s3manager.NewUploader(sess)
	_, err := uploader.Upload(&s3manager.UploadInput{
		Bucket: aws.String("damienstanton-test/childpath/"), // <- trailing slash
		Key:    aws.String("test.txt"),
		Body:   file,
	})
	if err != nil {
		t.Fatal("💥 ")
	}
}

// newSession creates and validates the test credentials
func newSession(key string, secret string) (*session.Session, error) {
	b := true
	creds := credentials.NewStaticCredentials(key, secret, "")
	sess, err := session.NewSession(&aws.Config{
		Region:      aws.String("us-east-1"),
		Credentials: creds,
		DisableSSL:  &b,
		LogLevel:    aws.LogLevel(aws.LogDebugWithRequestErrors | aws.LogDebugWithHTTPBody | aws.LogDebugWithSigning), // DEBUG
	})
	if err != nil {
		return nil, errors.New("Error creating AWS session: %v", err)
	}
	_, err = sess.Config.Credentials.Get()
	if err != nil {
		return nil, fmt.Errorf("Error getting AWS Credentials: %v", err)
	}

	return sess, nil
}

I will venture to guess that this could be related to #1385. We are aware that our version of the SDK is slightly old, but it seems URL encoding is still an open topic and since this particular issue caused upload failures in a live scenario, we believe it is worth discussing.

[jasdel]

Thanks for reaching out to us @damienstanton. The issue in differing functionality here that you're seeing is an artifact of the CLI's legacy implementation. The CLI hides some of the details of a S3 bucket and object.

Based on the the example it looks like damienstanton-test is the bucket name. Is this correct?

S3 does not natively the concept of founders. This is an artifact of the CLI's representation of folders due to the nature of using a CLI on a in a command shell. All objects in S3 are referenced by a key. These keys are unique within a bucket. The way the CLI simulates folders is to add additional meaning to the / character, by creating empty Objects at that key. In addition to mixing the Object's bucket and key value into a CLI specific s3:// protocol.

In this case the example is adding a path to the bucket name. This is incorrect. Instead of prefixing the key. A prefix (aka sub folders in this example) can only exist in the Key api field. The SDKs do not perform any additional parsing on these values.

A correct API request would be the following with the Bucket name field separate from the Object's Key. If the object key's in your bucket are prefixed with / you'll need to add that to the Key as well. (e.g childpath/test.txt vs /childpath/test.txt).

uploader := s3manager.NewUploader(sess)
	_, err := uploader.Upload(&s3manager.UploadInput{
		Bucket: aws.String("damienstanton-test"),
		Key:    aws.String("childpath/test.txt"),
		Body:   file,
	})

[damienstanton]

Got it, this does describe the scenario we found and explains the unfortunate behavior of the CLI tool.

I’ll close this issue as a change would have to come from the boto/cli side and not the Go SDK. Thanks 🙏

[jasdel]

Thanks for the update @damienstanton. Let us know if you run into any additional issues or have feedback for the SDK.