Add support for signature extensions

[adityasaky]

First draft of signature extensions including a stab at defining the sigstore extension in the repo. I think we'd perhaps be better off not listing extensions here.

#59

[TomHennen]

Should we also request a pointer to code/libraries that can validate the extensions/sigs?

[haydentherapper]

+1

[mnm678]

Pending the couple of "FIXME"s this looks good to me!

[znewman01]

CC Sigstore clients who might be consumers of this: @woodruffw @haydentherapper @codysoyland @bdehamer (and me, I'll take a look at some point soon)

[haydentherapper]

+1

[TomHennen]

I think this generally looks good to me, but I don't trust my crypto expertise to say one way or the other. @haydentherapper how do you think this is looking from a Sigstore perspective?

[haydentherapper]

I’m also generally good with this. I do expect some changes once this is actually used though, so I’d prefer we mark it as alpha or wip so it’s easy to make breaking changes.

[TomHennen]

Is this PR just waiting for me at this point? :)

[adityasaky]

@TomHennen I'm going to remove the sigstore.md file, I retained it while we checked if just a table was enough. Once I do that and squash my commits, I think we should be good to go with a review from you?

[adityasaky]

@TomHennen done!

[TomHennen]

Thank you!

[TomHennen]

Are we still waiting for @haydentherapper and @MarkLodato's last thoughts or should we go ahead and merge?

[haydentherapper]

I think we're just waiting on sigstore/protobuf-specs#145?

[adityasaky]

Yes, sorry, I'm going to be addressing that thread this afternoon / evening and then updating this PR.

[woodruffw]

Sorry for the belated comment here, but to make sure I understand: the intended outcome here is to allow a DSSE envelope to contain a Sigstore bundle (or whatever else), correct?

In other words: ext will contain the bundle's fields, modulo the signature that would otherwise be in the bundle?

[jkjell]

As Aditya mentioned, signing with SPIFFE x509-SVID issued cert was another likely extension format. It has a bundle format defined as well. That would also probably have a timestamp signature associated with it.

I think the proposal looks good as is, and we should move forward with it.

Besides multiple signatures and multiple extensions, the sigstore verification process is a good example of a reason to not sign over the extension type. There are many possible verification flows for the single extension. What version is the bundle? Is it using a Rekor entry or not? Are you doing an online verification of the CTL?

From a practical standpoint, I can see other consumers, outside of Sigstore (Witness, SLSA, etc), supporting the Sigstore extension-type but, I doubt Sigstore would have any use for supporting any other extension type.

[haydentherapper]

If we end up not signing over the extension kind

I might have forgotten, but was this discussed previously? I assume we wouldn't want to sign over the extension to avoid this being a breaking change and mandating clients understand the extension format.

The invoking clients still benefit from having extensions for the multi-sig, mixed signing scenarios as they wouldn't have to duplicate envelopes / payloads and reason about all of them. WDYT?

I agree that I think this approach is better for multi-sigs given the current Sigstore bundle, as the bundle doesn't support a mapping between sigs and verification keys/certs. It's a little hard for me to predict what the exact use case currently since we haven't had a need for multi-sig DSSEs, but we should probably document somewhere that if the need does arise, we should revisit using this format.

[trishankatdatadog]

I might have forgotten, but was this discussed previously? I assume we wouldn't want to sign over the extension to avoid this being a breaking change and mandating clients understand the extension format.

If signing over kind is a breaking change, then it should be a V2 requirement (if at all) @SantiagoTorres @JustinCappos

[adityasaky]

If signing over kind is a breaking change, then it should be a V2 requirement (if at all)

To clarify, this PR does not propose signing over the extension kind, though it was discussed in the comments. As it stands, this PR does not introduce a breaking change, and one of the points against signing over the kind was to avoid breaking backwards compatibility.

I propose we move ahead with this PR (pending reviews from @SantiagoTorres, @JustinCappos, @lukpueh) but hold off on creating a release of the specification. Before we actually release with extensions, we can nail down whether we want to sign over the kind + anything else that needs to be discussed. Thoughts? 😄

[trishankatdatadog]

I propose we move ahead with this PR (pending reviews from @SantiagoTorres, @JustinCappos, @lukpueh) but hold off on creating a release of the specification. Before we actually release with extensions, we can nail down whether we want to sign over the kind + anything else that needs to be discussed. Thoughts?

That might be too confusing to casual observers who need to realise that just because something is there in a commit, doesn't mean it's been released. I argue for pushing for consensus (been too long, sorry on behalf of everyone) and releasing ASAP. Any objections from the other 8 maintainers?

[MarkLodato]

That might be too confusing to casual observers who need to realise that just because something is there in a commit, doesn't mean it's been released. I argue for pushing for consensus (been too long, sorry on behalf of everyone) and releasing ASAP. Any objections from the other 8 maintainers?

I agree. We don't have a formal release process, so we're currently living at head. Submitted == released. Perhaps an alternative is marking the extension field as "experimental", with a warning that the design is subject to change? I don't foresee any changes, but it probably is a good idea to get some real-world feedback before committing.

[adityasaky]

Marked extensions as experimental. I also bumped the version to 1.1.0-draft, I'm not sure if we have a process there.

[adityasaky]

As discussed in #61 (comment), #67 has been merged with a versioning scheme and v1.0.1 has been tagged. I've updated this PR to specify the version as just v1.1.0 in each .md file, and I've updated the base branch to be devel, which points to the current tip of the master branch.

[trishankatdatadog]

Conditional on this key text:

Additionally, extensions MUST NOT contain any information such that the
signature verification fails in its presence and passes in its absence.

I am approving this PR.

[adityasaky]

Are we ready to merge this into devel? I see the outstanding review request is for @SantiagoTorres. 😄

[adityasaky]

Thank you everyone!