sigstore / sigstore-python

A Sigstore client written in Python
https://pypi.org/p/sigstore
Other
220 stars 42 forks source link

verification and signing need to support different sets of keys/certs #375

Closed jku closed 5 months ago

jku commented 1 year ago

Current TrustUpdater implementation (the tuf component in sigstore-python) always returns keys/certs that have status "Active". Verification should also use status "Expired". I'm not 100% sure if this applies to all target types but I think all certs used in verification at least.

I will have a look at this myself.

jku commented 1 year ago

I don't think I can figure this out on my own -- I don't believe the problems are TUF issue as such, I just don't understand how a sigstore client should do these things.

One issue is fulcio cert chains. Current set of certs is:

I assume client is supposed to form cert chains here but what's the logic: how do you select the certs that you put in one chain? I can do that for the active chain (just put all active keys in there) but what about the others?

The other issue is just the multi-key usage: I think this is just not implemented in sigstore-python yet? does cosign have examples already -- like using multiple rekor keys to verify a signature?

jku commented 1 year ago

After looking at cosign:

EDIT: have not found a definitive answer to the last question but it does seem like we can just stuff all certs we have into a openssl X509Store and it will figure out the chains.

woodruffw commented 1 year ago

EDIT: have not found a definitive answer to the last question but it does seem like we can just stuff all certs we have into a openssl X509Store and it will figure out the chains.

Yep! On the cert side at least, we can punt that complexity to X509Store 🙂

We already support multiple CTFE keys in the form of the CTKeyring; we should probably make sure we're fully populating it with every CTFE key we find in the TUF repo.

woodruffw commented 1 year ago

For Rekor, I don't think there's been a key rotation yet. But we'll probably want a similar approach to CTKeyring (and maybe even just to make that implementation generic).

woodruffw commented 1 year ago

For Rekor, I don't think there's been a key rotation yet. But we'll probably want a similar approach to CTKeyring (and maybe even just to make that implementation generic).

Looking at the code, the only thing we currently use/need the Rekor pubkey for is for SET verification. Each SET is represented within a RekorEntry that references the expected log_id (i.e., Rekor key ID), so we can make the current CTKeyring generic and reuse the current implementation for both CTFE and Rekor keys.

jku commented 1 year ago

So the case for fulcio certs seems clear. This should be easy to fix (and a bit harder to test):

My understanding of the key cases is not as good:

woodruffw commented 1 year ago
  • Even with production, I don't know how to find an old signature (that uses a cert that uses an expired chain)

The good news is that at least some of the sigstore-python releases should be signed and chained up to the old Fulcio root, as we've been publishing releases since well before the GA

  • which status keys are supposed to be used in the keyrings? all of them?

All of them, yeah -- my understanding is that statuses for the CTFE and Rekor keys mean the same thing as for the Fulcio cert chain(s): "Active" means that the key/material is actively being used to sign things on the given instance, while "Expired" means that the key/material is no longer being used to sign but may be needed to verify older signatures/Rekor entries/etc.

why are there two active CTFE keys in the production metadata -- what do the statuses and URIs really mean?

I'm not 100% sure about this 🤔 -- my understanding is that only one should be active at a time, but IIRC the maintenance team performed two CTFE rotations in 2022 (hence ctfe_2022.pub and ctfe_2022_2.pub). Maybe @asraa or @haydentherapper knows more?

is there any difference between signing and verify WRT the keys that should be included in the keyrings?

For signing, technically the only key we need in the keyring is the latest active CTFE key, since we need that to verify the SCT we get back from the transparency log. But I don't think it hurts to have all of them loaded, since the SCT identifies the key needed to verify it by key_id. But I could be underthinking that.

For verification, we need all active and expired keys included in their respective keyrings.

haydentherapper commented 1 year ago

I'm not 100% sure about this 🤔 -- my understanding is that only one should be active at a time, but IIRC the maintenance team performed two CTFE rotations in 2022 (hence ctfe_2022.pub and ctfe_2022_2.pub). Maybe @asraa or @haydentherapper knows more?

For CTFE in staging, yes, there were two sharding events in 2022, hence the key namings. Even though I added status awhile ago, I'm a little skeptical it's useful. Older CT logs are still up even though we might mark the public keys as "expired", signifying it's a frozen log. As a user, what should I do with that knowledge? In Cosign, we just print a warning you're verifying with an old key, but I'm not sure this is the right behavior. It's not a risk to use an old key if the log is still up. If the key isn't trustworthy, we'd remove it from the targets.

woodruffw commented 1 year ago

Yeah, a warning doesn't make sense to me either (per the same reasoning).

This is now a tangent, but are there other status states available to us? It sounds like the actual states here are "Active" and "Rotated," or some other word to describe the fact that the log is frozen but still considered valid.

asraa commented 1 year ago

This is now a tangent, but are there other status states available to us? It sounds like the actual states here are "Active" and "Rotated,"

We are going to move them to NotBefore/NotAfter, I think that makes it a lot more clear that it's forzen but valid for verification.

. It's not a risk to use an old key if the log is still up.

yeah, it's not a security issue but a robustness issue. We still trust the keys, but it should not happen that we sign and have a usage that's expired.

my understanding is that only one should be active at a time, but IIRC the maintenance team performed two CTFE rotations in 2022 (hence ctfe_2022.pub and ctfe_2022_2.pub). Maybe @asraa or @haydentherapper knows more?

It still holds to check on the signing path that the key is Active (even if there is two during a rotation). But I agree that the status being binary is problematic for these cases. Technically it should have been better to have a slightly overlapping validity range.

jku commented 1 year ago

collecting the current state of things from this discussion (and from some further education from asra):

So proposing these practical TODO items:

woodruffw commented 1 year ago

Sounds like a good list! I have @jleightcap tagged to handle the Rekor keyring side.

woodruffw commented 5 months ago

I believe this is done, both with #458 and @javanlacerda's "purpose" refactor.