Address WGLC comments

[jricher]

• make @query and @path use raw values before percent-decoding
• add trailer fields and clarify list-based field combinations (Closes Accept-Signature: if it is a header, it must be stated explicitly #2277)
• add IANA section for registering new header field names
• add warning note about content digest depending on encoding (Closes Addessing missing content-encoding in signature input with content-digest #2279)
• add keys in JWK format (Closes Signatures 13: Problem with test-key-rsa-pss in Browser JS #2290)

[jricher]

@bblfish : please check these JWK-formatted keys for all the test vectors

[bblfish]

You may have already dealt with these typos but I'll just post those I found

• p20: double "the"

  The component value of a single named parameter is the the

• 2.2.8 as a language error:
  it looks like "in" is too much or a word is missing on p 25

  Indicating the baz, qux and param named query parameters in would
  result in the following @query-param component values:

• 2.1.0 page 15 "are be defined" ("should be..."?)

  Additional parameters are be defined

[ioggstream]

@jricher Here we are implying that not signing content-encoding means that there is no content coding. @reschke do you think it is a reasonable assertion? In general, this could be a slippery slope...

Suggested change

	As discussed in {{DIGEST}}, the value of the Content-Digest field is dependent on the content encoding of the message. If an intermediary changes the content encoding, the resulting Content-Digest value would change, which would in turn invalidate the signature. Any intermediary performing such an action would need to apply a new signature with the updated Content-Digest field value, similar to the reverse proxy use case discussed in {{signature-multiple}}.
	As discussed in {{DIGEST}}, the value of the Content-Digest field is dependent on the content encoding of the message.
	The above exchange implies that when the Content-Encoding is not included in the signature, then no content coding is applied.
	If an intermediary changes the content encoding, the resulting Content-Digest value would change, which would in turn invalidate the signature. Any intermediary performing such an action would need to apply a new signature with the updated Content-Digest field value, similar to the reverse proxy use case discussed in {{signature-multiple}}.

[reschke]

Not sure what the question is.

If Content-Digest is signed, and somebody changes the content by changing the content encoding, the signature will be invalid, right? Isn't that what we want here?

[jricher]

No, that's not the implication -- if Content-Encoding is not included in the signature, it doesn't mean that it's not applied, it only means that the encoding type is not signed. It could still be applied to the message if the field is there. And if it gets changed, the Content-Digest also gets changed, which fails the signature. This is desired behavior but something we want to warn implementors about, which is what the existing text does.

[ioggstream]

In the above example we have that:

1. content-digest is signed
2. content-encoding is not present
3. content-encoding is not signed

If I add a content-encoding header, the signature is still valid, since content-encoding is not signed and the client does not know whether:

1. content-encoding in the original request was present but not signed by accident
2. content-encoding was missing and the request was tampered

[jricher]

Correct, you cannot sign an absent header. That feature was discussed at length and rejected by the wg. The original message could have a content encoding header, whether or not that's covered by the signature. Absence from the signature does not imply absence from the message, as you claim above. These are independent.

If you add a content encoding header, and this changes the content, the signature is not valid because the digest is no longer valid - while the signature itself would pass the digest would not, as is pointed out in this section. Now this is assuming you change the content and don't just add a meaningless header. If you don't change the content but claim a different encoding, this should fail when the message is processed because the encoding makes no sense. Signature and digest won't fix a badly formed http message.

In all of these cases, I believe the system is behaving as intended and failing where we'd expect it to fail, and how we expect it to fail. So, I am with @reschke here in that I don't see what the issue is with this situation.

[LPardue]

I'm with Julian and Justin here. Not signing a field is tangential to that field applying to the message.

Signatures doesn't support covering the absence of a field. And it's sad that there's no formal definition that allows "Content-encoding: nothing". But these are things we are stuck with. The rough edge, of signing content-digest and having someone screw up the message isn't nice but will fail closed and will be quickly noticeable. The workaround is gzip (or whatever) the content if your application really can't live this sharp edge, then you have a proper content-encoding to send.

I don't think Signstures needs to do anything special here. There's lots of ways applications can screw up signing unrelated to content digest.

[ioggstream]

I agree that the issue is we can't say content-encoding: nothing. The example could use a content-encoding as a workaround.

[ioggstream]

@jricher pls see comments and a minor tweak. Not sure this addresses the content-digest concerns.

[reschke]

I believe the only robust way to sign a field is to actually make sure that only one instance of the field is present in the signed message (thus canonicalize not only the signed value, but the actual message).

[jricher]

That's a good point, I will add that to the recommendation here.

[jricher]

@reschke done, please check the new text to see if it aligns with your point

[reschke]

There are stimm multiple issues here:

• this not just about "list based" fields, but any case where there are multiple instances of the same field name - that may soundlike nitpicking, but the combination rule applies to any field
• "Note that intermediaries are allowed to separate values of list-based HTTP fields with any amount of whitespace between commas." - that sounds a bit like if this was about uncombining lists, but that's no whay you mean here , right?
• "For robustness, it is RECOMMENDED that messages include only a single value for any field covered under the signature, with the value for list-based fields serialized using the algorithm below." - once it's a single value, the actua algorithm doesn't matter at all.
• "This approach increases the chances of the field value remaining untouched through intermediaries." - it sort of guarantees it, unless the intermediary rewrites values it shouldn't touch (in which case we want the sig to fail)
• "Where that approach is not possible and multiple field lines need to be sent separately, it is RECOMMENDED that signers and verifiers process list-based fields starting with the individual field values based on the strict algorithm below, where possible, to account for possible intermediary behavior." - that makes it sound as if that would help - but if the intermediary combines in a different way (such as no SP), the sig will definitively fail.

[jricher]

Thanks, I think I'm still getting hung up on getting the precise terminology here -- noted about "list-based" fields, but I do think it's worth pointing it out as these are the fields that specifically fall into this category.

One thing I'm confused about: Aren't intermediaries allowed to process lists as lists? Part of the concern here is that I send Foo: a, b (one space) and an intermediary turns that into Foo: a,b (no space) in transit, which would fail the signature w/o the semantics of the field changing. I believe this is allowed, but am I wrong here? If I'm wrong then we can lean harder on the single field instance's value. The algorithm given is for creating the single field value.

The final recommendation is actually to split the values apart and re-combine them with "comma ", so an intermediary messing with the spacing actually wouldn't affect the value in the signature base, since it would be re-created.

[reschke]

No, intermediaries are not supposed to rewrite field values. In particular, an intermediary cannot assume that a comma is indeed a list delimiter in general -- consider quoted strings. Or ETags.

[jricher]

I've taken another shot at this -- please take a look at the new text.

[reschke]

As a general comment: it would be awesome if this PR could be split to address the individual issues separately.

[jricher]

@reschke I looked at that (and generally prefer that approach) but the trailer addition was intertwined with a few things so I opted to put everything into one batch instead to avoid conflicts.

[bblfish]

I have implemented the tests with JWK in the bobcats library in this commit bblfish/bobcats@450d299

• Java compiles for all except for the ed25519 key, even though the tests work for the PEM key. This is because I am using Nimbus jose JWT, which does not implement the transformation of ed25519 to java.security because java only added that in jdk15, and nimbus wants to be backward compatible to jdk8 in part because of Android. (see nimbus issue 359 and others).
• for JS: All tests pass (if I remove ed25519). BUT I have to first filter out all the private key fields from the JSON you submitted. (search for filterKeys in the commit above). It may be worth mentioning which keys appear only in the public key.

So I have not yet really tested the ed25519 JWK key.

[jricher]

@bblfish thanks. We explicitly say that the jwk format is for the key pair. Many libraries, including nimbus, have a method of easily extracting a public key from a key pair object, so to me it doesn't make sense to list that separately.

[bblfish]

Ok, I have succeeded in adding support for ED25519 in Java with commit bblfish/bobcats@ef15857 and the tests now all pass.
bblfish/bobcats#9

[jricher]

Merging based on overall positive feedback.