aws-sdk-ruby icon indicating copy to clipboard operation
aws-sdk-ruby copied to clipboard

Aws:STS:Presigner sometimes sets X-Amz-Expires < 900

Open rittneje opened this issue 11 months ago • 17 comments

Describe the bug

After upgrading from 3.173.0 to 3.178.0 of aws-sdk-core, get_caller_identity_presigned_url sometimes sets X-Amz-Expires to a value < 900. A value other than 900 does not work for anything other than S3.

We use these presigned URLs for authn, and the server strictly enforces the value of this parameter for safety.

I believe it is due to this "feature" from aws-sdk-s3 v1.127.0:

Feature - Select minimum expiration time for presigned urls between the expiration time option and the credential expiration time.

I don't see anything of relevance in the changelog for aws-sdk-core.

Expected Behavior

see above

Current Behavior

see above

Reproduction Steps

Generate a presigned url when role creds will expire in < 15 minutes

Possible Solution

No response

Additional Information/Context

No response

Gem name ('aws-sdk', 'aws-sdk-resources' or service gems like 'aws-sdk-s3') and its version

aws-sdk-core 3.178.0

Environment details (Version of Ruby, OS environment)

n/a

rittneje avatar Jul 25 '23 18:07 rittneje

Thanks for opening up an issue. The change was in aws-sigv4:

1.6.0 (2023-06-28)
------------------

* Feature - Select the minimum expiration time for presigned urls between the expiration time option and the credential expiration time.

Selecting the minimum credentials time should have been safe.

We use these presigned URLs for authn, and the server strictly enforces the value of this parameter for safety.

Can you be specific about what server is enforcing this? Do you have a reproduction case?

mullermp avatar Jul 25 '23 19:07 mullermp

It's a server that works very much like Kubernetes does, where we leverage the presigned URL to prove the client's identity.

rittneje avatar Jul 25 '23 19:07 rittneje

Who owns the software where that validation occurs? I tested that a GetCallerIdentity presigned url will work for an expiration < 900, so any other validation that is occurring is happening elsewhere and not on any AWS service. You can verify the client's identity but perhaps not specifically look at an expiration of 900 seconds, that seems very fragile.

mullermp avatar Jul 25 '23 19:07 mullermp

The feature of selecting minimum expiration time is very much intended. Otherwise, there are cases where a pre-signed url with expiration of 900 seconds may not work if the credentials expire any time sooner.

mullermp avatar Jul 25 '23 19:07 mullermp

@mullermp No other AWS SDK works that way. And for anything other than S3, an expiration time other than 900 doesn't actually work in general, in that AWS ignores it and uses 900 anyway. Attempting to use that field to convey when the underlying role creds expire is kind of a hack that is not supported by SigV4 spec.

rittneje avatar Jul 25 '23 21:07 rittneje

Internally the SDK teams talked about using the minimum of credential expiration and expiration time config - we need to do this for an upcoming feature. Do you have any supporting documentation for AWS ignoring the value and using 900 anyway? As I said in the previous message - expiration in the URL was not always a guarantee if the credentials were expiring before 900 seconds (or whatever was configured for the URL)

mullermp avatar Jul 25 '23 21:07 mullermp

@mullermp It is mentioned in the Go SDK, and confirmed by our own experimentation. https://pkg.go.dev/github.com/aws/aws-sdk-go/aws/request#Request.Presign

The expire parameter is only used for presigned Amazon S3 API requests. All other AWS services will use a fixed expiration time of 15 minutes.

As has always been the case for presigning, if the role creds expire first, then the presigned URL is no longer valid. However, if, say, your role creds are valid for one hour, and you presign STS GetCallerIdentity with X-Amz-Expires=600, it is still is valid for 15 minutes. Hence, attempting to use X-Amz-Expires to hold the role cred expiration is kind of a hack, and in general no one can rely on a value other than 900 to mean anything.

rittneje avatar Jul 25 '23 22:07 rittneje

I think STS is an exceptional case where the credentials, even when expired, will still work when calling GetCallerIdentity. Other presigned URLs like for RDS and Polly will not work this way.

mullermp avatar Jul 25 '23 22:07 mullermp

I'll ask around internally.

mullermp avatar Jul 25 '23 22:07 mullermp

@mullermp The other thing I want to point out is I don't think this new behavior of "clamping" the presigned expiration is the right solution, even for S3. Let's say my role creds expire in 10 minutes and I try to generate a presigned S3 url for 30 minutes. As per this change, you will instead generate a presigned url with X-Amz-Expires set to 10 minutes. But that doesn't really help as the presigned URL was only valid for 10 minutes anyway. I think the better thing to do in that situation is to just refresh the credentials first, so the resulting presigned url is valid for the intended duration.

rittneje avatar Jul 26 '23 03:07 rittneje

When vending presigned urls to another party (a very common case of presigned urls), the recipient doesn't know anything about what credentials were used and when they may expire. Previously the vended url would indicate 30 minutes expiration in your example and the third party would be surprised when it doesn't work. Both the vendor and the recipient will know exactly when the URL will no longer be valid.

mullermp avatar Jul 26 '23 11:07 mullermp

@mullermp Only if they know the presigned url came from this SDK, using role credentials whose expiration is known to you. And as I mentioned, I believe the desired behavior would be to refresh the credentials to honor the requested expiration, not to clamp it.

rittneje avatar Jul 26 '23 11:07 rittneje

Refreshing does not guarantee that credentials will be valid for longer than 900 seconds, the previous default. Some credential types only last 5 minutes.

mullermp avatar Jul 26 '23 13:07 mullermp

Sure, but that's less common, and would consistently not work as expected. This change still does not address the original problem, which is that I want a presigned URL valid for a certain duration, but sometimes the SDK fails to do so. Hence the SDK should first check if credential expiration < presigned expiration. If so, force a refresh. Only after that would clamping be reasonable.

rittneje avatar Jul 26 '23 14:07 rittneje

Some clarification I got from STS is that, GetCallerIdentity will always succeed if the access key id is still known (hasn't been fully rotated) - this means that the expiration parameter is not considered.

I talked with some other engineers on other SDKs. This is another potential approach we can take - introduce a presigner option that has three modes:

  • ignore_expiration (default) - This is the previous behavior (I would have to partially revert the change) where the URL always had the expiration that was configured by the presigner, ignoring credential expiration.

  • refresh_if_will_expire - This would refresh the credentials before generating a presigned url. It would potentially have the same issue where credentials may expire before the URL indicates that it may expire.

  • require_full_signing_duration - This would refresh the credentials before generating a presigned url AND compare expiration to the configured amount. If the credentials expire before the configured amount, raise an error.

mullermp avatar Jul 28 '23 17:07 mullermp

@rittneje Did you have any preference on the above?

mullermp avatar Aug 07 '23 18:08 mullermp

require_full_signing_duration might be a little dicey if you are off by a few seconds. For example, if my role assumption is valid for one hour, and I try to generated a presigned URL valid for one hour. Maybe it should add some buffer to the role expiration before taking the minimum. Also raising an error sounds like a harsh penalty. Perhaps the SDK should just log a warning under refresh_if_will_expire instead?

rittneje avatar Aug 07 '23 21:08 rittneje