sigstore / sigstore-python

A Sigstore client for Python
https://pypi.org/p/sigstore
Other
214 stars 41 forks source link

_internal/tuf: Avoid constant metadata updates #376

Open jku opened 1 year ago

jku commented 1 year ago

Description

Joshuas original TUF PR and the "sigstore TUF client" design doc contain the idea of not fetching remote metadata if it's not required:

I did not include either approach in the merged TUF implementation as it felt unnecessary for v1 and not completely ready. The rough idea still makes sense: e.g. a CI calling sigstore verify a thousand times in a row should work, and there should be no need to make the root and timestamp requests that TUF clients normally do at startup more than once during those thousand calls.

I've made an experimental PR for python-tuf that tries to implement the timestamp expiry idea a bit more robustly but I don't think this is purely a python-tuf feature, sigstore-python is going to need changes as well:

jku commented 1 year ago

how do we deal with e.g. verify failure because the signature uses a key/cert that we don't know about because we're using three days old certs and new ones were published yesterday?

Very careful repository/instance maintenance would actually solve this:

but maybe this is too much to ask from maintenance?

woodruffw commented 1 year ago
  • what is the policy that sigstore-python actually wants? is "timestamp is valid and verify succeeds" good enough for sigstore-python, or should there be an additional "we've updated tuf metadata less than X days ago" requirement

IMO the latter makes sense -- the 99% case here will probably be performing a whole bunch of sign/verify operations in a short timeframe (e.g., a release workflow's lifetime on GitHug Actions) within virtual environments. Given that, I think 48 to 72 hours is a reasonable cache period.

Very careful repository/instance maintenance would actually solve this:

I like this idea! But yeah, it does add quite a burden -- we should shop this with the Sigstore service maintainers (cc @asraa and @haydentherapper?) to see what they think..

Even without that, I think we can do a little better than "try to verify and fail": we could instead track the key IDs for our current Rekor and CTFE keys, and re-fetch from TUF if the incoming verification materials need keys (by ID) that we don't currently know. That would be a little trickier for certs (we'd probably end up having to partially re-implement X509Store?) though.

haydentherapper commented 1 year ago

repository could publish keys/certs for verifiers X weeks (timestamp expiry period) before they are made available to signers -- e.g. fulcio cert could be added in the metadata before the instance starts signing with that root cert

I don't think this is a burden on maintainers, this is what's necessary to not break clients. We did this with the CT log key rotation for example. We first included the key in the target metadata, waited until all clients' local timestamps should have expired (the timestamp expiration period of 1 week), and then rotated the log.

jku commented 1 year ago

this is what's necessary to not break clients.

Note that with TUF spec-compliant clients it explicitly is not necessary-- in TUF timestamp expiration does not define the time that clients can avoid updating, it defines the the time that repository can avoid creating new metadata. Clients must update every single time...

But I do agree that (once we have this ironed out) it might make sense for sigstore to

  1. document that clients can avoid updating metadata every time (even if it's non-compliant with TUF spec)
  2. promise to maintain services in a way that ensures clients will not encounter unknown key material when doing this
woodruffw commented 1 year ago

xref https://github.com/theupdateframework/python-tuf/pull/2251 (now closed, but a good example of a working start/basis here.)

IMO we can consider this lower-priority now that Sigstore's TUF repo will contain "bundled" trust roots -- that'll the number of network round-trips before first Sigstore sign/verify substantially.

Edit: xref #488 for the above.