Re: [secdir] Secdir review of draft-ietf-webpush-encryption-08

[Martin Thomson <[e-mail]>]

Hi Frank,

Thanks for the review. I've put changes on github here:
https://github.com/webpush-wg/webpush-encryption/pull/16

On 24 July 2017 at 20:38, Xialiang (Frank) <[e-mail]> wrote:
> 1.       The word “falsification” is used in the section 1, I am not sure if
> you see any essential difference between it and the “modification”. Can you
> clarify it?

The word was used intentionally to include protection against the
ability to generate an inauthentic message.

> 2.       In section 2.1, the sentence “Most applications that use push
> messaging have a pre-existing relationship with an Application Server”: what
> is the exact meaning of “pre-existing relationship”? From the context, I
> assume it’s a mutual authenticated and encrypted connection between the UA
> and AS, right? More texts to clarify this term seem good here;

I've expanded this a little.

> 3.       The second phase listed in section 3 (“The shared secret is then
> combined with the application secret to produce the input keying material”)
> seems to be described in details in section 3.4, not section 3.3. Please
> check it. And the term “application secret” can be changed to
> “authentication secret” for accuracy;

Section 3.3 is correct.  Section 3.4 is an overview of the entire process.

"authentication secret" is a good catch there.

> 4.       In last paragraph of section 3.1, is “An Application Server” more
> appropriate?

Yes, thanks.

> 5.       For the HKDF, should the salt be the authentication secret, or a
> random (16)?

In the case that you refer to, the authentication secret is correct.
There are two randomized inputs in the entire process: the
authentication secret is generated by the User Agent, and the salt is
generated by the Application Server.  Both are random(16) when
generated, but then they are distributed to the other side.

> 6.       For formula of IKM = HMAC-SHA-256(PRK_cek, key_info || 0x01),
> should the “PRK_cek” be “PRK_key” which is calculated before from
> auth_secret and ecdh_secret?

Obviously PRK_cek isn't generated until after this point, so you are
right.  (That's an editing error on my part.)

> 7.       In Security Considerations section, the potential threats (e.g.,
> eavesdropping, tempering, etc) of unprotected HTTP header fields have been
> identified, but the according protection measures are not discussed here.
> Would it be better to have them?

This is implied, but the mitigation is to discard them.  I've made this clearer.