_cli: Add --certificate-chain and support --rekor-url for verification
#323
Conversation
…ation Signed-off-by: Alex Cameron <asc@tetsuo.sh>
tetsuo-cpp
force-pushed
the
alex/certificate-chain
branch
from
415213c
to
2596a2f
Compare
sigstore/_cli.py
Outdated
| pass | ||
|
|
||
|
|
||
| def _split_certificate_chain(chain_pem: str) -> List[bytes]: |
There was a problem hiding this comment.
This is pretty nasty. Not sure if there is a cleaner way to do it.
I believe that the load_pem...functions in cryptography load the first valid PEM entry and leave the rest so I don't think they can help us here.
There was a problem hiding this comment.
Yeah, this isn't ideal, but it looks like neither cryptography nor pyOpenSSL exposes a "load an entire chain from PEMs" API.
I think the corresponding OpenSSL API is SSL_CTX_get0_chain_certs, so this might be worth a patch to pyOpenSSL in the medium term. But for now, something like this approach is fine 🙂
There was a problem hiding this comment.
Looks like the OpenSSL APIs that accomplish this all work in the context of SSL contexts or connections, so they're probably not a great fit...
There was a problem hiding this comment.
NB: I opened pyca/cryptography#7878 to add this API, so we'll be able to use it starting with cryptography 39 (assuming they accept it).
There was a problem hiding this comment.
Awesome, thanks @woodruffw.
| verifier = Verifier( | ||
| rekor=RekorClient( | ||
| url=args.rekor_url, | ||
| pubkey=args.rekor_root_pubkey.read(), |
There was a problem hiding this comment.
I wonder whether we should warn if a user supplies --rekor-root-pubkey but don't set the --rekor-url. Same with the new --certificate-chain. At the moment they just get ignored which might be confusing.
Same thing when we set --rekor-url but don't set --rekor-root-pubkey. It defaults to the rekor.pub that comes bundled with sigstore-python (which is almost certainly not what you want when you're using a custom Rekor) instead of warning.
Similar issues also exist in the signing code path.
There was a problem hiding this comment.
Yeah, it would be good to catch this and flag it as a warning at the minimum (or maybe even an error, since we expect it to fail anyways).
There was a problem hiding this comment.
Let's track this with a follow-up issue.
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Co-authored-by: William Woodruff <william@trailofbits.com> Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
tetsuo-cpp
force-pushed
the
alex/certificate-chain
branch
from
d8b25d5
to
b3f3847
Compare
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Closes #296