Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"SignatureDoesNotMatch" error with PresignGetObject #2531

Closed
dchoma-sy opened this issue Mar 5, 2024 · 4 comments
Closed

"SignatureDoesNotMatch" error with PresignGetObject #2531

dchoma-sy opened this issue Mar 5, 2024 · 4 comments

Comments

@dchoma-sy
Copy link

Describe the bug

After upgrading aws-sdk-go-v2 beyond v1.24.0, the PresignGetObject op adds to X-Amz-SignedHeaders which seems to always produce "SignatureDoesNotMatch" errors.

Expected Behavior

https://s3.us-east-2.amazonaws.com/
some_bucket/some_file.png
?X-Amz-Algorithm=AWS4-HMAC-SHA256&
X-Amz-Credential=REDACTED%2Fus-east-2%2Fs3%2Faws4_request&
X-Amz-Date=20240305T040452Z&
X-Amz-Expires=900&
X-Amz-Security-Token=REDACTED&
X-Amz-SignedHeaders=host&
x-id=GetObject&
X-Amz-Signature=REDACTED

URL retrieves object.

Current Behavior

https://s3.us-east-2.amazonaws.com/
some_bucket/some_file.png
?X-Amz-Algorithm=AWS4-HMAC-SHA256&
X-Amz-Credential=REDACTED%2Fus-east-2%2Fs3%2Faws4_request&
X-Amz-Date=20240305T040232Z&
X-Amz-Expires=900&
X-Amz-Security-Token=REDACTED&
X-Amz-SignedHeaders=amz-sdk-request%3Bhost&
x-id=GetObject&
X-Amz-Signature=REDACTED

SignatureDoesNotMatch
The request signature we calculated does not match the signature you provided. Check your key and signing method.

Reproduction Steps

go get github.com/aws/aws-sdk-go-v2@v1.24.1

Any version between v1.24.1 -> v1.25.2 exhibits the new behavior.

Possible Solution

Seems like it's introduced here...

diff --git a/aws/retry/middleware.go b/aws/retry/middleware.go
index 722ca34c6a..dc703d482d 100644
--- a/aws/retry/middleware.go
+++ b/aws/retry/middleware.go
@@ -328,10 +328,12 @@ func AddRetryMiddlewares(stack *smithymiddle.Stack, options AddRetryMiddlewaresO
                middleware.LogAttempts = options.LogRetryAttempts
        })

-       if err := stack.Finalize.Add(attempt, smithymiddle.After); err != nil {
+       // index retry to before signing, if signing exists
+       if err := stack.Finalize.Insert(attempt, "Signing", smithymiddle.Before); err != nil {
                return err
        }
-       if err := stack.Finalize.Add(&MetricsHeader{}, smithymiddle.After); err != nil {
+
+       if err := stack.Finalize.Insert(&MetricsHeader{}, attempt.ID(), smithymiddle.After); err != nil {
                return err
        }
        return nil

Reverting if err := stack.Finalize.Add(&MetricsHeader{}, smithymiddle.After); err != nil { fixes the issue for me.

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

github.com/aws/aws-sdk-go-v2 v1.24.1
github.com/aws/aws-sdk-go-v2/config v1.25.8
github.com/aws/aws-sdk-go-v2/credentials v1.16.6
github.com/aws/aws-sdk-go-v2/service/s3 v1.46.0

Compiler and Version used

go version go1.21.3 darwin/arm64

Operating System and version

MacOS 13.6.3 (22G436)

@dchoma-sy dchoma-sy added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 5, 2024
@lucix-aws
Copy link
Contributor

@dchoma-sy --

Your service/s3 module is out-of-date:

  • you are on v1.46.0
  • the latest release is v1.51.2

You can upgrade all of your sdk v2 dependencies by running

go get -u github.com/aws/aws-sdk-go-v2/...

This behavior does not occur against the latest set of released modules. I will provide my reproduction go.mod and sample program in a subsequent comment.

There was a set of recent changes that introduced compatibility issues with modules across releases. We are tracking long-term correction of this behavior in #2507. In general, updating all of your modules via the above command will ensure you do not hit this compatibility edge.

@lucix-aws lucix-aws removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 5, 2024
Copy link

github-actions bot commented Mar 5, 2024

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

@lucix-aws
Copy link
Contributor

go.mod:

module main

go 1.21.0

require (
	github.com/aws/aws-sdk-go-v2 v1.25.2
	github.com/aws/aws-sdk-go-v2/config v1.27.5
	github.com/aws/aws-sdk-go-v2/service/s3 v1.51.2
)

require (
	github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.1 // indirect
	github.com/aws/aws-sdk-go-v2/credentials v1.17.5 // indirect
	github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.15.2 // indirect
	github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.2 // indirect
	github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.2 // indirect
	github.com/aws/aws-sdk-go-v2/internal/ini v1.8.0 // indirect
	github.com/aws/aws-sdk-go-v2/internal/v4a v1.3.2 // indirect
	github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.1 // indirect
	github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.3.3 // indirect
	github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.3 // indirect
	github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.17.2 // indirect
	github.com/aws/aws-sdk-go-v2/service/sso v1.20.1 // indirect
	github.com/aws/aws-sdk-go-v2/service/ssooidc v1.23.1 // indirect
	github.com/aws/aws-sdk-go-v2/service/sts v1.28.2 // indirect
	github.com/aws/smithy-go v1.20.1 // indirect
)

main.go:

package main

import (
	"context"
	"fmt"
	"log"

	"github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/config"
	"github.com/aws/aws-sdk-go-v2/service/s3"
)

func main() {
	cfg, err := config.LoadDefaultConfig(context.Background())
	if err != nil {
		log.Fatal(err)
	}

	svc := s3.NewFromConfig(cfg)
	presign := s3.NewPresignClient(svc)

	req, err := presign.PresignGetObject(context.Background(), &s3.GetObjectInput{
		Bucket: aws.String("..."),
		Key:    aws.String("..."),
	})
	if err != nil {
		log.Fatal(err)
	}

	fmt.Println(req.URL)
}

@dchoma-sy
Copy link
Author

Thanks @lucix-aws! Much appreciated help. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants