Versioning

[lrvick]

One thing that would be -really- good to agree on nowish would be versioning.

If we can agree on versioning it will allow early implementations of signing to easily migrate to new signing formats.

I propose the following very limited scope:

1. We agree on json (but not nessesarily everything outlined in Signature Format #4 yet)
2. We agree on the key: {version:"$SEMVER"}
3. We agree on storing signature wrapper as plaintext single lines
4. We agree on the git notes path: "refs/notes/signatures" for a given object we are signing.

If we can at least agree on the above, then early WIP implementations of git-signatures that I need to ship to multiple organizations ASAP can cleanly migrate to an official spec when there is one.

Even if we never include anything in the spec but the the commit hash alone offically with the rest being implementation specific extensions... versioning will still be useful when/if migrates to other hashing formats, or introduces other hash types in the future we might want to use (lots of ideas in mailing list that might materialize at some point).

In https://github.com/hashbang/git-signatures I can do this right away and use {version:"0.1.0-gs",...} until we can form agreement on the rest of the spec.

Thoughts?

[Ekleog]

1. I think this can't be solved before at least deciding on Signature Format #4. Because, as you say, we would need to agree on JSON, and to me this is the core of the debate in Signature Format #4.

2. Also, both JSON and git commit object format are extensible, so, so long as we don't change the meaning of an existing key and we keep the following warning in the RFC, I think we don't need a version key (which would be unnecessary flutter):

   Failure to interpret the contents of “GOODSIG” review MUST cause a warning and MUST cause such a review to be ignored.

   (the change to SHOULD has already been suggested)
   Because we can just make a v2 by suffixing all keys to .v2 and not having v1 keys around any longer anyway (if we can't find any more elegant way of doing it). Actually, we can always retro-fit a version argument and make it default to 1.0.0, if we ever find out we actually need it.

3. I think we all agree on storing review+signature on a plaintext single line. The question of whether said lines should be ordered or not is not solved though, for the time being I think conforming to the current draft would be most stringent and thus most likely to be forward-compatible: the RFC's strict commit and author dates ordering, as well as forcing each review to be written in order. I can't think of a future scheme that'd not accept this, future evolutions could only loosen some requirements.

4. Finally, bikeshed: I prefer refs/notes/reviews, because we're trying to allow people to review code they didn't necessarily write, while “signatures” makes me think of someone signing their own code :)

[lrvick]

Also, both JSON and git commit object format are extensible, so, so long as we don't change the meaning of an existing key and we keep the following warning in the RFC, I think we don't need a version key (which would be unnecessary flutter):

But we should assume even the value of a known key could change. Even if only to a hashing scheme stronger than sha1 in the not too distant future. In my experience anything you think won't change, will eventually change. API versioning as become the standard way to easily know under what rules to parse a given blob in, allowing backwards compatibility.

Finally, bikeshed: I prefer refs/notes/reviews, because we're trying to allow people to review code they didn't necessarily write, while “signatures” makes me think of someone signing their own code :)

Well the first author is signing their own code. Others are signing off on approval to release that code. Also the "reviews" key would semi-clobber with the git-appraise project: https://github.com/google/git-appraise#code-review-requests

[lrvick]

Because we can just make a v2 by suffixing all keys to .v2 and not having v1 keys around any longer anyway (if we can't find any more elegant way of doing it). Actually, we can always retro-fit a version argument and make it default to 1.0.0, if we ever find out we actually need it.

Thinking more about it, doing it in the path actually is the classic way that web APIs do it. The version number does not need to be signed as it is simply an helper to make tooling easier.

We could simply agree that the version number for the signature format for a given ref as a single line at refs/notes/signatures/version. I could put "0.1.0-gs" there so signatures I start doing today won't break tools later that only decide to support 1.0.0+

This assumes one won't mix signatures of different formats on the same ref, which I think is acceptable as that would be a nightmare to support.

Given this simple setup, the data format and structure of refs/notes/signatures could entirely change in the future without issue.

[Ekleog]

But we should assume even the value of a known key could change.

When this happen, we can rename the known key in v2 to key.v2 and specify that key cannot be output. This way an old verifier MUST disregard this signature as being invalid.

Even if only to a hashing scheme stronger than sha1 in the not too distant future

We nowhere specify SHA-1. :) We only consider git OIDs, and I'm pretty sure git will keep the concept of OIDs, doing whatever it needs to keep this abstraction for the reader (though the number of characters would likely change, but it's specified nowhere in the current draft and I don't think it needs to)

Thinking more about it, doing it in the path actually is the classic way that web APIs do it. The version number does not need to be signed as it is simply an helper to make tooling easier.

Hmm… Well, if we do it in the path then there is no reason to have a version number? If using an unsigned version number then there is IMO a potential security risk, as implementations could receive conflicting information from the two version numbers, and may end up confused about which one to pick.

Also, with the only-in-path version, we can mix signatures of different versions on the same reference, as the verifier would just accumulate information on the signatures by parsing them each independently, and then checking #9 to figure out whether to trust the commit.

Also, TBH I think currently, if you're willing to release git-signatures before the RFC release, there are going to be likely issues. I'd think the easiest way to do it would be to just use whichever format you want, and to put the signatures into refs/notes/signatures-preview or similar, and to, once the RFC is released, provide a tool to re-sign in a RFC-conforming way all reviews that had been signed before. This would require a one-time effort of everyone already having signed something in the repo to re-run this tool (not more as the commits in the new branch won't need to be in the same order as in the old branch), but it should do the trick? Likely better than waiting on whether we should use JSON, as I guess that's going to be a truckshed to paint :)

[lrvick]

