Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CDK cross-span vuln fix (WIP) #1436

Merged
merged 2 commits into from
Jan 8, 2025
Merged

CDK cross-span vuln fix (WIP) #1436

merged 2 commits into from
Jan 8, 2025

Conversation

rBangay
Copy link
Contributor

@rBangay rBangay commented Dec 27, 2024

What does this PR change?

Fix security vulnerability in cross-spawn in CDK land.

As part of this pr I have also updated the version of CDK we use by quite a few major versions.

@rBangay rBangay changed the title CDK cross-span vuln fix CDK cross-span vuln fix (WIP) Dec 27, 2024
@rBangay rBangay force-pushed the cross-spawn-cd-vuln-fix branch from 0c209d9 to bd7943e Compare January 7, 2025 10:42
@@ -21,6 +21,7 @@ Object {
"GuEc2App",
"GuCertificate",
"GuInstanceRole",
"GuSsmSshPolicy",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We think this replaces the old AmazonSSMManagedInstanceCore policy below to ssh into the ec2 instance.

TODO: test in code to make sure we can still ssh into the instance

@@ -1921,6 +1913,7 @@ Object {
},
"Port": 443,
"Protocol": "HTTPS",
"SslPolicy": "ELBSecurityPolicy-TLS13-1-2-2021-06",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as fastly user TLS v1.2 or later this should be fine (https://docs.aws.amazon.com/elasticloadbalancing/latest/application/describe-ssl-policies.html)

Comment on lines +1931 to +1934
Object {
"Key": "routing.http.drop_invalid_header_fields.enabled",
"Value": "true",
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not sure what constitutes an invalid header, but this sounds sensible.

Comment on lines +2241 to +2254
"ec2messages:AcknowledgeMessage",
"ec2messages:DeleteMessage",
"ec2messages:FailMessage",
"ec2messages:GetEndpoint",
"ec2messages:GetMessages",
"ec2messages:SendReply",
"ssm:UpdateInstanceInformation",
"ssm:ListInstanceAssociations",
"ssm:DescribeInstanceProperties",
"ssm:DescribeDocumentParameters",
"ssmmessages:CreateControlChannel",
"ssmmessages:CreateDataChannel",
"ssmmessages:OpenControlChannel",
"ssmmessages:OpenDataChannel",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These match the current permissions needed according to https://github.com/guardian/ssm-scala?tab=readme-ov-file#in-aws

Comment on lines +2402 to +2404
"MetadataOptions": Object {
"InstanceMetadataTags": "enabled",
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is to allow access to https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/work-with-tags-in-IMDS.html
This is probably useful for kibana logs or wazuh or something but shouldn't break anything.

@rBangay rBangay marked this pull request as ready for review January 7, 2025 16:34
@rBangay rBangay merged commit bba7454 into main Jan 8, 2025
13 checks passed
@rBangay rBangay deleted the cross-spawn-cd-vuln-fix branch January 8, 2025 10:01
@prout-bot
Copy link
Collaborator

Seen on PROD (merged by @rBangay 9 minutes and 34 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants