Unverified Commit 2c372ed4 authored by John DiSanti's avatar John DiSanti Committed by GitHub
Browse files

Fix signing when Hyper ALPN negotiates to h2 (#674) 

The endpoint middleware was adding a host header to the request, and
this worked when using HTTP/1.1. However, when the connection gets
upgraded to h2, this results in the remote service expecting the
canonical request to have two `host` headers.

This change moves the addition of the host header into the signing code,
and lets hyper take charge of the host header.

* Update changelog

* Remove `transcribestreaming` exclusion from full SDK build

* Fix failing tests

* CR feedback
parent c6143ec5

CHANGELOG.md

+1 −0
Original line number Diff line number Diff line
@@ -34,6 +34,7 @@ vNext (Month Day, Year) 
- Improve documentation on collection-aware builders (#664)

- Add support for Transcribe `StartStreamTranscription` and S3 `SelectObjectContent` operations (#667)

- Add support for shared configuration between multiple services (#673)

- :bug: Fix sigv4 signing when request ALPN negotiates to HTTP/2. (#674)



**Internal Changes**

- Add NowOrLater future to smithy-async (#672)

aws/rust-runtime/aws-auth-providers/src/test_case.rs

+1 −1
Original line number Diff line number Diff line
@@ -139,7 +139,7 @@ impl TestEnvironment { 
        self.log_info();

        self.check_results(&result);

        // todo: validate bodies

        match connector.validate(&["CONTENT-TYPE", "HOST"], |_expected, _actual| Ok(())) {

        match connector.validate(&["CONTENT-TYPE"], |_expected, _actual| Ok(())) {

            Ok(()) => {}

            Err(e) => panic!("{}", e),

        }

aws/rust-runtime/aws-endpoint/src/lib.rs

+8 −27
Original line number Diff line number Diff line
@@ -11,21 +11,16 @@ pub use partition::Partition; 
#[doc(hidden)]

pub use partition::PartitionResolver;



use std::error::Error;

use std::fmt;

use std::fmt::{Debug, Display, Formatter};

use std::sync::Arc;



use http::HeaderValue;



use aws_types::region::{Region, SigningRegion};

use aws_types::SigningService;

use http::header::HOST;

use smithy_http::endpoint::{Endpoint, EndpointPrefix};

use smithy_http::middleware::MapRequest;

use smithy_http::operation::Request;

use smithy_http::property_bag::PropertyBag;

use std::convert::TryFrom;

use std::error::Error;

use std::fmt;

use std::fmt::{Debug, Display, Formatter};

use std::sync::Arc;



/// Endpoint to connect to an AWS Service

///
@@ -202,14 +197,6 @@ impl MapRequest for AwsEndpointStage { 
            endpoint

                .endpoint

                .set_endpoint(http_req.uri_mut(), props.get::<EndpointPrefix>());

            // host is only None if authority is not. `set_endpoint` guarantees that authority is not None

            let host = http_req

                .uri()

                .host()

                .expect("authority is guaranteed to be non-empty after `set_endpoint`");

            let host = HeaderValue::try_from(host)

                .expect("authority must only contain valid header characters");

            http_req.headers_mut().insert(HOST, host);

            Ok(http_req)

        })

    }
@@ -219,6 +206,7 @@ impl MapRequest for AwsEndpointStage { 
mod test {

    use std::sync::Arc;



    use http::header::HOST;

    use http::Uri;



    use aws_types::region::{Region, SigningRegion};
@@ -229,7 +217,6 @@ mod test { 


    use crate::partition::endpoint::{Metadata, Protocol, SignatureVersion};

    use crate::{set_endpoint_resolver, AwsEndpointStage, CredentialScope};

    use http::header::HOST;



    #[test]

    fn default_endpoint_updates_request() {
@@ -249,10 +236,7 @@ mod test { 
            set_endpoint_resolver(&mut props, provider);

        };

        let req = AwsEndpointStage.apply(req).expect("should succeed");

        assert_eq!(

            req.properties().get(),

            Some(&SigningRegion::from(region.clone()))

        );

        assert_eq!(req.properties().get(), Some(&SigningRegion::from(region)));

        assert_eq!(

            req.properties().get(),

            Some(&SigningService::from_static("kinesis"))
@@ -263,10 +247,7 @@ mod test { 
            req.uri(),

            &Uri::from_static("https://kinesis.us-east-1.amazonaws.com")

        );

        assert_eq!(

            req.headers().get(HOST).expect("host header must be set"),

            "kinesis.us-east-1.amazonaws.com"

        );

        assert!(req.headers().get(HOST).is_none());

    }



    #[test]
@@ -285,7 +266,7 @@ mod test { 
        let mut req = operation::Request::new(req);

        {

            let mut props = req.properties_mut();

            props.insert(region.clone());

            props.insert(region);

            props.insert(SigningService::from_static("kinesis"));

            set_endpoint_resolver(&mut props, provider);

        };

aws/rust-runtime/aws-hyper/tests/e2e_test.rs

+29 −25
Original line number Diff line number Diff line
@@ -13,7 +13,7 @@ use aws_sig_auth::signer::OperationSigningConfig; 
use aws_types::region::Region;

use aws_types::SigningService;

use bytes::Bytes;

use http::header::{AUTHORIZATION, HOST, USER_AGENT};

use http::header::{AUTHORIZATION, USER_AGENT};

use http::{self, Uri};

use smithy_client::test_connection::TestConnection;

use smithy_http::body::SdkBody;
@@ -71,7 +71,12 @@ impl ParseHttpResponse for TestOperationParser { 
}



fn test_operation() -> Operation<TestOperationParser, AwsErrorRetryPolicy> {

    let req = operation::Request::new(http::Request::new(SdkBody::from("request body")))

    let req = operation::Request::new(

        http::Request::builder()

            .uri("https://test-service.test-region.amazonaws.com/")

            .body(SdkBody::from("request body"))

            .unwrap(),

    )

    .augment(|req, mut conf| {

        set_endpoint_resolver(

            &mut conf,
@@ -109,7 +114,6 @@ async fn e2e_test() { 
    let expected_req = http::Request::builder()

        .header(USER_AGENT, "aws-sdk-rust/0.123.test os/windows/XPSP3 lang/rust/1.50.0")

        .header("x-amz-user-agent", "aws-sdk-rust/0.123.test api/test-service/0.123 os/windows/XPSP3 lang/rust/1.50.0")

        .header(HOST, "test-service.test-region.amazonaws.com")

        .header(AUTHORIZATION, "AWS4-HMAC-SHA256 Credential=access_key/20210215/test-region/test-service-signing/aws4_request, SignedHeaders=host;x-amz-date;x-amz-user-agent, Signature=da249491d7fe3da22c2e09cbf910f37aa5b079a3cedceff8403d0b18a7bfab75")

        .header("x-amz-date", "20210215T184017Z")

        .uri(Uri::from_static("https://test-service.test-region.amazonaws.com/"))

aws/rust-runtime/aws-sig-auth/src/middleware.rs

+4 −1
Original line number Diff line number Diff line
@@ -152,7 +152,10 @@ mod test { 


    #[test]

    fn places_signature_in_property_bag() {

        let req = http::Request::new(SdkBody::from(""));

        let req = http::Request::builder()

            .uri("https://test-service.test-region.amazonaws.com/")

            .body(SdkBody::from(""))

            .unwrap();

        let region = Region::new("us-east-1");

        let req = operation::Request::new(req)

            .augment(|req, properties| {