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

BUILDKIT_SBOM_SCAN build args #3249

Merged
merged 4 commits into from
Nov 18, 2022
Merged

BUILDKIT_SBOM_SCAN build args #3249

merged 4 commits into from
Nov 18, 2022

Conversation

jedevc
Copy link
Member

@jedevc jedevc commented Nov 1, 2022

🛠️ Fixes #3185
⬆️ Follows up #2983

This patch adds BUILDKIT_SBOM_SCAN_CONTEXT and BUILDKIT_SBOM_SCAN_STAGE which can configure scanning of the build context and intermediate dockerfile stages respectively.

To use these, the underlying Dockerfile must declare these args, and optionally assign a default value. BUILDKIT_SBOM_SCAN_CONTEXT must either be set in the global meta args before a FROM or in the target stage, while BUILDKIT_SBOM_SCAN_STAGE must be set in each target stage.

The user can additionally override the values set in the Dockerfile to change the behavior.

@tonistiigi
Copy link
Member

@sudo-bmitch wdyt?

Do we have good example cases? I guess most multi-stage builds with compilation stage except Go which embeds dependency data in final binary?

@sudo-bmitch
Copy link

That's the main use case I've seen. Having multiple SBOMs, and possibly merging them, looks like it will be a common use case. There will be the SBOMs for any upstream dependencies. Then an SBOM for the build environment (which could include previous stages of a multi-stage build), the output image (application + OS packages), and the runtime environment for SaaS solutions.

@jedevc jedevc marked this pull request as ready for review November 2, 2022 12:30
@jedevc
Copy link
Member Author

jedevc commented Nov 2, 2022

The added test should show an example of how it's used. In a practical scenario I imagine a dockerfile author adding the SBOM_SCAN_CONTEXT arg if scanning the context is likely to reveal anything of value and adding SBOM_SCAN_STAGE to stages that might benefit from scanning (the dockerfile author is also able to control the default values), as an incredibly rough untested example:

ARG SBOM_SCAN_CONTEXT=true

FROM golang:latest AS builder
ARG SBOM_SCAN_STAGE=true
WORKDIR /src
COPY . .
RUN go build .

FROM scratch
COPY --from=builder /src/binary .
CMD ["/binary"]

The end user can then manually adjust the exact parameters setting build args for SBOM_SCAN_CONTEXT and SBOM_SCAN_STAGE to true/false to override the default values. The user can more narrowly select exact stages to build with comma-separate values with something like --build-arg SBOM_SCAN_STAGE=foo,bar to scan only the foo and bar stages. However, a user can never enable scanning on a stage/the context that the dockerfile author has not explicitly allowed for (though maybe we'd like to allow that?).

@crazy-max
Copy link
Member

Should we not prefix with BUILDKIT_ for extra safety?

@jedevc
Copy link
Member Author

jedevc commented Nov 9, 2022

Have rebased, changed the prefix to include BUILDKIT_ and also refactored to allow BUILDKIT_SBOM_SCAN_CONTEXT to take comma-separated args (which represents which stages should include the context when built as --target).

frontend/dockerfile/dockerfile2llb/convert.go Outdated Show resolved Hide resolved
frontend/dockerfile/dockerfile2llb/convert.go Outdated Show resolved Hide resolved
frontend/dockerfile/dockerfile2llb/convert.go Outdated Show resolved Hide resolved
frontend/dockerfile/builder/build.go Show resolved Hide resolved
if err != nil {
return nil, err
}
o := ds.Outline(dt)
return &o, nil
}

type SBOMTargets struct {
Core llb.State
Extras map[string]llb.State
Copy link
Member

Choose a reason for hiding this comment

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

Is the name in this map retrievable from the final attestation (descriptor)?

Copy link
Member Author

Choose a reason for hiding this comment

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

So by default this is the last part of the path component - I think we should probably document this, so that scanners can use this as the name in the SBOM.

@jedevc jedevc changed the title SBOM_SCAN build args BUILDKIT_SBOM_SCAN build args Nov 16, 2022
Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

This doesn't seem to work for me, at least with the default scanner. I have defined args in Dockerfile but only get one sbom.

Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
@jedevc
Copy link
Member Author

jedevc commented Nov 17, 2022

Ugh, GitHub actions hadn't run the CI, have fixed the issue, it should work now.

It also looks like https://github.com/tonistiigi/buildkit/blob/5d5a6b93e09c466aece20f9835b07138a95660fe/cmd/buildctl/build.go#L281-L289 changed some critical behavior in buildctl, so that the FrontendOpts for attestations were never actually passed to the dockerfile.v0 frontend (since the call to .Solve was being made without any of the attestation options) - I've reverted it to the previous behavior, though I'm not quite sure of the context for why the change needed to be made in the first place.

This patch adds BUILDKIT_SBOM_SCAN_CONTEXT and BUILDKIT_SBOM_SCAN_STAGE
which configure scanning the build context and stages respectively.

To use these, the underlying Dockerfile must declare these args, and
optionally assign a default value. BUILDKIT_SBOM_SCAN_CONTEXT must
either be set in the global meta args before a FROM or in the target
stage, while BUILDKIT_SBOM_SCAN_STAGE must be set in each target stage.

The user can additionally override the values set in the Dockerfile to
change the behavior.

Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
@tonistiigi
Copy link
Member

I've reverted it to the previous behavior

Doesn't this mean that attestations not generated in frontend do not run. The attestation parameters are sent with the inner solve that only returns inner result, not with the c.Build() call that captures the result sent to the exporter. Not sure what the correct solution would be. Maybe we just should send them for both c.Build() and gateway.Solve()?

@jedevc
Copy link
Member Author

jedevc commented Nov 18, 2022

The previous behaviour passed all parameters to both the outer solve and the inner solve, so both the frontend and the solver have the chance to generate attestations. The provenance PR updated this to pass only the attestation parameters to the solver, so none of them ever reached the frontend.

@tonistiigi tonistiigi merged commit 41c989c into moby:master Nov 18, 2022
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.

Dockerfile SBOM scan points
4 participants