Fix presigning bug with content-length and content-type in S3

CHANGELOG.next.toml

@@ -46,3 +46,15 @@ message = "`ClientBuilder` helpers `rustls()` and `native_tls()` now return `Dyn
 references = ["smithy-rs#1217", "aws-sdk-rust#467"]
 meta = { "breaking" = false, "tada" = false, "bug" = true }
 author = "jdisanti"
+
+[[aws-sdk-rust]]
+message = "`aws-sigv4` no longer skips the `content-length` and `content-type` headers when signing with `SignatureLocation::QueryParams`"
+references = ["smithy-rs#1216"]
+meta = { "breaking" = true, "tada" = false, "bug" = false }
+author = "jdisanti"
+
+[[aws-sdk-rust]]
+message = "Fixed a bug in S3 that prevented the `content-length` and `content-type` inputs from being included in a presigned request signature. With this fix, customers can generate presigned URLs that enforce `content-length` and `content-type` for requests to S3."
+references = ["smithy-rs#1216", "aws-sdk-rust#466"]
+meta = { "breaking" = false, "tada" = false, "bug" = true }
+author = "jdisanti"

aws/rust-runtime/aws-inlineable/src/presigning.rs

@@ -219,7 +219,7 @@ pub mod request {
 pub(crate) mod service {
     use crate::presigning::request::PresignedRequest;
     use aws_smithy_http::operation;
-    use http::header::{CONTENT_LENGTH, CONTENT_TYPE, USER_AGENT};
+    use http::header::USER_AGENT;
     use std::future::{ready, Ready};
     use std::marker::PhantomData;
     use std::task::{Context, Poll};
@@ -262,12 +262,6 @@ pub(crate) mod service {
         fn call(&mut self, req: operation::Request) -> Self::Future {
             let (mut req, _) = req.into_parts();
 
-            // Remove headers from input serialization that shouldn't be part of the presigned
-            // request since the request body is unsigned and left up to the person making the final
-            // HTTP request.
-            req.headers_mut().remove(CONTENT_LENGTH);
-            req.headers_mut().remove(CONTENT_TYPE);
-
             // Remove user agent headers since the request will not be executed by the AWS Rust SDK.
             req.headers_mut().remove(USER_AGENT);
             req.headers_mut().remove("X-Amz-User-Agent");

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

@@ -10,7 +10,7 @@ use crate::http_request::sign::SignableRequest;
 use crate::http_request::url_escape::percent_encode_path;
 use crate::http_request::PercentEncodingMode;
 use crate::sign::sha256_hex_string;
-use http::header::{HeaderName, CONTENT_LENGTH, CONTENT_TYPE, HOST, USER_AGENT};
+use http::header::{HeaderName, HOST, USER_AGENT};
 use http::{HeaderMap, HeaderValue, Method, Uri};
 use std::borrow::Cow;
 use std::cmp::Ordering;
@@ -223,11 +223,6 @@ impl<'a> CanonicalRequest<'a> {
                 continue;
             }
             if params.settings.signature_location == SignatureLocation::QueryParams {
-                // Exclude content-length and content-type for query param signatures since the
-                // body is unsigned for these use-cases, and the size is not known up-front.
-                if name == CONTENT_LENGTH || name == CONTENT_TYPE {
-                    continue;
-                }
                 // 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) {
                     continue;
@@ -675,7 +670,7 @@ mod tests {
         assert_eq!(expected, actual);
     }
 
-    // It should exclude user-agent, content-type, content-length, and x-amz-user-agent headers from presigning
+    // It should exclude user-agent and x-amz-user-agent headers from presigning
     #[test]
     fn presigning_header_exclusion() {
         let request = http::Request::builder()
@@ -688,15 +683,20 @@ mod tests {
             .unwrap();
         let request = SignableRequest::from(&request);
 
-        let mut settings = SigningSettings::default();
-        settings.signature_location = SignatureLocation::QueryParams;
-        settings.expires_in = Some(Duration::from_secs(30));
+        let settings = SigningSettings {
+            signature_location: SignatureLocation::QueryParams,
+            expires_in: Some(Duration::from_secs(30)),
+            ..Default::default()
+        };
 
         let signing_params = signing_params(settings);
         let canonical = CanonicalRequest::from(&request, &signing_params).unwrap();
 
         let values = canonical.values.into_query_params().unwrap();
-        assert_eq!("host", values.signed_headers.as_str());
+        assert_eq!(
+            "content-length;content-type;host",
+            values.signed_headers.as_str()
+        );
     }
 
     #[test]

aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsPresigningDecorator.kt

@@ -154,22 +154,23 @@ class AwsInputPresignedMethod(
         val operationError = operationShape.errorSymbol(symbolProvider)
         val presignableOp = PRESIGNABLE_OPERATIONS.getValue(operationShape.id)
 
-        var makeOperationFn = "make_operation"
-        if (presignableOp.hasModelTransforms()) {
-            makeOperationFn = "_make_presigned_operation"
-
-            val syntheticOp =
-                codegenContext.model.expectShape(syntheticShapeId(operationShape.id), OperationShape::class.java)
-            val protocol = section.protocol
-            MakeOperationGenerator(
-                codegenContext,
-                protocol,
-                HttpBoundProtocolPayloadGenerator(codegenContext, protocol),
-                // Prefixed with underscore to avoid colliding with modeled functions
-                functionName = makeOperationFn,
-                public = false,
-            ).generateMakeOperation(this, syntheticOp, section.customizations)
+        val makeOperationOp = if (presignableOp.hasModelTransforms()) {
+            codegenContext.model.expectShape(syntheticShapeId(operationShape.id), OperationShape::class.java)
+        } else {
+            section.operationShape
         }
+        val makeOperationFn = "_make_presigned_operation"
+
+        val protocol = section.protocol
+        MakeOperationGenerator(
+            codegenContext,
+            protocol,
+            HttpBoundProtocolPayloadGenerator(codegenContext, protocol),
+            // Prefixed with underscore to avoid colliding with modeled functions
+            functionName = makeOperationFn,
+            public = false,
+            includeDefaultPayloadHeaders = false
+        ).generateMakeOperation(this, makeOperationOp, section.customizations)
 
         documentPresignedMethod(hasConfigArg = true)
         rustBlockTemplate(

aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4SigningDecorator.kt

@@ -157,10 +157,7 @@ class SigV4SigningFeature(
         return when (section) {
             is OperationSection.MutateRequest -> writable {
                 rustTemplate(
-                    """
-                    ##[allow(unused_mut)]
-                    let mut signing_config = #{sig_auth}::signer::OperationSigningConfig::default_config();
-                    """,
+                    "let mut signing_config = #{sig_auth}::signer::OperationSigningConfig::default_config();",
                     *codegenScope
                 )
                 if (needsAmzSha256(service)) {

aws/sdk/integration-tests/s3/tests/presigning.rs

@@ -4,39 +4,49 @@
  */
 
 use aws_sdk_s3 as s3;
+use aws_sdk_s3::presigning::request::PresignedRequest;
+use http::header::{CONTENT_LENGTH, CONTENT_TYPE};
+use http::{HeaderMap, HeaderValue};
 use s3::presigning::config::PresigningConfig;
 use std::error::Error;
 use std::time::{Duration, SystemTime};
 
+/// Generates a `PresignedRequest` from the given input.
+/// Assumes that that input has a `presigned` method on it.
+macro_rules! presign_input {
+    ($input:expr) => {{
+        let creds = s3::Credentials::new(
+            "ANOTREAL",
+            "notrealrnrELgWzOk3IfjzDKtFBhDby",
+            Some("notarealsessiontoken".to_string()),
+            None,
+            "test",
+        );
+        let config = s3::Config::builder()
+            .credentials_provider(creds)
+            .region(s3::Region::new("us-east-1"))
+            .build();
+
+        let req: PresignedRequest = $input
+            .presigned(
+                &config,
+                PresigningConfig::builder()
+                    .start_time(SystemTime::UNIX_EPOCH + Duration::from_secs(1234567891))
+                    .expires_in(Duration::from_secs(30))
+                    .build()
+                    .unwrap(),
+            )
+            .await?;
+        req
+    }};
+}
+
 #[tokio::test]
 async fn test_presigning() -> Result<(), Box<dyn Error>> {
-    let creds = s3::Credentials::new(
-        "ANOTREAL",
-        "notrealrnrELgWzOk3IfjzDKtFBhDby",
-        Some("notarealsessiontoken".to_string()),
-        None,
-        "test",
-    );
-    let config = s3::Config::builder()
-        .credentials_provider(creds)
-        .region(s3::Region::new("us-east-1"))
-        .build();
-
-    let input = s3::input::GetObjectInput::builder()
+    let presigned = presign_input!(s3::input::GetObjectInput::builder()
         .bucket("test-bucket")
         .key("test-key")
-        .build()?;
-
-    let presigned = input
-        .presigned(
-            &config,
-            PresigningConfig::builder()
-                .start_time(SystemTime::UNIX_EPOCH + Duration::from_secs(1234567891))
-                .expires_in(Duration::from_secs(30))
-                .build()
-                .unwrap(),
-        )
-        .await?;
+        .build()?);
 
     let pq = presigned.uri().path_and_query().unwrap();
     let path = pq.path();
@@ -63,3 +73,42 @@ async fn test_presigning() -> Result<(), Box<dyn Error>> {
 
     Ok(())
 }
+
+#[tokio::test]
+async fn test_presigning_with_payload_headers() -> Result<(), Box<dyn Error>> {
+    let presigned = presign_input!(s3::input::PutObjectInput::builder()
+        .bucket("test-bucket")
+        .key("test-key")
+        .content_length(12345)
+        .content_type("application/x-test")
+        .build()?);
+
+    let pq = presigned.uri().path_and_query().unwrap();
+    let path = pq.path();
+    let query = pq.query().unwrap();
+    let mut query_params: Vec<&str> = query.split('&').collect();
+    query_params.sort();
+
+    assert_eq!("PUT", presigned.method().as_str());
+    assert_eq!("/test-bucket/test-key", path);
+    assert_eq!(
+        &[
+            "X-Amz-Algorithm=AWS4-HMAC-SHA256",
+            "X-Amz-Credential=ANOTREAL%2F20090213%2Fus-east-1%2Fs3%2Faws4_request",
+            "X-Amz-Date=20090213T233131Z",
+            "X-Amz-Expires=30",
+            "X-Amz-Security-Token=notarealsessiontoken",
+            "X-Amz-Signature=6a22b8bf422d17fe25e7d9fcbd26df31397ca5e3ad07d1cec95326ffdbe4a0a2",
+            "X-Amz-SignedHeaders=content-length%3Bcontent-type%3Bhost",
+            "x-id=PutObject"
+        ][..],
+        &query_params
+    );
+
+    let mut expected_headers = HeaderMap::new();
+    expected_headers.insert(CONTENT_LENGTH, HeaderValue::from_static("12345"));
+    expected_headers.insert(CONTENT_TYPE, HeaderValue::from_static("application/x-test"));
+    assert_eq!(&expected_headers, presigned.headers());
+
+    Ok(())
+}

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpProtocolGenerator.kt

@@ -76,7 +76,13 @@ class ServerHttpProtocolGenerator(
 ) : ProtocolGenerator(
     codegenContext,
     protocol,
-    MakeOperationGenerator(codegenContext, protocol, HttpBoundProtocolPayloadGenerator(codegenContext, protocol)),
+    MakeOperationGenerator(
+        codegenContext,
+        protocol,
+        HttpBoundProtocolPayloadGenerator(codegenContext, protocol),
+        public = true,
+        includeDefaultPayloadHeaders = true
+    ),
     ServerHttpProtocolImplGenerator(codegenContext, protocol),
 ) {
     // Define suffixes for operation input / output / error wrappers
@@ -468,7 +474,7 @@ private class ServerHttpProtocolImplGenerator(
                 """,
                 *codegenScope,
             )
-        } ?:run {
+        } ?: run {
             val payloadGenerator = HttpBoundProtocolPayloadGenerator(codegenContext, protocol, httpMessageType = HttpMessageType.RESPONSE)
             withBlockTemplate("let body = #{SmithyHttpServer}::body::to_boxed(", ");", *codegenScope) {
                 payloadGenerator.generatePayload(this, "output", operationShape)