Skip to content

Commit b55abe4

Browse files
authored
Add Confused Deputy Prevention (#1503)
1 parent 77aa32a commit b55abe4

File tree

4 files changed

+131
-4
lines changed

4 files changed

+131
-4
lines changed

.github/workflows/ec2-integration-test.yml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,11 @@ jobs:
9292
9393
terraform init
9494
if terraform apply --auto-approve \
95-
-var="ssh_key_value=${{env.PRIVATE_KEY}}" -var="github_test_repo=${{ inputs.test_repo_url }}" \
95+
-var="ssh_key_value=${{env.PRIVATE_KEY}}" \
96+
-var="github_test_repo=${{ inputs.test_repo_url }}" \
9697
-var="test_name=${{ matrix.arrays.os }}" \
97-
-var="cwa_github_sha=${{inputs.github_sha}}" -var="install_agent=${{ matrix.arrays.installAgentCommand }}" \
98+
-var="cwa_github_sha=${{inputs.github_sha}}" \
99+
-var="install_agent=${{ matrix.arrays.installAgentCommand }}" \
98100
-var="github_test_repo_branch=${{inputs.test_repo_branch}}" \
99101
-var="ec2_instance_type=${{ matrix.arrays.instanceType }}" \
100102
-var="user=${{ matrix.arrays.username }}" \

cfg/aws/credentials.go

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@ import (
1616
"github.com/aws/aws-sdk-go/aws/credentials/ec2rolecreds"
1717
"github.com/aws/aws-sdk-go/aws/credentials/stscreds"
1818
"github.com/aws/aws-sdk-go/aws/endpoints"
19+
"github.com/aws/aws-sdk-go/aws/request"
1920
"github.com/aws/aws-sdk-go/aws/session"
2021
"github.com/aws/aws-sdk-go/service/sts"
2122

23+
"github.com/aws/amazon-cloudwatch-agent/cfg/envconfig"
2224
"github.com/aws/amazon-cloudwatch-agent/extension/agenthealth/handler/stats/agent"
2325
)
2426

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

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

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

208+
const (
209+
SourceArnHeaderKey = "x-amz-source-arn"
210+
SourceAccountHeaderKey = "x-amz-source-account"
211+
)
212+
213+
// newStsClient creates a new STS client with the provided config and options.
214+
// Additionally, if specific environment variables are set, it also appends the confused deputy headers to requests
215+
// made by the client. These headers allow resource-based policies to limit the permissions that a service has to
216+
// a specific resource. Note that BOTH environment variables need to contain non-empty values in order for the headers
217+
// to be set.
218+
//
219+
// See https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html#cross-service-confused-deputy-prevention
220+
func newStsClient(p client.ConfigProvider, cfgs ...*aws.Config) *sts.STS {
221+
222+
sourceAccount := os.Getenv(envconfig.AmzSourceAccount)
223+
sourceArn := os.Getenv(envconfig.AmzSourceArn)
224+
225+
client := sts.New(p, cfgs...)
226+
if sourceAccount != "" && sourceArn != "" {
227+
client.Handlers.Sign.PushFront(func(r *request.Request) {
228+
r.HTTPRequest.Header.Set(SourceArnHeaderKey, sourceArn)
229+
r.HTTPRequest.Header.Set(SourceAccountHeaderKey, sourceAccount)
230+
})
231+
232+
log.Printf("I! Found confused deputy header environment variables: source account: %q, source arn: %q", sourceAccount, sourceArn)
233+
}
234+
235+
return client
236+
}
237+
206238
// The partitional STS endpoint used to fallback when regional STS endpoint is not activated.
207239
func getFallbackEndpoint(region string) string {
208240
partition := getPartition(region)

cfg/aws/credentials_test.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package aws
5+
6+
import (
7+
"testing"
8+
9+
"github.com/aws/aws-sdk-go/aws"
10+
"github.com/aws/aws-sdk-go/aws/credentials"
11+
"github.com/aws/aws-sdk-go/awstesting/mock"
12+
"github.com/aws/aws-sdk-go/service/sts"
13+
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
15+
16+
"github.com/aws/amazon-cloudwatch-agent/cfg/envconfig"
17+
)
18+
19+
func TestConfusedDeputyHeaders(t *testing.T) {
20+
tests := []struct {
21+
name string
22+
envSourceArn string
23+
envSourceAccount string
24+
expectedHeaderArn string
25+
expectedHeaderAccount string
26+
}{
27+
{
28+
name: "unpopulated",
29+
envSourceArn: "",
30+
envSourceAccount: "",
31+
expectedHeaderArn: "",
32+
expectedHeaderAccount: "",
33+
},
34+
{
35+
name: "both populated",
36+
envSourceArn: "arn:aws:ec2:us-east-1:474668408639:instance/i-08293cd9825754f7c",
37+
envSourceAccount: "539247453986",
38+
expectedHeaderArn: "arn:aws:ec2:us-east-1:474668408639:instance/i-08293cd9825754f7c",
39+
expectedHeaderAccount: "539247453986",
40+
},
41+
{
42+
name: "only source arn populated",
43+
envSourceArn: "arn:aws:ec2:us-east-1:474668408639:instance/i-08293cd9825754f7c",
44+
envSourceAccount: "",
45+
expectedHeaderArn: "",
46+
expectedHeaderAccount: "",
47+
},
48+
{
49+
name: "only source account populated",
50+
envSourceArn: "",
51+
envSourceAccount: "539247453986",
52+
expectedHeaderArn: "",
53+
expectedHeaderAccount: "",
54+
},
55+
}
56+
for _, tt := range tests {
57+
t.Run(tt.name, func(t *testing.T) {
58+
59+
t.Setenv(envconfig.AmzSourceAccount, tt.envSourceAccount)
60+
t.Setenv(envconfig.AmzSourceArn, tt.envSourceArn)
61+
62+
client := newStsClient(mock.Session, &aws.Config{
63+
// These are examples credentials pulled from:
64+
// https://docs.aws.amazon.com/STS/latest/APIReference/API_GetAccessKeyInfo.html
65+
Credentials: credentials.NewStaticCredentials("AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", ""),
66+
Region: aws.String("us-east-1"),
67+
})
68+
69+
request, _ := client.AssumeRoleRequest(&sts.AssumeRoleInput{
70+
// We aren't going to actually make the assume role call, we are just going
71+
// to verify the headers are present once signed so the RoleArn and RoleSessionName
72+
// arguments are irrelevant. Fill them out with something so the request is valid.
73+
RoleArn: aws.String("arn:aws:iam::012345678912:role/XXXXXXXX"),
74+
RoleSessionName: aws.String("MockSession"),
75+
})
76+
77+
// Headers are generated after the request is signed (but before it's sent)
78+
err := request.Sign()
79+
require.NoError(t, err)
80+
81+
headerSourceArn := request.HTTPRequest.Header.Get(SourceArnHeaderKey)
82+
assert.Equal(t, tt.expectedHeaderArn, headerSourceArn)
83+
84+
headerSourceAccount := request.HTTPRequest.Header.Get(SourceAccountHeaderKey)
85+
assert.Equal(t, tt.expectedHeaderAccount, headerSourceAccount)
86+
})
87+
}
88+
89+
}

cfg/envconfig/envconfig.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ const (
3232
CWConfigContent = "CW_CONFIG_CONTENT"
3333
CWOtelConfigContent = "CW_OTEL_CONFIG_CONTENT"
3434
CWAgentMergedOtelConfig = "CWAGENT_MERGED_OTEL_CONFIG"
35+
36+
// confused deputy prevention related headers
37+
AmzSourceAccount = "AMZ_SOURCE_ACCOUNT" // populates the "x-amz-source-account" header
38+
AmzSourceArn = "AMZ_SOURCE_ARN" // populates the "x-amz-source-arn" header
3539
)
3640

3741
const (

0 commit comments

Comments
 (0)