Skip to content

Commit

Permalink
Add Confused Deputy Prevention (#1503)
Browse files Browse the repository at this point in the history
  • Loading branch information
dricross authored Feb 3, 2025
1 parent 77aa32a commit b55abe4
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 4 deletions.
6 changes: 4 additions & 2 deletions .github/workflows/ec2-integration-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,11 @@ jobs:
terraform init
if terraform apply --auto-approve \
-var="ssh_key_value=${{env.PRIVATE_KEY}}" -var="github_test_repo=${{ inputs.test_repo_url }}" \
-var="ssh_key_value=${{env.PRIVATE_KEY}}" \
-var="github_test_repo=${{ inputs.test_repo_url }}" \
-var="test_name=${{ matrix.arrays.os }}" \
-var="cwa_github_sha=${{inputs.github_sha}}" -var="install_agent=${{ matrix.arrays.installAgentCommand }}" \
-var="cwa_github_sha=${{inputs.github_sha}}" \
-var="install_agent=${{ matrix.arrays.installAgentCommand }}" \
-var="github_test_repo_branch=${{inputs.test_repo_branch}}" \
-var="ec2_instance_type=${{ matrix.arrays.instanceType }}" \
-var="user=${{ matrix.arrays.username }}" \
Expand Down
36 changes: 34 additions & 2 deletions cfg/aws/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ import (
"github.com/aws/aws-sdk-go/aws/credentials/ec2rolecreds"
"github.com/aws/aws-sdk-go/aws/credentials/stscreds"
"github.com/aws/aws-sdk-go/aws/endpoints"
"github.com/aws/aws-sdk-go/aws/request"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/sts"

"github.com/aws/amazon-cloudwatch-agent/cfg/envconfig"
"github.com/aws/amazon-cloudwatch-agent/extension/agenthealth/handler/stats/agent"
)

Expand Down Expand Up @@ -174,7 +176,7 @@ func (s *stsCredentialProvider) Retrieve() (credentials.Value, error) {

func newStsCredentials(c client.ConfigProvider, roleARN string, region string) *credentials.Credentials {
regional := &stscreds.AssumeRoleProvider{
Client: sts.New(c, &aws.Config{
Client: newStsClient(c, &aws.Config{
Region: aws.String(region),
STSRegionalEndpoint: endpoints.RegionalSTSEndpoint,
HTTPClient: &http.Client{Timeout: 1 * time.Minute},
Expand All @@ -188,7 +190,7 @@ func newStsCredentials(c client.ConfigProvider, roleARN string, region string) *
fallbackRegion := getFallbackRegion(region)

partitional := &stscreds.AssumeRoleProvider{
Client: sts.New(c, &aws.Config{
Client: newStsClient(c, &aws.Config{
Region: aws.String(fallbackRegion),
Endpoint: aws.String(getFallbackEndpoint(fallbackRegion)),
STSRegionalEndpoint: endpoints.RegionalSTSEndpoint,
Expand All @@ -203,6 +205,36 @@ func newStsCredentials(c client.ConfigProvider, roleARN string, region string) *
return credentials.NewCredentials(&stsCredentialProvider{regional: regional, partitional: partitional})
}

const (
SourceArnHeaderKey = "x-amz-source-arn"
SourceAccountHeaderKey = "x-amz-source-account"
)

// newStsClient creates a new STS client with the provided config and options.
// Additionally, if specific environment variables are set, it also appends the confused deputy headers to requests
// made by the client. These headers allow resource-based policies to limit the permissions that a service has to
// a specific resource. Note that BOTH environment variables need to contain non-empty values in order for the headers
// to be set.
//
// See https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html#cross-service-confused-deputy-prevention
func newStsClient(p client.ConfigProvider, cfgs ...*aws.Config) *sts.STS {

sourceAccount := os.Getenv(envconfig.AmzSourceAccount)
sourceArn := os.Getenv(envconfig.AmzSourceArn)

client := sts.New(p, cfgs...)
if sourceAccount != "" && sourceArn != "" {
client.Handlers.Sign.PushFront(func(r *request.Request) {
r.HTTPRequest.Header.Set(SourceArnHeaderKey, sourceArn)
r.HTTPRequest.Header.Set(SourceAccountHeaderKey, sourceAccount)
})

log.Printf("I! Found confused deputy header environment variables: source account: %q, source arn: %q", sourceAccount, sourceArn)
}

return client
}

// The partitional STS endpoint used to fallback when regional STS endpoint is not activated.
func getFallbackEndpoint(region string) string {
partition := getPartition(region)
Expand Down
89 changes: 89 additions & 0 deletions cfg/aws/credentials_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: MIT

package aws

import (
"testing"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/credentials"
"github.com/aws/aws-sdk-go/awstesting/mock"
"github.com/aws/aws-sdk-go/service/sts"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/aws/amazon-cloudwatch-agent/cfg/envconfig"
)

func TestConfusedDeputyHeaders(t *testing.T) {
tests := []struct {
name string
envSourceArn string
envSourceAccount string
expectedHeaderArn string
expectedHeaderAccount string
}{
{
name: "unpopulated",
envSourceArn: "",
envSourceAccount: "",
expectedHeaderArn: "",
expectedHeaderAccount: "",
},
{
name: "both populated",
envSourceArn: "arn:aws:ec2:us-east-1:474668408639:instance/i-08293cd9825754f7c",
envSourceAccount: "539247453986",
expectedHeaderArn: "arn:aws:ec2:us-east-1:474668408639:instance/i-08293cd9825754f7c",
expectedHeaderAccount: "539247453986",
},
{
name: "only source arn populated",
envSourceArn: "arn:aws:ec2:us-east-1:474668408639:instance/i-08293cd9825754f7c",
envSourceAccount: "",
expectedHeaderArn: "",
expectedHeaderAccount: "",
},
{
name: "only source account populated",
envSourceArn: "",
envSourceAccount: "539247453986",
expectedHeaderArn: "",
expectedHeaderAccount: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

t.Setenv(envconfig.AmzSourceAccount, tt.envSourceAccount)
t.Setenv(envconfig.AmzSourceArn, tt.envSourceArn)

client := newStsClient(mock.Session, &aws.Config{
// These are examples credentials pulled from:
// https://docs.aws.amazon.com/STS/latest/APIReference/API_GetAccessKeyInfo.html
Credentials: credentials.NewStaticCredentials("AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", ""),
Region: aws.String("us-east-1"),
})

request, _ := client.AssumeRoleRequest(&sts.AssumeRoleInput{
// We aren't going to actually make the assume role call, we are just going
// to verify the headers are present once signed so the RoleArn and RoleSessionName
// arguments are irrelevant. Fill them out with something so the request is valid.
RoleArn: aws.String("arn:aws:iam::012345678912:role/XXXXXXXX"),
RoleSessionName: aws.String("MockSession"),
})

// Headers are generated after the request is signed (but before it's sent)
err := request.Sign()
require.NoError(t, err)

headerSourceArn := request.HTTPRequest.Header.Get(SourceArnHeaderKey)
assert.Equal(t, tt.expectedHeaderArn, headerSourceArn)

headerSourceAccount := request.HTTPRequest.Header.Get(SourceAccountHeaderKey)
assert.Equal(t, tt.expectedHeaderAccount, headerSourceAccount)
})
}

}
4 changes: 4 additions & 0 deletions cfg/envconfig/envconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ const (
CWConfigContent = "CW_CONFIG_CONTENT"
CWOtelConfigContent = "CW_OTEL_CONFIG_CONTENT"
CWAgentMergedOtelConfig = "CWAGENT_MERGED_OTEL_CONFIG"

// confused deputy prevention related headers
AmzSourceAccount = "AMZ_SOURCE_ACCOUNT" // populates the "x-amz-source-account" header
AmzSourceArn = "AMZ_SOURCE_ARN" // populates the "x-amz-source-arn" header
)

const (
Expand Down

0 comments on commit b55abe4

Please sign in to comment.