diff --git a/.github/workflows/ec2-integration-test.yml b/.github/workflows/ec2-integration-test.yml index 2a31c4d468..5d1eb6245c 100644 --- a/.github/workflows/ec2-integration-test.yml +++ b/.github/workflows/ec2-integration-test.yml @@ -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 }}" \ diff --git a/cfg/aws/credentials.go b/cfg/aws/credentials.go index c0867ba258..7701d6b7f3 100644 --- a/cfg/aws/credentials.go +++ b/cfg/aws/credentials.go @@ -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" ) @@ -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}, @@ -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, @@ -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) diff --git a/cfg/aws/credentials_test.go b/cfg/aws/credentials_test.go new file mode 100644 index 0000000000..722590db60 --- /dev/null +++ b/cfg/aws/credentials_test.go @@ -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) + }) + } + +} diff --git a/cfg/envconfig/envconfig.go b/cfg/envconfig/envconfig.go index 3fd4b637d2..afbf4918de 100644 --- a/cfg/envconfig/envconfig.go +++ b/cfg/envconfig/envconfig.go @@ -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 (