Message Encryption for Web Push
draft-ietf-webpush-encryption-09
Yes
(Adam Roach)
No Objection
(Deborah Brungard)
(Suresh Krishnan)
Note: This ballot was opened for revision 08 and is now closed.
Warren Kumari
No Objection
Comment
(2017-08-15 for -08)
Unknown
Firstly, thanks to Tim Chown for his helpful OpsDir review ( https://datatracker.ietf.org/doc/review-ietf-webpush-encryption-08-opsdir-lc-chown-2017-08-01/ ) and for your response. I only have nits on this document: 1: I reviewed this and draft-ietf-webpush-vapid together. This document uses title case for "User Agent" (and many other terms), while draft-ietf-webpush-vapid and RFC8030 uses lower-case. Consistency would be nice here. 2: Section 2: "In addition to the reasons described in [I-D.ietf-webpush-protocol], this ensures that the authentication secret is not revealed to unauthorized entities, which can be used to generate push messages that will be accepted by the User Agent." -- this is ambiguous / confusing. It is unclear which which is which. I'd suggest rewording to something like "... to unauthorized entities, which would allow that entities to generate push messages that would be accepted by the User Agent as valid" (or similar) 3: Section 7. Security Considerations "In particular, any HTTP header fields are not protected by the content encoding scheme." -- I think you may mean "In particular, no HTTP header fields are protected ..." (or similar)
Adam Roach Former IESG member
Yes
Yes
(for -08)
Unknown
Alexey Melnikov Former IESG member
(was No Objection)
Yes
Yes
(2017-08-14 for -08)
Unknown
This is a fine document. One nit: 4. Restrictions on Use of "aes128gcm" Content Coding An Application Server MUST encrypt a push message with a single record. This allows for a minimal receiver implementation that handles a single record. An application server MUST set the "rs" parameter in the "aes128gcm" content coding header to a size that is greater than the some of the length of the plaintext, the padding s/some/sum ? delimiter (1 octet), any padding, and the authentication tag (16 octets).
Ben Campbell Former IESG member
Yes
Yes
(2017-08-15 for -08)
Unknown
Just a few minor and editorial comments: Substantive: - 1: "For efficiency reasons, multiple users of Web Push often share a central agent that aggregates push functionality." Is the "central agent" a push server, application server, or something else? Editorial: -1: "Web Push messages are the payload of an HTTP message " - Plural disagreement. -1.1: Please consider using the boilerplate from RFC 8174. -4, first paragraph: s/ "... some of the length..." / "... sum of the length ..."
Spencer Dawkins Former IESG member
Yes
Yes
(2017-08-15 for -08)
Unknown
This is really well written and clear. Thank you for that. I found “for efficiency reasons” in this text For efficiency reasons, multiple users of Web Push often share a central agent that aggregates push functionality. To be so broad that I wasn’t sure what you were telling the reader. Are there any specific efficiencies that you could call out, so that we’d better understand why central agents are used? And if that’s already written down someplace, adding a pointer would be even better. I’m curious about algorithm agility, but I’m not the person to ask that question ...
Deborah Brungard Former IESG member
No Objection
No Objection
(for -08)
Unknown
Eric Rescorla Former IESG member
(was Discuss)
No Objection
No Objection
(2017-08-15 for -08)
Unknown
Moving to No Objection because my DISCUSS is fixed in: https://github.com/webpush-wg/webpush-encryption/commit/645a04b3b86ffe10322134e27a3d3c5eb5a8b06b Note, I think technically only the UA needs to do point verification if the app generates a fresh key as implied by S 2. It would also be nice to have a cite to how to do the point verification. This text can be stolen from TLS 1.3. S 1. This document describes how messages sent using this protocol can be secured against inspection, modification and falsification by a Push Service. "forgery" is more customary than falsification. S 3.3. key_info = "WebPush: info" || 0x00 || ua_public || as_public You should make clear that the string is not null-terminated. Ugh, I know. S 3.4. You should clearly separate which pieces are defined in this document and which are defined in the HTTP encryption document.
Kathleen Moriarty Former IESG member
No Objection
No Objection
(2017-08-15 for -08)
Unknown
Thank you for addressing the SecDir review comments. https://mailarchive.ietf.org/arch/msg/secdir/6wE0iKyBOoUHKsWILu7fdTPHsHw
Mirja Kühlewind Former IESG member
No Objection
No Objection
(2017-08-14 for -08)
Unknown
That would have been fixed by the RFC editor but anyway s/[I-D.ietf-webpush-protocol]/[RFC8030]/
Suresh Krishnan Former IESG member
No Objection
No Objection
(for -08)
Unknown