Re: [MLS] MLSPlaintext packets aren't authenticated using symmetric key schedule

[Raphael Robert <[e-mail]>]

I thought about this some more and now I’m not sure that the membership_token should only be implicit:

The whole reason for HS messages to not be encrypted is that they can be inspected along the way by an external party (e.g. the Delivery Service). If this is not a requirement, MLSCiphertext should really be used instead.
Hiding the membership_token from external inspector means the inspector can not verify the signature anymore. I think that defies the point of HS messages being inspectable.

The obvious fix is to include the membership_token in MLSPlaintext as well.

Raphael

> On 18 Aug 2020, at 21:01, Richard Barnes <[e-mail]> wrote:
> 
> Here ya go: https://github.com/mlswg/mls-protocol/pull/396 <https://github.com/mlswg/mls-protocol/pull/396>
> On Tue, Aug 18, 2020 at 1:30 PM Joel Alwen <[e-mail] <mailto:[e-mail]>> wrote:
> > Joël, do you want to write a PR?  If not, I could probably get to it in the
> > next couple days.
> 
> Thanks. :-) If you could that would be really nice.
> 
> - Joël
> 
> On 18/08/2020 19:20, Richard Barnes wrote:
> > Sounds like we're converging here.  The only question in my mind is what goes in
> > the MAC -- seems like the easy option is probably "the remainder of the
> > MLSPlaintextTBS", i.e., everything from group_id to the end.  That seems like it
> > minimizes multiple serialization:
> > 
> > tbs_content = serialize(group_id, ...)
> > membership_token = KDF.Extract(confirmation_key, tbs_content)
> > tbs = group_context || membership_token || tbs_content
> > 
> > So the PR would be to basically pop the context off of MLSPlaintextTBS and add
> > the three lines above (with the switch for internal/external in prose).
> > 
> > Joël, do you want to write a PR?  If not, I could probably get to it in the next
> > couple days.
> > 
> > --Richard
> > 
> > On Tue, Aug 18, 2020 at 12:24 PM Raphael Robert <[e-mail] <mailto:[e-mail]>
> > <mailto:[e-mail] <mailto:[e-mail]>>> wrote:
> > 
> >     I think what you just described is indeed a combination of option 1 & 2.
> >     It’s a MAC over the payload we want to authenticate, but it’s implicit and
> >     we only include it in the MLSPlaintextTBS. Or in other words, we stripped it
> >     from MLSPlaintext because it is implicitly known to any valid member of the
> >     group.
> > 
> >     Raphael
> > 
> >>     On 18 Aug 2020, at 18:16, Joel Alwen <[e-mail] <mailto:[e-mail]>
> >>     <mailto:[e-mail] <mailto:[e-mail]>>> wrote:
> >>
> >>     Yeah, I like Option 2 here. I like that it avoids growing packet size.
> >>
> >>     One caveat though: I'd go for the MAC(...) version rather than the
> >>     confirmation_key. For starters thing including confirmation_key doesnt
> >>     authenticate the contents of the packet. But even if it did, signatures aren't
> >>     meant to hide the contents of what was signed. (Amend you favorite sig
> >>     scheme to
> >>     tack on the message at the end a signature and you've still got a secure
> >>     signature scheme. But clearly not a message hiding one.) That means that AFAIK
> >>     neither ECDSA nor EdDSA etc. were designed or analyzed for such a property. So
> >>     this would amount to a very non-standard use of a signature schemes by
> >>     MLS. Not
> >>     saying it doesn't work for the particular sig schemes in our ciphersuite. But
> >>     its def. not how signatures are "meant" to be used.
> >>
> >>     But that leaves Option 2 with, say, including tag = MAC(conf_key,
> >>     conf_trans_hash || MLSPlaintext.content) into whats being signed which I like
> >>     and think gets the job done. Both conf_* values are taken from the current
> >>     epoch. By MLSPlaintext.content I mean whats now called MLSPlaintextTBS.
> >>
> >>>     I would propose that we do need something additional on Commit messages as
> >>>     well as Proposals.
> >>
> >>     @Richard: For Proopsals I think this works. Is that about what you had in mind
> >>     for commits too?
> >>
> >>     - Joël
> >>
> >>
> >>     On 18/08/2020 17:26, Richard Barnes wrote:
> >>>     Thanks for pointing this out, Joël.  I agree that the attacks you're
> >>>     describing
> >>>     should work as things are currently specified.  And they're salient,
> >>>     especially
> >>>     the "replace Alice in the group" one.
> >>>
> >>>     Also agree with Raphael is correct that Commit is not affected by this, since
> >>>     someone who is not a member won't be able to generate the right confirmation
> >>>     value.  However, I don't think this is actually the right design to adopt
> >>>     for a
> >>>     general solution to this problem.  Confirmation verifies group membership
> >>>     *after* processing the handshake message; the point here is that we
> >>>     should also
> >>>     have a membership check *before* processing handshake messages.  In
> >>>     particular,
> >>>     I would propose that we do need something additional on Commit messages
> >>>     as well
> >>>     as Proposals.
> >>>
> >>>     Thinking about solutions here, a couple of options come to mind:
> >>>
> >>>     1. Use MLSCiphertext, but with an integrity-only encapsulation
> >>>     2. Incorporate in the signature something that is only known to the group
> >>>     (e.g.,
> >>>     confirmation_key or MAC(confirmation_key; confirmed_transcript_hash ||
> >>>     Proposal/Commit))
> >>>
> >>>     Option (1) has the appeal that you would only ever send MLSCiphertext, though
> >>>     switching between encrypted/not could be problematic.  Option (2) seems a lot
> >>>     more appealing: It doesn't add any overhead, since the group-secret value
> >>>     doesn't need to be sent.  And we already switch between the signature context
> >>>     that is added for group members vs. external.  In fact, I think option
> >>>     (2) would
> >>>     just amount to a one-line change to include an extra, secret value in the
> >>>     context at the top of the MLSPlaintextTBS struct.
> >>>     https://github.com/mlswg/mls-protocol/blob/master/draft-ietf-mls-protocol.md#content-signing-and-encryption <https://github.com/mlswg/mls-protocol/blob/master/draft-ietf-mls-protocol.md#content-signing-and-encryption>
> >>>
> >>>     The only thing that seems odd about (2) is overloading signature
> >>>     verification in
> >>>     that way, i.e., using the ability to generate a signature over a secret
> >>>     thing to
> >>>     prove you know the secret thing.  That doesn't seem obviously flawed to
> >>>     me, but
> >>>     worth thinking about.
> >>>
> >>>     Does that make sense to folks?
> >>>
> >>>     --Richard
> >>>
> >>>
> >>>     On Tue, Aug 18, 2020 at 10:55 AM Raphael Robert
> >>>     <raphael=[e-mail] <mailto:[e-mail]>
> >>>     <mailto:raphael <mailto:raphael>=[e-mail] <mailto:[e-mail]>> <mailto:[e-mail] <mailto:[e-mail]>>>
> >>>     wrote:
> >>>
> >>>        Hi Joel,
> >>>
> >>>        For context: this would only apply when applications use cleartext
> >>>        MLSPlaintext for HS messages. The recommendation is still to encrypt them
> >>>        and send them around as MLSCiphertext.
> >>>        That being said, we said we would like to support scenarios where HS
> >>>        messages are not necessarily encrypted.
> >>>
> >>>        Question: would this attack work with Commit messages? I’m thinking that
> >>>        they would be rejected because the attacker cannot compute the
> >>>     confirmation_tag.
> >>>
> >>>        As you mention in the PS, the easy target would be Proposal messages.
> >>>
> >>>        I’d be interested to see what exactly you would propose as a mitigation
> >>>        mechanism.
> >>>
> >>>        Raphael
> >>>
> >>>>     On 18 Aug 2020, at 16:36, Joel Alwen <[e-mail] <mailto:[e-mail]>
> >>>>     <mailto:[e-mail] <mailto:[e-mail]>>
> >>>        <mailto:[e-mail] <mailto:[e-mail]>>> wrote:
> >>>>
> >>>>     Hey everyone,
> >>>>
> >>>>     Something thats been bugging Marta Mularczyk and Daniel Jost and me for
> >>>>     a bit
> >>>>     now is that handshake messages sent out as MLSPlaintext packets are only
> >>>>     authenticated using signatures, but not using the group's key schedule. For
> >>>>     non-members that makes sense but for group members that's weaker than
> >>>>     need be.
> >>>>
> >>>>     Suppose Alice is in a group using signing key pair (spk, ssk). I corrupt
> >>>        her to
> >>>>     learn ssk. Now I loose access to her device again. Later she generates a
> >>>>     fresh
> >>>>     key package with her same spk but a new HPKE key for her leaf. She sends
> >>>        out and
> >>>>     update proposal for her new key package and someone commits to the update.
> >>>>
> >>>>     Expected result: she (and the group at large) has achieved PCS again.
> >>>>
> >>>>     Actual result: using her stolen ssk I can still forge a new proposal's
> >>>        (sent as
> >>>>     MLSPlaintext packets) coming from Alice. Some things I could do with this
> >>>        power:
> >>>>     - I can generate a new key package kp for Alice using her spk and some
> >>>        HPKE key
> >>>>     she doesn't know. Then I forge an update proposal for Alice with kp. If it
> >>>        gets
> >>>>     committed I've effectively kicked her out of the group.
> >>>>     - I could forge Add's and Remove's coming from Alice, so I could trick the
> >>>>     group into thinking Alice is trying to Add my account to the group or remove
> >>>>     some other group member.
> >>>>
> >>>>     Lemme know if I've missed something here in that scenario...
> >>>>
> >>>>
> >>>>     If I didn't miss anything and the attacks really work as advertised then IMO
> >>>>     this is kinda weak sauce and worth avoiding if possible. So to that end, how
> >>>>     about we modify MLS such that MLSPlaintext packets coming from group members
> >>>>     must also be authenticated using something from the application key
> >>>>     schedule.
> >>>>     Now the above attacks fail. As soon as Alice's update is gets committed I no
> >>>>     longer know the group's key schedule and so can't forged packet from
> >>>        Alice. More
> >>>>     generally, this brings the PCS guarantees when using MLSPlaintexts
> >>>>     frameing in
> >>>>     line with what we're getting from MLSCiphertext packets.
> >>>>
> >>>>     Any thoughts?
> >>>>
> >>>>     - Joël
> >>>>
> >>>>
> >>>>
> >>>>     PS. For concreteness, we could probably extend the current mechanism for
> >>>        getting
> >>>>     concistancy (the confirmation_tag) to also provide symmetric key
> >>>        authentication.
> >>>>     E.g. include most of the MLSPlaintext content into whats being tagged by
> >>>>     confirmation_tag. That would cover the case of a commit packet and doesn't
> >>>        even
> >>>>     grow the size of MLSPlaintext packets over the current design.
> >>>>
> >>>>     For a proposal packet we could also have a confirmation_tag but this one is
> >>>>     computed using the *current* epoch's confirmation_key and
> >>>        confirmed_transcript_hash.
> >>>>
> >>>>     _______________________________________________
> >>>>     MLS mailing list
> >>>>     [e-mail] <mailto:[e-mail]> <mailto:[e-mail] <mailto:[e-mail]>> <mailto:[e-mail] <mailto:[e-mail]>>
> >>>>     https://www.ietf.org/mailman/listinfo/mls <https://www.ietf.org/mailman/listinfo/mls>
> >>>
> >>>        _______________________________________________
> >>>        MLS mailing list
> >>>        [e-mail] <mailto:[e-mail]> <mailto:[e-mail] <mailto:[e-mail]>> <mailto:[e-mail] <mailto:[e-mail]>>
> >>>        https://www.ietf.org/mailman/listinfo/mls <https://www.ietf.org/mailman/listinfo/mls>
> >