service/s3: make streaming operations use unsigned payload by default (#1354)

Adds SwapPayloadSHA256ResolverMiddleware that swaps computPayloadSHA256 middleware if an S3 operation uses streaming payload.

For S3 operations with streaming payload, we use unsigned payload strategy if TLS is enabled.

Author: skotambkar

.changelog/41daeba51bff488e951008a60dfa2229.json

@@ -0,0 +1,8 @@
+{
+    "id": "41daeba5-1bff-488e-9510-08a60dfa2229",
+    "type": "feature",
+    "description": "Updates S3 streaming operations - PutObject, UploadPart, WriteGetObjectResponse to use unsigned payload signing auth when TLS is enabled.",
+    "modules": [
+        "service/s3"
+    ]
+}

.changelog/e5ca03635cf246a6927b2c99ec0f2fc2.json

@@ -0,0 +1,8 @@
+{
+    "id": "e5ca0363-5cf2-46a6-927b-2c99ec0f2fc2",
+    "type": "feature",
+    "description": "Adds dynamic signing middleware that switches to unsigned payload when TLS is enabled.",
+    "modules": [
+        "."
+    ]
+}

aws/signer/v4/middleware.go

@@ -6,13 +6,14 @@ import (
 	"encoding/hex"
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	awsmiddleware "github.com/aws/aws-sdk-go-v2/aws/middleware"
 	v4Internal "github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4"
 	"github.com/aws/aws-sdk-go-v2/internal/sdk"
 	"github.com/aws/smithy-go/middleware"
-	smithyHTTP "github.com/aws/smithy-go/transport/http"
+	smithyhttp "github.com/aws/smithy-go/transport/http"
 )
 
 const computePayloadHashMiddlewareID = "ComputePayloadHash"
@@ -46,6 +47,48 @@ func (e *SigningError) Unwrap() error {
 	return e.Err
 }
 
+// UseDynamicPayloadSigningMiddleware swaps the compute payload sha256 middleware with a resolver middleware that
+// switches between unsigned and signed payload based on TLS state for request.
+// This middleware should not be used for AWS APIs that do not support unsigned payload signing auth.
+// By default, SDK uses this middleware for known AWS APIs that support such TLS based auth selection .
+//
+// Usage example -
+// S3 PutObject API allows unsigned payload signing auth usage when TLS is enabled, and uses this middleware to
+// dynamically switch between unsigned and signed payload based on TLS state for request.
+func UseDynamicPayloadSigningMiddleware(stack *middleware.Stack) error {
+	_, err := stack.Build.Swap(computePayloadHashMiddlewareID, &dynamicPayloadSigningMiddleware{})
+	return err
+}
+
+// dynamicPayloadSigningMiddleware dynamically resolves the middleware that computes and set payload sha256 middleware.
+type dynamicPayloadSigningMiddleware struct {
+}
+
+// ID returns the resolver identifier
+func (m *dynamicPayloadSigningMiddleware) ID() string {
+	return computePayloadHashMiddlewareID
+}
+
+// HandleBuild sets a resolver that directs to the payload sha256 compute handler.
+func (m *dynamicPayloadSigningMiddleware) HandleBuild(
+	ctx context.Context, in middleware.BuildInput, next middleware.BuildHandler,
+) (
+	out middleware.BuildOutput, metadata middleware.Metadata, err error,
+) {
+	req, ok := in.Request.(*smithyhttp.Request)
+	if !ok {
+		return out, metadata, fmt.Errorf("unknown transport type %T", in.Request)
+	}
+
+	// if TLS is enabled, use unsigned payload when supported
+	if strings.EqualFold(req.URL.Scheme, "https") {
+		return (&unsignedPayload{}).HandleBuild(ctx, in, next)
+	}
+
+	// else fall back to signed payload
+	return (&computePayloadSHA256{}).HandleBuild(ctx, in, next)
+}
+
 // unsignedPayload sets the SigV4 request payload hash to unsigned.
 //
 // Will not set the Unsigned Payload magic SHA value, if a SHA has already been
@@ -120,7 +163,7 @@ func (m *computePayloadSHA256) HandleBuild(
 ) (
 	out middleware.BuildOutput, metadata middleware.Metadata, err error,
 ) {
-	req, ok := in.Request.(*smithyHTTP.Request)
+	req, ok := in.Request.(*smithyhttp.Request)
 	if !ok {
 		return out, metadata, &HashComputationError{
 			Err: fmt.Errorf("unexpected request middleware type %T", in.Request),
@@ -195,7 +238,7 @@ func (m *contentSHA256Header) HandleBuild(
 ) (
 	out middleware.BuildOutput, metadata middleware.Metadata, err error,
 ) {
-	req, ok := in.Request.(*smithyHTTP.Request)
+	req, ok := in.Request.(*smithyhttp.Request)
 	if !ok {
 		return out, metadata, &HashComputationError{Err: fmt.Errorf("unexpected request middleware type %T", in.Request)}
 	}
@@ -241,7 +284,7 @@ func (s *SignHTTPRequestMiddleware) HandleFinalize(ctx context.Context, in middl
 		return next.HandleFinalize(ctx, in)
 	}
 
-	req, ok := in.Request.(*smithyHTTP.Request)
+	req, ok := in.Request.(*smithyhttp.Request)
 	if !ok {
 		return out, metadata, &SigningError{Err: fmt.Errorf("unexpected request middleware type %T", in.Request)}
 	}

aws/signer/v4/middleware_test.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"strconv"
 	"strings"
 	"testing"
@@ -306,6 +307,69 @@ func TestSwapComputePayloadSHA256ForUnsignedPayloadMiddleware(t *testing.T) {
 	}
 }
 
+func TestUseDynamicPayloadSigningMiddleware(t *testing.T) {
+	cases := map[string]struct {
+		content      io.Reader
+		url          string
+		expectedHash string
+		expectedErr  error
+	}{
+		"TLS disabled": {
+			content: func() io.Reader {
+				br := bytes.NewReader([]byte("some content"))
+				return br
+			}(),
+			url:          "http://localhost.com/",
+			expectedHash: "290f493c44f5d63d06b374d0a5abd292fae38b92cab2fae5efefe1b0e9347f56",
+		},
+		"TLS enabled": {
+			content: func() io.Reader {
+				br := bytes.NewReader([]byte("some content"))
+				return br
+			}(),
+			url:          "https://localhost.com/",
+			expectedHash: "UNSIGNED-PAYLOAD",
+		},
+	}
+
+	for name, tt := range cases {
+		t.Run(name, func(t *testing.T) {
+			c := &dynamicPayloadSigningMiddleware{}
+
+			next := middleware.BuildHandlerFunc(func(ctx context.Context, in middleware.BuildInput) (out middleware.BuildOutput, metadata middleware.Metadata, err error) {
+				value := GetPayloadHash(ctx)
+				if len(value) == 0 {
+					t.Fatalf("expected payload hash value to be on context")
+				}
+				if e, a := tt.expectedHash, value; e != a {
+					t.Errorf("expected %v, got %v", e, a)
+				}
+
+				return out, metadata, err
+			})
+
+			req := smithyhttp.NewStackRequest().(*smithyhttp.Request)
+			req.URL, _ = url.Parse(tt.url)
+			stream, err := req.SetStream(tt.content)
+			if err != nil {
+				t.Fatalf("expected no error, got %v", err)
+			}
+
+			_, _, err = c.HandleBuild(context.Background(), middleware.BuildInput{Request: stream}, next)
+			if err != nil && tt.expectedErr == nil {
+				t.Errorf("expected no error, got %v", err)
+			} else if err != nil && tt.expectedErr != nil {
+				e, a := tt.expectedErr, err
+				if !errors.As(a, &e) {
+					t.Errorf("expected error type %T, got %T", e, a)
+				}
+			} else if err == nil && tt.expectedErr != nil {
+				t.Errorf("expected error, got nil")
+			}
+		})
+	}
+}
+
 type nonSeeker struct{}
 
 func (nonSeeker) Read(p []byte) (n int, err error) {

codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/customization/S3ContentSHA256Header.java

@@ -1,12 +1,18 @@
 package software.amazon.smithy.aws.go.codegen.customization;
 
 import java.util.List;
+import java.util.Optional;
 import software.amazon.smithy.aws.go.codegen.AwsGoDependency;
 import software.amazon.smithy.aws.traits.auth.UnsignedPayloadTrait;
 import software.amazon.smithy.go.codegen.SymbolUtils;
 import software.amazon.smithy.go.codegen.integration.GoIntegration;
 import software.amazon.smithy.go.codegen.integration.MiddlewareRegistrar;
 import software.amazon.smithy.go.codegen.integration.RuntimeClientPlugin;
+import software.amazon.smithy.model.shapes.MemberShape;
+import software.amazon.smithy.model.shapes.Shape;
+import software.amazon.smithy.model.shapes.ShapeId;
+import software.amazon.smithy.model.shapes.StructureShape;
+import software.amazon.smithy.model.traits.StreamingTrait;
 import software.amazon.smithy.utils.ListUtils;
 
 
@@ -42,6 +48,37 @@ public List<RuntimeClientPlugin> getClientPlugins() {
                                         AwsGoDependency.AWS_SIGNER_V4
                                 ).build())
                                 .build())
+                        .build(),
+                RuntimeClientPlugin.builder()
+                        // If a S3 operation has a streaming payload but is not event stream payload,
+                        // client swaps signing middleware to use dynamic payload signing middleware.
+                        // This enables client to use unsigned payload when TLS is enabled, and switch
+                        // to signed payload for security when TLS is disabled.
+                        .operationPredicate(((model, service, operation) -> {
+                            if (!(S3ModelUtils.isServiceS3(model, service))) {
+                                return false;
+                            }
+
+                            Optional<ShapeId> input = operation.getInput();
+                            if (!input.isPresent()) {
+                                return false;
+                            }
+
+                            StructureShape inputShape = model.expectShape(input.get(), StructureShape.class);
+                            for (MemberShape memberShape : inputShape.getAllMembers().values()) {
+                                Shape targetShape = model.expectShape(memberShape.getTarget());
+                                if (targetShape.hasTrait(StreamingTrait.class)
+                                        && !StreamingTrait.isEventStream(model, memberShape)) {
+                                    return true;
+                                }
+                            }
+                            return false;
+                        }))
+                        .registerMiddleware(MiddlewareRegistrar.builder()
+                                .resolvedFunction(SymbolUtils.createValueSymbolBuilder(
+                                        "UseDynamicPayloadSigningMiddleware", AwsGoDependency.AWS_SIGNER_V4
+                                ).build())
+                                .build())
                         .build()
         );
     }

service/internal/integrationtest/s3/api_test.go

@@ -4,7 +4,6 @@
 package s3
 
 import (
-	"bytes"
 	"strings"
 	"testing"
 )
@@ -14,7 +13,6 @@ func TestInteg_WriteToObject(t *testing.T) {
 		"seekable body":     {Body: strings.NewReader("hello world"), ExpectBody: []byte("hello world")},
 		"empty string body": {Body: strings.NewReader(""), ExpectBody: []byte("")},
 		"nil body":          {Body: nil, ExpectBody: []byte("")},
-		"unseekable body":   {Body: bytes.NewBuffer([]byte("hello world")), ExpectError: "failed to compute payload hash"},
 	}
 
 	for name, c := range cases {

service/s3/api_op_PutObject.go

@@ -3,6 +3,7 @@ package customizations_test
 import (
 	"bytes"
 	"context"
+	"crypto/tls"
 	"fmt"
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/service/s3"
@@ -191,11 +192,15 @@ func TestWriteGetObjectResponse(t *testing.T) {
 
 	for name, tt := range cases {
 		t.Run(name, func(t *testing.T) {
-			server := httptest.NewServer(tt.Handler(t))
+			server := httptest.NewTLSServer(tt.Handler(t))
 			defer server.Close()
-
 			client := s3.New(s3.Options{
 				Region: "us-west-2",
+				HTTPClient: &http.Client{
+					Transport: &http.Transport{
+						TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
+					},
+				},
 				EndpointResolver: s3.EndpointResolverFunc(func(region string, options s3.EndpointResolverOptions) (aws.Endpoint, error) {
 					return aws.Endpoint{
 						URL:               server.URL,