Unverified Commit 80de569d authored by Sam Bartlett's avatar Sam Bartlett Committed by GitHub
Browse files

Expand skipped headers for sigv4 canonical request signing to include... 


Expand skipped headers for sigv4 canonical request signing to include 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: default avatarSam Bartlett <sbartl@amazon.com>
Co-authored-by: default avatarZelda Hessler <zhessler@amazon.com>
parent 9c95803a

CHANGELOG.next.toml

+6 −0
Original line number Diff line number Diff line
@@ -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"]

aws/rust-runtime/aws-sigv4/src/http_request/canonical_request.rs

+33 −1
Original line number Diff line number Diff line
@@ -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();

aws/rust-runtime/aws-sigv4/src/http_request/settings.rs

+22 −5
Original line number Diff line number Diff line
@@ -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,

        }