aws/signer/v4: Remove logic determining if request needs to be resign…

…ed (#876)

Improves the relyability of the request signature by resigning the
request each retry. In addition logic was added to the Send request
handler chain to ensure the delay between Sign and Send will not prevent
expired signature. This also brings the SDK in line with the other AWS
SDKs in their behavior of resigning the request each retry.

Related to #822

aws/corehandlers/handlers.go

@@ -10,9 +10,11 @@ import (
 	"regexp"
 	"runtime"
 	"strconv"
+	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
+	"github.com/aws/aws-sdk-go/aws/credentials"
 	"github.com/aws/aws-sdk-go/aws/request"
 )
 
@@ -67,6 +69,34 @@ var SDKVersionUserAgentHandler = request.NamedHandler{
 
 var reStatusCode = regexp.MustCompile(`^(\d{3})`)
 
+// ValidateReqSigHandler is a request handler to ensure that the request's
+// signature doesn't expire before it is sent. This can happen when a request
+// is built and signed signficantly before it is sent. Or signficant delays
+// occur whne retrying requests that would cause the signature to expire.
+var ValidateReqSigHandler = request.NamedHandler{
+	Name: "core.ValidateReqSigHandler",
+	Fn: func(r *request.Request) {
+		// Unsigned requests are not signed
+		if r.Config.Credentials == credentials.AnonymousCredentials {
+			return
+		}
+
+		signedTime := r.Time
+		if !r.LastSignedAt.IsZero() {
+			signedTime = r.LastSignedAt
+		}
+
+		// 10 minutes to allow for some clock skew/delays in transmission.
+		// Would be improved with aws/aws-sdk-go#423
+		if signedTime.Add(10 * time.Minute).After(time.Now()) {
+			return
+		}
+
+		fmt.Println("request expired, resigning")
+		r.Sign()
+	},
+}
+
 // SendHandler is a request handler to send service request using HTTP client.
 var SendHandler = request.NamedHandler{Name: "core.SendHandler", Fn: func(r *request.Request) {
 	var err error

aws/corehandlers/handlers_test.go

@@ -8,6 +8,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 
@@ -117,6 +118,45 @@ func TestSendHandlerError(t *testing.T) {
 	assert.NotNil(t, r.HTTPResponse)
 }
 
+func TestValidateReqSigHandler(t *testing.T) {
+	cases := []struct {
+		Req    *request.Request
+		Resign bool
+	}{
+		{
+			Req: &request.Request{
+				Config: aws.Config{Credentials: credentials.AnonymousCredentials},
+				Time:   time.Now().Add(-15 * time.Minute),
+			},
+			Resign: false,
+		},
+		{
+			Req: &request.Request{
+				Time: time.Now().Add(-15 * time.Minute),
+			},
+			Resign: true,
+		},
+		{
+			Req: &request.Request{
+				Time: time.Now().Add(-1 * time.Minute),
+			},
+			Resign: false,
+		},
+	}
+
+	for i, c := range cases {
+		resigned := false
+		c.Req.Handlers.Sign.PushBack(func(r *request.Request) {
+			resigned = true
+		})
+
+		corehandlers.ValidateReqSigHandler.Fn(c.Req)
+
+		assert.NoError(t, c.Req.Error, "%d, expect no error", i)
+		assert.Equal(t, c.Resign, resigned, "%d, expected resigning to match", i)
+	}
+}
+
 func setupContentLengthTestServer(t *testing.T, hasContentLength bool, contentLength int64) *httptest.Server {
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		_, ok := r.Header["Content-Length"]

aws/defaults/defaults.go

@@ -72,6 +72,7 @@ func Handlers() request.Handlers {
 	handlers.Build.PushBackNamed(corehandlers.SDKVersionUserAgentHandler)
 	handlers.Build.AfterEachFn = request.HandlerListStopOnError
 	handlers.Sign.PushBackNamed(corehandlers.BuildContentLengthHandler)
+	handlers.Send.PushBackNamed(corehandlers.ValidateReqSigHandler)
 	handlers.Send.PushBackNamed(corehandlers.SendHandler)
 	handlers.AfterRetry.PushBackNamed(corehandlers.AfterRetryHandler)
 	handlers.ValidateResponse.PushBackNamed(corehandlers.ValidateResponseHandler)

aws/signer/v4/v4.go

@@ -247,11 +247,6 @@ func (v4 Signer) signWithBody(r *http.Request, body io.ReadSeeker, service, regi
 	}
 
 	if ctx.isRequestSigned() {
-		if !v4.Credentials.IsExpired() && currentTimeFn().Before(ctx.Time.Add(10*time.Minute)) {
-			// If the request is already signed, and the credentials have not
-			// expired, and the request is not too old ignore the signing request.
-			return ctx.SignedHeaderVals, nil
-		}
 		ctx.Time = currentTimeFn()
 		ctx.handlePresignRemoval()
 	}

aws/signer/v4/v4_test.go

@@ -2,6 +2,7 @@ package v4
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"net/http"
@@ -189,8 +190,12 @@ func TestIgnoreResignRequestWithValidCreds(t *testing.T) {
 	SignSDKRequest(r)
 	sig := r.HTTPRequest.Header.Get("Authorization")
 
-	SignSDKRequest(r)
-	assert.Equal(t, sig, r.HTTPRequest.Header.Get("Authorization"))
+	signSDKRequestWithCurrTime(r, func() time.Time {
+		// Simulate one second has passed so that signature's date changes
+		// when it is resigned.
+		return time.Now().Add(1 * time.Second)
+	})
+	assert.NotEqual(t, sig, r.HTTPRequest.Header.Get("Authorization"))
 }
 
 func TestIgnorePreResignRequestWithValidCreds(t *testing.T) {
@@ -210,10 +215,15 @@ func TestIgnorePreResignRequestWithValidCreds(t *testing.T) {
 	r.ExpireTime = time.Minute * 10
 
 	SignSDKRequest(r)
-	sig := r.HTTPRequest.Header.Get("X-Amz-Signature")
+	sig := r.HTTPRequest.URL.Query().Get("X-Amz-Signature")
 
-	SignSDKRequest(r)
-	assert.Equal(t, sig, r.HTTPRequest.Header.Get("X-Amz-Signature"))
+	fmt.Println(sig)
+	signSDKRequestWithCurrTime(r, func() time.Time {
+		// Simulate one second has passed so that signature's date changes
+		// when it is resigned.
+		return time.Now().Add(1 * time.Second)
+	})
+	assert.NotEqual(t, sig, r.HTTPRequest.URL.Query().Get("X-Amz-Signature"))
 }
 
 func TestResignRequestExpiredCreds(t *testing.T) {