Also, TBH I think currently, if you're willing to release git-signatures before the RFC release, there are going to be likely issues. I'd think the easiest way to do it would be to just use whichever format you want, and to put the signatures into refs/notes/signatures-preview or similar, and to, once the RFC is released, provide a tool to re-sign in a RFC-conforming way all reviews that had been signed before.

Using "signatures-preview" in effect versioning, and is the same as my proposed "0.1.0-gs" which respects semver. (plus I might have multiple iterations pre final RFC). Also we should assume that we won't get everything right in the very first draft of the RFC. There is a reason even major specs like HTTP have version numbers ;)

As long as the core security assumptions are the same (commit id signing in format A vs commit signing in format b) then the tool could simply interpret pre-release signature format in the 0.1.0-gs way, 1.0.0 signatures in that way, and future 2.0.0 signatures in that way.

Asking several engineering teams across multiple orgs to re-sign all their work every time there is a new spec change is not practical imo.

I do agree actually signing the version number is better than putting it in path so people can't force a signature to be interpreted under older rules. No attacks come to mind but we don't know what we don't know yet. If we do stick with json, keeping it as a key is an easy and obvious solution imo.

Unless someone has strong objections I suppose I'll just truck on with {body:{version:"0.1.0-gs",...},sig:{...}}.

If we do decide on somethin other than JSON that is going to be pretty easy to detect, and I could keep my json implementation around for validating signatures created today.... so I guess I am good.

[Ekleog]

Asking several engineering teams across multiple orgs to re-sign all their work every time there is a new spec change is not practical imo.

We do agree :) however, that would be required for these signatures to be compatible with the RFC, and thus with other tools that might respect it. Now, so long as these orgs only use git-signatures and git-signatures keeps backwards-compatibility, then this is all good for them :)

I do agree actually signing the version number is better than putting it in path so people can't force a signature to be interpreted under older rules

Hmm… The fact is, I personally dislike forcing each signature to have “version 1”, because it'd be useless cruft for all the time the version 1 is live, and maybe there won't ever be a version 2 (well… hopefully there'll never be a version 2, as that'd mean we got the RFC perfect at the first try :D).

So, idea: say that key version MUST NOT be present in v1 reviews. (well, that's dependent on #10, as if we decide not to accept then instead we'll have a paragraph stating the verifier MUST reject any review that includes unknown keys, in which case version would naturally be forbidden). Then, version being absent would mean (unambiguously) v1, and version being present would mean the included version.

If we do decide on somethin other than JSON that is going to be pretty easy to detect, and I could keep my json implementation around for validating signatures created today.... so I guess I am good.

Well, the issue of using JSON if we decide on something other than JSON is that, a priori, if the line doesn't even parse as a signed review then it'll be a hard error, while a review with an unknown version would be simply ignored. This would mean that other RFC-compliant tools would not be able to do anything even with the reviews that were created after its completion.

So, suggestion: use a refs/reviews-preview branch to put your JSON signatures, that can either (if we decide on JSON) be later renamed to refs/reviews, or (if we don't) left there and taken into account by git-signatures only, and other tools would be able to handle reviews that were done after, and would naturally ignore refs/reviews-preview.

[oxij]

On git-notes issue see #13. The rest I'll comment later (I don't want to even try to understand and comment that much stuff outside of Emacs).

[oxij]

Okay, so this is really a sub-issue of #4. Hence my thoughts in #4 (comment) apply here too. Concerning re-signing, nothing prevents your tool from merging reviews from different branches that use different signature formats... Again, like I said in #10, I now think `git-wotr` should just be the simplest possible thing with a static dumb-simple data format (i.e. git-object-like with fixed field order) that only verifies the correct chaining of commits in review-branch and specifies and verifies "result" of the review. Everything else should be made into extensions.

[Ekleog]

So I think current wording of #10 makes it possible to:

• Not have a version field for v1
• Keep the possibility to add a version field to future versions, as all reviews made with a version field MUST be ignored by tools compliant with v1 spec

However, the format of the message is still not subject to change, and a line that doesn't parse as a message will trigger the MUST fatal error… well, actually I'm saying that, and it's not explicitly stated in the spec. Do you feel it would be worth adding? I think it'd be a good thing, would it only be for better enforcing the lockstep between dates (I think? still don't have it all as a whole in my brain)

As a consequence, I'd advocate for:

• git-signatures uses custom branch for pre-RFC, merges reviews from custom branch and from standardized branch later on
• This issue is closed as solved by Ignore unknown metadata fields #10
• Someone opens an issue / PR for specifying either “a line that doesn't parse as a message MUST trigger a fatal error” or “MUST be ignored” and we discuss it :)

[Ekleog]

OK, so I've opened #15 for the question of lines that don't parse as a message.

@lrvick, would you be OK using a separate branch for reviews done by git-signatures in a non-RFC-compliant (or not-yet-RFC-compliant) scheme? If so this issue is solved by the current state of #10 :)

[lrvick]

I can use a separate branch if there is a use case I am missing, but tbh I am now not sure that it matters now that I am getting on board with the maximally KISS "git romans" format of just a minimum set of hashes + signature in one line. That said any signatures done today are very potentially not valid against future compliant git-signatures... or if we get lucky maybe they are.

Any reasonable future tool should simply refuse to validate a note in a format it does not recognize. If people really want to migrate signatures manually or verify them by hand for forensics later they can, else it is just history... as all that generally matters in practice are recent commits.

[Ekleog]

Well, I think the point of the separate branch is to protect against the two possible outcomes of #15 :) Agree with you with all other points (and maybe we can even get to an agreement before your MVP is due? that'd be great!)