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

From: "Xialiang (Frank)" <[email-masked]>
Date: Mon, 24 Jul 2017 10:38:15 +0000

Hi,

I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG.  These comments were written primarily for the benefit of the security area directors.  Document editors and WG chairs should treat these comments just like any other last call comments.

This document describes a message encryption scheme for the Web Push protocol. This scheme provides confidentiality and integrity for messages sent from an Application Server to a User Agent.

In general, I think it's well written and prepared for the WGLC, in addition to some nits and little problems:

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?

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;

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;

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

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

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?

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?

B.R.
Frank

---

From: Martin Thomson <[email-masked]>
Date: Tue, 25 Jul 2017 12:16:24 +1000

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) <[email-masked]> 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

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.

---

From: "Xialiang (Frank)" <[email-masked]>
Date: Tue, 25 Jul 2017 02:28:18 +0000

Hi Martin,
Thanks for your quick response and actions to my comments.

I only have one question left: so you mean there are two authentication secrets generated respectively in UA and AS? 

B.R.
Frank

-----邮件原件-----
发件人: Martin Thomson [mailto:[email-masked]] 
发送时间: 2017年7月25日 10:16
收件人: Xialiang (Frank)
抄送: [email-masked]; [email-masked]; [email-masked]
主题: Re: Secdir review of draft-ietf-webpush-encryption-08

Hi Frank,

On 24 July 2017 at 20:38, Xialiang (Frank) <[email-masked]> wrote:
> 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.

> 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;

> 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;

Yes, thanks.

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.

> 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.)

> eavesdropping, tempering, etc) of unprotected HTTP header fields have 
> been identified, but the according protection measures are not discussed here.

---

From: Martin Thomson <[email-masked]>
Date: Tue, 25 Jul 2017 14:31:41 +1000

No, there is only one authentication secret.  The second salt doesn't
carry any significance (it just helps avoid key and IV reuse).

---

From: "Xialiang (Frank)" <[email-masked]>
Date: Tue, 25 Jul 2017 05:47:33 +0000

I see, thanks!
No more comments...

-----邮件原件-----
发件人: Martin Thomson [mailto:[email-masked]] 
发送时间: 2017年7月25日 12:32
收件人: Xialiang (Frank)
抄送: [email-masked]; [email-masked]; [email-masked]
主题: Re: 答复: Secdir review of draft-ietf-webpush-encryption-08

No, there is only one authentication secret.  The second salt doesn't carry any significance (it just helps avoid key and IV reuse).

On 25 July 2017 at 12:28, Xialiang (Frank) <[email-masked]> wrote:
> 抄送: [email-masked]; [email-masked]; 
> [email-masked]
>> section 3.3. Please check it. And the term “application secret” can 
>> be changed to “authentication secret” for accuracy;
>