cosign icon indicating copy to clipboard operation
cosign copied to clipboard

CLI exit status should be improved

Open Itxaka opened this issue 2 years ago • 1 comments

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!

Itxaka avatar Oct 25 '21 11:10 Itxaka

+1 on this! We could switch to a different exit code.

dlorenc avatar Dec 19 '21 03:12 dlorenc

@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

ChrisJBurns avatar Dec 20 '22 17:12 ChrisJBurns

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 :)

Itxaka avatar Dec 21 '22 13:12 Itxaka

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)

znewman01 avatar Dec 21 '22 13:12 znewman01

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

Itxaka avatar Dec 21 '22 14:12 Itxaka

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.

ChrisJBurns avatar Jan 15 '23 21:01 ChrisJBurns

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/error to cmd/cosign/error.
  • Cosign pkg/error errors should get wrapped as they get handled in cmd/*

Otherwise I'm pretty happy with a cleaned-up version of what you proposed

znewman01 avatar Jan 15 '23 22:01 znewman01

@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.

ChrisJBurns avatar Jan 28 '23 16:01 ChrisJBurns

@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

ChrisJBurns avatar Apr 10 '23 18:04 ChrisJBurns

Looking really good to me, thank you folks for implementing this!

Itxaka avatar Apr 10 '23 19:04 Itxaka