sigstore / sigstore-python

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

Improve our internal key management #927

Closed woodruffw closed 5 months ago

woodruffw commented 6 months ago

sigstore-python's internal key management is currently a bit of a mess. The following parties are at play:

  1. TUF/the trust root/bundle: this is the ultimate source of all of our "a priori" key material.
  2. Fulcio: the trust root distributes one or more Fulcio CAs as chains of X.509 certificates. Internally, we mash these into a single pool and return certificates with expired timeranges at the trustroot level (which is probably incorrect).
  3. Certificate Transparency ("CTFE"): the trust root distributes one or more CTFE keys, which we load so long as they are valid or expired (but not from the future).
  4. Rekor: the trust root distributes one or more Rekor keys, which we load similarly to CTFE keys.

Now, the status quo:

  1. Our internal RekorClient has _ct_keyring and _rekor_keyring members, representing the CTFE and Rekor keyrings respectively as loaded from the trust root. The latter makes a little sense; the former makes no sense (since CTFE has nothing to do with Rekor).
  2. Our internal FulcioClient has no keyrings or certificate pools internally.
  3. We currently make no distinction in loaded keys/certs between signing and verifying, which is almost certainly incorrect.

Now, a solution:

  1. We should keep key management entirely within the TrustedRoot and its APIs. In practice, that means that TrustedRoot should have APIs roughly like the following:

    TrustedRoot.rekor_keyring(disposition="sign") # a suitable set of Rekor keys for use in the signing process
    TrustedRoot.ctfe_keyring(disposition="verify") # a suitable set of CTFE keys for use during verification

    ...and so forth. The disposition kwarg is pretty ugly; we just need some way to differentiate between the set of keys that are valid in each context.

  2. SigningContext and Verifier should probably take a TrustedRoot directly.
  3. RekorClient should have no key material within it at all.

CC @jku for thoughts.

jku commented 6 months ago

This sounds very much like I was imagining it. Thanks for writing this down.

The main obstacle to implementation are the legacy CLI flags... I wonder if we could workaround that by modifying the TrustedRoot in _cli.py: So we change all of the actual APIs to work with TrustedRoot only and then inside _cli.py we just somehow produce a reasonable TrustedRoot from the arguments we're given.

woodruffw commented 6 months ago

The main obstacle to implementation are the legacy CLI flags... I wonder if we could workaround that by modifying the TrustedRoot in _cli.py: So we change all of the actual APIs to work with TrustedRoot only and then inside _cli.py we just somehow produce a reasonable TrustedRoot from the arguments we're given.

Yeah, that's what I was thinking as well! I'm also not opposed to violently ripping those legacy flags out 😉

woodruffw commented 6 months ago

@javanlacerda would you be interested in taking a stab at this? If not, I can set aside some time for it (otherwise I'll be working on DSSE verification support).

javanlacerda commented 6 months ago

@javanlacerda would you be interested in taking a stab at this? If not, I can set aside some time for it (otherwise I'll be working on DSSE verification support).

Sure!!! I'll go on it asap