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

Polling causes SignatureDoesNotMatch error #521

Closed
3 tasks done
akupila opened this issue Apr 2, 2020 · 6 comments · Fixed by #537 or #539
Closed
3 tasks done

Polling causes SignatureDoesNotMatch error #521

akupila opened this issue Apr 2, 2020 · 6 comments · Fixed by #537 or #539
Labels
bug This issue is a bug. investigating This issue is being investigated and/or work is in progress to resolve the issue.

Comments

@akupila
Copy link

akupila commented Apr 2, 2020

Confirm by changing [ ] to [x] below to ensure that it's a bug:

Describe the bug
In a polling scenario, a repeated request may return the following error:

SignatureDoesNotMatch: The request signature we calculated does not match the signature you provided. Check your AWS Secret Access Key and signing method. Consult the service documentation for details.

The request that fails is identical to a request that previously succeeded. See below for code.

Version of AWS SDK for Go?
v0.20.0. This does not happen with v0.19.0

Version of Go (go version)?
go version go1.14.1 linux/amd64

To Reproduce (observed behavior)

package main

import (
	"context"
	"flag"
	"fmt"
	"log"
	"os"
	"time"

	"github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/aws/external"
	"github.com/aws/aws-sdk-go-v2/service/cloudformation"
)

func main() {
	stack := flag.String("stack", "", "Stack name")
	wait := flag.Duration("wait", 100*time.Millisecond, "Poll wait time")
	flag.Parse()

	if *stack == "" {
		flag.Usage()
		os.Exit(2)
	}

	log.SetFlags(log.Lmicroseconds)

	if err := run(*stack, *wait); err != nil {
		fmt.Fprintln(os.Stderr, err)
		os.Exit(1)
	}
}

func run(stackName string, wait time.Duration) error {
	cfg, err := external.LoadDefaultAWSConfig()
	if err != nil {
		return fmt.Errorf("load aws config: %v", err)
	}
	cf := cloudformation.New(cfg)

	n := 100
	for i := 0; i < n; i++ {
		log.Printf("Request %d", i)
		t := time.Now()
		req := cf.DescribeStackEventsRequest(&cloudformation.DescribeStackEventsInput{
			StackName: aws.String(stackName),
		})
		_, err := req.Send(context.TODO())
		if err != nil {
			return fmt.Errorf("describe stack events: %v", err)
		}
		log.Printf("Completed in %s", time.Since(t))
		time.Sleep(wait)
	}

	return nil
}
$ go run . -stack <valid cloudformation stack>

After ~18 requests, this returns an error with v0.20.0 but all 100 requests succeed on v0.19.0.

Expected behavior
The code above should work the same as in v0.19.0, no error should be returned.

Additional context
I'm not sure if CloudFormation is significant here (that's how i encountered this). As changes to signing and retries were done in v0.20.0 and the issue only appears after some time, i'm guessing the server throttles the request, which causes the client to retry, but the retry doesn't doesn't create the correct signature. Changing the wait time to longer than the default 100ms also allows the code above to run on v0.20.0, which further supports the theory that this is rate limiting/retry related.

@akupila
Copy link
Author

akupila commented Apr 2, 2020

Enabling logging (cfg.LogLevel = aws.LogDebug) shows that the request in v0.20.0 has more signed headers

v0.19.0

2020/04/02 11:40:47 DEBUG: Request AWS CloudFormation/DescribeStackEvents Details:
---[ REQUEST POST-SIGN ]-----------------------------
POST / HTTP/1.1
Host: cloudformation.eu-central-1.amazonaws.com
User-Agent: aws-sdk-go/0.19.0 (go1.14.1; linux; amd64)
Content-Length: 64
Authorization: AWS4-HMAC-SHA256 Credential=AKIAICHWxxxxxxxxxxxx/20200402/eu-central-1/cloudformation/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-date, Signature=9a5e41bc3cf30e4ea69a34a722a734e3f9cc2816ef33e8c16ecbxxxxxxxxxxxx
Content-Type: application/x-www-form-urlencoded; charset=utf-8
X-Amz-Date: 20200402T094047Z
Accept-Encoding: gzip


