aws-sdk-go-v2 icon indicating copy to clipboard operation
aws-sdk-go-v2 copied to clipboard

can't create presigned url with canned acl, can't limit content type

Open congbaoyangrou opened this issue 3 years ago • 1 comments

Describe the bug

  1. don't know how to set canned ACL in presigned url.
  2. even if I set content type in PutObjectInput, the SignedHeaders doesn't contain it.
  3. misleading error and different results in different clients.

Expected Behavior

when using the presigned url, a file should be uploaded successfully.

Current Behavior

1.when I set acl:types.ObjectCannedACLPublicRead in PutObjectInput,the client should set x-amz-acl in header,which is not expected. it should be one of presigned url's query params.(I found this issue https://github.com/aws/aws-sdk-java-v2/issues/1849 exactly the same as my confusion, but I don't know how to override it in go) 2. set content type="image/png" in PutObjectInput that limit didn't appear in signedheaders, so the client can successfully upload a txt file with content type text/plain(it returns HTTP status 200) 3. got NotImplemented error, I found that I didn't set the content length. the "NotImplemented error" is misleading. the same presigned URL can be used in go client, but in PostMan, it returns the SignatureDoesNotMatch error. Copy the curl code exported by postman,it works too. why?

Reproduction Steps

server-side code: generate upload URL

preReq, err := s.presvc.PresignPutObject(context.Background(), &s3.PutObjectInput{
		Bucket: aws.String(bucket),
		Key:    aws.String(key),
		ContentType: aws.String("image/png"),
	}, s3.WithPresignExpires(time.Hour))

client-side 1:

stats, _ := os.Stat(path)
f, _ := os.Open(path)
req, err := http.NewRequest("PUT", uploadURL, f)
// we can't change this 2 lines ,as client are not under our control, they got resigned URL from java server before.
req.Header.Set("Content-Type", "image/png")
req.Header.Set("Cache-Control", "public, max-age=31536000")
req.ContentLength = stats.Size()
response, err := http.DefaultClient.Do(req)

it works.

client-side 2: use Postman I copy that URL in postman, set the method to PUT, in the body, I choose binary and upload a file, then I get a SignatureDoesNotMatch error. (when i set acl in putObjectInput and generate a presigned URL, i can use postman to upload a file with header X-Amz-acl)

client-side 3: just use the curl code exported by postMan in client-side 2, it works. why?it both works with or without Content-Type header. curl --location --request PUT 'https://<my-bucket>.s3.us-east-1.amazonaws.com/<my-key>?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=<xxx>%2F20220430%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220430T103611Z&X-Amz-Expires=3600&X-Amz-SignedHeaders=host&x-id=PutObject&X-Amz-Signature=7f9a9a6fd1652ad483aadbe87fe7d20c564ddbb209bfccf8aaa92a2e5a4cb8fa ' \ --header 'Content-Type: image/png' \ --data-binary '@path/redis.png'

BTW: I don't know how to set that object's ACL in presigned URL, the options just contain a few like WithPresignExpires.

I searched in test.go, go examples, document in AWS, but they just told me how to generate a resigned URL, never told me how to use it.I see this issue: https://github.com/aws/aws-sdk-go-v2/issues/1134,but it seems not to work(if that means I should inclued headers that X-Amz-SignedHeaders provided when I use that presinged URL), that can't explain the failure in postman.

Possible Solution

No response

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

github.com/aws/aws-sdk-go-v2 v1.16.3 github.com/aws/aws-sdk-go-v2/credentials v1.12.0 github.com/aws/aws-sdk-go-v2/service/s3 v1.26.7

Compiler and Version used

1.18

Operating System and version

macOs monterey

congbaoyangrou avatar Apr 30 '22 10:04 congbaoyangrou

btw i solved this by using raw presign:

httpSigner := v4.NewSigner()
	uri := "xxx"
	req, _ := http.NewRequest("PUT", uri, nil)
	params := url.Values{
		"X-Amz-Credential": {fmt.Sprintf("%s/%s/us-east-1/s3/aws4_request", AccessKeyID, time.Now().Format("20060102"))},
		"X-Amz-Expires":    {"3600"},
		"x-id":             {"PutObject"},
		"x-amz-acl":        {"public-read"},
	}
	req.Header.Set("Content-Type", mimeType)
	req.URL.RawQuery = params.Encode()
	auth, _ := s.auth.Retrieve(ctx)
	u, _, e := httpSigner.PresignHTTP(ctx, auth, req, "UNSIGNED-PAYLOAD", "s3", "us-east-1", time.Now())
	

congbaoyangrou avatar Jun 16 '22 09:06 congbaoyangrou

This works and is simpler than doing the whole thing raw: https://stackoverflow.com/a/74585074/13383986

tyliggity avatar Aug 17 '23 18:08 tyliggity

Are there any updates on this? Without the ability to set Conditions so we can set content type and content length range, similar to JS SDK and Boto, there is a security issue. Meaning, anyone with a valid presigned URL can upload a large file of any content type. Why is this not being investigated?

