Subject: [Webpush] Major change to encryption

From: Martin Thomson <[email-masked]>
Date: Mon, 31 Oct 2016 21:38:19 +1100

Discussion in the HTTP working group has lead to some fairly
substantial changes to the spec that we rely on.  These are breaking
changes.  See the changes here:
https://github.com/httpwg/http-extensions/pull/252

In short, several of the parameters that were in header fields are now
in the body of the message and the Encryption header field is now
gone.

This completely messes with the use of that spec in Webpush.  It's
easy to detect which version is in use because the identifier has
changed, and there are small gains to be had.  The overall message
size is now slightly smaller, and the key derivation is now slightly
simpler.  The specs also have fewer interdependencies as a result.

I've put together a revision of the webpush-encryption draft.  I've
taken this opportunity to simplify things a little.  You can see a
preview in the editor's draft:

  https://webpush-wg.github.io/webpush-encryption/

I realize that this is a fairly big (and late) change.  I remain
optimistic that it will be the last. Feedback on the changes are
positive so far [1].

I plan to submit this doc very soon, ahead of the draft submission
deadline.  I realize that's short notice, but I'm fully prepared to
back out this change if necessary.

--Martin

[1] Costin suggested that we might also remove Crypto-Key.  That is
technically possible, though it's probably excessively kludgy, the DH
key could be moved to the keyid field.  I'm leery of that sort of
optimization, but I'm willing to be convinced that this is a special
enough case (I don't think that it is that special, but have at it).

---

From: Costin Manolache <[email-masked]>
Date: Mon, 31 Oct 2016 21:40:36 +0000

I'm not suggesting removing the Crypto-Key as optimization - but for
consistency.
If we send the salt and the keyid in the binary payload, we should also
include the dh key.

In particular, it makes it much easier to send webpush binary messages with
alternate transports - like Websocket or WiSH (https://tools.ietf.org/html/
draft-yoshino-wish-00 <https://tools.ietf.org/html/draft-yoshino-wish-00>)
if push promises are not available.
Technically having headers or binary prefix is equivalent for single
message with push
promises, or for the send API. But if the message has to be layered on a
different transport -
it gets tricky, with more complexity and chances of bugs.

Costin

On Mon, Oct 31, 2016 at 3:38 AM Martin Thomson <[email-masked]>
wrote:

--Martin

_______________________________________________
Webpush mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/webpush

---

From: jr conlin <[email-masked]>
Date: Mon, 31 Oct 2016 15:47:43 -0700

Perhaps I'm just confused by the various PRs and comments, but if I may,
i'd like to make sure I'm very clear on what the change is:

The crux of the change is:
1) Encrypted content would be identified as "aes128gcm", which should
not be confused with the now, long obsolete "aesgcm128".

2) salt, rs, and key_id are now prefixed to the encrypted content as:
`salt(16)|rs(4)|id_len(1)|key_id(id_len)|encrypted_content`

3) The content encoding key (CEK) is set to
```
HMAC-SHA-256(
    HMAC-SHA-256(salt, key[key_id].secret),
    "Content-Encoding: aes128gcm\x00\x01")  # from 2.2 of
http://httpwg.org/http-extensions/encryption-preview.html
```
The majority case will be that `key_id` is not defined (or is ''), in
which case, we'd use the locally derived key.

4) There's no longer a need for "context" to be appended to the key info
and nonce info, although the Content-Encoding for the new content type
will use the now obsolete "aesgcm128"
https://github.com/martinthomson/encrypted-content-encoding/pull/28/files#diff-6ee19a23c153fa68b2910aeb69bde1ddR213

5) The DH secret is now derived from running an HMAC-SHA-256 over
```'WebPush: info\x00' + receiverPublicKey + senderPublicKey```

Is that correct? Am I missing something?

---

From: Martin Thomson <[email-masked]>
Date: Tue, 1 Nov 2016 10:04:57 +1100

That is (to the best of my limited ability to check) 100% correct.

---

From: jr conlin <[email-masked]>
Date: Mon, 31 Oct 2016 16:07:44 -0700

One small comment, then? Can we change the transmitted Content-Encoding
type to match the new Content-type of "aes128gcm" instead of the long
abandoned "aesgcm128"? (See point #4)

I'm betting that's going to be a pain point for a number of folks.

---

From: Martin Thomson <[email-masked]>
Date: Tue, 1 Nov 2016 10:19:20 +1100

> abandoned "aesgcm128"? (See point #4)

Ouch, that's going to hurt.  I'll have to redo the examples :*(  40
minutes until the deadline, go!

---

From: Peter Beverloo <[email-masked]>
Date: Mon, 31 Oct 2016 23:31:27 +0000

Hi Martin,

Thanks for the update and the proposal!

I've reviewed these today, and have minor some points of feedback. I'll
deliberately avoid the topics of interoperability and upgrade cost here.

First of all, this indeed vastly improves layering between the drafts. I
very much like how webpush-encryption is now built on top of encryption-
encoding as opposed to being some sort of fork.

>> A push message MUST include a zero length keyid parameter in the
>> content coding header. This allows implementations to ignore the first
>> 21 octets of a push message.

I don't think this is right. The `salt` and the `rs` must still be known,
and those are included in the header.

>> A push service is not required to support more than 4096 octets of
>> payload body (see Section 7.2 of [I-D.ietf-webpush-protocol]), which
>> equates to at most 4059 octets of cleartext.

I think this forgot about the padding --

  4096 - 16 (auth) - 2 (padding length) - 21 (header w/o keyid) = 4,057

May also want to s/cleartext/plaintext/ for consistency with encryption-
encoding.

>> An Application Server MUST include exactly one aes128gcm content
>> coding, and at most one entry in the Crypto-Key field. This allows the
>> keyid parameter to be omitted.

This means the draft is incompatible with VAPID again. It must have at
most one Crypto-Key entry that has a `dh` value.

I haven't yet been able to validate the examples in the draft, but it
sounds like you're changing these anyway per jr's feedback (+1 to that).

Thanks,
Peter

---

From: Peter Beverloo <[email-masked]>
Date: Mon, 31 Oct 2016 23:39:10 +0000

Hi jr,

Your understanding matches mine too.

> http://httpwg.org/http-extensions/encryption-preview.html

Yes, if key[key_id].secret contains the IKM that has already seen the
HMAC with the "WebPush: info" info (the context replacement).

   PRK_key = HMAC-SHA-256(auth_secret, ecdh_secret)
   key_info = "WebPush: info" || 0x00 || ua_public || as_public
   IKM = HMAC-SHA-256(PRK_cek, key_info || 0x01)

(So it's not just the shared ECDH secret.)

> which case, we'd use the locally derived key.

To be clear, section 4 of webpush-encryption mandates that the `id` field
in the header is the empty string.

Thanks,
Peter

> https://github.com/martinthomson/encrypted-content-encoding/pull/28/
> files#diff-6ee19a23c153fa68b2910aeb69bde1ddR213
>

---

From: Martin Thomson <[email-masked]>
Date: Tue, 1 Nov 2016 10:56:27 +1100

Hi Peter and JR,

Thanks both for picking up on my stupid errors.  Hasty is not always
careful enough, but I was working to a time limit.  With 5 minutes
left, I think that I managed to get all your input integrated.

And your reminder about vapid was timely.  I think that we can
dispense with any attempt to remove Crypto-Key, since we have to have
it for vapid anyway.

Now I need to think about getting the python implementation updated.

--Martin