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.