Fix(aws-connector): Limit signing to signed headers from original req…

…uest

Prior to this change the AWS connector was signing requests using all the headers present on the original request. This was resulting signature mismatches and failed auth, particularly visible when creating a new s3 bucket. With this change the aws connector will sign only the headers on the original request, it achieves this by temporarily hiding the rest of the headers before signing, then them after signing.

CHANGELOG.md

@@ -6,6 +6,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
+### Fixed
+- Request-signing on the AWS connector was updated to address a bug that was
+  causing failed integrity checks, where the request-signing by Secretless was
+  incorporating more headers than were used on the original request-signing. The
+  fix limits the headers used by Secretless to those used in the original
+  request. [cyberark/secretless-broker#1432](https://github.com/cyberark/secretless-broker/issues/1432)
+
 ## [1.7.6] - 2021-09-10
 
 ### Added

internal/plugin/connectors/http/aws/aws.go

@@ -39,52 +39,50 @@ func newAmzDate(amzDateStr string) (time.Time, error) {
 }
 
 // requestMetadataFromAuthz parses an authorization header string and create a
-// requestMetadata instance populated with the associated region and service
-// name
+// requestMetadata instance populated with the associated region, service
+// name and signed headers
 func requestMetadataFromAuthz(authorizationStr string) (*requestMetadata, error) {
-	// extract credentials section of authorization header
-	credentialsComponent, err := extractCredentialsComponent(authorizationStr)
-	if err != nil {
-		return nil, err
+	// Parse the following (line breaks added):
+	// AWS4-HMAC-SHA256
+	// Credential=AKIAJC5FABNOFVBKRWHA/20171103/us-east-1/ec2/aws4_request, \
+	// SignedHeaders=content-type;host;x-amz-date, \
+	// Signature=c4a8ade09a5e0c644cc282311c36aae6c834596076ffde7db7d1195c7b454ed0
+
+	// validate form of entire authorization header
+	tokens := strings.Split(authorizationStr, ", ")
+	if len(tokens) != 3 || tokens[0] == "" || tokens[1] == "" || tokens[2] == "" {
+		return nil, fmt.Errorf("malformed Authorization header")
 	}
-	// validate credential component of authorization header, then extract region
-	// and service name
-	matches := reForCredentialComponent.FindStringSubmatch(credentialsComponent)
-	if len(matches) != 3 {
+
+	// extract region and service name from credential component
+	credentialParts := strings.SplitN(tokens[0], "/", 5)
+	if len(credentialParts) != 5 {
 		return nil, fmt.Errorf("malformed credential component of Authorization header")
 	}
 
+	region := credentialParts[2]
+	serviceName := credentialParts[3]
+
+	// extract signed headers from signed headers component
+	signedHeaders := strings.Split(
+		strings.TrimPrefix(tokens[1], "SignedHeaders="),
+		";",
+	)
+
 	return &requestMetadata{
-		region:      matches[1],
-		serviceName: matches[2],
+		region:        region,
+		serviceName:   serviceName,
+		signedHeaders: signedHeaders ,
 	}, nil
 }
 
-// requestMetadata captures the metadata of a signed AWS request: date, region
-// and service name
+// requestMetadata captures the metadata of a signed AWS request: date, region, service
+// name and signed headers
 type requestMetadata struct {
 	date        time.Time
 	region      string
 	serviceName string
-}
-
-// extractCredentialsComponent extracts the credentials component from an
-// authorization header string
-func extractCredentialsComponent(authorizationStr string) (string, error) {
-	// Parse the following (line breaks added):
-	// AWS4-HMAC-SHA256
-	// Credential=AKIAJC5FABNOFVBKRWHA/20171103/us-east-1/ec2/aws4_request, \
-	// SignedHeaders=content-type;host;x-amz-date, \
-	// Signature=c4a8ade09a5e0c644cc282311c36aae6c834596076ffde7db7d1195c7b454ed0
-
-	// validate form of entire authorization header
-	tokens := strings.Split(authorizationStr, ", ")
-	if len(tokens) != 3 || tokens[0] == "" || tokens[1] == "" || tokens[2] == "" {
-		return "", fmt.Errorf("malformed Authorization header")
-	}
-
-	// trim prefix and return credential component
-	return strings.TrimPrefix(tokens[0], "AWS4-HMAC-SHA256 "), nil
+	signedHeaders []string
 }
 
 // newRequestMetadata parses the request headers to extract the metadata
@@ -176,7 +174,7 @@ func signRequest(
 	return nil
 }
 
-// setAmzEndpoint, when the request URL is http://secretless.empty, sets the
+// maybeSetAmzEndpoint, when the request URL is http://secretless.empty, sets the
 // request endpoint using the default AWS endpoint resolver. The resolver allows
 // the connector to mimic a typical AWS client and provides a TLS endpoint where
 // possible.
@@ -193,7 +191,7 @@ func signRequest(
 //
 // Note: There is a plan to add a configuration option to instruct Secretless to
 // upgrade the connect between Secretless and the target endpoint to TLS.
-func setAmzEndpoint(req *gohttp.Request, reqMeta *requestMetadata) error {
+func maybeSetAmzEndpoint(req *gohttp.Request, reqMeta *requestMetadata) error {
 	shouldSetEndpoint := req.URL.Scheme == "http" &&
 		req.URL.Host == "secretless.empty"
 

internal/plugin/connectors/http/aws/connector.go

@@ -25,7 +25,7 @@ func (c *Connector) Connect(
 ) error {
 	var err error
 
-	// Extract metadata of a signed AWS request: date, region and service name
+	// Extract metadata of a signed AWS request: date, region and service name.
 	reqMeta, err := newRequestMetadata(req)
 	if err != nil {
 		return err
@@ -40,7 +40,7 @@ func (c *Connector) Connect(
 	// Set AWS endpoint
 	// NOTE: this must be done before signing the request, otherwise the modified request
 	// will fail the integrity check.
-	err = setAmzEndpoint(req, reqMeta)
+	err = maybeSetAmzEndpoint(req, reqMeta)
 	if err != nil {
 		return err
 	}
@@ -51,5 +51,58 @@ func (c *Connector) Connect(
 		reqMeta.serviceName,
 		reqMeta.region,
 	)
-	return signRequest(req, reqMeta, credentialsByID)
+
+	// Temporarily remove any headers that were not signed in the original request.
+	unsignedHeaders := removeUnsignedHeaders(req, reqMeta)
+
+	// Sign the request.
+	err = signRequest(req, reqMeta, credentialsByID)
+	if err != nil {
+		return err
+	}
+
+	// Reinstate unsigned headers without clobbering the effects of signing.
+	reinstateUnsignedHeaders(req, unsignedHeaders)
+
+	return nil
+}
+
+func removeUnsignedHeaders(req *gohttp.Request, reqMeta *requestMetadata) map[string][]string {
+	var signedHeadersMap = map[string]struct{}{}
+	for _, key := range reqMeta.signedHeaders {
+		signedHeadersMap[key] = struct{}{}
+	}
+
+	var unsignedHeaders = map[string][]string{}
+	for key, value := range req.Header {
+		if _, ok := signedHeadersMap[key]; ok {
+			unsignedHeaders[key] = value
+			continue
+		}
+
+		req.Header.Del(key)
+	}
+
+	return unsignedHeaders
+}
+
+func reinstateUnsignedHeaders(req *gohttp.Request, unsignedHeaders map[string][]string) {
+	// Reserved meaning the headers already on the request. We shouldn't touch those because
+	// they should only be the signed headers and those generated by signing
+	var reservedHeaders = map[string]struct{}{}
+	for key, _ := range req.Header {
+		reservedHeaders[key] = struct{}{}
+	}
+
+	for key, values := range unsignedHeaders {
+		// Ignore reserved headers, we don't want to mess with those!
+		if _, ok := reservedHeaders[key]; ok {
+			continue
+		}
+
+		// Add back all the values for each non-reserved unsigned header
+		for _, value := range values {
+			req.Header.Add(key, value)
+		}
+	}
 }