aws/signer/v4: Fix signer to not double sign Content-Length header (#…

…1743)


Fixes the SDK's AWS SigV4 signer to not double sign the content-length header. If the Content-Length header was manually set on the http.Request, that value would be included along with the Request.ContentLength value as a common separated list when computing the string to sign.

This fix updates the signer to always ignore the content-length header, and only use the Request.ContentLength parameter. This change also matches http.Request's behavior of ignoring the Content-Length header if set.

Fixes #1728
Replaces: #1729

.changelog/c4a428a2d14d44febd314da7b9edfb28.json

@@ -0,0 +1,8 @@
+{
+    "id": "c4a428a2-d14d-44fe-bd31-4da7b9edfb28",
+    "type": "bugfix",
+    "description": "Fix aws/signer/v4 to not double sign Content-Length header. Fixes [#1728](https://github.com/aws/aws-sdk-go-v2/issues/1728). Thanks to @matelang for creating the issue and PR.",
+    "modules": [
+        "."
+    ]
+}

aws/signer/v4/v4.go

@@ -407,8 +407,8 @@ func (s *httpSigner) buildCanonicalHeaders(host string, rule v4Internal.Rule, he
 	headers = append(headers, hostHeader)
 	signed[hostHeader] = append(signed[hostHeader], host)
 
+	const contentLengthHeader = "content-length"
 	if length > 0 {
-		const contentLengthHeader = "content-length"
 		headers = append(headers, contentLengthHeader)
 		signed[contentLengthHeader] = append(signed[contentLengthHeader], strconv.FormatInt(length, 10))
 	}
@@ -417,6 +417,10 @@ func (s *httpSigner) buildCanonicalHeaders(host string, rule v4Internal.Rule, he
 		if !rule.IsValid(k) {
 			continue // ignored header
 		}
+		if strings.EqualFold(k, contentLengthHeader) {
+			// prevent signing already handled content-length header.
+			continue
+		}
 
 		lowerCaseKey := strings.ToLower(k)
 		if _, ok := signed[lowerCaseKey]; ok {

aws/signer/v4/v4_test.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"net/http"
@@ -246,6 +247,38 @@ func TestRequestHost(t *testing.T) {
 		t.Errorf("canonical host header invalid")
 	}
 }
+
+func TestSign_buildCanonicalHeadersContentLengthPresent(t *testing.T) {
+	body := `{"description": "this is a test"}`
+	req, _ := buildRequest("dynamodb", "us-east-1", body)
+	req.URL.RawQuery = "Foo=z&Foo=o&Foo=m&Foo=a"
+	req.Host = "myhost"
+
+	contentLength := fmt.Sprintf("%d", len([]byte(body)))
+	req.Header.Add("Content-Length", contentLength)
+
+	query := req.URL.Query()
+	query.Set("X-Amz-Expires", "5")
+	req.URL.RawQuery = query.Encode()
+
+	ctx := &httpSigner{
+		ServiceName:  "dynamodb",
+		Region:       "us-east-1",
+		Request:      req,
+		Time:         v4Internal.NewSigningTime(time.Now()),
+		KeyDerivator: v4Internal.NewSigningKeyDeriver(),
+	}
+
+	build, err := ctx.Build()
+	if err != nil {
+		t.Fatalf("expected no error, got %v", err)
+	}
+
+	if !strings.Contains(build.CanonicalString, "content-length:"+contentLength+"\n") {
+		t.Errorf("canonical header content-length invalid")
+	}
+}
+
 func TestSign_buildCanonicalHeaders(t *testing.T) {
 	serviceName := "mockAPI"
 	region := "mock-region"