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

[Richard Barnes <[e-mail]>]

Here ya go: https://github.com/mlswg/mls-protocol/pull/396

On Tue, Aug 18, 2020 at 1:30 PM Joel Alwen <[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]>> 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]>> 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
> >>>
> >>>     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:raphael=[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]>> 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]>
> >>>>     https://www.ietf.org/mailman/listinfo/mls
> >>>
> >>>        _______________________________________________
> >>>        MLS mailing list
> >>>        [e-mail] <mailto:[e-mail]> <mailto:[e-mail]>
> >>>        https://www.ietf.org/mailman/listinfo/mls
> >
>