providers/aws: Refactor + fix 2x Authorization header append issue.

internal/providers/aws.go

@@ -74,8 +74,8 @@ func sortKeys(strMap map[string][]string) []string {
 	return keys
 }
 
-// AWSSignV4 modifies a map[string][]string of headers to include an AWS V4 signature based on the config/credentials provided.
-func AWSSignV4(headers map[string][]string, method string, theURL *url.URL, body []byte, service string, awsCreds AWSCredentials, theTime time.Time) map[string][]string {
+// AWSSignV4 modifies a map[string][]string of headers to generate an AWS V4 signature + headers based on the config/credentials provided.
+func AWSSignV4(headers map[string][]string, method string, theURL *url.URL, body []byte, service string, awsCreds AWSCredentials, theTime time.Time) (string, map[string]string) {
 	// General ref. https://docs.aws.amazon.com/general/latest/gr/sigv4_signing.html
 	// S3 ref. https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-auth-using-authorization-header.html
 	// APIGateway ref. https://docs.aws.amazon.com/apigateway/api-reference/signing-requests/
@@ -104,16 +104,18 @@ func AWSSignV4(headers map[string][]string, method string, theURL *url.URL, body
 	}
 
 	headersToSign := map[string][]string{}
-	// sign all of the aws headers that aren't on the ignore list.
+	// sign all of the aws headers.
+	for k, v := range awsHeaders {
+		headersToSign[k] = []string{v}
+	}
+
+	// sign all of the request's headers, except for those in the ignore list
 	for k, v := range headers {
 		lowercaseHeader := strings.ToLower(k)
 		if _, ok := awsSigv4IgnoredHeaders[lowercaseHeader]; !ok {
 			headersToSign[lowercaseHeader] = v
 		}
 	}
-	for k, v := range awsHeaders {
-		headersToSign[k] = []string{v}
-	}
 
 	// the "canonical request" is the normalized version of the AWS service access
 	// that we're attempting to perform; in this case, a GET from an S3 bucket
@@ -153,19 +155,10 @@ func AWSSignV4(headers map[string][]string, method string, theURL *url.URL, body
 
 	// required format of Authorization header; n.b. the access key corresponding to
 	// the secret key is included here
-	authHdr := "AWS4-HMAC-SHA256 Credential=" + awsCreds.AccessKey + "/" + dateNow
-	authHdr += "/" + awsCreds.RegionName + "/" + service + "/aws4_request,"
-	authHdr += "SignedHeaders=" + headerList + ","
-	authHdr += "Signature=" + fmt.Sprintf("%x", signature)
-
-	// add the computed Authorization
-	out := make(map[string][]string, len(awsHeaders)+1)
-	out["Authorization"] = []string{authHdr}
-
-	// populate the other signed headers into the request
-	for k, v := range awsHeaders {
-		out[k] = []string{v}
-	}
+	authHeader := "AWS4-HMAC-SHA256 Credential=" + awsCreds.AccessKey + "/" + dateNow
+	authHeader += "/" + awsCreds.RegionName + "/" + service + "/aws4_request,"
+	authHeader += "SignedHeaders=" + headerList + ","
+	authHeader += "Signature=" + fmt.Sprintf("%x", signature)
 
-	return out
+	return authHeader, awsHeaders
 }

plugins/rest/aws.go

@@ -507,11 +507,11 @@ func signV4(req *http.Request, service string, credService awsCredentialService,
 
 	now := theTime.UTC()
 
-	signedHeaders := providers.AWSSignV4(req.Header, req.Method, req.URL, body, service, creds, now)
+	authHeader, awsHeaders := providers.AWSSignV4(req.Header, req.Method, req.URL, body, service, creds, now)
 
-	req.Header.Set("Authorization", strings.Join(signedHeaders["Authorization"], ""))
-	for k, v := range signedHeaders {
-		req.Header.Add(k, strings.Join(v, ","))
+	req.Header.Set("Authorization", authHeader)
+	for k, v := range awsHeaders {
+		req.Header.Add(k, v)
 	}
 
 	return nil

plugins/rest/aws_test.go

@@ -717,6 +717,10 @@ func TestV4SigningWithMultiValueHeaders(t *testing.T) {
 		"AWS4-HMAC-SHA256 Credential=MYAWSACCESSKEYGOESHERE/20190424/us-east-1/execute-api/aws4_request,"+
 			"SignedHeaders=accept;host;x-amz-date;x-amz-security-token,"+
 			"Signature=0237b0c789cad36212f0efba70c02549e1f659ab9caaca16423930cc7236c046", t)
+	// Ensure 'authorization' is not multi-valued.
+	if len(req.Header.Values("Authorization")) != 1 {
+		t.Fatal("Authorization header is multi-valued. This will break AWS v4 signing.")
+	}
 	// The multi-value headers are preserved
 	assertEq(req.Header.Values("Accept")[0], "text/plain", t)
 	assertEq(req.Header.Values("Accept")[1], "text/html", t)

topdown/providers.go

@@ -7,7 +7,6 @@ package topdown
 import (
 	"encoding/json"
 	"net/url"
-	"strings"
 	"time"
 
 	"github.com/open-policy-agent/opa/ast"
@@ -173,10 +172,11 @@ func builtinAWSSigV4SignReq(ctx BuiltinContext, operands []*ast.Term, iter func(
 	}
 
 	// Sign the request object's headers, and reconstruct the headers map.
-	signedHeadersMap := providers.AWSSignV4(objectToMap(headers), method, theURL, body, service, awsCreds, signingTimestamp)
+	authHeader, signedHeadersMap := providers.AWSSignV4(objectToMap(headers), method, theURL, body, service, awsCreds, signingTimestamp)
 	signedHeadersObj := ast.NewObject()
+	signedHeadersObj.Insert(ast.StringTerm("Authorization"), ast.StringTerm(authHeader))
 	for k, v := range signedHeadersMap {
-		signedHeadersObj.Insert(ast.StringTerm(k), ast.StringTerm(strings.Join(v, ",")))
+		signedHeadersObj.Insert(ast.StringTerm(k), ast.StringTerm(v))
 	}
 
 	// Create new request object with updated headers.