sigstore-python icon indicating copy to clipboard operation
sigstore-python copied to clipboard

Fulcio: Switch to new-style claim extensions

Open woodruffw opened this issue 2 years ago • 8 comments

Just filing this for tracking purposes: https://github.com/sigstore/fulcio/pull/945 will change Fulcio's certificate extensions to make them more generic, avoiding unnecessary references to implementation details for e.g. GitHub.

These changes will follow Fulcio's deprecation policy, so no action is immediately required on our part.

woodruffw avatar Jan 11 '23 22:01 woodruffw

Looks like these changes have been merged upstream, so we should check to see whether the production or staging instances have deployed them. If so, we should follow suit.

woodruffw avatar Mar 08 '23 15:03 woodruffw

This changes have been fully deployed, so we should begin supporting them.

woodruffw avatar Apr 26 '23 14:04 woodruffw

I did some quick tests in https://github.com/jku/sigstore-python/commit/50418201b5ade7ddaa7dc2fdd9e86a7bccf04b42:

  • backwards compat decision needs to be made: likely we should support verifying already existing certificates that contain github OIDs only, like the test data currently in tree? My WIP patch does not do this but there's at least three ways about it:
    • Add more policies to policy module (e.g. OIDCIssuer is a AnyOf policy over both OIDCIssuerV1 and OIDCIssuerV2) -- this is quite a lot of new classes
    • make _SingleX509ExtPolicy support "alternative oids": I like this option, it's likely the least amount of work
    • keep complexity out of policy module completely (all policies are just _SingleX509ExtPolicy with a specific OID). Instead put the complexity in "verify github" code
  • GitHubWorkflowRepository no longer exists: the CLI can paper over the difference between that and the current Source Repository URI, but a single policy can't cover both
  • GitHubWorkflowName no longer exists: it probably should never have been used anyway since it has no real claim value -- it's not unique at all and SAN already contains the workflow path. We may want to expose Build Signer Digest as policy instead (?) and the CLI could use "sha" argument as default value for that
  • I'm not sure what is the value of Build Signer URI -- should the cli use the cert identity argument there too?

jku avatar Sep 07 '23 08:09 jku

FWIW, I also have an initial stab at this up at #715: it keeps the old extension handling in place while adding new APIs for the new extensions. My thinking was that backwards compatibility could be maintained by using the AnyOf building block to compose the old and new extensions.

(More generally, using these new extensions is blocked by the fact that they contain DER encodings for their values, which pyca/cryptography doesn't support for arbitrary third-party extensions yet. That's being tracked here: https://github.com/pyca/cryptography/issues/9283.)

woodruffw avatar Sep 07 '23 15:09 woodruffw

FWIW, I also have an initial stab at this up at #715: it keeps the old extension handling in place while adding new APIs for the new extensions. My thinking was that backwards compatibility could be maintained by using the AnyOf building block to compose the old and new extensions.

Yeah this works but does lead to a lot of classes (e.g. GitHubWorkflowTrigger(_SingleX509ExtPolicy), _BuildTrigger(_SingleX509ExtPolicy), BuildTrigger(Anyof)). Supporting "deprecations" in _SingleX509ExtPolicy makes that more manageable since you could just have BuildTrigger(_SingleX509ExtPolicy) -- this is with the assumption that API stability of the policy module is not that important, but supporting old certificates is very important (which is my intuition). But either solution definitely works.

(More generally, using these new extensions is blocked by the fact that they contain DER encodings for their values, which pyca/cryptography doesn't support for arbitrary third-party extensions yet.

ooh, good to know

jku avatar Sep 08 '23 07:09 jku

After reading up on the situation in cryptography:

  • it looks like this should not block a sigstore-python release as solving this will likely take some time?
  • is this going to be a scheduling issue: what's the plan if fulcio does start thinking about a 2.0 release and the cryptography DER API is not close to being implemented:
    • ask fulcio to extend support for the deprecated OIDs or
    • do DER parsing crimes in sigstore-python (with pyasn1 or something)

jku avatar Sep 08 '23 08:09 jku

this is with the assumption that API stability of the policy module is not that important, but supporting old certificates is very important (which is my intuition)

Yeah, this is a fair point. IMO it'd be okay to break the API here between major versions if we think the resulting code will be smaller (especially since the migration path will be smooth).

it looks like this should not block a sigstore-python release as solving this will likely take some time?

Yes, agreed -- I have this as a 2.0 milestone ATM but I don't think it should block at 2.0 release of sigstore-python (since the legacy extensions continue to work just fine). We can move it to 3.0, I think.

is this going to be a scheduling issue: what's the plan if fulcio does start thinking about a 2.0 release and the cryptography DER API is not close to being implemented:

DER crimes with pyasn1 is an option, but not one I find appealing 😅

Short of a full declarative ASN.1 API in Python becoming available, I can think of two other options:

  • Convince PyCA to include upstream support for the Sigstore X.509v3 extensions (no idea whether they'll consider this; I'll ask)
  • Create another PyPI package that uses PyCA Cryptography's ASN.1 APIs to parse these extensions (which should be very simple, since they're mostly no more complicated than OCTET STRING wrappers).

Regardless I think we have options available to us, so we won't need to ask Fulcio to prolong its support for the deprecated extensions 🙂

woodruffw avatar Sep 08 '23 14:09 woodruffw

Dropped this from 2.0, per #766.

woodruffw avatar Sep 11 '23 14:09 woodruffw