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

[Joel Alwen <[e-mail]>]

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.)

- 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
>>     >
>>
>