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

[Richard Barnes <[e-mail]>]

To close the loop on context: As mentioned on the call, the content of the
GroupContext is public, so it shouldn't interfere with the DS verifying
signatures.

After the discussion on the interim call today, I made two updates to this
PR:
1. Use an explicit MAC and separate keys
https://github.com/mlswg/mls-protocol/pull/396/commits/10e1ba799e44a7e858d7832ec5c6c39bd214010a
2. Have the membership MAC cover the signature
https://github.com/mlswg/mls-protocol/pull/396/commits/52a06e4021d654c60afea85681f4cb2c91bd05f1

We didn't really close on the latter change on the call, but it seems like
it adds a bit more rigidity than the parallel MAC-and-Sign construction we
had before, and makes things more parallel to MLSCiphertext.  (Puts me in
mind of the encrypt-then-MAC / MAC-then-encrypt / encrypt-and-MAC
discussions of yore.)

On Fri, Aug 21, 2020 at 5:49 AM Raphael Robert <[e-mail]> wrote:

> Thanks Richard, I think this now meets all requirements as such. However,
> I stumbled upon the (group) context field that is already in
> MLSPlaintextTBS. Wouldn’t this
>
> a) mean that the server cannot verify a signature of a member, since the
> group context is not known outside of the group?
> b) address Joel’s concerns in the first place?
>
> I’m a bit confused as to what the intention was initially when we
> introduced that. Right now it doesn’t seem to serve any extra purpose and I
> think we could remove it.
>
> Raphael
>
> On 20 Aug 2020, at 20:55, Richard Barnes <[e-mail]> wrote:
>
> On Thu, Aug 20, 2020 at 10:20 AM Joel Alwen <[e-mail]> wrote:
>
>> Yeah, good point. While I'm not sure every case of server inspection would
>> require checking sigs on packets I am sure it would be useful for some
>> cases.
>>
>> > The obvious fix is to include the membership_token in MLSPlaintext as
>> well.
>>
>> Yup, I'm all for it. (Or at least for making it an optional field in that
>> struct.)
>>
>
> The PR has been updated.  There is now a member_token field before the
> signature, which non-member senders set to zero length.
>
> https://github.com/mlswg/mls-protocol/pull/396/files
>
> Note that this allows the server to verify the signature, but the
> membership_token is opaque to it; the server can't authenticate the
> sender's membership.
>
> --Richard
>
>
>>
>> - Joël
>>
>> On 19/08/2020 09:24, Raphael Robert wrote:
>> > 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] <mailto:
>> [e-mail]>> wrote:
>> >>
>> >> Here ya go: 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
>> >>     >>>
>> >>     >>>     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
>> >>     >>>
>> >>     >>>        _______________________________________________
>> >>     >>>        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
>> >>     >
>> >>
>> >
>>
>
>