-
Notifications
You must be signed in to change notification settings - Fork 711
StreamSigner uses signingTime.UTC() #3105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Is the event stream signer broken in the context of using an actual AWS service (i.e. not using the signer directly)? |
Based on my own grep'ing around the code, it looks like all the cases where I see StreamSigner.GetSignature() called in the sdk, they're all using |
So what is the context you're using this in where not using |
I'm using the signing logic directly to write an S3 proxy, which uses a custom external authn / authz mechanism. The proxy is meant to be transparent to the s3 client in question, so the s3 client is configured with an endpoint pointed at the proxy and some fake credentials. When the proxy accepts a request, it forwards the request to s3 signed with its own aws credentials. The reason the stream signing is important is that the proxy must not require buffering an entire blob upload (which might be huge) before signing and forwarding the request. The failure observed is that if you use |
OK, so it's basically because the short date gets formatted wrong (compared to the full version that's used elsewhere in the signature which I guess the format string forces to be UTC). I also see we're doing this in request-response sigv4 so this should be fine really. |
codegen/integration tests fail in forks, those can be ignored |
Cool. I rebased my work onto main. Is there anything else I can do, or is the rest up to you? |
thank you! |
The workaround for the sdk consumer is easy (always apply
.UTC()
before passingsigningTime
), but this feels like an unnecessary footgun. It cost me a couple of hours, at least.