source policy resolves image digest before matching

[tianon]

I was testing the new source policy support (#3332 + follow-ups for format changes) via buildx version 0.11.0-rc1 (docker/buildx#1628), and found it's got some interesting quirks that seem to make the instructions in https://github.com/moby/buildkit/blob/e4683730676f8788ae3ad108375f278dda148fad/docs/build-repro.md not actually do what is expected. 😅

In short, it appears that it resolves image references in a Dockerfile all the way to a full digest before matching against policy lines, so a tag-only selector will actually never match. 😬

Here's a policy I've been using against a Dockerfile of just FROM bash that shows the issue pretty clearly:

{
  "rules": [
    {
      "action": "DENY",
      "selector": {
        "identifier": "*"
      }
    },
    {
      "action": "ALLOW",
      "selector": {
        "identifier": "local://dockerfile"
      }
    },
    {
      "action": "CONVERT",
      "selector": {
        "identifier": "docker-image://docker.io/library/bash:latest"
      },
      "updates": {
        "identifier": "docker-image://docker.io/library/debian:bullseye-slim"
      }
    }
  ]
}

$ ./buildx create --name foo
$ printf 'FROM bash:latest' | EXPERIMENTAL_BUILDKIT_SOURCE_POLICY=<(jq -nc '{ rules: [ { action: "DENY", selector: { identifier: "*" } }, { action: "ALLOW", selector: { identifier: "local://dockerfile" } }, { action: "CONVERT", selector: { identifier: "docker-image://docker.io/library/bash:latest" }, updates: { identifier: "docker-image://docker.io/library/debian:bullseye-slim" } } ] }') ./buildx --builder foo build --progress=plain -
#0 building with "foo" instance using docker-container driver

#1 [internal] load build definition from Dockerfile
#1 transferring dockerfile: 53B done
#1 DONE 0.0s

#2 [auth] library/bash:pull token for registry-1.docker.io
#2 DONE 0.0s

#3 [internal] load metadata for docker.io/library/bash:latest
#3 DONE 0.6s
WARNING: No output specified with docker-container driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load
ERROR: failed to solve: failed to load LLB: error evaluating the source policy: source "docker-image://docker.io/library/bash:latest@sha256:d6f71002f8b3cb7660d5bec2436a850564e79673c43be75f31908c250f808458" denied by policy: source denied by policy

[tianon]

If I run my reproducer several times, I can sometimes get it to fail claiming local://dockerfile is denied by policy too, but that feels like a slightly different issue. 😬

WARNING: No output specified with docker-container driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load
ERROR: failed to solve: failed to read dockerfile: failed to load LLB: error evaluating the source policy: source "local://dockerfile" denied by policy: source denied by policy

If I adjust the policy to match on docker-image://docker.io/library/bash:latest@* instead, I then get denied on pulling debian:bullseye-slim, as expected:

$ printf 'FROM bash:latest' | EXPERIMENTAL_BUILDKIT_SOURCE_POLICY=<(jq -nc '{ rules: [ { action: "DENY", selector: { identifier: "*" } }, { action: "ALLOW", selector: { identifier: "local://dockerfile" } }, { action: "CONVERT", selector: { identifier: "docker-image://docker.io/library/bash:latest@*" }, updates: { identifier: "docker-image://docker.io/library/debian:bullseye-slim" } } ] }') ./buildx --builder foo build --progress=plain -
#0 building with "foo" instance using docker-container driver

#1 [internal] load build definition from Dockerfile
#1 transferring dockerfile: 53B done
#1 DONE 0.0s

#2 [internal] load metadata for docker.io/library/bash:latest
#2 DONE 0.3s
WARNING: No output specified with docker-container driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load
ERROR: failed to solve: failed to load LLB: error evaluating the source policy: source "docker-image://docker.io/library/debian:bullseye-slim" denied by policy: source denied by policy

And finally updating the policy to allow debian:bullseye-slim (even without a digest wildcard), which then works (after one instance of not working claiming local://dockerfile is denied again 😭):

$ printf 'FROM bash:latest' | EXPERIMENTAL_BUILDKIT_SOURCE_POLICY=<(jq -nc '{ rules: [ { action: "DENY", selector: { identifier: "*" } }, { action: "ALLOW", selector: { identifier: "local://dockerfile" } }, { action: "CONVERT", selector: { identifier: "docker-image://docker.io/library/bash:latest@*" }, updates: { identifier: "docker-image://docker.io/library/debian:bullseye-slim" } }, { action: "ALLOW", selector: { identifier: "docker-image://docker.io/library/debian:bullseye-slim" } } ] }') ./buildx --builder foo build --progress=plain -
#0 building with "foo" instance using docker-container driver

#1 [internal] load build definition from Dockerfile
#1 transferring dockerfile: 53B done
#1 DONE 0.0s

#2 [internal] load metadata for docker.io/library/bash:latest
#2 DONE 0.3s

#3 [auth] library/debian:pull token for registry-1.docker.io
#3 DONE 0.0s

#4 docker-image://docker.io/library/debian:bullseye-slim
#4 resolve docker.io/library/debian:bullseye-slim
#4 resolve docker.io/library/debian:bullseye-slim 0.9s done
#4 DONE 0.9s
WARNING: No output specified with docker-container driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load
ERROR: failed to solve: missing provenance for zycemdpyd1jtjlfmk6lnjrs7x

(ooh, and a cute missing provenance error 👀)

[tonistiigi]

@cpuguy83 The issue seems to be that the ResolveImageConfig does not go through the policy as well. In Dockerfile the image names user sets don't actually end up in LLB but are extended to contain the digest as well before while the ENV are loaded from image config. If this is true then it also means that for example for a digest ref inside Dockerfile(or wildcard in policy) the convert rule would change it but image config metadata is still pulled from the wrong image.

(ooh, and a cute missing provenance error 👀)

I fixed some cases for this recently(PRs open). If it still shows up after fixing the rest then I can take this one.

[cpuguy83]

What's not apparent to me is how to get this into the resolver because when the frontend calls the resolver the only policy it knows about is whatever policy the frontend came with.
Whatever policy the client passed in is not known to the frontend.

My first inclination was to put it into a ResolveImageConfigRequest but its not very helpful since it could never have the client supplied policy when called from a frontend.

[tonistiigi]

@cpuguy83 The frontend should not itself shouldn't validate the policy and can only add additional policies in https://github.com/moby/buildkit/blob/master/frontend/gateway/client/client.go#L133 . For the policies from client, they are already loaded to the server. In https://github.com/moby/buildkit/blob/master/solver/llbsolver/bridge.go#L293 I think you can just call the same https://github.com/moby/buildkit/blob/master/solver/llbsolver/bridge.go#L76-L79 to get policy from the *llbBridge.

One thing I was thinking about though is what if:

• there is a conversion from alpine:latest to busybox:latest
• ResolveImageConfig does the conversion and returns busybox's digest and config
• Now Dockerfile frontend makes up a digest ref. That would be alpine:latest@sha256:busyboxsha.
• This is sent to the sourcepolicy via LLB and may not match again if the conversion rule is only alpine:latest based? (is this case possible?)

To solve that case I think we would need to extend ResolveImageConfig so it can also return a new ref string that has passed policy, not only the resolved digest.

[cpuguy83]

@tonistiigi Should the policy matcher discard digests in the past in source unless the match selector also has a digest?

[tonistiigi]

@cpuguy83 I'm not sure I understand. Could you add an example you have in mind?

[cpuguy83]

Ref passed in to evaluate: docker.io/library/busybox:latest@sha256:deadbeef
Policy match selector: docker.io/library/busybox:latest

For the purposes of matching a rule should the matcher drop that digest?

[tonistiigi]

I think by default, it should not, but it may be useful to make it configurable (in an easier way than adding a wildcard to the ref).

[cpuguy83]

Why I was thinking this:

The default matcher is the wildcard matcher.
While the example does not provide a literal wildcard it seems like that would be the desired outcome unless the exact matcher is specified.
So really this change would be to modify the wildcard matcher to assume a wildcard after a tag if none is supplied.

[tianon]

Similar edge case for a selector of docker.io/library/busybox:latest@sha256:deadbeef and a reference being matched of docker.io/library/busybox@sha256:deadbeef (or vice versa).