Skip to content

Add public key verification for aws cli download #715

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

PotatoWKY
Copy link

@PotatoWKY PotatoWKY commented Jun 20, 2025

Description

Add a file that stores the public key the AWS CLI is using (public resource can be find in their documents).
Add logic in the Dockerfile to read the public key and validate the downloaded file.

Type of Change

  • Image update - Bug fix
  • Image update - New feature
  • Image update - Breaking change
  • SMD image build tool update
  • Documentation update

Release Information

Does this change need to be included in patch version releases? By default, any pull requests will only be added to the next SMD image minor version release once they are merged in template folder. Only critical bug fix or security update should be applied to new patch versions of existed image minor versions.

  • Yes (Critical bug fix or security update)
  • No (New feature or non-critical change)
  • N/A (Not an image update)

If yes, please explain why:
Appsec requirement, not urgent

How Has This Been Tested?

Tested in a test Dockerfile

Checklist:

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

Test Screenshots (if applicable):

Screenshot 2025-06-20 at 3 36 35 PM

test generate SMD docker image succeed:

dev-dsk-wukeyu-2c-3078dfb2 % python ./src/main.py build \                                                                        
  --target-patch-version=$NEXT_VERSION --skip-tests
Successfully built an image with id: sha256:0e20fcd65cdbb6d14b4a5ae5a28433d801180c455b3093d2024144356703ca1c
Successfully built an image with id: sha256:a8cf421fd4bf697d36064b5978bfc979b552163cd6d9fa745644e19097b5c4de
Will skip tests.

Related Issues

N/A

Additional Notes

N/A

@PotatoWKY PotatoWKY requested a review from a team as a code owner June 20, 2025 22:37
Comment on lines 62 to 66
RUN curl -O "https://awscli.amazonaws.com/awscli-exe-linux-x86_64.zip" && \
curl -O "https://awscli.amazonaws.com/awscli-exe-linux-x86_64.zip.sig"
RUN gpg --import aws-cli-public-key.asc
RUN gpg --fingerprint FB5DB77FD5C118B80511ADA8A6310ACC4672475C
RUN gpg --verify awscli-exe-linux-x86_64.zip.sig awscli-exe-linux-x86_64.zip && \
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's avoid additional "RUN" commands in the Dockerfile because each "RUN" command will generate a new image layer and may cause increase in image size. One way is to move the "COPY" command to the beginning and connect these commands with "&"

sudo ./aws/install && \
rm -rf aws awscliv2.zip && \

COPY aws-cli-public-key.asc .
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's copy the file to "/tmp/" folder so it will be automatically cleaned up. Example: https://github.com/aws/sagemaker-distribution/blob/main/template/v2/Dockerfile#L85

RUN curl -O "https://awscli.amazonaws.com/awscli-exe-linux-x86_64.zip" && \
curl -O "https://awscli.amazonaws.com/awscli-exe-linux-x86_64.zip.sig"
RUN gpg --import aws-cli-public-key.asc
RUN gpg --fingerprint FB5DB77FD5C118B80511ADA8A6310ACC4672475C
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this fingerprint generated? Will it change in the future or is it fixed? Let's avoid to have such hard-coded value here. If this is indeed needed, try to add some comment to explain what this is

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