Add image signature verification support for images signed by non-default ecdsa cosign keys#1398
Conversation
esposem
force-pushed
the
image_signature
branch
2 times, most recently
from
5323148 to
c4a7ae3
Compare
There was a problem hiding this comment.
There are really two different things being fixed here. I didn't look much at the first one, but the second one is something that I am already working on fixing. The fix here does not seem correct. See comments.
We've got to be really careful with the signature validation code.
Also we've got some unhappy CI jobs.
|
Fine for me @fitzthum, your fix is much better. I can drop the last two commits (even though I didn't realize the last is related to the second one). Is that ok? The main problem is that it seems that image-rs only accepts a RSA key algorithm, so if anything else is being used then it will fail. Which CI job is unhappy? The only failing I see it's unrelated, and failing in other PRs too. |
|
Yes, can drop the last two and I will take a look at the first one. CI could be unrelated. Let me see. This PR may actually go beyond what is supported in that crate. |
|
Ok @fitzthum @rajatchopra I investigated a bit, basically we can figure the key algo using the spki crate, but when it comes to RSA, we need to guess if it's PKCSI or PSS. Therefore the approach remains the same, but it's cleaner. We only get >1 result in the vector if it's RSA, and we need to try all of them. |
|
Please see this method. Looks like it will detect the key info automatically for the keys that are supported by the sigstore crate. |
+1. When we designed sigstore-rs API, we take into consideration that a un-recognized key should be supported. But note that only PEM can be supported as the input. |
image-rs should auto-detect and support multiple RSA keys when verifying the image signature. When trying to verify a container signed with a RSA PKCSI key, image-rs fails with "Ecdsa-P256 from der bytes to public key failed: unknown/unsupported algorithm OID: 1.2.840.10045.2.1" This allows support for multiple types of keys. Unfortunately RSA algo cannot be inferred from just the key bytes, so we need to try all of them. Signed-off-by: Emanuele Giuseppe Esposito <[email protected]> Assisted-by: AI
|
Done, thanks for the suggestion, much cleaner |
fitzthum
left a comment
fitzthum
left a comment
There was a problem hiding this comment.
LGTM
seems like the test cases don't exercise all the possible combinations, but as long as you've got what you need it should be ok
|
Yeah I tried with the RH public key and it goes into the other error ( |
Xynnn007
left a comment
Xynnn007
left a comment
There was a problem hiding this comment.
The latest rand_core is 0.10.0. 0.6 is a very old version. Let me update in a following PR.
4 participants
The goal of this PR is to support image-rs to verify images signed by cosign with a non-Ecdsa default key.
Which means:
The concrete example that this PR is based on is to be able to verify
registry.access.redhat.com/ubi9/ubi:latestwithhttps://security.access.redhat.com/data/63405576.txtkey.Doing this verification by hand works:
But with image-rs, it fails first with
"Ecdsa-P256 from der bytes to public key failed: unknown/unsupported algorithm OID: 1.2.840.10045.2.1", then it complains aboutSigPayload's manifest digest does not match, the input is sha256:.., but in SigPayload it is sha256:...and after fixing that, it works only by digest and not by tag.