Skip to content

CIS-DI-0009 #1455

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 2 commits into
base: develop
Choose a base branch
from
Open

CIS-DI-0009 #1455

wants to merge 2 commits into from

Conversation

waja
Copy link

@waja waja commented May 15, 2025

Related Issue:

New Behavior

Download nginx-keyring.gpg and verify checksum at build time instead of use ADD.

Contrast to Current Behavior

Fixing https://github.com/goodwithtech/dockle/blob/master/CHECKPOINT.md#cis-di-0009 which was introduced with 1c8cdfa.

Discussion: Benefits and Drawbacks

ADD instruction introduces risks such as adding malicious files from URLs without scanning and unpacking procedure vulnerabilities.

Changes to the Wiki

Not needed.

Proposed Release Note Entry

Not needed really.

Double Check

  • I have read the comments and followed the PR template.
  • I have explained my PR according to the information in the comments.
  • My PR targets the develop branch.

@waja waja changed the title Cis di 0009 CIS-DI-0009 May 15, 2025
@waja
Copy link
Author

waja commented May 15, 2025

Hmm .. I see the problem (source can't be a URL for COPY) ... the issue mentioned in CIS-DI-0009 is still valid. But I can't imagine a way to solve this easily yet.

@waja waja marked this pull request as draft May 15, 2025 08:23
@waja waja force-pushed the CIS-DI-0009 branch 3 times, most recently from 7323f3e to 0018c16 Compare May 15, 2025 09:04
@tobiasge
Copy link
Member

ADD has a parameter to verify the checksum: https://docs.docker.com/reference/dockerfile/#add---checksum

@waja waja marked this pull request as ready for review May 15, 2025 09:06
Copy link
Member

@tobiasge tobiasge left a comment

Choose a reason for hiding this comment

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

This can be simplified by using the --checksum parameter of the ADD command.

@waja waja requested a review from tobiasge May 15, 2025 09:22
@waja
Copy link
Author

waja commented May 15, 2025

This can be simplified by using the --checksum parameter of the ADD command.

Okay ... this seems "pretty" new ... :) Didn't recognised it yet. Changed the PR.

@waja
Copy link
Author

waja commented May 15, 2025

Seems like hadolint doesn't like (or know) about --checksum:

DOCKERFILE_HADOLINT
  2025-05-15 10:24:06 [INFO]   Linting DOCKERFILE_HADOLINT items...
  Error: -15 10:24:07 [ERROR]   Found errors when linting DOCKERFILE_HADOLINT. Exit code: 1.
  2025-05-15 10:24:07 [INFO]   Command output for DOCKERFILE_HADOLINT:
  ------
  /github/workspace/Dockerfile:52:5 invalid flag: --checksum 

What's you suggestion? Doesn't look like a simple "ignore" is solving this.

@tobiasge
Copy link
Member

Seems to be that problem: hadolint/hadolint#985
I think we can ignore that for now.

@waja waja requested a review from tobiasge May 15, 2025 11:44
@waja
Copy link
Author

waja commented May 15, 2025

I moved the --checksum to the end of the ADD statement and hadolint is working well about this, but my local test is complaining about DL3020 error: Use COPY instead of ADD for files and folders now.

I can add it to .hadolint.yaml, but I'm unsure if this is the way you intend to fix it.

@waja waja force-pushed the CIS-DI-0009 branch 2 times, most recently from e3354b5 to 2ba9014 Compare May 15, 2025 12:05
@waja
Copy link
Author

waja commented May 15, 2025

The latest CI run revealed that hadolint is also complaining there. But the bigger issue is, it seems the build environment seems to have issues with the ADD syntax.

@waja
Copy link
Author

waja commented May 16, 2025

@tobiasge I added a inline hadolint ignore, so this check is not disabled globally

@waja waja force-pushed the CIS-DI-0009 branch 4 times, most recently from db9067c to b26fa2d Compare May 16, 2025 13:38
@tobiasge
Copy link
Member

If you look at the documentation of ADD https://docs.docker.com/reference/dockerfile/#add you can see that the options need to be placed before the source and destination.

Add hadolint inline ignore to prevent ignoring it globally
@waja
Copy link
Author

waja commented May 16, 2025

#10 [main  3/18] ADD https://unit.nginx.org/keys/nginx-keyring.gpg /usr/share/keyrings/nginx-keyring.gpg --chmod=444 --chown=0:0 --checksum=sha256:7d3d5a7adf37e17d6882e2f6f55324b9a8f978ef3c99c50fe801af67c9847c91
  #10 ERROR: failed to calculate checksum of ref cgo21278a3kk11hgospq0tdf7::pimc1tpuoeefuzy1txmbj1vnq: "/--chown=0:0": not found

When using the workaround not complaining hadolint, the ADD fails. When using the correct syntax of ADD hadolint is complaining about the flag --checksum:

Dockerfile:51:75 invalid flag: --checksum

So I don't know how to deal about that without doing the checksum check on buildtime at the moment.

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.

2 participants