Re: [Tsv-art] Tsvart early review of draft-ietf-mls-protocol-16

[Gorry Fairhurst <[e-mail]>]

On 23/09/2022 21:12, Richard Barnes wrote:
> Hi Gorry,
>
> Thanks for taking a look at the document.  I've posted a PR with some 
> edits:
>
> https://github.com/mlswg/mls-protocol/pull/747
>
> A couple of replies inline...
>
> On Fri, Sep 23, 2022 at 10:36 AM Gorry Fairhurst via Datatracker 
> <[e-mail]> wrote:
>
>     Reviewer: Gorry Fairhurst
>     Review result: On the Right Track
>
>     This is an early review of draft-ietf-mls-protocol-16, which seeks
>     to specify a
>     key establishment protocol to provide scalable efficient
>     asynchronous group key
>     establishment. This document seems to define a component that
>     could be used to
>     build or operate over an Internet transport. The focus is on security
>     algorithms and procedures. There are some issues that
>
>     Major Issue: This would have been easier to understand if there
>     was a section
>     that described the transport requirements of the messages. This
>     makes me wonder
>     what transport service is actually needed? - Can the underlying
>     transport
>     reorder messages? Is loss tolerated or reliability required? Are
>     there any
>     requirements on message size? etc. A brief paragraph explicitly
>     explaining the
>     requirements would help.
>     A later statement says, which appears to suggest any service can
>     be used: "MLS
>     is an encrypted messaging layer designed to be transmitted over
>     arbitrary lower
>     layer protocols....."
>
>     I note draft-ietf-mls-architecture states: "MLS is not intended as
>     a full
>     instant messaging protocol but rather is intended to be embedded
>     in concrete
>     protocols, such as XMPP [RFC6120]." This could form a part of such an
>     explanation, if it applies to this ID also?
>
>
> Yes, all of the transport considerations in 
> draft-ietf-mls-architecture apply here.  The documents are a pair, as 
> noted here 
> <https://www.ietf.org/archive/id/draft-ietf-mls-protocol-16.html#section-3>. 
> If you have thoughts on how to express that more clearly, suggestions 
> would be welcome.

I am unsure what "broadcast" means in that list, i.e.:

  * Broadcast delivery of Proposal and Commit messages to members of a group

As a transport/network person, "Broadcast to a group" seems to be 
"multicast to a group" .

>
> Basically, the main constraint that MLS imposes is that all group 
> messages process the same sequence of Commit messages in the same 
> order.  (Proposal and Application messages can arrive out of order.)
>
> https://www.ietf.org/archive/id/draft-ietf-mls-protocol-16.html#name-sequencing-of-state-changes
>
> I think for the most part, folks are envisioning MLS being run at the 
> application layer, carried over something like HTTP / WebSockets (this 
> is how Webex does things, for example), with the ordering above 
> provided by an application-layer broker and normal "transport-layer" 
> requirements addressed lower much in the stack.   In principle, you 
> could design a system that ran MLS directly over say TCP, but you 
> would need a way to meet the Commit criterion.

As a transport person, knowing whether there are message ordering 
requirements and whether the are message reliability requirements, maybe 
this could be said somewhere.

  I thought that could be a case,  you will know if it is appropriate to 
