improve KindVersion compatibility

[ramonpetgrave64]

Client support for Rekor V2: sigstore-python

Resolves #1369

Summary

• Changes _from_response() to parse the KindVersion from the responses canonicalized json body and cache it.
• Cahnges _from_dict_rekor() and _to_rekor() to store and reuse the cached KindVersion.

Release Note

• Added LogEntry.kind_version, which is now parsed earlier upon receipt from the rekor API,
  either from the root of the response, or from the reponse's inner base64-encoded JSON body.

Documentation

None

[ramonpetgrave64]

@woodruffw

[jku]

Can you confirm I've understood correctly: we have two ways to deserialize some sort of log entry:

• _from_response() that deserializes the rekor v1 response (that is not an actual v1.TransparencyLogEntry) while signing
• _from_dict_rekor() deserializer for actual v1.TransparencyLogEntry, currently used by verification when parsing a bundle

And you want to use _from_dict_rekor() to also parse the responses from rekor v2 when signing (because now the response is an actual v1.TransparencyLogEntry)

The changes are required because previously kindversion was only parsed when serializing:

• in rekor v1 response only canonicalized_body contains the kindversion?
• with rekor v2, you want to get kindversion from v1.TransparencyLogEntry -- I think that makes sense since that way you don't have to parse the canonicalized body before you know what kind it is?

This seems to make sense, there is one part here I don't understand:

In rekor V2, we may not be including the KindVersion in the "canonicalized body"

I didn't think this is possible: don't the log witnesses operate only with canonicalized body so need the kindversion there?

[ramonpetgrave64]

@jku I checked and with these two patches, KindVersion is now in the canonicalized_body for Rerkor V2. Still the new canonicalized body has a different layout in rekorV2, so I think this patch is simpler.

• Include KindVersion in TLE response rekor-tiles#286
• Include type and version of entry for client verification rekor-tiles#287

[jku]

I had another look:

• I realized why kindVersion looks weird here: this PR adds a betterproto type (KindVersion) into LogEntry: this feels against the design since otherwise LogEntry is a "native" type and does not use betterproto... I suppose that's fine. William as original author might have opinions?
• It's a bit odd that we go through the trouble of handling KindVersion but then never use it to validate canonical body in any way... but that validation currently would belong in Verifier.verify_* (since we're avoiding parsing canonical body before that) so maybe it's not part of this PR

I'm willing to approve this but would appreciate another opinion.

[woodruffw]

Sorry for the delay on my end!

• I realized why kindVersion looks weird here: this PR adds a betterproto type (KindVersion) into LogEntry: this feels against the design since otherwise LogEntry is a "native" type and does not use betterproto... I suppose that's fine. William as original author might have opinions?

Yeah, I think I would prefer to not have a betterproto type become part of the public API here -- we've mostly successfully avoided exposing the betterproto types, both for stability (we don't consider them stable) and idiomatic reasons (exposing a foreign type through a library's API is usually not recommended).

I suppose we could address that by making it private for now (_kind_version) but to take a step back I think I'd prefer if we could devolve the states here: is there a way we can tell which version of Rekor instance we're talking to, and devolve our marshalling/unmarshalling of the LogEntry that way? IMO that's probably something we'll need to do in the medium term anyways, to give ourselves a clear v1/v2 division (so that, in turn, we can deprecate and then remove v1 support when the time comes).

[ramonpetgrave64]

@woodruffw Changed to ._kind_version.

Yes, @jku's changes for the singing_config will inform which version of rekor to use.

• https://github.com/sigstore/sigstore-python/pull/1363/files#diff-eeff00b0846e751e3b890d4ca4a444621e63d7a4f4800690e1555dee5c172c6cR21-R29

  "rekorTlogUrls": [
    {
      "url": "https://rekor.sigstage.dev",
      "majorApiVersion": 1,
      "validFor": {
        "start": "2021-01-12T11:53:27Z"
      }
    }
  ],

The shape of the response is also different in v2: It will be an actual TLE. So it's my intention to use _from_dict_rekor to handle rekorV2's reponse.

• https://github.com/sigstore/rekor-tiles/pull/255/files#diff-eb568acf84d583e4d3734b07773e96912277776bad39c560392aa33ea2cf2210R95-R98

The response will be a
[`TransparencyLogEntry` message](https://github.com/sigstore/protobuf-specs/blob/5296f13d62e7fad428581d969f664c30cc52f549/protos/sigstore_rekor.proto#L94),
which should be persisted in a bundle (example at the bottom of this doc).
Clients no longer need to transform the Rekor response into a `TLE` message to store in the bundle.

[jku]

Yes, @jku's changes for the singing_config will inform which version of rekor to use.

I think William is also asking if we know which rekor version created the entry in the bundle that we are parsing and validating during verification, and I don't think we do with current rekor design... However, after flip-flopping a few times on the TLE versioning I believe the current plan (sigstore/rekor-tiles#255) is that the "canonical body" remains compatible (just with fewer types) and this means we should be able to deserialize a TLE safely with a single code path regardless of the source as long as we're careful -- it's not trivial but also does not seem unfeasible.

WRT removing v1 support:

• On the signing side I think we can plan that as soon as the PGI has rekor-tiles in its signing config: This would allow us to remove any RekorClient code that is for Rekor v1 and Bundle._from_response()
• verification (and Bundle._from_dict_rekor()) will have to support the different TLE variations for a relatively long time

[ramonpetgrave64]

Yes, so far the plan is to also have the KindVersion information in the reponse's inner canonicaliedBody, which will also be stored in the Bundle. Rekor V2 will be using dsse v0.0.2 and hashedrekord v0.0.2, while rekorV1 still only supports v0.0.1 of both kinds.

• https://github.com/sigstore/rekor-tiles/pull/255/files#diff-eb568acf84d583e4d3734b07773e96912277776bad39c560392aa33ea2cf2210R585

[jku]

@woodruffw I'll leave this for you since you requested changes but it seems sensible to me.

I would like us to fail in an understandable way when we see unsupported kindVersions (see e.g. #1384)... but so far LogEntry has been agnostic about kindVersions and the canonical body, and maybe that makes sense: We could do a followup PR to make Bundle._verify() check that KindVersion has values the bundle can work with?

[jku]

I'd prefer if we could devolve the states here: is there a way we can tell which version of Rekor instance we're talking to, and devolve our marshalling/unmarshalling of the LogEntry that way?

I think this is not needed: the only real changes are:

• the canonicalized body types change -- this was always a possibility, just hidden in reference docs
• inclusion promise may not there anymore

everything else is something we should already have prepared for in v1... but maybe hadn't.

[ramonpetgrave64]

@woodruffw , please take a look

[jku]

/gcbrun

[jku]

Let's go with this: any followup comments should go to new issues.