cosign
cosign copied to clipboard
CLI exit status should be improved
Description Currently if you run cosign verify against a non existing image, against a not signed image, against a signed image with a different key, the exit status is the same (1)
This makes difficult to automate using cosign to verify stuff, especially in projects where the signing is starting or in which all artifacts may not be signed and those are out of your control.
cosign should probably try to exit with a different status code if the verify failed, or if the image is not verified.
Examples:
Image without signature:
error: fetching signatures: remote image: GET https://index.docker.io/v2/library/ubuntu/manifests/sha256-626ffe58f6e7566e00254b638eb7e0f3b11d4da9675088f4781a50ae288f3322.sig: MANIFEST_UNKNOWN: manifest unknown; unknown tag=sha256-626ffe58f6e7566e00254b638eb7e0f3b11d4da9675088f4781a50ae288f3322.sig
$ echo $?
1
Non existing tag
error: GET https://index.docker.io/v2/library/ubuntu/manifests/latestf: MANIFEST_UNKNOWN: manifest unknown; unknown tag=latestf
$ echo $?
1
Signature not matching
error: no matching signatures:
failed to verify signature
$ echo $?
1
Cheers!
+1 on this! We could switch to a different exit code.
@dlorenc @Itxaka Am happy to work on this, although there are some reserved exit codes that have their own definition across Windows and Linux. I was hoping to piggy back off existing codes that may have a common definition across operating systems (in the same way that HTTP Status Codes do), but the fact that each operating system has its own defined set, we may have to define our own and (most likely) override the ones that may exist for the underlying operating systems.
3,4,5 for the three cases above? If so, do we think this will have any knock-on effects on the system to which the cosign binary runs on? If not, am happy to put up a PR on this with:
3 - Image without signature
4 - Non existing tag
5 - Signature not matching
IMO any numbers not reserved will do, as long as there is a differentiation on the errors that are generated so automating things can work with those exit codes. for example, I can deal with error 4 (non existing tag) as that migth be a user error, but If Im checking images in an automated fashion and I get error 5 (signature not matching) that should raise an alert as the image might be compromised. Same with 4 (image without signature), as I can make an automated job that checks for that and signs the images if they dont have a signature.
This is real scenario BTW, we check for signatures on our github.com/rancher/elemental-toolkit packages and fi signatures are missing our job will sign it. Currently we have no way of knowing if the error is missing signature or other :)
IMO any numbers not reserved will do, as long as there is a differentiation on the errors that are generated so automating things can work with those exit codes
Strong +1 to this. @ChrisJBurns I'll assign this issue to you for now; if you come up with a convention, please document it (maybe README.md? I think long run we'll want to have this in the equivalent of a man page)
FYI for elemental-cli we also had to implement exit codes under a cobra cli and ended up with a constants file which had all the exit codes with an explanation comment and we have a job to autogenerate the docs with some go code
That means that we never forget to update the docs with the exit codes when new ones are added, and the docs always have a small explanation of what the exit code means, very handy :)
You could probably hook the exit-code docs autogeneration here https://github.com/sigstore/cosign/blob/main/cmd/help/main.go#L34 as part of the gendocs command
All, I have implemented a very draft-ish version of this in the following commit. It's not perfect, still needs a cleanup, but I've managed to get a custom exit code thrown back (12) for unmatching signature, in addition to doc generation via the example from elemental-cli (@Itxaka hope you don't mind, I quite literally copied and pasted the GenerateDocs func as it worked perfectly just to get something).
The only thing left now is to probably conform to a pattern of how we do errors. Currently, I can see there is a mix and max of how we do this. One of the ways is that there is a VerificationError object and a NewVerificationError method in the errors.go file. But moving forward, we should probably conform around a CosignError object with a Message and Code variable, with a more generic RaiseError function that allows for the passing of a specific Error type struct (e.g. ErrNoMatchingSignatures = &CosignError{....}.
Welcome your thoughts @znewman01 @Itxaka @dlorenc. If we move towards the above approach, there's going to be a few changes around the code to ensure the rest of the errors are thrown the same way.
Awesome, thanks @ChrisJBurns! Something like that SGTM.
A couple main points of feedback:
- I'd like to separate the CLI errors (which include exit status etc.) from general Cosign errors. So basically I'd suggest moving a lot of
pkg/errortocmd/cosign/error. - Cosign
pkg/errorerrors should get wrapped as they get handled incmd/*
Otherwise I'm pretty happy with a cleaned-up version of what you proposed
@znewman01 Feel free to take a look (https://github.com/sigstore/cosign/pull/2673). Managed to keep code to minimum to reduce side affects on other functionality. Should be backwards compatible also and only affect code that explicitly has mapped error to exit codes, otherwise the standard exit code 1 is returned.
Should be somewhat extensible also, so for other errors and exit codes, it should be relatively easy to add the functionality for them. The PR contains not only the throwing of a custom exit code for "no matching signatures", but also for all supporting code to make this work. The next PR's that address the other 2 errors in the issue description, should be a matter of just adding the correct error types and exit codes and returning them in the places the error happens.
@Itxaka @znewman01
I think this can be closed as completed with fixes implemented in:
- https://github.com/sigstore/cosign/pull/2766 (fixed scenarios where image has no signature & verifying when the image tag doesn't exist)
- https://github.com/sigstore/cosign/pull/2673 (fixed scenarios where there were images with no matching signatures).
If you run cosign through the following examples you'll get the correct exit codes as defined in https://github.com/sigstore/cosign/blob/main/doc/cosign_exit_codes.md.
$ cosign version
...
GitVersion: 2.0.0
GitCommit: d6b9001f8e6ed745fb845849d623274c897d55f2
GitTreeState: "clean"
BuildDate: 2023-02-23T19:26:35Z
GoVersion: go1.20.1
Compiler: gc
Platform: darwin/amd64
Image with no signature
$ SRC_IMAGE=busybox
$ SRC_DIGEST=$(crane digest busybox)
$ IMAGE_URI=ttl.sh/$(uuidgen | head -c 8 | tr 'A-Z' 'a-z')
$ crane cp $SRC_IMAGE@$SRC_DIGEST $IMAGE_URI:1h
$ IMAGE_URI_DIGEST=$IMAGE_URI@$SRC_DIGEST
$ cosign verify --key cosign.pub $IMAGE_URI_DIGEST
Error: no signatures found for image
main.go:69: error during command execution: no signatures found for image
$ echo $?
10
Image with non existing tag
$ cosign verify --key cosign.pub ttl.sh/37708c4:1h
Error: image tag not found: GET https://ttl.sh/v2/37708c4/manifests/1h: MANIFEST_UNKNOWN: manifest unknown; map[Tag:1h]
main.go:69: error during command execution: image tag not found: GET https://ttl.sh/v2/37708c4/manifests/1h: MANIFEST_UNKNOWN: manifest unknown; map[Tag:1h]
$ echo $?
11
Image with no matching signature
$ cosign verify --key blue.pub $IMAGE_URI_DIGEST
Error: no matching signatures:
error verifying bundle: comparing public key PEMs, expected -----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEI2cpYDnNla7+kidLeulgtt3ZwAyC
m4PWQZO3mZA3Rxu6VRwtYpGTCflxteDrwgmQcrhHSyqyKxwQloLdPtHeaQ==
-----END PUBLIC KEY-----
, got -----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEi0+0MRDdb1d5A9dSEVJ/84fVjSVQ
IFYZl6hON0gEaFhzMogfyLT4lUVVoPc9LhwsQAeIWpamm6oInU5+FEGEfw==
-----END PUBLIC KEY-----
$ echo $?
12
Looking really good to me, thank you folks for implementing this!