-----------------------------------------------------
2020/04/02 11:40:47 DEBUG: Response AWS CloudFormation/DescribeStackEvents Details:
---[ RESPONSE ]--------------------------------------
HTTP/1.1 200 OK
Transfer-Encoding: chunked
Content-Type: text/xml
Date: Thu, 02 Apr 2020 09:41:19 GMT
Vary: accept-encoding
X-Amzn-Requestid: a7655173-9548-490a-9b11-a0bfe621653c


-----------------------------------------------------

v0.20.0

2020/04/02 11:43:00 DEBUG: Request AWS CloudFormation/DescribeStackEvents Details:
---[ REQUEST POST-SIGN ]-----------------------------
POST / HTTP/1.1
Host: cloudformation.eu-central-1.amazonaws.com
User-Agent: aws-sdk-go/0.20.0 (go1.14.1; linux; amd64)
Content-Length: 64
Amz-Sdk-Invocation-Id: A91B0AA1-5E80-4357-B1E6-D16A84D502A5
Amz-Sdk-Request: attempt=1; max=3
Authorization: AWS4-HMAC-SHA256 Credential=AKIAICHWxxxxxxxxxxxx/20200402/eu-central-1/cloudformation/aws4_request, SignedHeaders=amz-sdk-invocation-id;amz-sdk-request;content-length;content-type;host;x-amz-date, Signature=0728d528338902d1b9e77f19a96473318e3ce7505f6321ff05c0xxxxxxxxxxxx
Content-Type: application/x-www-form-urlencoded; charset=utf-8
X-Amz-Date: 20200402T094300Z
Accept-Encoding: gzip


-----------------------------------------------------
2020/04/02 11:43:00 DEBUG: Response AWS CloudFormation/DescribeStackEvents Details:
---[ RESPONSE ]--------------------------------------
HTTP/1.1 403 Forbidden
Content-Length: 441
Content-Type: text/xml
Date: Thu, 02 Apr 2020 09:43:33 GMT
X-Amzn-Requestid: 1dc1d7ce-4961-41a5-ad74-a58b77a43a5c


-----------------------------------------------------

@dudududi
Copy link

I have observed the same issue with v0.20.0, however in my opinion it is not related to wait time between the subsequent requests - it is failing randomly with random clients (not only cfn, I have reproduced it with dynamodb as well)

@dudududi
Copy link

Any update on this? In my opinion this makes the v.0.20.0 not usable at all...

@jasdel
Copy link
Contributor

jasdel commented Apr 21, 2020

Hi @dudududi thanks for reaching out and letting us know about this issue. We're investigating this issue. This issue and #533 seem to be related to something in the signature getting changed after signing on retries.

We're able to reproduce the issue using the sample code you provided.

@jasdel jasdel added bug This issue is a bug. investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Apr 21, 2020
jasdel added a commit to jasdel/aws-sdk-go-v2 that referenced this issue Apr 21, 2020
Fixes the SDK's adding the request metadata headers in the wrong
location within the request handler stack. This created a situation
where a request that was retried would sign the new attempt using the
old value of the header. The header value would then be changed before
sending the request.

This moves the request invocation id to only occur during build,
maintaining its value across all requests attempts. Also moves request
retry metadata header to be before sign, and after build. This ensures
that a new value for each attempt can be set, and included in each
request attempt.

Fix aws#533
Fix aws#521
@jasdel
Copy link
Contributor

jasdel commented Apr 21, 2020

I've created PR #537 that fixes this bug, and allows the SDK to retry failed correctly without signature errors. Once this is merged in we'll include it in our latest release.

