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
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
@@ -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"]
Original file line number Diff line number Diff line change
@@ -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();
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
@@ -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]
@@ -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,
}