diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f2694375..6c1b1e0cd 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/internal/plugin/connectors/http/aws/aws.go b/internal/plugin/connectors/http/aws/aws.go index 43fc8a59d..663f81e83 100644 --- a/internal/plugin/connectors/http/aws/aws.go +++ b/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" diff --git a/internal/plugin/connectors/http/aws/connector.go b/internal/plugin/connectors/http/aws/connector.go index 40c756b5c..9d26c2a3f 100644 --- a/internal/plugin/connectors/http/aws/connector.go +++ b/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) + } + } }