Skip to content

Commit

Permalink
Fix(aws-connector): Limit signing to signed headers from original req…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
doodlesbykumbi committed Oct 21, 2021
1 parent e870b33 commit 5e02e2e
Show file tree
Hide file tree
Showing 5 changed files with 320 additions and 67 deletions.
10 changes: 6 additions & 4 deletions .gitleaks.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ description = "AWS Client ID"
regex = '''(A3T[A-Z0-9]|AKIA|AGPA|AIDA|AROA|AIPA|ANPA|ANVA|ASIA)[A-Z0-9]{16}'''
tags = ["key", "AWS"]
[[rules.whitelist]]
description = "sample AWS key in AWS HTTP connector comments"
regex = '''AKIAJC5FABNOFVBKRWHA'''
description = "sample AWS key in AWS HTTP connector"
regex = '''AKIAIOSFODNN7EXAMPLE'''
[[rules.whitelist]]
description = "since-removed sample AWS key"
regex = '''AKIAJADDJE4Q4JVX3HAA'''
Expand Down Expand Up @@ -214,6 +214,7 @@ files = [
"test/ssh/id_(.*)", # since-removed ssh test certs
"test/util/ssl/(.*)", # test ssl certs
"internal/plugin/connectors/tcp/mssql/connection_details_test.go", # fake cert string
"internal/plugin/connectors/http/aws/(.*)", # fake AWS credentials
]
# As of v4, gitleaks can whitelist paths to accommodate no longer using
# paths in the `files` whitelist.
Expand All @@ -232,10 +233,11 @@ paths = [
"test/connector/tcp/mssql/certs",
"test/ssh",
"test/util/ssl",
"internal/plugin/connectors/tcp/mssql"
"internal/plugin/connectors/tcp/mssql",
"internal/plugin/connectors/http/aws",
]
regexes = [
"AKIAJC5FABNOFVBKRWHA", # sample AWS key in AWS HTTP connector comments
"AKIAIOSFODNN7EXAMPLE", # sample AWS key in AWS HTTP connector
"AKIAJADDJE4Q4JVX3HAA", # since-removed sample AWS key
"SuperSecure", # dummy password used in conjur integration test docker-compose
]
Expand Down
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

### Security
- Updated containerd to v1.4.11 to close CVE-2020-15257 (Not vulnerable)
[cyberark/secretless-broker#1431](https://github.com/cyberark/secretless-broker/pull/1431)
Expand Down
104 changes: 45 additions & 59 deletions internal/plugin/connectors/http/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"io/ioutil"
gohttp "net/http"
"net/url"
"regexp"
"strings"
"time"

Expand All @@ -20,15 +19,6 @@ import (
// From https://github.com/aws/aws-sdk-go/blob/master/aws/signer/v4/v4.go#L77
const timeFormat = "20060102T150405Z"

// reForCredentialComponent matches headers strings like:
//
// Credential=AKIAJC5FABNOFVBKRWHA/20171103/us-east-1/ec2/aws4_request
//
// See https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-auth-using-authorization-header.html
var reForCredentialComponent = regexp.MustCompile(
`^Credential=\w+\/\d+\/([\w-_]+)\/(\w+)\/aws4_request$`,
)

// newAmzDate parses a date string using the AWS signer time format
func newAmzDate(amzDateStr string) (time.Time, error) {
if amzDateStr == "" {
Expand All @@ -39,52 +29,52 @@ 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 for readability):
// AWS4-HMAC-SHA256 \
// Credential=AKIAIOSFODNN7EXAMPLE/20130524/us-east-1/s3/aws4_request, \
// SignedHeaders=host;range;x-amz-date, \
// Signature=fe5f80f77d5fa3beca038a248ff027d0445342fe2855ddc963176630326f1024
//
// See https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-auth-using-authorization-header.html

// 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
date time.Time
region string
serviceName string
signedHeaders []string
}

// newRequestMetadata parses the request headers to extract the metadata
Expand All @@ -99,21 +89,21 @@ func newRequestMetadata(r *gohttp.Request) (*requestMetadata, error) {
return nil, nil
}

// parse date string
// Parse date string
//
date, err := newAmzDate(amzDateStr)
if err != nil {
return nil, err
}

// create request metadata by extracting service name and region from
// Create request metadata by extracting service name and region from
// Authorization header
reqMeta, err := requestMetadataFromAuthz(authorizationStr)
if err != nil {
return nil, err
}

// populate request metadata with date
// Populate request metadata with date
reqMeta.date = date

return reqMeta, nil
Expand Down Expand Up @@ -169,14 +159,10 @@ func signRequest(
reqMeta.region,
reqMeta.date,
)
if err != nil {
return err
}

return nil
return err
}

// 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.
Expand All @@ -188,12 +174,12 @@ func signRequest(
// endpoint and then this connector can use the AWS SDK to resolve the endpoint
// in the same way the client might via a direct call to Amazon over HTTPS.
//
// Note that if the client to specifies an HTTP (not HTTPS) endpoint that is not
// http://secretless.empty it will be respected.
// Note that if the client specifies an HTTP (not HTTPS, because Secretless does not proxy
// HTTPS requests) endpoint that is not http://secretless.empty it will be respected.
//
// 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 {
// upgrade the connect between Secretless and the target endpoint to TLS
func maybeSetAmzEndpoint(req *gohttp.Request, reqMeta *requestMetadata) error {
shouldSetEndpoint := req.URL.Scheme == "http" &&
req.URL.Host == "secretless.empty"

Expand Down
63 changes: 59 additions & 4 deletions internal/plugin/connectors/http/aws/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package aws

import (
gohttp "net/http"
"strings"

"github.com/cyberark/secretless-broker/pkg/secretless/log"
"github.com/cyberark/secretless-broker/pkg/secretless/plugin/connector"
Expand All @@ -25,7 +26,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
Expand All @@ -40,16 +41,70 @@ 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
}

// Use metadata and credentials to sign request
c.logger.Debugf(
"Signing for service=%s region=%s",
"Signing for service=%s region=%s signedHeaders=%s",
reqMeta.serviceName,
reqMeta.region,
strings.Join(reqMeta.signedHeaders, ","),
)
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 _, isSignedHeader := signedHeadersMap[key]; isSignedHeader {
continue
}

unsignedHeaders[key] = value
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 _, isReservedHeader := reservedHeaders[key]; isReservedHeader {
continue
}

// Add back all the values for each non-reserved unsigned header
for _, value := range values {
req.Header.Add(key, value)
}
}
}
Loading

0 comments on commit 5e02e2e

Please sign in to comment.