Re: [MLS] More clarity around validating proposals

[Richard Barnes <[e-mail]>]

Yeah, I think this makes sense, and points to a good way to tighten things
up.

Actually, I think there might be three levels of filtering / assembly:

1. Does each individual proposal make sense for this group? (e.g., not: an
Add with the wrong ciphersuite, or a Remove for a non-existent member)
2. Sender assembles list / receiver verifies list according to
application-defined policy
3. Is the resulting list a legal sequence of proposals?  (e.g., not: Update
and Remove for the same leaf, multiple GroupContextExtensions)

In terms of spec text, I think you could define rules for (1) and (3), and
say that any Commit MUST obey those rules.  The individual-proposal rules
(1) could probably go in the proposal type sections.  The list-of-proposal
rules (3) probably need a new subsection, as Marta suggests.  Then the
commit processing would lay out the above.

How does that strike you?

--RLB

On Fri, May 13, 2022 at 9:36 PM thomas leavy <[e-mail]> wrote:

> I think I get what you are saying, what do you think of the logic below?
>
> On both sending and receiving there would be a two stage filter. First
> stage is filtering based on rules defined by the RFC (arbitrary choosing of
> dupe updates, etc). Second stage is filtering based on rules defined by the
> application.
>
> On sending a commit, you would pass the complete list of proposals through
> the RFC filter stage, and then pass the remaining proposals to the
> application stage for further filtering. The resulting proposal set would
> be committed to. Not sure about MUST be SHOULD wording here.
>
> On receiving, you MUST reject the commit in the case either your RFC rule
> filter, or your application rule filter would select any of the committed
> proposals for removal, as this is either a protocol or application rule
> violation.
>
> Thanks,
>
> - Tom
>
> On May 13, 2022, at 3:45 PM, Richard Barnes <[e-mail]> wrote:
>
> 
> Fair point, but I think what that mainly argues is for harder
> receiver-side requirements, not sender-side ones.  So maybe we upgrade some
> of the SHOULDs discussed below into MUSTs, especially where the result of
> !SHOULD is nonsensical (e.g., Remove after Update, 2x
> GroupContextExtension).  That way, even if the send selection / order is up
> to the application, the receivers will all react the same way.
>
> The only reason that comes to mind why one wouldn't want to rule out some
> of these cases is if you were using inclusion in a commit as a signal that
> a proposal was received and accepted but overwritten by a later proposal.
> But it seems like that's something that can be safely done at the
> application level, reserving the Commit for only things that have actual
> impact on the group.
>
> --RLBa
>
> On Fri, May 13, 2022 at 2:31 PM thomas leavy <[e-mail]>
> wrote:
>
>> Hey Richard,
>>
>> One thing I think we should be careful about is just ensuring that
>> whatever the spec says that it is prescriptive enough as to not break basic
>> interoperability. What I mean by basic interoperability is that if the same
>> application wanted to use one MLS implementation for a web client, and
>> another implementation for a native client that interoperability would be a
>> success assuming that the application implemented each required
>> customization point the same way on both clients.
>>
>> If we leave it too open where one MLS implementation allows filtering
>> proposals on receive, and another only inside processing a commit, then it
>> becomes likely that it is not possible to make multiple MLS implementations
>> work the same way across even across multiple clients of same application.
>> I think the RFC does a good job with this around credential validation, it
>> is very explicit about when the application needs to review, and what
>> should happen if that review fails.
>>
>> We would be specifying a flow for first processing the set of available
>> proposals to fixed rules that are already defined, and then saying that the
>> remainder of proposals should be given to the application for additional
>> filtering before the commit is created. On the receiving side we would just
>> be looking for the application to identify rule violations that should
>> result in error given the proposals in the commit.
>>
>> Thanks,
>>
>> - Tom
>>
>> On May 13, 2022, at 2:04 PM, Mularczyk Marta <[e-mail]>
>> wrote:
>>
>> 
>> Hi Richard,
>>
>> Thanks for the input! In fact, we’re all for downgrading MUSTs to
>> SHOULDs. I think it still makes sense to collect all the rules / guidance
>> from the RFC into a single section 13.4. It would contain:
>>
>>
>>    1. Properties that a committed vector of proposals MUST satisfy. They
>>    are required for correctness or security of MLS.E.g.
>>       1. There MUST NOT be a proposal removing the committer
>>       2. ExternalInit MUST only be used in external commit
>>       3. The signature MUST verify
>>    2. Properties that a committed vector of proposals SHOULD satisfy,
>>    i.e., general guidance). E.g.
>>       1. There SHOULD be at most one proposal affecting a leaf
>>
>>
>>
>> > Though I have to say I don't love the idea of trying to implement the
>> logic to build such a maximal list.
>>
>> Very simple :) Say you have a proposal list L. To build a maximal valid
>> list, start with an empty list LVal. Then for each P in L, do: if LVal with
>> P appended is valid, append P to LVal. Else, continue.
>>
>> Best,
>> Marta
>>
>> ------------------------------
>> *From:* MLS <[e-mail]> on behalf of Richard Barnes <
>> [e-mail]>
>> *Sent:* Friday, May 13, 2022 5:05:28 PM
>> *To:* Mularczyk Marta
>> *Cc:* [e-mail]
>> *Subject:* Re: [MLS] More clarity around validating proposals
>>
>> Hi Marta,
>>
>> Thanks for thinking about this stuff, these are good issues to clarify.
>> I agree with your analysis, and the conclusion that the current text is
>> ambiguous.  Though I think some of the ambiguity may have been intentional,
>> to allow for the sorts of policies you describe.
>>
>> It seems like there's a prerequisite question as to whether we need to be
>> prescriptive about these things.  One could imagine leaving the set of
>> proposals committed up to the application.  Different choices by the
>> application would have implications that would be worth discussing in the
>> spec (e.g., the members could refuse to commit a Remove sent by an external
>> proposer), but both the risks and the possibilities to mitigate them seem
>> more specific to application deployment contexts, as opposed to core parts
>> of the protocol.  Personally, I have a pretty strong preference for this
>> option.
>>
>> If we were going to do that, I think the major impact would just be to
>> downgrade the MUSTs on this front to SHOULDs.  We should probably provide
>> some notes of the general character you describe, but we wouldn't need to
>> be quite so precise because we would just be providing guidance, not hard
>> requirements.
>>
>> If we're going to go down the path you suggest, the text changes you
>> propose sound about right.  Though I have to say I don't love the idea of
>> trying to implement the logic to build such a maximal list.  It might be
>> worth noting that it's possible for there to be multiple lists that are
>> maximal in this sense, e.g. if two proposals are mutually exclusive with
>> each other but compatible with the others.
>>
>> --Richard
>> On Thu, May 12, 2022 at 12:44 PM Mularczyk Marta <
>> [e-mail]> wrote:
>>
>>> Hi all,
>>>
>>> Me, Tom Leavy and Joël Alwen think that the text in the RFC around
>>> validating proposals, choosing which proposals go into a commit and how to
>>> validate a received commit could do with a bit more work.
>>>
>>> The root issue is that Section 13.2 states “The sender of a Commit MUST
>>> include all valid proposals that it has received during the current epoch.
>>> Invalid proposals include, for example, proposals with an invalid signature
>>> or proposals that are semantically invalid, such as an Add when the sender
>>> does not have the application-level permission to add new users.” There are
>>> two intertwined issues with this text:
>>>
>>>    1. The qualifier “MUST”
>>>    2. The text is too vague about what a “valid Proposal” is. E.g. it
>>>    could be interpreted to mean a Proposal is “valid” relative to the current
>>>    group state: Can it be applied to the group as it stands now? Then it’s
>>>    “valid”.
>>>
>>>
>>> Put simply, it doesn’t make sense to only validate proposals
>>> individually. Instead one should validate if the full list of proposals can
>>> be committed. Indeed, other parts of the RFC enforce validation steps that
>>> apply to the full list. E.g.
>>>
>>>    - A ReInit proposal MUST be the only one in the commit, and other
>>>    existing proposals SHOULD be preferred (13.2.1).
>>>
>>>
>>>    - Out of multiple proposals affecting the same leaf, the committer
>>>    MUST choose an arbitrary one (13.2, “If there are multiple (…)”).
>>>
>>>
>>>    - Out of multiple PSK proposals with the same ID, the committer MUST
>>>    choose an arbitrary one (13.2).
>>>
>>>
>>> In fact, once extensions come into play there are likely many more
>>> similar situations, even ones affecting lists of otherwise benign proposals
>>> (e.g. Add/Remove). Suppose a chat app is built on MLS with a custom room
>>> moderator roles extension. The app wants MLS to enforce two rules: Only
>>> mods can Add/Remove. Rooms should always have at least 1 Mod. Here’s how
>>> the RFC’s current text can become a problem: A and B are the only mods in a
>>> room. A makes proposal P1 that removes B. Meanwhile, B makes prop P2 that
>>> removes A. C gets both proposals. What should C do?
>>>
>>>    - Option 1: validate & cache both proposals. → Problem: C can no
>>>    longer issue a commit as it “MUST include all valid proposals”.
>>>    (Alternatively, it does commit to both as instructed by the RFC which
>>>    leaves the room without a mod.)
>>>
>>>
>>>    - Option 2: validate & cache the first prop received (say, P1), but
>>>    reject P2 as validating both means C can’t commit. → Problem: If someone
>>>    else commits to P2 by ref. C can’t process the commit as it doesn’t “know”
>>>    P2.
>>>
>>>
>>> So, to clean all this up a bit more we are working on a PR with 2 main
>>> changes:
>>>
>>>    1. In 13.2, replace “The sender of a Commit MUST include all valid
>>>    proposals that it has received“ by ”The sender of a Commit MUST commit to a
>>>    maximal valid list of proposals. The conditions making a list of proposals
>>>    'valid' for an epoch are described in Section 13.4. A valid list is
>>>    'maximal' if adding any other received proposal would result in an invalid
>>>    list.“
>>>    2. Add Section 13.4 which collects all rules for validating
>>>    proposals (currently in 13.2, 13.1, 11.1, 8.3) into 3 procedures:
>>>    validating an incoming proposal, validating a list of proposals on creating
>>>    a commit and on receiving a commit.
>>>
>>>
>>> We’d be happy to hear what people think about this!
>>>
>>> Marta
>>>
>>> _______________________________________________
>>> MLS mailing list
>>> [e-mail]
>>> https://www.ietf.org/mailman/listinfo/mls
>>>
>> _______________________________________________
>> MLS mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/mls
>>
>>