add these to the list.
>
>     Major Issue: The document targets PS, but the version I reviewed
>     added a
>     "DISCLAIMER: This is a work-in-progress draft of MLS and has not
>     yet seen
>     significant security analysis.  It should not be used as a basis
>     for building
>     production systems." - Why is the final document proposed as a PS
>     rather than
>     EXP, if this is not for general deployment? Or is this clause to
>     be removed by
>     RFC-Ed?
>
>
> That disclaimer is standard on security protocols that are in-progress 
> (following the pattern of TLS 1.3).  MLS has received significant 
> security analysis by this point, so I'll remove it.
>
OK
>
>     Major Issue: From a transport perspective, there are issues in the
>     statement
>     providing insufficient guidance on selecting a rate: "Update
>     messages SHOULD be
>     sent at regular intervals of time as long
>        as the group is active, and members that don't update SHOULD
>        eventually be removed from the group.  It's left to the application
>        to determine an appropriate amount of time between Updates."
>     - One possible way to address this is to state that the messages
>     are either
>     sent over a congestion-controlled transport or the maximum
>     aggregate rate of
>     messages falls within a specific guideline as defined in RFC8085,
>     or some other
>     BCP with similar advice on rate selection.
>
>
> As noted above, I expect that in most cases, MLS will be operated a 
> few layers above a transport that has congestion control.  And I think 
> the intervals people have in mind here are on the order of days, not 
> milliseconds.  But I'll try to clarify this.
>
That would likely address the issue.
>
>     Minor: " Application messages MAY be padded to provide some
>     resistance against
>     traffic analysis techniques over encrypted traffic [CLINIC]
>     [HCJ16]." - Padding
>     is a well known technique - it can have benefits and also have
>     drawbacks, e.g.
>     in efficiency of transmission and for message-based transfers in
>     reducing
>     robustness to paths that have limited PMTU, where larger messages
>     can be
>     dropped.
>
>
> Added a note about this.
>
>     Minor: " A receiver MUST verify that there are no non-zero
>        bytes in the padding field, and if this check fails, the enclosing
>        MLSCiphertext MUST be rejected as malformed." - I don't doubt
>     that this is
>        useful, but it would be helpful to add a sentence to explain
>     why these are
>        required to be encoded as zero.
>
>
> Added a note about this.
>
> On Fri, Sep 23, 2022 at 10:36 AM Gorry Fairhurst via Datatracker 
> <[e-mail]> wrote:
>
>     Reviewer: Gorry Fairhurst
>     Review result: On the Right Track
>
>     This is an early review of draft-ietf-mls-protocol-16, which seeks
>     to specify a
>     key establishment protocol to provide scalable efficient
>     asynchronous group key
>     establishment. This document seems to define a component that
>     could be used to
>     build or operate over an Internet transport. The focus is on security
>     algorithms and procedures. There are some issues that
>
>     Major Issue: This would have been easier to understand if there
>     was a section
>     that described the transport requirements of the messages. This
>     makes me wonder
>     what transport service is actually needed? - Can the underlying
>     transport
>     reorder messages? Is loss tolerated or reliability required? Are
>     there any
>     requirements on message size? etc. A brief paragraph explicitly
>     explaining the
>     requirements would help.
>
>     A later statement says, which appears to suggest any service can
>     be used: "MLS
>     is an encrypted messaging layer designed to be transmitted over
>     arbitrary lower
>     layer protocols....."
>
>     I note draft-ietf-mls-architecture states: "MLS is not intended as
>     a full
>     instant messaging protocol but rather is intended to be embedded
>     in concrete
>     protocols, such as XMPP [RFC6120]." This could form a part of such an
>     explanation, if it applies to this ID also?
>
>     Major Issue: The document targets PS, but the version I reviewed
>     added a
>     "DISCLAIMER: This is a work-in-progress draft of MLS and has not
>     yet seen
>     significant security analysis.  It should not be used as a basis
>     for building
>     production systems." - Why is the final document proposed as a PS
>     rather than
>     EXP, if this is not for general deployment? Or is this clause to
>     be removed by
>     RFC-Ed?
>
>     Major Issue: From a transport perspective, there are issues in the
>     statement
>     providing insufficient guidance on selecting a rate: "Update
>     messages SHOULD be
>     sent at regular intervals of time as long
>        as the group is active, and members that don't update SHOULD
>        eventually be removed from the group.  It's left to the application
>        to determine an appropriate amount of time between Updates."
>     - One possible way to address this is to state that the messages
>     are either
>     sent over a congestion-controlled transport or the maximum
>     aggregate rate of
>     messages falls within a specific guideline as defined in RFC8085,
>     or some other
>     BCP with similar advice on rate selection.
>
>     Minor: " Application messages MAY be padded to provide some
>     resistance against
>     traffic analysis techniques over encrypted traffic [CLINIC]
>     [HCJ16]." - Padding
>     is a well known technique - it can have benefits and also have
>     drawbacks, e.g.
>     in efficiency of transmission and for message-based transfers in
>     reducing
>     robustness to paths that have limited PMTU, where larger messages
>     can be
>     dropped.
>
>     Minor: " A receiver MUST verify that there are no non-zero
>        bytes in the padding field, and if this check fails, the enclosing
>        MLSCiphertext MUST be rejected as malformed." - I don't doubt
>     that this is
>        useful, but it would be helpful to add a sentence to explain
>     why these are
>        required to be encoded as zero.
>
>
It appears this would be resolved in the PR that I reviewed.
>
> _______________________________________________
> Tsv-art mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/tsv-art

I hope this was helpful, and look forward to seeing a new version of 
this in the future,

Best wishes,

Gorry