-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fail when valid provided cert chain is not on log #119
Conversation
Ensure clients are reconstructing the rekor/log entry with the whole provided ceritificate rather than just the leaf. This fails because the entry is written with just the leaf but a validation event is requested with the full chain. Signed-off-by: Appu Goundan <[email protected]>
@woodruffw I don't know enough about the python client, but I believe the reconstructed rekor object should not be found on rekor because the user provided a full chain (rather than just the leaf)? |
Oh this is a fun issue! It's currently undefined in the sigstore spec if a client must upload the entire chain or just the leaf. Cosign for example only uploads the leaf, to save on storage. For certificate transparency, it mandates uploading the entire chain, though I think CT v2 (unimplemented) removed this to save on storage. |
I think clients should verify with what they were given though. So if they uploaded a full chain, that's what's used for client verification and that's what a user should provide to the client. |
Yeah, I think this is a spec gap 🙂 Making sure I understand: the entry in question is the Rekor entry, correct? If so, I believe sigstore-python also only uploads the leaf: (I haven't fully thought this through yet, but is anything gained by having the client attempt to validate the Rekor entry with the "full" chain? My intuition is that chain construction ensures that this is sound regardless.) |
It's not necessarily the full chain, but I guess any provided cert chain during verification. If we're lopping off part of it would that lead to inconsistencies? How does one decided how much of the chain to remove? I'm not exactly sure this provides any strong value except ensuring all clients behave the same way. But it could help in the future if there are untrusted intermediates? I'm pretty bad at navigating python, but I believe with no provided rekor entry, reconstructing the UUID should lead to a rekor entry not found error (if we use the provided chain, instead of a leaf)
|
Yeah, I think this needs to be clarified in the spec. One relevant part: the bundle specification at minimum says that the This test case isn't formed as a bundle, but my (maybe incorrect) interpretation of that language was that clients only ever use and interchange the minimum viable chain state, which for the Public Good Instance is just the leaf signing certs themselves. |
Yeah, I guess my confusion is how do we define the |
So I think this test might need to become 2 tests:
|
Yep. Specifically, we'll probably need the following cases:
I think this should be silently ignored? For better or worse, PEM parsers tend to be malleable in this way (they tend to scan the input looking for the first appropriate PEM tag, ignoring anything before and after). Producing an error here would probably require excessively specializing each implementation's PEM parsing/loading behavior. (But I'm not confident about this.) |
I think this has been resolved by v0.3.0 spec and documentation |
Ensure clients are reconstructing the rekor/log entry with the whole provided ceritificate rather than just the leaf. This fails because the entry is written with just the leaf but a validation event is requested with the full chain.