Re: [MLS] Roman Danyliw's No Objection on draft-ietf-mls-protocol-17: (with COMMENT)

[Richard Barnes <[e-mail]>]

Hi Roman,

Thanks for the review.  A few responses inline.  I have filed a PR for
these changes:

https://github.com/mlswg/mls-protocol/pull/850

On Tue, Jan 31, 2023 at 9:30 AM Roman Danyliw via Datatracker <
[e-mail]> wrote:

> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> ** Section 2.
>    Handshake Message:  A PublicMessage or PrivateMessage carrying an MLS
>       Proposal or Commit object, as opposed to application data.
>
> This definition of a handshake message is being defined in terms of a
> Proposal
> or Commit object, neither of which are defined in this session.
>  Furthermore,
> PublicMessage or PrivateMessage aren’t defined either as this point in the
> text.  To improve clarity, consider defining these key terms.
>

I added definitions for "Proposal", "Commit", "PublicMessage", and
"PrivateMessage".  This turned out to help with the next comment...


> ** Section 2.  Editorial.
>    The PublicMessage and PrivateMessage formats are defined in
>    Section 6; they represent integrity-protected and confidentiality-
>    protected messages, respectively.
>
> The use of “respectively” in the text is suggesting that PrivateMessages
> don’t
> have integrity protection which is not accurate.
>

... because I could put more detailed descriptions in the definitions and
just remove this text.



> ** Section 3.1.  Typo. s/the the/the/
>

Done.



> ** Section 5.1.1.
> (see the Cryptographic Dependencies section of
>    the HPKE specification for more information).
>
> Is this Section 4 of RFC9180?  If so, why not say that?
>

Done.



> ** Section 5.3.3.
>
>    However, applications SHOULD NOT rely on the data in an
>    application_id extension as if it were authenticated by the
>    Authentication Service,
>
> What would be the circumstance where the application_id extension would be
> treated with equal standing as something from the AS?  Why can’t this be a
> “MUST”?
>

I think this probably makes sense as a MUST, so I've put it in the PR.  But
other folks in the WG might have different opinions.



> ** Section 12.2.
>
>    A group member creating a commit and a group member processing a
>    commit MUST verify that the list of committed proposals is valid
>    using one of the following procedures, depending on whether the
>    commit is external or not.
>
> Unsaid is what to do if the proposal list is not valid.  Please clarify.
>

Clarified that an invalid proposal list means the Commit is invalid.



> ** Section 12.2
>
>    *  It contains multiple Update and/or Remove proposals that apply to
>       the same leaf.  If the committer has received multiple such
>       proposals they SHOULD prefer any Remove received, or the most
>       recent Update if there are no Removes.
>
> Is this normative behavior required, or could it be left up to
> application?
> The mixing of updates/removes on the same node and the SHOULD guidance
> already
> means that this “recovery from an invalid node” may not be consistent
> across
> clients/DS-es.
>
> Same observation on recovering from ReInit with any other proposals.
>

Sorry, I'm unclear on what you mean by "recovering from" here.

The underlying principles here are:

* The Commit sender may not have received all the Proposals that were sent
in an epoch
* Aside from the constraints here, the Commit sender may freely choose
which Proposals to commit

For example, in this case, a Remove might be rejected because application
policy says that its sender isn't authorized to remove the subject

Net of all that, I think the answer to your question is that the normative
behavior is not required, and that the possible inconsistency is
deliberately allowed to create space for application-level policies.



> ** Section 12.4.
>
>    Due to the asynchronous nature of proposals, receivers of a Commit
>    SHOULD NOT enforce that all valid proposals sent within the current
>    epoch are referenced by the next Commit.  In the event that a valid
>    proposal is omitted from the next Commit, and that proposal is still
>    valid in the current epoch, the sender of the proposal MAY resend it
>    after updating it to reflect the current epoch.
>
> Is there any additional retry mechanism?  Let’s say the sender of the
> proposal
> doesn’t see it reflect in the epoch after it sent proposal.  And then not
> in
> the next epoch.  Does it try resending indefinitely?
>

This is an application-level consideration, since it interacts with
potential application-layer constraints on what proposals are allowed.  For
example, in an "moderated" group where only certain "moderator" members are
allowed to add or remove members, a Add/Remove proposal by a non-moderator
might never be committed because it is forbidden by application policy.



> ** Section 15.1.
>    If Alice asks
>    Bob "When are we going to the movie?", then the answer "Wednesday"
>    could be leaked to an adversary solely by the ciphertext length.
>
> This setup seems a little simplistic.  If the application data is
> encrypted to
> begin with, how does the adversary know the question being posed by Alice?
>

For example, maybe the attacker can tell from application context that Bob
is sending his question to a movie scheduling bot.  But I don't think this
document is the place for a complete primer on the risks of traffic
analysis.  We already have citations to studies that cover these risks in
more detail.

Best,
--Richard