Skip to content

Commit

Permalink
Expand skipped headers for sigv4 canonical request signing to include…
Browse files Browse the repository at this point in the history
… x-amzn-trace-id and authorization headers. (#2815)

## 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
- [x] 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._

---------

Co-authored-by: Sam Bartlett <[email protected]>
Co-authored-by: Zelda Hessler <[email protected]>
  • Loading branch information
3 people authored Jun 28, 2023
1 parent 9c95803 commit 80de569
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 6 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@
# meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}
# author = "rcoh"

[[aws-sdk-rust]]
message = "Automatically exclude X-Ray trace ID headers and authorization headers from SigV4 canonical request calculations."
references = ["smithy-rs#2815"]
meta = { "breaking" = false, "tada" = false, "bug" = true }
author = "relevantsam"

[[aws-sdk-rust]]
message = "Add accessors to Builders"
references = ["smithy-rs#2791"]
Expand Down
34 changes: 33 additions & 1 deletion aws/rust-runtime/aws-sigv4/src/http_request/canonical_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -774,14 +774,46 @@ mod tests {
assert_eq!(creq.values.signed_headers().as_str(), "host;x-amz-date");
}

// It should exclude user-agent and x-amz-user-agent headers from presigning
// It should exclude authorization, user-agent, x-amzn-trace-id headers from presigning
#[test]
fn non_presigning_header_exclusion() {
let request = http::Request::builder()
.uri("https://some-endpoint.some-region.amazonaws.com")
.header("authorization", "test-authorization")
.header("content-type", "application/xml")
.header("content-length", "0")
.header("user-agent", "test-user-agent")
.header("x-amzn-trace-id", "test-trace-id")
.header("x-amz-user-agent", "test-user-agent")
.body("")
.unwrap();
let request = SignableRequest::from(&request);

let settings = SigningSettings {
signature_location: SignatureLocation::Headers,
..Default::default()
};

let signing_params = signing_params(settings);
let canonical = CanonicalRequest::from(&request, &signing_params).unwrap();

let values = canonical.values.as_headers().unwrap();
assert_eq!(
"content-length;content-type;host;x-amz-date;x-amz-user-agent",
values.signed_headers.as_str()
);
}

// It should exclude authorization, user-agent, x-amz-user-agent, x-amzn-trace-id headers from presigning
#[test]
fn presigning_header_exclusion() {
let request = http::Request::builder()
.uri("https://some-endpoint.some-region.amazonaws.com")
.header("authorization", "test-authorization")
.header("content-type", "application/xml")
.header("content-length", "0")
.header("user-agent", "test-user-agent")
.header("x-amzn-trace-id", "test-trace-id")
.header("x-amz-user-agent", "test-user-agent")
.body("")
.unwrap();
Expand Down
27 changes: 22 additions & 5 deletions aws/rust-runtime/aws-sigv4/src/http_request/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
* SPDX-License-Identifier: Apache-2.0
*/

use http::header::{HeaderName, USER_AGENT};
use http::header::{HeaderName, AUTHORIZATION, USER_AGENT};
use std::time::Duration;

/// HTTP signing parameters
pub type SigningParams<'a> = crate::SigningParams<'a, SigningSettings>;

const HEADER_NAME_X_RAY_TRACE_ID: &str = "x-amzn-trace-id";

/// HTTP-specific signing settings
#[derive(Debug, PartialEq)]
#[non_exhaustive]
Expand Down Expand Up @@ -97,15 +99,30 @@ pub enum SessionTokenMode {

impl Default for SigningSettings {
fn default() -> Self {
// The user agent header should not be signed because it may be altered by proxies
const EXCLUDED_HEADERS: [HeaderName; 1] = [USER_AGENT];

// Headers that are potentially altered by proxies or as a part of standard service operations.
// Reference:
// Go SDK: <https://github.com/aws/aws-sdk-go/blob/v1.44.289/aws/signer/v4/v4.go#L92>
// Java SDK: <https://github.com/aws/aws-sdk-java-v2/blob/master/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4Signer.java#L70>
// JS SDK: <https://github.com/aws/aws-sdk-js/blob/master/lib/signers/v4.js#L191>
// There is no single source of truth for these available, so this uses the minimum common set of the excluded options.
// Instantiate this every time, because SigningSettings takes a Vec (which cannot be const);
let excluded_headers = Some(
[
// This header is calculated as part of the signing process, so if it's present, discard it
AUTHORIZATION,
// Changes when sent by proxy
USER_AGENT,
// Changes based on the request from the client
HeaderName::from_static(HEADER_NAME_X_RAY_TRACE_ID),
]
.to_vec(),
);
Self {
percent_encoding_mode: PercentEncodingMode::Double,
payload_checksum_kind: PayloadChecksumKind::NoHeader,
signature_location: SignatureLocation::Headers,
expires_in: None,
excluded_headers: Some(EXCLUDED_HEADERS.to_vec()),
excluded_headers,
uri_path_normalization_mode: UriPathNormalizationMode::Enabled,
session_token_mode: SessionTokenMode::Include,
}
Expand Down

0 comments on commit 80de569

Please sign in to comment.