Skip to content
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

Expand skipped headers for sigv4 canonical request signing to include x-amzn-trace-id and authorization headers. #2815

Merged
merged 4 commits into from
Jun 28, 2023

Conversation

relevantsam
Copy link
Contributor

@relevantsam relevantsam commented Jun 28, 2023

Motivation and Context

When customers add x-ray headers to requests, the SigV4 signer should exclude them, or the generated canonical signature will not match the remote service's, since many services being called are written with non-rust SDKs that automatically exclude these common headers.

The Rust SDK should exclude a similar set of headers to the other popular AWS SDKs. While this is not uniform across the SDKs, a minimal set should be excluded and others should be considered to be excluded in future PRs.

Description

  • Expands the set of headers excluded from canonical request calculation to include "x-amzn-trace-id" and "authorization" (since authorization will be added as a part of this process).

Testing

  • Added headers to exclusion test & validated with cargo test
  • ./gradlew :aws:sdk:test

Checklist

  • I have updated CHANGELOG.next.toml if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@relevantsam relevantsam requested review from a team as code owners June 28, 2023 17:47
CHANGELOG.next.toml Outdated Show resolved Hide resolved
Copy link
Collaborator

@jdisanti jdisanti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing this!

aws/rust-runtime/aws-sigv4/src/http_request/settings.rs Outdated Show resolved Hide resolved
CHANGELOG.next.toml Outdated Show resolved Hide resolved
CHANGELOG.next.toml Outdated Show resolved Hide resolved
… x-amzn-trace-id and authorization headers.
@relevantsam
Copy link
Contributor Author

12 | const X_RAY_TRACE_HEADER: HeaderName = HeaderName::from_static("x-amzn-trace-id");

We looked at suppressing the clippy warning and adding a const array. The Vec input for SigningSettings reduces the value for this, so we went with the original approach.

@jdisanti jdisanti added this pull request to the merge queue Jun 28, 2023
Merged via the queue into smithy-lang:main with commit 80de569 Jun 28, 2023
@relevantsam relevantsam deleted the canonical-request-exclusions branch June 28, 2023 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants