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

[Gorry Fairhurst via Datatracker <[e-mail]>]

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.