jasdel added a commit to jasdel/aws-sdk-go-v2 that referenced this issue Apr 21, 2020
Fixes the SDK's adding the request metadata headers in the wrong
location within the request handler stack. This created a situation
where a request that was retried would sign the new attempt using the
old value of the header. The header value would then be changed before
sending the request.

This moves the request invocation id to only occur during build,
maintaining its value across all requests attempts. Also moves request
retry metadata header to be before sign, and after build. This ensures
that a new value for each attempt can be set, and included in each
request attempt.

Fix aws#533
Fix aws#521
jasdel added a commit that referenced this issue Apr 21, 2020
…537)

Fixes the SDK's adding the request metadata headers in the wrong
location within the request handler stack. This created a situation
where a request that was retried would sign the new attempt using the
old value of the header. The header value would then be changed before
sending the request.

This moves the request invocation id to only occur during build,
maintaining its value across all requests attempts. Also moves request
retry metadata header to be before sign, and after build. This ensures
that a new value for each attempt can be set, and included in each
request attempt.

Fix #533
Fix #521
jasdel added a commit that referenced this issue Apr 21, 2020
===

Breaking Change
---
* `aws/endpoints`: Several functions and types have been removed
  * Removes `DecodeModel` and `DecodeModelOptions` from the package ([#509](#509))
  * Remove Region Constants, Partition Constants, and types use for exploring the endpoint data model ([#512](#512))
* `service/s3/s3crypto`: Package and associated encryption/decryption clients have been removed from the SDK ([#511](#511))
* `aws/external`: Removes several export constants and types ([#508](#508))
  * No longer exports AWS environment constants used by the external environment configuration loader
  * `DefaultSharedConfigProfile` is now defined an exported constant
* `aws`: `ErrMissingRegion`, `ErrMissingEndpoint`, `ErrStaticCredentialsEmpty` are now concrete error types ([#510](#510))

Services
---
* Synced the V2 SDK with latest AWS service API definitions.

SDK Features
---
* `aws/signer/v4`: New methods `SignHTTP` and `PresignHTTP` have been added ([#519](#519))
  * `SignHTTP` replaces `Sign`, and usage of `Sign` should be migrated before it's removal at a later date
  * `PresignHTTP` replaces `Presign`, and usage of `Presign` should be migrated before it's removal at a later date
  * `DisableRequestBodyOverwrite` and `UnsignedPayload` are now deprecated options and have no effect on `SignHTTP` or `PresignHTTP`. These options will be removed at a later date.
* `aws/external`: Add Support for setting a default fallback region and resolving region from EC2 IMDS ([#523](#523))
  * `WithDefaultRegion` helper has been added which can be passed to `LoadDefaultAWSConfig`
    * This helper can be used to configure a default fallback region in the event a region fails to be resolved from other sources
  * Support has been added to resolve region using EC2 IMDS when available
    * The IMDS region will be used if region as not found configured in either the shared config or the process environment.
  * Fixes [#244](#244)
  * Fixes [#515](#515)
SDK Enhancements
---
* `service/dynamodb/expression`: Add IsSet helper for ConditionBuilder and KeyConditionBuilder ([#494](#494))
  * Adds a IsSet helper for ConditionBuilder and KeyConditionBuilder to make it easier to determine if the condition builders have any conditions added to them.
  * Implements [#493](#493).
* `internal/ini`: Normalize Section keys to lowercase ([#495](#495))
  * Update's SDK's ini utility to store all keys as lowercase. This brings the SDK inline with the AWS CLI's behavior.

SDK Bugs
---
* `internal/sdk`: Fix SDK's UUID utility to handle partial read ([#536](#536))
  * Fixes the SDK's UUID utility to correctly handle partial reads from its crypto rand source. This error was sometimes causing the SDK's InvocationID value to fail to be obtained, due to a partial read from crypto.Rand.
  * Fix [#534](#534)
* `aws/defaults`: Fix request metadata headers causing signature errors ([#536](#536))
    * Fixes the SDK's adding the request metadata headers in the wrong location within the request handler stack. This created a situation where a request that was retried would sign the new attempt using the old value of the header. The header value would then be changed before sending the request.
    * Fix [#533](#533)
    * Fix [#521](#521)
jasdel added a commit that referenced this issue Apr 22, 2020
Breaking Change
---
* `aws/endpoints`: Several functions and types have been removed
  * Removes `DecodeModel` and `DecodeModelOptions` from the package ([#509](#509))
  * Remove Region Constants, Partition Constants, and types use for exploring the endpoint data model ([#512](#512))
* `service/s3/s3crypto`: Package and associated encryption/decryption clients have been removed from the SDK ([#511](#511))
* `aws/external`: Removes several export constants and types ([#508](#508))
  * No longer exports AWS environment constants used by the external environment configuration loader
  * `DefaultSharedConfigProfile` is now defined an exported constant
* `aws`: `ErrMissingRegion`, `ErrMissingEndpoint`, `ErrStaticCredentialsEmpty` are now concrete error types ([#510](#510))

Services
---
* Synced the V2 SDK with latest AWS service API definitions.

SDK Features
---
* `aws/signer/v4`: New methods `SignHTTP` and `PresignHTTP` have been added ([#519](#519))
  * `SignHTTP` replaces `Sign`, and usage of `Sign` should be migrated before it's removal at a later date
  * `PresignHTTP` replaces `Presign`, and usage of `Presign` should be migrated before it's removal at a later date
  * `DisableRequestBodyOverwrite` and `UnsignedPayload` are now deprecated options and have no effect on `SignHTTP` or `PresignHTTP`. These options will be removed at a later date.
* `aws/external`: Add Support for setting a default fallback region and resolving region from EC2 IMDS ([#523](#523))
  * `WithDefaultRegion` helper has been added which can be passed to `LoadDefaultAWSConfig`
    * This helper can be used to configure a default fallback region in the event a region fails to be resolved from other sources
  * Support has been added to resolve region using EC2 IMDS when available
    * The IMDS region will be used if region as not found configured in either the shared config or the process environment.
  * Fixes [#244](#244)
  * Fixes [#515](#515)

SDK Enhancements
---
* `service/dynamodb/expression`: Add IsSet helper for ConditionBuilder and KeyConditionBuilder ([#494](#494))
  * Adds a IsSet helper for ConditionBuilder and KeyConditionBuilder to make it easier to determine if the condition builders have any conditions added to them.
  * Implements [#493](#493).
* `internal/ini`: Normalize Section keys to lowercase ([#495](#495))
  * Update's SDK's ini utility to store all keys as lowercase. This brings the SDK inline with the AWS CLI's behavior.


SDK Bugs
---
* `internal/sdk`: Fix SDK's UUID utility to handle partial read ([#536](#536))
  * Fixes the SDK's UUID utility to correctly handle partial reads from its crypto rand source. This error was sometimes causing the SDK's InvocationID value to fail to be obtained, due to a partial read from crypto.Rand.
  * Fix [#534](#534)
* `aws/defaults`: Fix request metadata headers causing signature errors ([#536](#536))
    * Fixes the SDK's adding the request metadata headers in the wrong location within the request handler stack. This created a situation where a request that was retried would sign the new attempt using the old value of the header. The header value would then be changed before sending the request.
    * Fix [#533](#533)
    * Fix [#521](#521)
@jasdel
Copy link
Contributor

jasdel commented Apr 22, 2020

We've released this fix as a tagged released, v0.21.0. You should be able to update to pull in these fixes and updates.

akupila added a commit to akupila/func that referenced this issue Apr 29, 2020
Fixes issue that caused retries to fail:
aws/aws-sdk-go-v2#521
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. investigating This issue is being investigated and/or work is in progress to resolve the issue.
Projects
None yet
3 participants