Skip to main content

Early Review of draft-ietf-mls-protocol-16
review-ietf-mls-protocol-16-artart-early-salz-2022-09-28-00

Request Review of draft-ietf-mls-protocol
Requested revision No specific revision (document currently at 20)
Type Early Review
Team ART Area Review Team (artart)
Deadline 2022-09-28
Requested 2022-09-21
Requested by Paul Wouters
Authors Richard Barnes , Benjamin Beurdouche , Raphael Robert , Jon Millican , Emad Omara , Katriel Cohn-Gordon
I-D last updated 2022-09-28
Completed reviews Opsdir Early review of -16 by Bo Wu (diff)
Tsvart Early review of -16 by Gorry Fairhurst (diff)
Artart Early review of -16 by Rich Salz (diff)
Intdir Telechat review of -17 by Suresh Krishnan (diff)
Comments
It would be nice if we could get a few early reviews in time for the MLS interim meeting on the 29th
Assignment Reviewer Rich Salz
State Completed
Request Early review on draft-ietf-mls-protocol by ART Area Review Team Assigned
Posted at https://mailarchive.ietf.org/arch/msg/art/cPgtxv10gxxU8_MXCai5iGHG8WE
Reviewed revision 16 (document currently at 20)
Result Ready w/nits
Completed 2022-09-28
review-ietf-mls-protocol-16-artart-early-salz-2022-09-28-00
I reviewed this draft for ART. I did not review the cryptography as I am
unqualified, compared with the draft authors (and other participants). Overall:
I think references should be uppercase, as "[art]" should be "[ART]"

Abstract:
Nicely describes the problem. Is 'forward secrecy and post-compromise security'
redundant? If not, there should be definitions in the draft for both terms and
perhaps a forward link toe the terminology section.

Introduction.
"pairwise broadcast of individual messages" seems to go to far for terseness to
make the sentence grammatical. The section on common strategy should have a
reference or two to implementations. And do you mean "unilaterally broadcast
*A*symmetric keys"  Or is the common technique to allow everyone to impersonate
anyone?

Sec 2, Terminology.
Alphabetical order please.
Maybe mention that MSLPlaintext, MLSCiphertext are message formats described in
section 4.1; I wondered why they didn't appear in the terminology.  And when I
searched forward to find where they are defined, I noticed that elsewhere they
are rendered as `_MLSPlaintext_` for example, and here the underscores aren't
present.  Consistency is a virtue. The last paragraph starts by saying "keys
and secrets are used interchangeably" which is contradicted by the last
sentence.

Sec 2.1.2
I was consistently confused by the term "variable-size vector headers"  Suggest
replacing "Headers" with "Sizes" The parenthetical should not be parenthesized,
the difference is important and not an aside. The example vector should be
"StructWithVector" not plural, right? The examples are useful, but should be
more clear that they are sample encodings of the *length* bytes and mark the
length of the following vector in bytes.

Sec 3.
Short and understandable.

Sec 4.
Not short :) but understandable.

Sec 4.2 is very useful and have a nice use of the railroad diagrams. The
section title should be plural tho. "Executions" Is there any guidance to be
offered on access control policies?  How does A know whether or not Z can
remove B? Are messages NAK'd or ignored or something else? I guess a forward
link to 6.3 makes sense.

Sec 5
The worked examples are very useful.  I like figure 9

Sec 6.2, nit no underscores around first use of `MLSAuthenticatedContent`

I skimmed sec 7 and 8.  The end of 8.4 'where lp and np[i] represent" confused
me as I don't see those notations in the diagram that follows. Doing a
spot-check of these sections it seems that overall the text is clear.

-----

Sorry, I ran out of time do finish the document.  Holy cow, this is a large
document with a great deal of technical detail. I'll be surprised if you got
much early feedback.  More time, next time, please.

Table 5 says "LVL the security level"  Should mention "strengh in bits" maybe.