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

Certs base #4079

Closed
wants to merge 5 commits into from
Closed

Certs base #4079

wants to merge 5 commits into from

Conversation

lewayne-aws
Copy link
Contributor

Summary

With this PR, ecs-agent will pull in a newer ca-certificates package, from the current stable release of Debian, and will no longer attempt to remove the CNNIC certs, which have already been removed upstream

Implementation details

The stable-20211201-slim image that is currently used to source ca-certificates is running Debian's bullseye release (a.k.a. oldstable), and as such, pulls in ca-certificates version 20210119. By using a newer stable image (stable-20240110-slim), the newer ca-certificates version 20230311 will be used.

I also noticed that the certs Dockerfile was performing an extra step to attempt to remove the CNNIC_ROOT certificate, however this cert no longer appears to be present in current versions of the package.

Testing

make test was run. There were some upgrade related tests that failed there (TestCNIPluginVersionUpgrade mainly), however it appears to be the same result from an unmodified master branch.

New tests cover the changes: no; There is no change in functionality to test

Description for the changelog

Update ca-certificates
Does this PR include breaking model changes? If so, Have you added transformation functions?

No functionality changes were made.

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

stable-20211201 is based on the current `oldstable` release, `bullseye`,
and has an older ca-certificates package (20210119), whereas 20240110 is
based on the current `stable` release, `bookworm`, and has a more up to
date ca-certificates package (20230311)
Current versions of the ca-certificates package no longer include
CNNIC_ROOT certificates, so the separate trimming step is no longer necessary
@lewayne-aws lewayne-aws requested a review from a team as a code owner January 27, 2024 01:40
@sparrc sparrc changed the base branch from master to dev April 22, 2024 23:16
@sparrc
Copy link
Contributor

sparrc commented Apr 23, 2024

Thanks for updating this @lewayne-aws, looks like there's no difference in the outputted certs config file from the updated container. I think we should merge this to stop doing the CNNIC hackaround and best practice of using an up-to-date container dependency.

Just as an aside, for reference on why to remove the CNNIC cert, see https://blog.mozilla.org/security/2015/03/23/revoking-trust-in-one-cnnic-intermediate-certificate/

I tested by building the container before and after this change and outputting the ca-certificates.conf file:

docker build -t certs .
docker run certs:latest cat /etc/ca-certificates.conf >/tmp/ca-certs

@sparrc
Copy link
Contributor

sparrc commented Apr 23, 2024

Our codebuild CI project is having issues finding this source code for some reason, so I've cherry-picked the commits and reopened as #4153

@sparrc sparrc closed this Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants