Conformance: sigstore-python's conformance runner should support `--trusted-root`

[woodruffw]

See https://github.com/sigstore/sigstore-conformance/pull/101.

[woodruffw]

Partial dupe of #779, closing that one in favor of this.

[jku]

From #779:

This will unify the current rats nest of flags for configuring custom Sigstore behavior: we'll be able to deprecate and eventually remove --rekor-url, --rekor-root-pubkey, --fulcio-url, --ctfe, and possibly some others. This in turn will allow us to close https://github.com/sigstore/sigstore-python/issues/324.

This sounds good:

• allow specifying --trusted-root <FILE> so TUF can be skipped without making a mess of the UI
• figure out rekor url and fulcio url from trusted root contents instead of hard coding (if the urls are not in there, they should be added)
• deprecate flags mentioned above to make it clear a complete trusted root should be provided instead (there might be some exceptions to this but I think that's a good general rule)

All of these can happen in separate issues/PRs

[woodruffw]

• deprecate flags mentioned above to make it clear a complete trusted root should be provided instead (there might be some exceptions to this but I think that's a good general rule)

Yeah, I think we can get all of them with this -- we could deprecate with 2.1.x and plan to fully remove with 3.x, which would make me very happy 🙂

[jku]

Some notes after a bit of thinking:

• We have essentially three cases (only first one is implemented today)
  • default: trustroot comes from TUF
  • offline: trustroot comes from TUF target cache
  • trustroot comes from user via --trusted-root
• Current TrustedRoot class handles two separate things: the TUF client operations and easy access to trustroot contents
  • We should separate them as the TUF operations are only needed in the default case above but the latter is needed in all three cases

[woodruffw]

• We should separate them as the TUF operations are only needed in the default case above but the latter is needed in all three cases

Makes sense to me!

[jku]

I'll take this. Plan is:

• Move all of the parsing and sanity checking into a class derived from sigstore_protobuf_specs TrustedRoot
• this class has multiple constructors:

  def from_file(path: str) -> MyTrustedRoot:
  def from_tuf(url: str, offline: bool = False) -> MyTrustedRoot:
  def production(offline: bool = False) -> MyTrustedRoot: # helper, calls from_tuf()
  def staging(offline: bool = False) -> MyTrustedRoot: # helper, calls from_tuf()

• from_tuf() (and the helpers production() and staging()) will use TrustUpdater by default but from_file() (and the others if offline) will just get trust root from a file
• the tuf module will now only be used from this new class
• adding UX for --trusted-root should be fairly trivial but I'll probably leave the offline UX support for a later PR if that looks complicated

[jku]

Okay I have the internal refactor done... but I'm not sure how to correctly add the --trusted-root flag:

• I should choose the rekor_url value based on data from trusted root: can I assume all values in tlogs array in the trusted root have the same baseURL? Maybe RekorClient should handle url per key?
• signing has some further issues (but then signing with --trusted-root might not be as significant a feature)
  • same url question for CAs...
  • oauth issuer url is not documented in the trusted_root -- maybe it should be

@woodruffw I'm guessing you'll have a hunch on this

[jku]

I should choose the rekor_url value based on data from trusted root: can I assume all values in tlogs array in the trusted root have the same baseURL? Maybe RekorClient should handle url per key?

I think I'll look into modifying the Keyring abstraction so that it keeps track of url-key pairs instead of just keys. Alternative ideas are welcome.

Alternatively I might stop here and make a refactor PR out of my current changes... So then we can do a spearate refactoring for keyring/rekorClient and then add the UI option

[woodruffw]

Maybe RekorClient should handle url per key?

This sounds right to me -- my read of the TrustedRoot definition is that the base URLs are not guaranteed to be equivalent between different log instances (and maybe even can't be, since the point is to have multiple distinct logs?)

same url question for CAs...

Yeah, this is confusing/ambiguous: my intuition there is that the TrustedRoot needs some way to signal the "CSR" CA, i.e. the one that's actually talked to for signing purposes. But that doesn't seem to exist yet, so maybe the right logic here (for now) is to fail unless there's exactly one CA listed and that CA has a URI?

Edit: Another maybe reasonable decision procedure here is to search through the listed CAs, and use the first that lists a URI? That one is presumably a sufficient one to submit CSRs to.

oauth issuer url is not documented in the trusted_root -- maybe it should be

This would be the Dex instance URL, right? If so, yes, agreed.

[woodruffw]

I think I'll look into modifying the Keyring abstraction so that it keeps track of url-key pairs instead of just keys. Alternative ideas are welcome.

Alternatively I might stop here and make a refactor PR out of my current changes... So then we can do a spearate refactoring for keyring/rekorClient and then add the UI option

That keyring refactor SGTM, but 👍 on creating an initial refactor PR first -- smaller changesets will be easier to test 🙂

[jku]

I think we agree on everything here.

• I'll make a refactoring PR #844
• I'll make an issue about refactoring KeyRing/RekorClient (#845)
• this issue (adding --trusted-root) can be fixed after that
• Since that all might take a while I added an xfail in the workflow in the mean time so we can start using sigstore-conformance 0.0.9: When this one is fixed the xfail should be removed. #825