Replace magic payloadType with explicit sigType. #13
Conversation
Previously the switch between normal and "backwards compatible json" mode was determined by a magic `payloadType`. The downside of this approach was that it was undiscoverable: readers would not know this magic value existed. Now this behavior is made explicit via a `signatures.sigType` field. The new string better explains the behavior, which is that the payload must be JSON and that `payloadType` is unauthenticated.
The reference implementation did not support backwards-compatible signatures anyway, so no update is needed.
|
Thanks! This PR is now ready for review. |
|
Random question: Honestly, if we're gonna do this far, why not just index signatures by keyids? Is there a good reason for redundant, possibly conflicting signatures from the same keyid? |
|
Hmm, good point. Let me rephrase to make sure I understand: Instead of having a sigType at all, the verifier knows that key X is always old-style and key Y is always new-style. A corollary is that it would be impossible to ever use the same key with both old- and new-style signatures. If you have an existing key, you need to provision a new key to get new-style signatures. No objections from me, but I don't know how difficult it will be for users to provision new keys. If you all are OK with it, I can make that change. Let me know. |
|
On second thought, I'm not sure that's a good idea. If we remove sigType, then the layout is going to need a new field stating whether each key will be used for old-style or new-style signatures. Plus, since it's a new field, it will have to be set explicitly on every new layout going forward. Given this, as well as the burden of having to re-provision new keys, I'm inclined to keep it as is. What do you think? |
|
No, that's not what I meant, but let me get back to you. |
Sorry this took a while. Leaving aside my earlier tangent, I don't think it's accurate to call canonical JSON signatures "backwards-compatible mode." There's two types of backwards-compatibility:
This PR does not maintain (2), which is harder to achieve, and so I think calling it "backwards-compatible" signatures is a misnomer. |
|
Right, "backward compatible" is perhaps too far a stretch. Maybe "legacy"? My intention is to make it clear that it is not recommended for new use cases. |
I figured this was the intent, too. Maybe it's best to call it "deprecated" or "canonical" (the most appropriate name TBH). |
|
I like the idea of calling it "legacy" or "deprecated". A question as a relative newcomer, but is there any danger of "canonical" being misinterpreted once the spec is accepted and is the new "canonical" way of doing things? I may be overthinking this, though. |
|
@MarkLodato I'm planning to accept and merge #13 and close #8 with the change in semantics re "Backwards Compatibility". Does that sound good to you? |
This was cruft from an earlier revision. It should have been "raw-json-no-payload-type".
|
@adityasaky Yes, sounds good. Thanks! FYI, I just pushed a new commit fixing a mistake. I agree with Aditya that "canoncial" sounds like "recommended" which is not what we want. I suggest "legacy" since that implies that it is not recommended for new use cases. To be clear, this PR does not add any new use of the term "backwards compatible." My latest commit (fba2968) removes an incorrect reference to the term. The sigType is "raw-json-no-payload-type". So, I suggest any replacing of the term be done in a separate PR. |
This sounds like a plan to me! |
There was a problem hiding this comment.
I'm merging #13 and I'll shortly be closing #8. Thank you, @MarkLodato and @trishankatdatadog!
This closes #5 and is an alternative to pull request #8. Here, we switch from a magic
payloadTypeto an explicitsigType, as discussed in #8. My preference is to use this PR and reject #8.Note: This PR depends on #11 and #12. Sadly, GitHub does not support dependent PRs, so those PRs are show in this one's diff. :-( The only commit in this PR is the last one, f0518cf.