Thanks for the review, Russ. Comments below (nothing major); pull request here for your review:
On Sat, Oct 20, 2018 at 4:24 AM Russ Housley <housley@xxxxxxxxxxxx> wrote:
Reviewer: Russ Housley
Review result: Almost Ready
I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair. Please treat these comments just
like any other last call comments.
For more information, please see the FAQ at
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
Document: draft-ietf-perc-double-10
Reviewer: Russ Housley
Review Date: 2018-10-20
IETF LC End Date: 2018-11-01
IESG Telechat date: unknown
Summary: Almost Ready
Major Concerns:
Section 10: Doesn't the IANA registry needs to specify the PRF to be
used with the ciphersuite as well?
I don't think so. I don't see a slot in the relevant registry for that, and the tabular summary in the IANA considerations section is really just a courtesy.
Minor Concerns:
Section 3: It would be useful to explain the Master Key before the
reader gets to Section 3.1.
Note that the "master key" concept comes from SRTP. I've added a bit of text to clarify.
Section 3.1: It is unclear what assistance is provided by the
additional level of indirection:
PRF_double_n(k_master,x) = PRF_inner_(n/2)(k_master,x) ||
PRF_outer_(n/2)(k_master,x)
PRF_inner_n(k_master,x) = PRF_n(inner(k_master),x)
PRF_outer_n(k_master,x) = PRF_n(outer(k_master),x)
It could just say:
PRF_double_n(k_master,x) = PRF_(n/2)(inner(k_master),x) ||
PRF_(n/2)(outer(k_master),x)
👍
Not sure what I was thinking.
Section 4: I suggest:
If the marker bit is not present, then B MUST be set to zero.
👍
Section 5, 1st paragraph: and endpoint cannot verify confidentiality.
Well, it can verify that the packet was encrypted with a key known only to the endpoints. But OK.
Nits:
The document uses "encryption" and "confidentiality" interchanagably.
Encryption is a mechanism or algorithm. Confidentiality is a security
service. While I do not think that the reader will be confused by the
current wording, it would be better to use the terms properly. In
addition, it is misleading to say:
... the receiving endpoint that can encrypt and authenticate ....
because the sending endpoint encrypts, and the recieving endpoints
decrypts. Also, the receiving endpoints check the authentication tag.
That's actually just some bad grammar. Reworded.
Abstract: s/authenticated encryption with associated data/
/authenticated encryption with associated data (AEAD)/
Abstract: s/scheme/algorithm/
Section 5.2: s/GCM/AES-GCM/
Section 7: s/HBH/hop-by-hop/
Section 7: s/E2E/end-to-end/
Section 7.1: s/HBH/hop-by-hop/
Section 7.2: The text is redundant. I suggest "etc" be dropped from
"such as SSRC, SEQ, CSRC, etc"
Section 7.2: s/non primary/non-primary/
Section 7.3: s/HBH/hop-by-hop/
Implemented all of the above...
Appendix A: s/HBH/hop-by-hop/
Appendix A: s/E2E/end-to-end/
... but I'm going to leave these last two as-is, for brevity.
