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

[Richard Barnes <[e-mail]>]

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.

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.


> 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.



> 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.



> 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.
>
>
>
>