Skip to content

Commit

Permalink
service/s3: make streaming operations use unsigned payload by default (
Browse files Browse the repository at this point in the history
…#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.
  • Loading branch information
skotambkar authored Oct 21, 2021
1 parent cf6f142 commit 98b705e
Show file tree
Hide file tree
Showing 10 changed files with 180 additions and 8 deletions.
8 changes: 8 additions & 0 deletions .changelog/41daeba51bff488e951008a60dfa2229.json
Original file line number Diff line number Diff line change
@@ -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"
]
}
8 changes: 8 additions & 0 deletions .changelog/e5ca03635cf246a6927b2c99ec0f2fc2.json
Original file line number Diff line number Diff line change
@@ -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": [
"."
]
}
51 changes: 47 additions & 4 deletions aws/signer/v4/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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)}
}
Expand Down Expand Up @@ -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)}
}
Expand Down
64 changes: 64 additions & 0 deletions aws/signer/v4/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"io"
"net/http"
"net/url"
"strconv"
"strings"
"testing"
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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;


Expand Down Expand Up @@ -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()
);
}
Expand Down
2 changes: 0 additions & 2 deletions service/internal/integrationtest/s3/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package s3

import (
"bytes"
"strings"
"testing"
)
Expand All @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions service/s3/api_op_PutObject.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions service/s3/api_op_UploadPart.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions service/s3/api_op_WriteGetObjectResponse.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 98b705e

Please sign in to comment.