Add Confused Deputy Prevention #1503
Conversation
| } | ||
|
|
||
| func TestConfusedDeputyHeaders(t *testing.T) { | ||
| mockProvider := mockConfigProvider{} |
There was a problem hiding this comment.
nit: Could we use the mock.Session from github.com/aws/aws-sdk-go/awstesting/mock?
There was a problem hiding this comment.
Yeah, I'll take a look at using that instead
| "x-amz-source-arn": sourceArn, | ||
| "x-amz-source-account": sourceAccount, |
There was a problem hiding this comment.
nit: Can we make the keys consts?
| client := sts.New(p, cfgs...) | ||
| if sourceAccount != "" && sourceArn != "" { | ||
| client.Handlers.Sign.PushFront(func(r *request.Request) { | ||
| r.ApplyOptions(request.WithSetRequestHeaders(map[string]string{ |
There was a problem hiding this comment.
nit: Could also add to the request headers directly like in
In the end, that's all the
WithSetRequestHeaders does.
There was a problem hiding this comment.
Makes sense, I'll switch the implementation
| sourceAccount = os.Getenv(envconfig.AMZ_SOURCE_ACCOUNT) // populates the "x-amz-source-account" header | ||
| sourceArn = os.Getenv(envconfig.AMZ_SOURCE_ARN) // populates the "x-amz-source-arn" header |
There was a problem hiding this comment.
So these should not be modifiable after start up?
There was a problem hiding this comment.
How do these get configured on EC2? The systemd service won't pick up external environment variables.
There was a problem hiding this comment.
Yeah, at this time these should not be modifiable after startup.
On EC2, when the agent is managed by systemd, these are set via the systemd service file, e.g.:
[Unit]
Description=Amazon CloudWatch Agent
After=network.target
[Service]
Type=simple
ExecStart=/opt/aws/amazon-cloudwatch-agent/bin/start-amazon-cloudwatch-agent
KillMode=process
Restart=on-failure
RestartSec=60s
Environment="AMZ_SOURCE_ACCOUNT=<some account>"
Environment="AMZ_SOURCE_ARN=<some ARN>"
[Install]
WantedBy=multi-user.target
Description of the issue
To support confused deputy prevention, the CloudWatch Agent needs to be able to pass confused deputy context keys in the headers of STS AssumeRole calls so that dependent service teams can allow their customers to use confused deputy context keys in their role policies.
For background on the confused deputy problem, see: https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html
Description of changes
Enable the CloudWatch Agent to resource confused deputy context keys from environment variables and include the key values in the STS AssumeRole request headers.
License
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Tests
New
assume_roleintegration test created in test repo: https://github.com/aws/amazon-cloudwatch-agent-test/tree/dricross/confused-deputy/test/assume_role. Example run: https://github.com/aws/amazon-cloudwatch-agent/actions/runs/12778135116/job/35673452648. See PR for amazon-cloudwatch-agent-test for more details: aws/amazon-cloudwatch-agent-test#449Note: the tests are currently failing as the test accounts needs to be specially onboarded with the STS service in order for STS to accept confused deputy keys in the request headers.
Requirements
Before commit the code, please do the following steps.
make fmtandmake fmt-shmake lint