[MLS] AD review of draft-ietf-mls-architecture-09

[Paul Wouters <[e-mail]>]

# Sec AD review of draft-ietf-mls-architecture-09

CC @paulwouters

Please refer to
https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/
for more information about how to handle DISCUSS and COMMENT positions.

I have used the same categories as if I was balloting as AD. That means,
I would like my DISCUSS items resolved. This could be as simple as saying
I misunderstood something (although then perhaps clarified text could be
useful). Comments and NITS are items that would be nice to see addressed
but aren't blocking.

Note that I have gone through this document and commented, before ever
reading the mls-protocol document, on purpose. So some questions might
be answered there, but I wanted to see if this document can stand on
its own as much as possible.

When the MLS WG started, I wasn't sure why such a formal and complex
system was being proposed. Having now read the architecture document, and
especially Section 7, I understand why. Many attacks I had not thought
of are covered and countered where possible and where not possible at
least defended as best as can be. So I do want to say kudos to everyone
involved. It made me an MLS enthousiast!

My AD review of the mls protocol document should follow in the next day or
two.

Paul


## Discuss

### Each client is its own entity
A user with multiple devices becomes a member of the group multiple times.
This
implies that group members can detect which of the user's clients was used
for
a specific message. Can this property be eliminated or at least reduced in
the possible cases (eg if the user's devices are all online, could these
work
together to hide the information from which device/client the user sent the
message? eg by always picking one of them to relay?)

### possibly detect?
```
   The
   protocol is designed such that this only results in a group no longer
   being functional and the group members possibly detecting this and
   requesting reinitialization.
```

It scares me a bit that this could possibly not be detected at all?
What is the scenario where it cannot be detected?

### Signed confirmations?

```
   Other forms of Delivery Service misbehavior are still possible that
   are not easy to detect.  For instance, a Delivery Service can simply
   refuse to relay messages to and from a given client.  Without some
   sort of side information, other clients cannot generally detect this
   form of Denial of Service (DoS) attack.
```

