Presigned URLs should use Query parameters for Additional request parameters (instead of headers)

[FlorianSW]

Describe the feature

AWS S3 allows additional request parameters to be set when a request is issued. In my case, I want to add the expected bucket owner to a GetObject request to S3. These parameters can either be added via a header or a query parameter.

When using presigned URLs, their main intended use case is to encapsulate the entire request in the URL for later usage by a thrid-party who should not need to know additional information to add to the request (except executing the presigned URL).[1]

When creating a presigned URL in the aws-go-sdk-v3 with the following code (a GetObject request with an expected bucket owner parameter set):

package main

import (
	"context"
	"fmt"
	"github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/config"
	"github.com/aws/aws-sdk-go-v2/service/s3"
	"os"
	"strings"
)

func main() {
	if len(os.Args) != 4 {
		println("Usage: go run test_bucket_owner.go ACCOUNT_ID BUCKET_NAME OBJECT_KEY")
		os.Exit(1)
	}

	account := os.Args[1]
	bucket := os.Args[2]
	key := os.Args[3]
	ctx := context.Background()
	cfg, err := config.LoadDefaultConfig(ctx, config.WithRegion("eu-west-1"))
	if err != nil {
		panic(err)
	}
	c := s3.NewFromConfig(cfg)
	p := s3.NewPresignClient(c)
	i := s3.GetObjectInput{
		ExpectedBucketOwner: aws.String(account),
		Bucket:              aws.String(bucket),
		Key:                 aws.String(key),
	}
	u, err := p.PresignGetObject(ctx, &i)
	if err != nil {
		panic(err)
	}
	println(u.URL)
	for k, vs := range u.SignedHeader {
		fmt.Printf("%s: %s\n", k, strings.Join(vs, ","))
	}
	println(u.Method)
}

the URL will look something like this:
https://<bucket-name>.s3.eu-west-1.amazonaws.com/<object-key>?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=<access-key-id>%2F20240209%2Feu-west-1%2Fs3%2Faws4_request&X-Amz-Date=20240209T095920Z&X-Amz-Expires=900&X-Amz-Security-Token=IQoJb3Jp...&X-Amz-SignedHeaders=host%3Bx-amz-expected-bucket-owner&x-id=GetObject&X-Amz-Signature=e856...

A caller of this URL is required to add the X-Amz-Expected-Bucket-Owner header with the same value as provided by the creator of the pre-signed URL. This adds a burden and requires additional information that needs to be passed from the creator of the pre-signed URL to the executor of the URL. It also seems to be in contrast to the intended behaviour of pre-signed URLs, as they are now not entirely expressed as a URL anymore.

When executing the above code to create a pre-signed URL, the URL should then look something like that, all parameters should be included as query parameters. This is supported by the S3 API:
https://<bucket-name>.s3.eu-west-1.amazonaws.com/<object-key>?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=<access-key-id>%2F20240209%2Feu-west-1%2Fs3%2Faws4_request&X-Amz-Date=20240209T100408Z&X-Amz-Expires=900&X-Amz-Security-Token=IQoJb3Jp...&X-Amz-SignedHeaders=host&x-amz-expected-bucket-owner=<aws-account-id>&x-id=GetObject&X-Amz-Signature=7ef0789...

Executing this URL will work without needing to add any additional header values.

[1] Refer to "Using query parameters to authenticate requests is useful when you want to express a request entirely in a URL." (https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-query-string-auth.html)

Use Case

I have two components, which work with objects in a S3 bucket, one of them having actual aws credentials to access objects from buckets (the buckets are owned by different accounts). The service that has credentials for these buckets creates pre-signed URLs for the other component to use and work with these objects. As the buckets are configured by customers of these both services, I want to use the expected bucket owner as an additional safety net to ensure that both services are talking to buckets that are owned by the customer we intend them to be owned by. This should protect against a customer deleting a bucket and it being re-created by a malicious actor.

The service that creates pre-signed URLs is expected to pass a URL only to the other service. No other information is communicated at this step. This does, as of now, not work with presigned URLs and the aws-sdk-go-v2.

Proposed Solution

No response

Other Information

No response

Acknowledgements

• I may be able to implement this feature request
• This feature might incur a breaking change

AWS Go SDK V2 Module Versions Used

v1.24.1

Go version used

go version go1.21.6 darwin/arm64

[lucix-aws]

Totally agree in principle that a "presigned" URL that requires you to sideload headers is pretty nonsense. This API predates me and the context for it being that way is lost on me. I'll have to dive deeper on this.

FWIW I do know that S3 as a service doesn't support every request parameter in a presigning context. Unclear if that fed into the design here or not, but it's come up before.

[FlorianSW]

FWIW I do know that S3 as a service doesn't support every request parameter in a presigning context. Unclear if that fed into the design here or not, but it's come up before.

Maybe a bit more context: I had an issue with with the expected bucket owner being passed as a query parameter in the aws-sdk-go (v1)[1]. Unfortunately, the real issue here seemed to be that the query parameter wasn't lowercased.

I had a talk with the AWS support about this issue and during this process also got the response, that request parameters can either be provided as a header, or as a query parameter, but not both. If that is true for each parameter, I don't know tbh :( The S3 API documentation, to my knowledge, does not mention anything about that. And despite the fact, that X-Amz-Expected-Bucket-Owner is documented as a header, it sure works with the lowercase variant in a pre-signed URL as a query parameter (and that is even supposed to be the case, as I understood it from the S3 team).

I'm also not sure how to proceed here, if possible, it should probably be rechecked if all parameters can be passed as query parameters. And if so, an update to the documentation would be nice as well :D

