Ballot for draft-ietf-mls-architecture
Summary: Has a DISCUSS. Has enough positions to pass once DISCUSS positions are resolved.
# Éric Vyncke, INT AD, comments for draft-ietf-mls-architecture-10 CC @evyncke Thank you for the work put into this document. I find it very difficult to understand as many concepts are not explained. Usually, I start reading the architecture I-D before the protocol one, and this document does not help understanding the architecture. We could even argue whether the I-D is about architecture or about security considerations. Please find below some blocking DISCUSS points (easy to address), some non-blocking COMMENT points (but replies would be appreciated even if only for my own education), and some nits. Thanks to Tatuya Jinmei, the Internet directorate reviewer (at my request), please consider this int-dir review with one nit worth considering: https://datatracker.ietf.org/doc/review-ietf-mls-architecture-10-intdir-telechat-jinmei-2023-01-29/ and as noted I share Jinmei's view about the clarity of the I-D. Please note that Dave Lawrence is the DNS directorate reviewer (at my request) and you may want to consider this dnsdir review as well when Dave will complete the review (no need to wait for it though): https://datatracker.ietf.org/doc/draft-ietf-mls-architecture/reviewrequest/16998/ Special thanks to Sean Turner for the shepherd's detailed write-up including the WG consensus *and* the justification of the intended status. I hope that this review helps to improve the document, Regards, -éric ## DISCUSS As noted in https://www.ietf.org/blog/handling-iesg-ballot-positions/, a DISCUSS ballot is a request to have a discussion on the following topics: ### Section 6 What is a `LeafNode` ? I.e., please define what it is before using the term ;-) Very similar issue in 5.1 for `GroupInfo`. And as noted by Jinmei "Commit" and "Proposal". ### Section 7.1 Is it appropriate for an IETF RFC (even if informal) to qualify WireGuard and TOR as `secure channel` ? This DISCUSS point is only to generate discussion among the IESG during the telechat. This discuss point will be removed anyway after the discussion.
## COMMENTS ### Abstract about 'scalable' This document and its companion appear to demonstrate the security properties of MLS, but do they also cover the scalability ones ? E.g., in section 2, there is `or as large as thousands`, good number but not enough for an IETF full remote plenary. Later in section 5, it is `groups with tens of thousands of members`, i.e., a different order of magnitude. ### Section 2 In `The Service Provider presents ....` is it unclear to me what service it is about ? The MLS service or the messaging service ? Is there a reason why sometimes AS/DS acronyms are used and at other places their expansions are used ? `she can use to send encrypted messages to Bob and Charlie` is the message also signed by Alice ? Hinted in section 3 but worth already warning the reader... `join an existing group;` should "(by asking to be added)" be specified ? As per `leave a group`. Does the MLS group intend to extend the MLS protocol itself to support group of moderators ? This sounds like a basic requirement to me (as a frequent videoconf user/moderator). ### Section 4.2 Why is "Partition-tolerant" mentionned in to the classes of DS ? It seems that it is useless to specify as a discriminator. I find this section difficult to read, e.g., what are the "Commit" or "Proposal" messages ? It seems that the flow is not natural. ### Section 5.4 Perhaps a mere rendering issue, but RECOMMENDATION appears in bold and is in uppercase while it is not a normative 'RECOMMEND'. Strongly suggest to use lowercase. ## NITS ### SEction 1 Isn't `enjoy some level of security` ambiguous or under specified ? ### Section 2 Please use the Oxford comma in `Alice, Bob and Charlie` ## Notes This review is in the ["IETF Comments" Markdown format][ICMF], You can use the [`ietf-comments` tool][ICT] to automatically convert this review into individual GitHub issues. [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md [ICT]: https://github.com/mnot/ietf-comments
# Internet AD comments for draft-ietf-mls-architecture-10 CC @ekline ## Nits ### S5.5 * "to other member" -> "to every other member"? ### S7.2.4 * "would have to be provide it" -> "would have to provide it", or "would have to be provided [with] it"
# GEN AD review of draft-ietf-mls-architecture-10 CC @larseggert Thanks to Meral Shirazipour for the General Area Review Team (Gen-ART) review (https://mailarchive.ietf.org/arch/msg/gen-art/M8UxyeH75I3l0lYx6DKtgipwPF0). Thanks for putting together a very readable MLS overview! ## Comments ### Inclusive language Found terminology that should be reviewed for inclusivity; see https://www.rfc-editor.org/part2/#inclusive_language for background and more guidance: * Terms `she` and `he`; alternatives might be `they`, `them`, `their` ## Nits All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. ### Outdated references Document references `draft-ietf-mls-protocol-16`, but `-17` is the latest available revision. ### URLs These URLs in the document did not return content: * https://hal.laas.fr/INRIA/hal-02425229/document ### Grammar/style #### Section 2.1, paragraph 3 ``` tions rely on users verifying each others' key fingerprints for authenticati ^^^^^^^ ``` Did you mean "other's"? #### Section 4.2, paragraph 1 ``` es; this can be detected only by out of band comparison (e.g., confirming tha ^^^^^^^^^^^ ``` Did you mean "out-of-band"? #### Section 5.1, paragraph 7 ``` he shared cryptographic material. However every service/infrastructure has c ^^^^^^^ ``` A comma may be missing after the conjunctive/linking adverb "However". #### Section 5.4, paragraph 8 ``` ly not allowed at the protocol level but applications can elect to provide s ^^^^ ``` Use a comma before "but" if it connects two independent clauses (unless they are closely connected and short). #### Section 6, paragraph 54 ``` layer. 7.1.3. DoS protection In general we do not consider Denial of Servic ^^^^^^^ ``` A comma is probably missing here. #### Section 7.1.3, paragraph 1 ``` ted traffic history combined with an access to all current keying material on ^^^^^^^^^ ``` Uncountable nouns are usually not used with an indefinite article. Use simply "access". #### Section 7.1.3, paragraph 3 ``` state is compromised at some time t1 but the group member subsequently perfo ^^^^ ``` Use a comma before "but" if it connects two independent clauses (unless they are closely connected and short). #### Section 7.2.1, paragraph 5 ``` , the application would have to be provide it through some other mechanism. ^^^^^^^ ``` Consider using either the past participle "provided" or the present participle "providing" here. #### Section 7.2.3, paragraph 1 ``` thin the epoch of the compromise. However the MLS protocol does not provide ^^^^^^^ ``` A comma may be missing after the conjunctive/linking adverb "However". #### Section 7.3.1, paragraph 1 ``` he attacker has compromised a client but the client signature keys are prote ^^^^ ``` Use a comma before "but" if it connects two independent clauses (unless they are closely connected and short). #### Section 7.3.3, paragraph 2 ``` this is the case for signature keys but similar concern exists for the encr ^^^^ ``` Use a comma before "but" if it connects two independent clauses (unless they are closely connected and short). #### Section 184.108.40.206, paragraph 7 ``` on client compromise, which helps recovering security faster in various case ^^^^^^^^^^ ``` The verb "helps" is used with an infinitive. ## Notes This review is in the ["IETF Comments" Markdown format][ICMF], You can use the [`ietf-comments` tool][ICT] to automatically convert this review into individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT]. [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md [ICT]: https://github.com/mnot/ietf-comments [IRT]: https://github.com/larseggert/ietf-reviewtool