Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add envelope version #34

Open
iamwillbar opened this issue Jun 3, 2021 · 8 comments
Open

Add envelope version #34

iamwillbar opened this issue Jun 3, 2021 · 8 comments

Comments

@iamwillbar
Copy link

Since the envelope may evolve over time it would be helpful if there was a simple version property on the envelope which is incorporated into the signature. This would support future changes to the envelope (for example, to add new fields) and this could specify what the PAE applies to.

@MarkLodato
Copy link
Collaborator

I agree that the version should be signed. The proposal in #27 achieves that.

In the interest of reducing attack surface, my inclination is to not transmit the version number for now since there's only one version (not counting 0.1, which is not a long-term supported thing). That said, I don't feel too strongly.

@dlorenc
Copy link

dlorenc commented Jun 6, 2021

I think @iamwillbar is talking about an unauthenticated version number on the envelope type itself - is that correct @iamwillbar ?

@MarkLodato
Copy link
Collaborator

Proposal: we omit the version number for now. When consuming the envelope, if the version number is not present, assume it is 1 (i.e. "DSSEv1" in the PAE as per #37). In the future, if we add a new version, we can add an explicit version field to the signature at the same time. If we never add a new version, then we have saved a field.

My concern with adding it now is that it increases attack surface. It is one more thing that clients can screw up.

@iamwillbar What do you think?

@iamwillbar
Copy link
Author

@dlorenc I imagined that it would be part of the signature payload, otherwise you could manipulate the version without invalidating the signatures which could introduce potential attacks in the future.

@MarkLodato I usually advocate for including schema versions from the outset because it's typically inevitable something gets versioned and thinking about it upfront can prevent pain later. That being said, nothing prevents deferring that decision.

@MarkLodato
Copy link
Collaborator

To make sure we're on the same page, the envelope version is definitely part of the signature payload (see #37). I 100% agree it needs to be authenticated. The question is whether we can omit it from the bytes that get transmitted.

Let's suppose we start with a version field, say signature[*].version. (I'm assuming it should be on the signature, not the overall message, to allow v1 and v2 signatures in the same message.)

  • Initially, the client requires the field to be present and have value 1, else it is rejected with an "unsupported version" error.
  • When we add a v2, the client then does a switch: if 1, use v1 (unless v1 is no longer acceptable); if 2, use v2; else reject with "unsupported version". Old clients will reject with "unsupported version".

Now let's consider omitting the version for now and then add signature[*].version once v2 comes out.

  • Initially, the client always assumes v1 format. If the signature fails, it returns "bad signature".
  • When we add a v2, the client then does the same switch as above, except it also considers an omitted version to be equivalent to 1. Old clients will reject with "bad signature" since they don't know the version field exists.

It seems like we have the following:

  • Benefit of omitting:
    • Less to define and implement now.
    • Slightly smaller signature.
    • If we never add a v2, the spec stays simpler.
  • Benefit of including now:
    • Easier migration if/when v2 happens (less to explain since we've already figured it out now).

What do you think?

@sudo-bmitch
Copy link

Old clients will reject with "bad signature" since they don't know the version field exists.

I don't see how that would be a valid behavior since the spec parsing rules state "Consumers MUST ignore unrecognized fields."

The way for old clients to reject a newer version field is if they know the field exists and what the maximum version number is that they support, which I believe means we need to add a version field. We could simplify with the assumption that a missing version field defaults to version 1. That means producers MAY omit the field from generated envelopes, but the consumers MUST check for the value.

@MarkLodato
Copy link
Collaborator

MarkLodato commented Aug 12, 2021

This all works because the version number is included in the PAE, which is the byte stream fed into the Sign/Verify crypto primitives.

Here's a concrete example. Suppose a hypothetical v2 adds authentication of keyid and also adds a version number.

{
  "dsseVersion": 2,
  "payload": "hello world",
  "payloadType": "my-type",
  "signatures": [{
    "keyid": "my-key",
    "sig:" "..."  // Sign("DSSEv2 6 my-key 7 my-type 11 hello world")
  }]
}

A V1 client would ignore dsseVersion (unrecognized field) and call

Verify("DSSEv1 7 my-type 11 hello world", sig)

This would fail because the byte stream is not what was signed.

@mnm678
Copy link
Collaborator

mnm678 commented Aug 12, 2021

Having the verification fail due to an 'invalid signature' when the real problem is the new version would be very confusing to the user, and I'm not convinced this would happen for every potential spec change. The version field is very small, and I think the benefits of defining behavior for future version changes outweigh the very small additional field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants