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
Show file tree
Hide file tree
Changes from 3 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
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."
relevantsam marked this conversation as resolved.
Show resolved Hide resolved
references = ["smithy-rs#2813"]
relevantsam marked this conversation as resolved.
Show resolved Hide resolved
meta = { "breaking" = false, "tada" = false, "bug" = false }
relevantsam marked this conversation as resolved.
Show resolved Hide resolved
author = "relevantsam"

[[aws-sdk-rust]]
message = "Add accessors to Builders"
references = ["smithy-rs#2791"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -774,14 +774,16 @@ 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-amz-user-agent, x-amzn-trace-id headers from presigning
relevantsam marked this conversation as resolved.
Show resolved Hide resolved
#[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
23 changes: 19 additions & 4 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 X_RAY_TRACE_HEADER: &str = "x-amzn-trace-id";
relevantsam marked this conversation as resolved.
Show resolved Hide resolved

/// HTTP-specific signing settings
#[derive(Debug, PartialEq)]
#[non_exhaustive]
Expand Down Expand Up @@ -97,15 +99,28 @@ 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.
let excluded_headers: Vec<HeaderName> = [
// 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(X_RAY_TRACE_HEADER),
]
.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: Some(excluded_headers),
uri_path_normalization_mode: UriPathNormalizationMode::Enabled,
session_token_mode: SessionTokenMode::Include,
}
Expand Down