Match a signature for any tag when pulling/coping by digest about image HOT 20 CLOSED

Excerpt from an IRC conversation so we don't forget:

09:58             runcom: hey Miloslav, doing some testing, I noticed I can sign a tag, then resolve to the trusted digest and pull a trusted image, but if I"m going to pull that image by 
                          digest the signature isn't valid
09:59             runcom: is this normal?
10:12               mitr: Yeah, the signature is “for” the specific signed tag
10:12             runcom: can't we associate a sig with his digest also? 
10:12               mitr: The policy allows relaxing signature matching to repo (signedIdentity/Type:{matchRepository,exactRepository} but that would be an admin action
10:13               mitr: You _can_ sign a digest (well, if you know it in advance to do (skopeo copy)
10:13               mitr: Matching a signature for any tag when pulling by digest... dunno
10:13             runcom: alright, so matchRepository would allow me to check against the image@digest I guess
10:13               mitr: It might make sense
10:13             runcom: yeah
10:13             runcom: because I'm now checking in docker CLI for valid signatures, I get OK and then go ahead and pull by digest
10:14             runcom: but I get "signature not accepted" because the reference is now image@digest
10:14               mitr: Another option would be to extend the policy to exempt pull by digest from signature checking altogether ... if you have a digest you might know what you are doing
10:14             runcom: YEAH!
10:14             runcom: please lol
10:14             runcom: (joking but that was my first thought)
10:14               mitr: OTOH if the admin wants to only run signed images then they REALLY want to only run signed images
10:15             runcom: I guess we can try to match a sig for any tag when pulling by digset then
10:15               mitr: I haven’t thought about this too deeply/carefully so far; and I’m afraid I didn’t realize how it affects the Docker plugin
10:17             runcom: docker pull runcom/busybox; the docker CLI checks signatures against "docker.io/runcom/busybox:latest"; signatures OK; the docker CLI resolves the trusted manifest 
                          Digest; the docker CLI pull by runcom/busybox@HASH; the plugin blocks server side because no signatures are accepted for runcom/busybo@HASH
10:17             runcom: (hope it's clear)
10:17               mitr: Yeah
10:18               mitr: Tentatively, an extra field to signedBy? untaggedReferences: {prohibit, matchPrecisely, matchSignatureForAnyTag} (modulo better naming)
10:18             runcom: I like that names as well :D
10:48             runcom: I'm going to open an issue in containers/image to track this - not sure how high is this in priority
10:48               mitr: Please do

from image.

The loosest match would be the best for default. Users will expect the TAG==Digest behaviour. Which I guess is matchSignatureForAnyTag

from image.

This is a blocking issue for the trust-plugin (and the docker branch with --trust=gpg for that matter) in case people the plugin tells the user he needs to pull by a trusted digest and then pulling by digest results in a not accepted signature for that digest (because the signature is only for the tag).

The only way right now to relax this is to use:

        "docker.io/runcom/busybox": [
            {
                "type": "signedBy",
                "keyType": "GPGKeys",
                "signedIdentity": {
                    "type": "matchRepository"
                },
                "keyPath": "/home/amurdaca/mykey.asc"
            }
        ],

but as noted by @mtrmac - this is an administrator thing

from image.

The loosest match would be the best for default. Users will expect the TAG==Digest behaviour. Which I guess is matchSignatureForAnyTag

agree here

from image.

I'm working on this, we can bikeshed on naming on the PR when I'll submit it - let's go the way Miloslav outlined in the chat above

from image.

"docker.io/runcom/busybox": [
{
"type": "signedBy",
"keyType": "GPGKeys",
"signedIdentity": {
"type": "matchRepository"
},
"keyPath": "/home/amurdaca/mykey.asc"
}
],

@mtrmac this isn't working anymore (but used to! testing with projectatomic/docker#200):

$ skopeo copy --sign-by [email protected] docker://alpine:3.1 docker://runcom/busybox2

$ docker pull runcom/busybox2
Using default tag: latest
Trying to pull repository docker.io/runcom/busybox2 ... 
docker.io/runcom/busybox2:latest isn't allowed: Signature for identity runcom/busybox2:latest is not accepted

$ cat /etc/containers/policy.json
{
    "default": [
        {
            "type": "insecureAcceptAnything"
        }
    ],
    "transports": {
        "docker": {
            "docker.io/runcom/busybox2": [
            {
                "type": "signedBy",
                "keyType": "GPGKeys",
                "signedIdentity": {
                    "type": "matchRepository"
                },
                "keyPath": "/home/amurdaca/mykey.asc"
            }
            ]
         }
    }
}

$ cat /etc/containers/registries.d/1.yaml 
docker:
    docker.io:
        sigstore: file:///home/amurdaca/signatures
        sigstore-staging: file:///home/amurdaca/signatures/staging

$ tree /home/amurdaca/signature/docker.io  
docker.io
└── runcom
    └── busybox2@sha256:ce3f5f3d27d79caa5e36f2a38d0f054dde1d5ce4b6d8e19be087d42310fdcd04
        └── signature-1

2 directories, 1 file

from image.

@mtrmac has something changed which makes the above failing?

from image.

docker.io/runcom/busybox2 vs. runcom/busybox2:latest ? Instrument signature/policy_reference_match.go to see how they are parsed, I guess.

from image.

(I can’t remember anything changing in this area FWIW.)

from image.

docker.io/runcom/busybox2 vs. runcom/busybox2:latest ? Instrument signature/policy_reference_match.go to see how they are parsed, I guess.

sorry, the point was that the above used to work - runcom/busybox2:latest get translated to docker.io/runcom/busybox2:latest and the policy was working correctly 2 weeks ago.....

from image.

Could this be because, within docker/docker + RH patches, docker/docker/reference is patched with a different semantics? Just guessing.

from image.

Could this be because, within docker/docker + RH patches, docker/docker/reference is patched with a different semantics? Just guessing.

it could - but projectatomic/docker#200 was working fine with containers/image 2 weeks ago so something happened in between :(

from image.

No idea. Can you perhaps bisect that? (Note that containers/image does not vendor anything, so docker/docker/reference comes from whatever your build environment is.)

from image.

@mtrmac you were right, I placed some logs around and:

"runcom intented: docker.io/runcom/busybox2:signed"
"runcom intented.Name(): docker.io/runcom/busybox2"
"runcom signature: runcom/busybox2:signed"
"runcom signature.Name(): runcom/busybox2"

So, we're signing with skopeo with the upstream docker/docker/reference where instead in projectatomic/docker#200 we qualify the image with upstream docker.io.

from image.

None of that should be a dealbreaker; upstream docker/docker/reference, in WithName, calls normalize, and that ensures that the semantically equivalent inputs also have string-equal .Name() return values.

Apparently the RH patch somehow breaks or removes the normalization logic? (Note that both removing it, and using a different one than upstream, would be very bad for signatures.)

Note that the signature match code does not really care what the normalization is, but there must be one. We can, easily enough, use a FullName form inside the signatures, but then we would have the opposite problem that abbreviated references would not match against a fully-expanded name in the signature.

from image.

@mtrmac yeah, figured that. RH code calls the check signatures with a fully qualified image reference, it should be easy to hack something into docker/docker to fix this I guess.

BTW, we may probably need an actual fork just for containers/image which uses upstream docker/docker reference?

from image.

BTW, we may probably need an actual fork just for containers/image which uses upstream docker/docker reference?

@mtrmac wdyt? I know this can be bad as well but at least it ensures us we use the same docker/docker/reference when checking signatures.

from image.

We didn’t have that, then we had that, now we don’t have that again.

If re-adding that again is the simplest solution for signatures, sure, why not.

I am, a bit, worried, though, that this lack of normalization will manifest elsewhere in docker as well. (See e.g. the “docker pull does not update some tags” issue discovered and discussed recently in email.)

from image.

I am, a bit, worried, though, that this lack of normalization will manifest elsewhere in docker as well. (See e.g. the “docker pull does not update some tags” issue discovered and discussed recently in email.)

I'm worried as well about this - that patch is almost 2 years old at least and apart from that bug discussed in the email I didn't spot anything significant till now. OTOH I'm not confident enough to change that code for the upcoming release :( but we definitely need an audit of that code wrt normalization.

from image.

@mtrmac git submodule? 😆

BTW, this change requires docs from #50 to be updated

from image.