I'm also looking forward to your deep dive of why this is actually build like it is right now 👍

[1] which resulted in the previously send query parameter to be moved to a header in aws/aws-sdk-go@d05d03f

[lucix-aws]

@FlorianSW So, the request with the bucket-owner parameter in the query string may succeed, but I want to double-triple-confirm: does the expected ownership constraint actually apply to the request? e.g. if you create a presigned PUT for some object, and specify a non-matching expected owner, does that presigned request fail when the constraint is on the query?

FWIW I do know that S3 as a service doesn't support every request parameter in a presigning context. Unclear if that fed into the design here or not, but it's come up before.

As far as I can tell, this is in fact at play here. We appear to maintain a list of required signed headers which cannot be hoisted.

Additionally, the addition of x-amz-expected-bucket-owner to this list is fairly recent: #2358. The PR description mentions that the relevant setting doesn't actually get enforced when hoisted as a query parameter, which is why I'd like to reconfirm that here.

[FlorianSW]

So, the request with the bucket-owner parameter in the query string may succeed, but I want to double-triple-confirm: does the expected ownership constraint actually apply to the request? e.g. if you create a presigned PUT for some object, and specify a non-matching expected owner, does that presigned request fail when the constraint is on the query?

I forgot to mention this: I added an example (modifying generated code), to use the SDK v2 code to generate me a pre-signed URL with the expected bucket owner used in the query parameter, which you can find here: https://github.com/FlorianSW/aws-sdk-go-v2/tree/presigned/expectedbucketowner
(most notably this commit: FlorianSW@244e5b9)
I did a tryout of the expected bucket owner to see if is actually enforced or not. I generated a presigned URL (for the GetObject operation, however, I assume that the api works the same for PutObject as well) for a bucket with the correct and once with an incorrect AWS Account ID.

The one with the correct Account ID works as expected. The one with the wrong Account ID results in an AccessDenied error, as one would expect as well.
Both requests were made with the same credential, which were still valid for more than an hour when I opened the link.

---

One caveat as well: The query parameter needs to be passed with a lowercase key. E.g. if the parameter X-Amz-Expected-Bucket-Owner is used (instead of x-amz-expected-bucket-owner), an error message like:

<Code>InvalidBucketOwnerAWSAccountID</Code>
<Message>The value of the expected bucket owner parameter must be an AWS Account ID... [null]</Message>

which is currently pending investigation on why this happens, if I understood the AWS support correctly.

---

Additionally, the addition of x-amz-expected-bucket-owner to this list is fairly recent: #2358.

This update was at the same timeframe where the change in the v1 SDK was made: aws/aws-sdk-go#5062
I feel that this is a response of a support case I made with AWS (back in October last year), where I was seeking help regarding the error message i mentioned above (InvalidBucketOwnerAWSAccountID). It seems that this change was a direct response to that (as it was also reported back to me in the support case), but it feels like a workaround for another problem, tbh. It definitely should work with a query parameter as well (that's what S3 told me as well).

[RanVaknin]

Hi @FlorianSW ,

Thanks for all of the context and investigation. I do remember working on this not too long ago. We have discussed this as a team and I will have to do a deep dive to determine the best path forward.

Thanks,
Ran~

[RanVaknin]

Hi @FlorianSW,

Thank you for your patience. I can confirm that lowercasing x-amz-expected-bucket-owner and supplying it as a query parameter works correctly.

We already know that when X-Amz-Expected-Bucket-Owner is hoisted to the query string, it does not work, which is the driving force behind #2358 .

Could you please provide us with the support ticket ID you have with S3? It will help us understand what has been done so far and allow the SDK team to share its findings with S3.

[FlorianSW]

Hi @RanVaknin,

glad to see that we came to the same results :)

Could you please provide us with the support ticket ID you have with S3? It will help us understand what has been done so far and allow the SDK team to share its findings with S3.

Sure, the Case ID is: 14086099621

[RanVaknin]

Hi @FlorianSW ,

Thanks. It seems like that ticket was closed. I reopened it (D111906415) asking for some clarification. I suggest that you reach out to the technical account manager you have worked with so that you have another set of eyes.

Thanks,
Ran~

[RanVaknin]

Hi @FlorianSW,

Thanks for your patience.

I have created another Github issue which is meant to align the behavior of the SDK with the behavior of S3. (adjusting the casing of the expected bucket owner header)

Now, this ticket calls out changing the behavior of the presigner to hoist headers into the query parameters instead of signing as headers. We cannot act on this as both can be (one or the other), and there are valid use cases of supplying headers in the presigned url as signed headers.

Please refer to #2508 for tracking.

Thanks,
Ran~

[github-actions]

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

[FlorianSW]

Hi @RanVaknin,

Sorry for my late reply. I looked for the case you linked above but was unable to find it in my account. I reached out to our TAM to assist in this, so that I can follow up with your open questions.

I'll certainly look into the linked issue to see the progress of that, thanks.

Best,
Florian

[lucix-aws]

there are valid use cases of supplying headers in the presigned url as signed headers.

To clarify (and circle back on what I said originally) it's now understood that the reason we have the headers "escape" hatch for presigning is for inputs that we know or at one point knew to not be supported by the S3 service in a query-hoisted context. In an ideal world, at some unspecified point in the future, S3 would recognize everything in the query and the returned header map would be vestigial.

The exact set of headers that we don't hoist is manually maintained by each SDK (ours is here). You've correctly identified that our handling of the bucket owner parameter is wrong, so we're tracking that on its own in #2508.