Would it not be possible for clients to report a signed receive receipt?
While
that might take forever for an offline client, it would allow tracking that
a specific user hasn't confirmed receiveing messages for a long time?
(receive receipt being different from a read receipt. I guess it is a
privacy
leak to know a user's device got the message...)

###
```
   External joins do
   not allow for more granular authorization checks to be done before
   the new member is added to the group, so if an application wishes to
   both allow external joins and enforce such checks, then the
   application will need to do such checks when a member joins, and
   remove them if checks fail.
```

This seems rather risky? What could such a member do in the short time
between getting added and getting kicked? Would they see message history?
Group membership? How many devices a user has? When members were last
online? Why support such a group type? Wouldn't it be better to not support
this and have an automated "special member" being a bot that handles
member addition?

### encrypted meaning object, not transport?

```
      Additional authenticated data, sent unencrypted in an otherwise
      encrypted message.
```
I assume this means object encryption, and that the unencrypted part of
the message would still be encrypted with a TLS transport? Can that be
written more explicitely ?

###
Padding is mentioned but not noop packets and/or iptfs style constant byte
streams that would hide whether there is any communication at all. Is this
considered only as part of a transport security (eg TLS layer) or also
something that should be considered at the MLS layer?

### Deniability

I assume that some thought has gone into this, and a decision was made
not to provide this as a core feature, likely due to the limited usefulness
in eg a court system, when logged messages (deniable or not) are recovered?
I assume it was not easy to add this property and it was therefor mostly
left
out? Because of course it would be nicer to have it than to not have it.


## Comments

### Section 1
```
   Clients may wish to do the following:
```
It is not really a client who wants this but a user (who can be using
multiple clients). I also find "member" confusing, because I keep thinking
of a user and not a client. Perhaps using "member client" would clarify
this?

### Section 2, bullet list

```
   Clients may wish to do the following:
   [bullet list]
```

It is a little unclear whether this is the entire list of operations
supported,
or a subset. Maybe make that more explicit?

### Section 2, use of client and member

The use of client, member and someone/everyone is a little inconsistent, eg
see:

```
        For instance, any member can add or remove another client in a
group.
```

I assume a member is identical to a client, but is a member of the group.
But
then the above "clients may wish to do the following:" should not be used
with
"remove one or more members from an existing group;"

Perhaps just adding a line saying "a member is a client that is within a
group"
might be useful? Or say that client and member are used interchangably, but
that a client could be no member of any group ?

I guess this is further explained in Section 2.1 afterwards. It might be
better to bring this up a little earlier?

### Served to who?

```
        The Delivery Service is responsible for ensuring that each
KeyPackage
        is only served once
```

It feels I am missing some context here. Who or whom is this KeyPackage
served too?

### Are different extension lists supported ?

```
        Clients may also want to support multiple protocol versions and
        ciphersuites.
```
What about different extensions? One client might be able to display images,
while the other cannot ? Is that not possible, or was it just not listed
here?

### What is a request?

```
        The Delivery
        Service should provide the minimum number of KeyPackages necessary
to
        satisfy a request.
```

Similarly, I feel I am missing context here. Why is minimum important, why
need more than one? What is the definition of request?

### members or devices?

```
   Regardless, MLS does not allow for or support
   addition or removal of group members without informing all other
   members.
```
It might become clear to me later on how the difference of member vs device
of
member works? I assume in this case it should be "informing all devices of
all
other members" ? Otherwise, one might be able to tweak a removal/addition if
one knows of a member who lost access to one of their devices?
For consistancy, probably this should say "informing all other clients"
(or "member clients")

###
```
   Membership of an MLS group is managed at the level of individual
   clients.  In most cases, a client corresponds to a specific device
   used by a user.
```
This should have been told to the reader WAY before in the document, before
it starts mentioning clients and users and devices.

###
```
   Messages sent
   during the client's membership are also secure as long as the client
   has properly implemented the MLS deletion schedule.
```
Is this the equivalent of proprietary IM clients "auto delete" or "view
once"
type messaging? "MLS deletation schedule" has not been defined before it is
mentioned here. What is this?

###
```
   External signers can submit proposals for changes to the group,
   and new joiners can use an external join to add themselves to the
   group.
```
Is there a method for always denying these? This seems like a DoS vector
where a group is spammed with change proposals or join requests?
### lost state and PSK
```
   Group members whose local MLS state is lost or corrupted can
   reinitialize their state by re-joining the group as a new member and
   removing the member representing their earlier state.  An application
   can require that a client performing such a reinitialization prove
   its prior membership with a PSK.
```
Wouldn't this PSK that is the result of a membership be part of the "state
lost or corrupted" ? What is the exact kind of failure mode one would
address with this? Also, how often would such a PSK get updated as it needs
to be known to add group members (and doing this too often would I assume
lead to long dormant group members from losing access?)
If one could remove once's old self without the PSK, than everyone could
delete any member?

### WireGuard but not IPsec?

It is a bit odd to mention a non-IETF VPN protool, but not the IETF
VPN protocol. Especially since IPsec also has constant byte stream
support via IPTFS that WireGuard does not have :P

### 7.1 reads odd

It states any "secure channel" can be used, but it is kinda says any
insecure channel can be used too.

It also feels a little odd to describe these properties of MLS in the
Security Considerations section, and not like in the introduction.

### 7.1.1. recommendation ?
```
      *RECOMMENDATION:* Never use the unencrypted mode for group
      operations without using a secure channel for the transport layer.
```
Seems like a MUST NOT downgraded to RECOMMENDED ?

### In scope or not?
```
      *RECOMMENDATION:* If unidirectional transport is used for the
      secure transport channel, prefer using a protocol which provides
      Forward Error Correction.
```
Is this saying a protocol within MLS, eg some application protocol? Or
is this saying in this case, do not use MLS but another protocol at
the messaging layer?

### 7.2.1 padding but not constant stream chatter ?

7.2.1 says there is support for padding but does not mention anything
about generating a (constant) flow of traffic that looks like messaging
and is expanded/exploded to all clients but is a NOOP. It would be good
to have this - or at the very least ensure the design allows for this
to be specified later. This would also hide changes in online status
being reported.

### offline clients
```
      any client which
      is persistently offline may still be holding old keying material
```
Should there be client recommendations to delete old keying material
even if offline for some long amount of time? Can the document recommend
some? Is 4 weeks reasonable ? The risk seems to be like "old phones" kept
in a drawer, likely depleted of power, but still likely to be powered up
without disassembly of flash storage, where it could still wipe the content
based on a (large) timer. I feel not giving ANY guidance is a hole in the
specification.

### 7.3 AEAD reference
```
      Recall that the MLS protocol provides chains of AEAD keys
```
This "recall" is misplaced. It is not mentioned before until this point.

### 7.3.2 PCS, but ...
```
   If the adversary is passive, it is expected from the PCS properties
   of the MLS protocol that, as soon as an honest Commit message is sent
   by the compromised party, the next epochs will provide message
   secrecy.
```
It feels a bit odd to think of a compromised user sending an honest Commit
message. If the user doesn't know they are compromised, they will still
be compromised and this new Commit message won't help. If the user did know
they were compromised and undid that, then sure this honest Commit message
helps. I think perhaps "honest Commit" should be expanded and say something
about de-compromised client sending a Commit message. My issue with "honest"
is that compromised user with a passive attacker is still "honest".

Similarly confusing is:

```
       an honest update from the compromised client happens
```

### Section 7.4.1.1
```
   In all these cases, the application will often connect to the device
   via a secure transport which leaks information about the origin of
   the request such as the IP address and depending on the protocol the
   MAC address of the device.
```
Why would a MAC address of a device ever be a part of the MLS protocol
or the transport security around the MLS protocol?

```
   More generally, using anonymous credentials in an MLS based
   architecture might not be enough to provide strong privacy or
   anonymity properties.
```

This all come down to interpreation of "strong privacy or anonymity",
but if the only leak is the source IP (and MAC?) then I would argue
that the protection of the MLS based architecture IS enough to provide
"strong privacy or anonymity". Sure, if I connect from my home DSL perhaps
that reveals it is me talking, but it would still not reveal what or to
whom I am taking, if I am right? That's pretty strong in my interpretation.

### Section 7.4.2

```
This threat can be mitigated by having initial keys expire.
```

Again I wish there would at least be some advise on the expiry period,
even if it is a ballpark. Are we talking 1h? 1d? 1m? If it depends on
things, what things? (perhaps this is explained in the protocol draft)
Maybe a *RECOMMENDATION:* ?

### Section 7.4.3
```
   Infrastructures that provide cryptographic material or credentials in
   place of the MLS client (which is under the control of the user) have
   often the ability to use the associated secrets to perform operations
   on behalf of the user, which is unacceptable in many situations.
   Other mechanisms can be used to prevent this issue, such as the
   service blessing cryptographic material used by an MLS client.
```

This seems a bit of gloating about the MLS protocol/client. Why not just
state why the attack is impossible and require that MLS clients do this?
I don't see other features being compared to other/older/worse solutions?

### Section 7.4.3.1
```
   Beware though, the link between the cryptographic identity of the
   Client and the real identity of the User is important.  With some
   Authentication Service designs, a private or centralized authority
   can be trusted to generate or validate signature keypairs used in the
   MLS protocol.  This is typically the case in some of the biggest
   messaging infrastructures.
```

Why allow this in the MLS protocol?
How can this "typically" be the case of some of the biggest messaging
infrastructures? Clearly those don't run the MLS protocol? Perhaps you
meant to compare a similar function to the AS in other messaging protocols?
That is, I would clarify what is the issue, what is possible in MLS and how
it compares to non-MLS existing deployments.

###
```
   Often, expectation from users is that the infrastructure will not
   retain the ability to constantly map the user identity to signature
   public keys of the MLS protocol.
```
Again I am confused there are users with expectations of the MLS protocol,
which is still in a draft form only? Is this mixing MLS and non-MLS?



## NITS

### RECOMMENDATION in txt rendering

I find *RECOMMENDATION:* not friendly to read, the "*" distract from
the actual capitalization of the important of RECOMMENDATION:
I understand this is a tooling issue, perhaps it can be addressed later?

###
```
        Clients may also want to support multiple protocol versions and
ciphersuites.
```

Maybe: Different clients of the same or different user might support
different protocol versions and ciphersuites.

### CAPBR
        [CAPBR] is not a proper reference

### t and t'

Reads a little odd, why not use t1 and t2 ?

### this hurts my brain
```
    authentication is not affected in neither of these cases
```

I cannot confirm neither deny that I don't not understand this

###

an legitimate client -> a legitimate client

###
```
    by additional security mechanism
```

either "by an", or by mechanisms (plural)

###

a DS can mount a variety of DoS attacks -> a malicious DS ....

###  This sentence does not parse for me
```
      If real time notifications are not necessary and
      that specific steps must be taken to improve privacy
```

### pet peeve: vis-a-vis

This is not a friendly language construct, especially for people
whose first language isn't English (or Latin, but those should all be dead
by now)

###

to extract the top-level signature key. -> to extract the top-level
signature private key.

###
One thing for which the MLS Protocol is designed for is -> One design
feature of the MLS protocol is

###
```
   The Authentication Service is facultative to design a working
   infrastructure
```

Please rewrite this without using the word "facultative". Maybe use
"essential" or "key element" or something?

### TOFU
Expand on its first (and only) use.

### fanout

Not a fan of the word fanout, but at least it is explained in the draft

### parse error
```
      it should be stored encrypted at rest and
      during the execution
```
Missing the word "decrypted" ?

### another hard to parse sentence
```
   This can benefit an adversary that has compromised the AS (or
   required access according to regulation) the ability of monitoring
   unencrypted traffic and correlating the messages exchanged within the
   same group.
```

Perhaps "to gain the ability of ....." ?

### avoid double negative
```
Always use encrypted group operation messages to
      reduce issues related to privacy.
```
Maybe "to limit privacy risks" ?