Unverified Commit b023426d authored by Martin Jesper Low Madsen's avatar Martin Jesper Low Madsen Committed by GitHub
Browse files

Add support for omitting session token in canonical requests (#2473)

* Add support for omitting session token in canonical request

* Add tests to cover session token exclusion in signed headers

* Remove redundant session token insertion

* Drop mut canonical_headers

* Skip adding x-amz-security-token to signed headers if excluded

* 📎



* cargofmt

* Update changelog

* Update CHANGELOG.next.toml

Co-authored-by: default avatarJohn DiSanti <johndisanti@gmail.com>

---------

Co-authored-by: default avatarRussell Cohen <rcoh@amazon.com>
Co-authored-by: default avatarJohn DiSanti <johndisanti@gmail.com>
parent d14357c2
Loading
Loading
Loading
Loading
+6 −0
Original line number Diff line number Diff line
@@ -68,3 +68,9 @@ message = "`AppName` is now configurable from within `ConfigLoader`."
references = ["smithy-rs#2513"]
meta = { "breaking" = false, "tada" = false, "bug" = true }
author = "ysaito1001"

[[aws-sdk-rust]]
message = "Add support for omitting session token in canonical requests for SigV4 signing."
references = ["smithy-rs#2473"]
meta = { "breaking" = false, "tada" = false, "bug" = false }
author = "martinjlowm"
+8 −5
Original line number Diff line number Diff line
@@ -6,8 +6,8 @@
use crate::middleware::Signature;
use aws_credential_types::Credentials;
use aws_sigv4::http_request::{
    sign, PayloadChecksumKind, PercentEncodingMode, SignableRequest, SignatureLocation,
    SigningParams, SigningSettings, UriPathNormalizationMode,
    sign, PayloadChecksumKind, PercentEncodingMode, SessionTokenMode, SignableRequest,
    SignatureLocation, SigningParams, SigningSettings, UriPathNormalizationMode,
};
use aws_smithy_http::body::SdkBody;
use aws_types::region::SigningRegion;
@@ -63,6 +63,7 @@ impl OperationSigningConfig {
                double_uri_encode: true,
                content_sha256_header: false,
                normalize_uri_path: true,
                omit_session_token: false,
            },
            signing_requirements: SigningRequirements::Required,
            expires_in: None,
@@ -90,10 +91,7 @@ pub struct SigningOptions {
    pub double_uri_encode: bool,
    pub content_sha256_header: bool,
    pub normalize_uri_path: bool,
    /*
    Currently unsupported:
    pub omit_session_token: bool,
     */
}

/// Signing Configuration for an individual Request
@@ -145,6 +143,11 @@ impl SigV4Signer {
            } else {
                UriPathNormalizationMode::Disabled
            };
        settings.session_token_mode = if operation_config.signing_options.omit_session_token {
            SessionTokenMode::Exclude
        } else {
            SessionTokenMode::Include
        };
        settings.signature_location = match operation_config.signature_type {
            HttpSignatureType::HttpRequestHeaders => SignatureLocation::Headers,
            HttpSignatureType::HttpRequestQueryParams => SignatureLocation::QueryParams,
+51 −3
Original line number Diff line number Diff line
@@ -5,6 +5,7 @@

use crate::date_time::{format_date, format_date_time};
use crate::http_request::error::CanonicalRequestError;
use crate::http_request::settings::SessionTokenMode;
use crate::http_request::settings::UriPathNormalizationMode;
use crate::http_request::sign::SignableRequest;
use crate::http_request::uri_path_normalization::normalize_uri_path;
@@ -122,6 +123,8 @@ impl<'a> CanonicalRequest<'a> {
    /// - If `settings.percent_encoding_mode` specifies double encoding, `%` in the URL will be re-encoded as `%25`
    /// - If `settings.payload_checksum_kind` is XAmzSha256, add a x-amz-content-sha256 with the body
    ///   checksum. This is the same checksum used as the "payload_hash" in the canonical request
    /// - If `settings.session_token_mode` specifies X-Amz-Security-Token to be
    ///   included before calculating the signature, add it, otherwise omit it.
    /// - `settings.signature_location` determines where the signature will be placed in a request,
    ///   and also alters the kinds of signing values that go along with it in the request.
    pub(super) fn from<'b>(
@@ -146,11 +149,17 @@ impl<'a> CanonicalRequest<'a> {
        let (signed_headers, canonical_headers) =
            Self::headers(req, params, &payload_hash, &date_time)?;
        let signed_headers = SignedHeaders::new(signed_headers);

        let security_token = match params.settings.session_token_mode {
            SessionTokenMode::Include => params.security_token,
            SessionTokenMode::Exclude => None,
        };

        let values = match params.settings.signature_location {
            SignatureLocation::Headers => SignatureValues::Headers(HeaderValues {
                content_sha256: payload_hash,
                date_time,
                security_token: params.security_token,
                security_token,
                signed_headers,
            }),
            SignatureLocation::QueryParams => SignatureValues::QueryParams(QueryParamValues {
@@ -170,10 +179,11 @@ impl<'a> CanonicalRequest<'a> {
                    .expect("presigning requires expires_in")
                    .as_secs()
                    .to_string(),
                security_token: params.security_token,
                security_token,
                signed_headers,
            }),
        };

        let creq = CanonicalRequest {
            method: req.method(),
            path,
@@ -232,6 +242,12 @@ impl<'a> CanonicalRequest<'a> {
                }
            }

            if params.settings.session_token_mode == SessionTokenMode::Exclude
                && name == HeaderName::from_static(header::X_AMZ_SECURITY_TOKEN)
            {
                continue;
            }

            if params.settings.signature_location == SignatureLocation::QueryParams {
                // The X-Amz-User-Agent header should not be signed if this is for a presigned URL
                if name == HeaderName::from_static(header::X_AMZ_USER_AGENT) {
@@ -240,6 +256,7 @@ impl<'a> CanonicalRequest<'a> {
            }
            signed_headers.push(CanonicalHeaderName(name.clone()));
        }

        Ok((signed_headers, canonical_headers))
    }

@@ -280,6 +297,7 @@ impl<'a> CanonicalRequest<'a> {
                param::X_AMZ_SIGNED_HEADERS,
                values.signed_headers.as_str(),
            );

            if let Some(security_token) = values.security_token {
                add_param(&mut params, param::X_AMZ_SECURITY_TOKEN, security_token);
            }
@@ -521,7 +539,7 @@ mod tests {
    };
    use crate::http_request::test::{test_canonical_request, test_request, test_sts};
    use crate::http_request::{
        PayloadChecksumKind, SignableBody, SignableRequest, SigningSettings,
        PayloadChecksumKind, SessionTokenMode, SignableBody, SignableRequest, SigningSettings,
    };
    use crate::http_request::{SignatureLocation, SigningParams};
    use crate::sign::sha256_hex_string;
@@ -726,6 +744,36 @@ mod tests {
        assert_eq!(expected, actual);
    }

    #[test]
    fn test_omit_session_token() {
        let req = test_request("get-vanilla-query-order-key-case");
        let req = SignableRequest::from(&req);
        let settings = SigningSettings {
            session_token_mode: SessionTokenMode::Include,
            ..Default::default()
        };
        let mut signing_params = signing_params(settings);
        signing_params.security_token = Some("notarealsessiontoken");

        let creq = CanonicalRequest::from(&req, &signing_params).unwrap();
        assert_eq!(
            creq.values.signed_headers().as_str(),
            "host;x-amz-date;x-amz-security-token"
        );
        assert_eq!(
            creq.headers.get("x-amz-security-token").unwrap(),
            "notarealsessiontoken"
        );

        signing_params.settings.session_token_mode = SessionTokenMode::Exclude;
        let creq = CanonicalRequest::from(&req, &signing_params).unwrap();
        assert_eq!(
            creq.headers.get("x-amz-security-token").unwrap(),
            "notarealsessiontoken"
        );
        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
    #[test]
    fn presigning_header_exclusion() {
+2 −2
Original line number Diff line number Diff line
@@ -53,7 +53,7 @@ pub(crate) mod test;

pub use error::SigningError;
pub use settings::{
    PayloadChecksumKind, PercentEncodingMode, SignatureLocation, SigningParams, SigningSettings,
    UriPathNormalizationMode,
    PayloadChecksumKind, PercentEncodingMode, SessionTokenMode, SignatureLocation, SigningParams,
    SigningSettings, UriPathNormalizationMode,
};
pub use sign::{sign, SignableBody, SignableRequest};
+18 −0
Original line number Diff line number Diff line
@@ -32,6 +32,11 @@ pub struct SigningSettings {

    /// Specifies whether the absolute path component of the URI should be normalized during signing.
    pub uri_path_normalization_mode: UriPathNormalizationMode,

    /// Some services require X-Amz-Security-Token to be included in the
    /// canonical request. Other services require only it to be added after
    /// calculating the signature.
    pub session_token_mode: SessionTokenMode,
}

/// HTTP payload checksum type
@@ -78,6 +83,18 @@ pub enum UriPathNormalizationMode {
    Disabled,
}

/// Config value to specify whether X-Amz-Security-Token should be part of the canonical request.
/// <http://docs.aws.amazon.com/general/latest/gr/sigv4-add-signature-to-request.html#temporary-security-credentials>
#[non_exhaustive]
#[derive(Debug, Eq, PartialEq)]
pub enum SessionTokenMode {
    /// Include in the canonical request before calculating the signature.
    Include,

    /// Exclude in the canonical request.
    Exclude,
}

impl Default for SigningSettings {
    fn default() -> Self {
        // The user agent header should not be signed because it may be altered by proxies
@@ -90,6 +107,7 @@ impl Default for SigningSettings {
            expires_in: None,
            excluded_headers: Some(EXCLUDED_HEADERS.to_vec()),
            uri_path_normalization_mode: UriPathNormalizationMode::Enabled,
            session_token_mode: SessionTokenMode::Include,
        }
    }
}
Loading