elousiv avatar Dec 01 '23 01:12 elousiv

Are there any updates on this? Without the ability to set Conditions so we can set content type and content length range, similar to JS SDK and Boto, there is a security issue. Meaning, anyone with a valid presigned URL can upload a large file of any content type. Why is this not being investigated?

The workaround is to manually set the headers (and they are case sensitive) as shown here: https://stackoverflow.com/a/74585074/13383986

But it's still a bit ridiculous that this workaround is necessary at all considering it does seem like a tiny fix 🤷‍♂️

tyliggity avatar Dec 01 '23 05:12 tyliggity

Are there any updates on this? Without the ability to set Conditions so we can set content type and content length range, similar to JS SDK and Boto, there is a security issue. Meaning, anyone with a valid presigned URL can upload a large file of any content type. Why is this not being investigated?

The workaround is to manually set the headers (and they are case sensitive) as shown here: https://stackoverflow.com/a/74585074/13383986

But it's still a bit ridiculous that this workaround is necessary at all considering it does seem like a tiny fix 🤷‍♂️

@tyliggity thanks for following up with a workaround! While this does address ACL and content type it doesn't address (unless i'm missing something of course) the ability to set conditions such as content-length-range. Can that be sent as a header as well?

elousiv avatar Dec 01 '23 14:12 elousiv

Hi all,

I'm able to set the content-type on the object I'm uploading via the presigned URL:

func presignPut() {
	cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRegion("us-east-1"), config.WithClientLogMode(aws.LogRequestWithBody|aws.LogSigning))
	if err != nil {
		panic(err)
	}

	client := s3.NewFromConfig(cfg)

	presigner := s3.NewPresignClient(client)

	data := bytes.NewBuffer([]byte("hello"))

	presignObject, err := presigner.PresignPutObject(context.TODO(), &s3.PutObjectInput{
		Bucket: aws.String(bucketName),
		Key:    aws.String("sample4.txt"),
		Body:   data,
		ContentType: aws.String("image/png"),
	})
	if err != nil {
		panic(err)
	}

	fmt.Println(presignObject.URL)
	

	request, err := http.NewRequest(presignObject.Method, presignObject.URL, data)
	if err != nil {
		panic(err)
	}

	// add all the signed headers from the presigned URL to the headers to the request.
	for k, vs := range presignObject.SignedHeader {
		for _, v := range vs {
			request.Header.Add(k, v)
		}
	}

	res, err := http.DefaultClient.Do(request)
	if err != nil {
		panic(err.Error())
	}

	fmt.Println(res.Status)

	bodyBytes, err := io.ReadAll(res.Body)
	if err != nil {
		panic(err.Error())
	}
	res.Body.Close()

	bodyString := string(bodyBytes)
	fmt.Println(bodyString)
}

You can log the signing proccess and see that the presigned URL does sign the content type as a header:

SDK 2023/12/18 13:02:05 DEBUG Request Signature:
---[ CANONICAL STRING  ]-----------------------------
PUT
/sample4.txt
X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=REDACTED%2F20231218%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20231218T210205Z&X-Amz-Expires=900&X-Amz-SignedHeaders=content-length%3Bcontent-type%3Bhost&x-id=PutObject
content-length:5
content-type:image/png
host:testbucket.s3.us-east-1.amazonaws.com

content-length;content-type;host
UNSIGNED-PAYLOAD
---[ STRING TO SIGN ]--------------------------------
AWS4-HMAC-SHA256
20231218T210205Z
20231218/us-east-1/s3/aws4_request
d1fc129b1dafe8b5f914c8d765298567657153b87b57e19d05e550528d58b927
---[ SIGNED URL ]------------------------------------
https://testbucket.s3.us-east-1.amazonaws.com/sample4.txt?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=REDACTED%2F20231218%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20231218T210205Z&X-Amz-Expires=900&X-Amz-SignedHeaders=content-length%3Bcontent-type%3Bhost&x-id=PutObject&X-Amz-Signature=REDACTED
-----------------------------------------------------
https://testbucket.s3.us-east-1.amazonaws.com/sample4.txt?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=REDACTED%2F20231218%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=REDACTED&X-Amz-Expires=900&X-Amz-SignedHeaders=content-length%3Bcontent-type%3Bhost&x-id=PutObject&X-Amz-Signature=REDACTED
200 OK

This will result in my text file uploaded with the defined content-type image/png. image

I understand that you are looking to restrict files based on the provided content type but unfortunately the S3 server itself does not enforce the content-type for presigned URLs. I think this is for flexibility reasons.

As far as content-length-range goes, that is a feature of a POST policy, and does not have support with Presigned Put requests. The S3 server does however enforce content-length. Thanks, Ran~

RanVaknin avatar Dec 18 '23 21:12 RanVaknin

This issue has not received a response in 1 week. If you want to keep this issue open, please just leave a comment below and auto-close will be canceled.

github-actions[bot] avatar Dec 21 '23 00:12 github-actions[bot]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

github-actions[bot] avatar Dec 26 '23 22:12 github-actions[bot]