Can KindVersion be an enum?

[loosebazooka]

It's not super clear in the proto itself what this type should be. The docs simply link to rekor.

https://github.com/sigstore/protobuf-specs/blob/main/protos/sigstore_rekor.proto#L29

Are they case sensitive? Is it up to the consumer to verify validity? I would like to propose that this should be more concrete, especially if/when rekor were to adopt this for on the wire transmissions.

Was the reasoning that these currently map to existing fields in rekor entries?

[haydentherapper]

Agreed, enums are definitely preferred over strings, less need for validation

[kommendorkapten]

Was the reasoning that these currently map to existing fields in rekor entries?

Yes, they are meant to be populated directly from the response from Rekor. And primarily used when e.g. verifying the SET from Rekor (which requires them to be exactly the same as returned from Rekor).

Beyond that, the kind is what drives the interpretation of the Rekor entry, exactly like it is today with the responses from Rekor.

If the bundle switches to an enum, we would need to keep the bundle format in sync whenever a new type (kind) is added to Rekor, which I think may be creating an artificial coupling that can make it hard to keep up.

Also enums in protobuf are represented as strings with uppercase letters, which would also make the value different from what any Rekor client expects. So any client must maintain a mapping table from the enum value to the Rekor value, or we bake this logic in to Rekor. But that would be even worse.

My suggestion is to leave it as-is, and clarify the documentation that this is intended to be an (for the bundle) opaque value from Rekor, to use when interacting with a Rekor client.

[kommendorkapten]

When Rekor gets an updated protobuf API, I think it would be good to use the same enum for both the bundle and the Rekor response type. But before that my feeling is that this will just cause issues for any client trying to work with the bundle.

[znewman01]

If the bundle switches to an enum, we would need to keep the bundle format in sync whenever a new type (kind) is added to Rekor, which I think may be creating an artificial coupling that can make it hard to keep up.

Another concern here is that we want people to be able to fork Rekor and add new types. This might require some extra thinking for how this will work generally.

[kommendorkapten]

Another concern here is that we want people to be able to fork Rekor and add new types. This might require some extra thinking for how this will work generally.

Without thinking too much, let's assume the bundle continues to use a (opaque) string:
Rekor switches to an enum, even if Rekor is forked and custom type XYZ is added. The type will still be serialized as a string, i.e the Bundle will continue to work. But, if all the protobuf definitions are in this repository, it will be quite a burden to fork Rekor and add a new type. So that may be an indicator that Rekor should return the kind as a string. And the bundle format can be used for Rekor instances using custom types.

With that said, clients must of course still know how to work with the custom Rekor type(s), but putting that restriction to the bundle format it self, feels a bit too restrictive/unflexible.

[loosebazooka]

Makes sense, but to clarify this a bit more?

• this situation might also apply to the public keys: https://github.com/sigstore/protobuf-specs/blob/main/protos/sigstore_common.proto#L47
• does a fork of rekor imply they are out of compliance from the standard protobuf and are required to also fork the protobuf spec?
• why not let non-standard types be determined by parsing the type out of the other data (I think it's in the canonicalized body somewhere?) and have a enum with a non-standard option, similar to the unknown type in the public-key

edit: I'm okay punting on this discussion for 0.1.0 though, things seem to work fine

[znewman01]

this situation might also apply to the public keys: https://github.com/sigstore/protobuf-specs/blob/main/protos/sigstore_common.proto#L47

Agreed in principle, but in practice I care a little less about that.

does a fork of rekor imply they are out of compliance from the standard protobuf and are required to also fork the protobuf spec?

IMO: ideally you could add pluggable types to Rekor using configuration only, so it wouldn't imply that you had actually forked Rekor (my last comment was sloppy). So no, the hope would be for the protobuf-spec to be "future compatible" in this way.

why not let non-standard types be determined by parsing the type out of the other data (I think it's in the canonicalized body somewhere?) and have a enum with a non-standard option, similar to the unknown type in the public-key

Yeah that's possible I think. I could be convinced to do that.

One consideration: we might want to enable non-JSON formats. (We can always wrap the non-JSON format in a JSON wrapper {"type": "otherFormat", "value": "<base64 nonsense here>"; it's a little bit ugly/inefficient but will work. But then our signing envelope is always going to be JSON)

[kommendorkapten]

why not let non-standard types be determined by parsing the type out of the other data (I think it's in the canonicalized body somewhere?

Some more background on this. The initial idea was (still is) to not capture the canonicalized body. It was added quite recently as in the general case, it's not possible to deterministically recreate the canonicalized body, so it was added to the bundle. There is an initiative in Rekor to change how the SET (Signed Entry Timestamp) is calcualted, so it can be deterministic. When that is in place, the canoncialized body should be removed from the bundle. So yes, today the kind is duplicated, but in the long run it should not.