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

[Raphael Robert <[e-mail]>]

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] <mailto:[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 <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] <mailto:[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]>
> >> <mailto:[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]>>
> >>     > <mailto:[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]>>
> >>     >>     <mailto:[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:[e-mail] <mailto:[e-mail]>>
> >>     >>>     <mailto:raphael <mailto:raphael> <mailto:raphael <mailto:raphael>>=[e-mail] <mailto:[e-mail]>
> >>     <mailto:[e-mail] <mailto:[e-mail]>>> <mailto:[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]> <mailto:[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]>
> >>     <mailto:[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]>
> >>     <mailto:[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>
> >>     >
> >>
> >