Subject: [MMUSIC] Benjamin Kaduk's Discuss on draft-ietf-mmusic-sdp-uks-06: (with DISCUSS and COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Mon, 05 Aug 2019 09:27:56 -0700

Benjamin Kaduk has entered the following ballot position for
draft-ietf-mmusic-sdp-uks-06: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-mmusic-sdp-uks/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

There are both pretty minor points, in the grand scheme of things, but I
do think it would be hazardous to publish the document without
addressing them.

The semantics surrounding the "external_id_hash" TLS extension seem
insufficiently specified to admit interoperable implementation.  In
Section 3.2 we read that it "carries a hash of the identity assertion that
communicating peers have exchanged", as if there was a single
distinguished identity assertion for the session.  But, if we read on,
we learn that there is not one identity assertion, but (in the general
case) two, one for each party, and that what seems to actually be
intended is that each party sends the hash of the identity assertion
corresponding to the sender's identity, with the requirements to send an
empty external_id_hash if the party in question is not providing
identity bindings.  Additionally, the text about having an empty
"external_id_hash" extension in ClientHello or
ServerHello/EncryptedExtensions is written in a way that implies that
all parties generate a ClientHello and all parties generate a
ServerHello or EncryptedExtensions message, whereas these are actually
conditional on whether the party is acting as (D)TLS client or server.

Similarly, the current text for the last sentence of Section 3.2 ("In
TLS 1.3, the "external_id_hash" extension MUST be sent in the
EncryptedExtensions message.") can be (mis)read as implying that all
EncryptedExtensions messages sent by TLS servers that implement this
specification must include this extension, which would violate the TLS
extension-negotiation model since it mandates the server sending an
extension without regard to the client having indicated support for the
extension.  Perhaps "MUST NOT be sent in the TLS 1.3 ServerHello message"
conveys the restriction more clearly?
(A similar comment applies to the corresponding statement in Section
4.3, which interestingly enough already has a "In TLS 1.3, the
"external_session_id" extension MUST NOT be included in a ServerHello."
disclaimer in addition to the problematic sentence.)

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Section 2

   The attacker obtains an identity assertion for an identity it
   controls, but binds that to the fingerprint of one peer.  The
   attacker is then able to cause a TLS connection to be established
   where two endpoints communicate.  The victim that has its fingerprint
   copied by the attack correctly believes that it is communicating with
   the other victim; however, the other victim incorrectly believes that
   it is communicating with the attacker.

nit: maybe this could be reworded for improved clarity.  Perhaps, "two
endponts other than the attacker communicate" or "two victim endpoints".

   A similar attack can be mounted without any communications
   established based on the SDP "fingerprint" attribute [FINGERPRINT].

At this point in the document, I don't know how to interpret "without
any communications established based on".

Section 2.1

   1.  An attacker can only modify the parts of the session signaling
       for a session that they are part of, which is limited to their
       own offers and answers.

nit(?): the first part of the sentence suggests that the attacker can
modify their peers' offers/answers, and it's not entirely clear (from a
rhetorical sense) how the latter clause is supposed to relate to the
first.

   The combination of these two constraints make the spectrum of
   possible attacks quite limited.  An attacker is only able to switch
   its own certificate fingerprint for a valid certificate that is
   acceptable to its peer.  Attacks therefore rely on joining two
   separate sessions into a single session.

nit: It's not clear to me (at this point in the document) whether this
is "victim A connects to attacker and also to victim B, and attacker
merges the first session into the second", or "victim A connects to
attacker and attacker connects to victim B, and attacker splices the two
together and steps out of the way".  (I assume the latter, but the text
hasn't clarified it yet.)

Section 2.3

   Third-party call control (3PCC) [RFC3725] is a technique where a
   signaling peer establishes a call that is terminated by a different
   entity.  This attack is very similar to the 3PCC technique, except
   where the TLS peers are aware of the use of 3PCC.

nit: Rhetorically-wise, I don't know what "except" is intended to mean
here.  Is the attack like 3PCC but in normal 3PCC the peers are unaware
of 3PCC use and in the attack they are?  The other way around?  ("except
that in the 3PCC case the TLS peers are aware of its use" would
disambiguate fine, I think.)

   It is understood that this technique will prevent the use of 3PCC if
   peers have different views of the involved identities, or the value
   of SDP "tls-id" attributes.

nit: understood by whom?  (I don't think that we need "It is understood
that" at all.)

Section 3

   The identity assertions used for WebRTC (Section 7 of [WEBRTC-SEC])
   and the SIP PASSPoRT using in SIP identity ([SIP-ID], [PASSPoRT]) are
   bound to the certificate fingerprint of an endpoint.  An attacker

nit: s/using/used/

   causes an identity binding to be created that binds an identity they
   control to the fingerprint of a first victim.

nit: I think we want "An attacker can cause" or "In an unknown-key-share
attack, an attacker causes".

   really talking to the first victim.  The attacker only needs to
   create an identity assertion that covers a certificate fingerprint of
   the first victim.

Well, and actually cause the traffic to shuffle around so the victims
are sending/receiving from each other.

   The problem might appear to be caused by the fact that the authority
   that certifies the identity binding is not required to verify that
   the entity requesting the binding controls the keys associated with
   the fingerprints.  Neither SIP nor WebRTC identity providers are not
   required to perform this validation.  However, validation of keys by
   the identity provided is not relevant because verifying control of
   the associated keys is not a necessary condition for a secure
   protocol, nor would it be sufficient to prevent attack [SIGMA].

nit: in the last sentence, I'm not sure that "validation of keys by the
identity provided" is correct; "identity provider" would make more
sense.

   This form of unknown key-share attack is possible without
   compromising signaling integrity, unless the defenses described in

nit: I'd suggest s/possible/even possible/

   Section 4 are used.  Endpoints MUST use the "external_session_id"
   extension (see Section 4.3) in addition to the "external_id_hash"
   (Section 3.2) so that two calls between the same parties can't be
   altered by an attacker.

nit(?): These normative requirements kind of come out of nowhere, in
terms of the flow of language.  Maybe "In order to prevent this attack,
endpoints MUST", or just move the normative requirements closer to the
mechanisms themselves?

Section 3.2

   A WebRTC identity assertion is provided as a JSON [JSON] object that
   is encoded into a JSON text.  The resulting string is then encoded
   using UTF-8 [UTF8].  The content of the "external_id_hash" extension

I don't really understand the separate UTF-8 step -- RFC 8259 already
requires text to be UTF-8 encoded.

I think this section would be easier to read if the different cases of
identity encoding/transmission were broken out into a bulleted or
enumerated list (the latter might make it easier to extend in the
future): right now I think we have (1) pure WebRTC, (2) SDP "identity",
and (3) SIP PASSPoRT, but I'm not 100% sure I'm reading the text
properly.

If that's done, it would also be a good opportunity to clarify that the
note about hash agility applies to the TLS extension as a whole, not
just the PASSPoRT case.

   Where a PASSPoRT is used, the compact form of the PASSPoRT MUST be
   expanded into the full form.  The base64 encoding used in the SIP

nit: this is written to assume that only compact PASSPoRTs will ever be
used, which IIUC is not the case.

                                                    This allows its peer
   to include a hash of its identity binding.  An endpoint without an
   identity binding MUST include an empty "external_id_hash" extension
   in its ServerHello or EncryptedExtensions message, to indicate
   support for the extension.

nit: and that it has validated the client's identity binding?

   A peer that receives an "external_id_hash" extension that does not
   match the value of the identity binding from its peer MUST
   immediately fail the TLS handshake with an error.  This includes
   cases where the binding is absent, in which case the extension MUST
   be present and empty.

nit: I'd suggest rewording the second sentence as follows (since the
conditional logic on "extension present but binding absent" could be
confusing:

% The absence of an identity binding does not relax this requirement --
% an extension received when the peer has not provided an identity
% binding on the signalling layer must still be validated to have the
% zero-length extension body.

   A peer that receives an identity binding, but does not receive an
   "external_id_hash" extension MAY choose to fail the connection,
   though it is expected that implementations written prior to the
   definition of the extensions in this document will not support both
   for some time.

nit: I don't think the comma after "binding" is needed.
Also, is the "not" intended?  I'm not entirely sure what "both" is
intended to refer to.

Section 4

nit(?): There's an annoying lack of parallelism in the Section titles
for Sections 3 and 4, though I don't have a good suggestion for Section
4's title -- "Attack on Raw Fingerprints" is the best I can do right
now.

   Even if the integrity session signaling can be relied upon, an

nit: s/integrity session signaling/session signaling integrity/?

Section 4.1

   another honest endpoint.  The attacker convinces the endpoint that
   their session has completed, and that the session with the other
   endpoint has succeeded.

Even with the benfit of the figure, I'm not sure I am properly
understanding the distinction between "completed" and "succeeded".  Is
the idea that the "completed" session finishes a DTLS handshake and then
immediately hangs up?  Or is this entirely at the signalling layer?

                 For this reason, it might be necessary to permit the
   signaling from Patsy to reach Norma to allow Patsy to receive a call
   setup completion signal, such as a SIP ACK.  Once the second session
   is established, Mallory might cause DTLS packets sent by Norma to
   Patsy to be dropped.  It is likely that these DTLS packets will be
   discarded by Patsy as Patsy will already have a successful DTLS
   connection established.

nit: Is this "it is likely these packets would be discarded even if
Mallory lets them through"?

   This attack creates an asymmetry in the beliefs about the identity of
   peers.  However, this attack is only possible if the victim (Norma)
   is willing to conduct two sessions nearly simultaneously, if the
   attacker (Mallory) is on the network path between the victims, and if
   the same certificate - and therefore SDP "fingerprint" attribute
   value - is used in both sessions.

This is the same certificate used by Norma in both sessions, right?

Section 4.3

   Where RTP and RTCP [RTP] are not multiplexed, it is possible that the
   two separate DTLS connections carrying RTP and RTCP can be switched.
   This is considered benign since these protocols are designed to be
   distinguishable.  RTP/RTCP multiplexing is advised to address this
   problem.

What does "switched" mean?  That Mallory could swap the data contents
around as an active MITM?

   This defense is not effective if an attacker can rewrite "tls-id"
   values in signaling.  Only the mechanism in "external_id_hash" is
   able to defend against an attacker that can compromise session
   integrity.

Please help me check my understanding: in terms of just the operation of
the TLS extensions, "external_id_hash" and "external_session_id" provide
similar protection, in that they are just validating that what's in the
TLS handshake matches what's in the signalling layer.  The added
protection from "external_id_hash" only comes when the endpoints
actually go and contact the peers' IdP to validate the identity
assertions that are transmitted in the signalling layer.
If my understanding is correct, we should probably add a bit more text
here indicating the need for more validation than just the validation of
the TLS extension contents that this document describes.

Section 5

   In the absence of any higher-level concept of peer identity, the use
   of session identifiers does not prevent session concatenation if the
   attacker is able to copy the session identifier from one signaling
   session to another.  This kind of attack is prevented by systems that
   enable peer authentication such as WebRTC identity [WEBRTC-SEC] or
   SIP identity [SIP-ID].  However, session concatenation remains
   possible at higher layers: an attacker can establish two independent
   sessions and simply forward any data it receives from one into the
   other.

And in such a case the attacker has access to the media plaintext, too,
right?

   Use of the "external_session_id" does not guarantee that the identity
   of the peer at the TLS layer is the same as the identity of the
   signaling peer.  The advantage an attacker gains by concatenating
   sessions is limited unless it is assumed that signaling and TLS peers
   are the same.  If a secondary protocol uses the signaling channel
   with the assumption that the signaling and TLS peers are the same
   then that protocol is vulnerable to attack unless they also validate
   the identity of peers at both layers.

Is this paragraph describing a case like in
draft-ietf-rtcweb-security-arch, where we do send (and verify) identity
assertions at the signalling layer?  That is, the verification at the
IdP counts for validation at the "secondary protocol" layer and the
verification that the TLS extension matches the signalling constitutes
verification at the TLS layer, thereby achieving the validation "at both
layers"?

   It is important to note that multiple connections can be created
   within the same signaling session.  An attacker might concatenate
   only part of a session, choosing to terminate some connections (and
   optionally forward data) while arranging to have peers interact
   directly for other connections.  It is even possible to have
   different peers interact for each connection.  This means that the
   actual identity of the peer for one connection might differ from the
   peer on another connection.

How do or could the mitigations specified in this document address these
attacks?

Section 8.2

I don't see how [BASE64] is only informative; we require base64-decoding
for some of the procedures.

---

From: "Martin Thomson" <[email-masked]>
Date: Tue, 06 Aug 2019 17:42:36 +1000

Hi Ben,

Thanks for the thorough review.  I've made a few clarifications based on this.

I've combined these changes with those suggested by Roman, which you can preview here:

   https://github.com/martinthomson/sdp-uks/pull/11

> disclaimer in addition to the problematic sentence.)

Yeah, that is pure gibberish.  Sorry about that.  Changed to:

A similar attack can be mounted based solely on the SDP `fingerprint` attribute
[FINGERPRINT] without compromising the integrity of the signaling channel.

> rhetorical sense) how the latter clause is supposed to relate to the

better?

   1. An attacker can only modify the parts of the session signaling that they are
   responsible for producing, namely their own offers and answers.

> hasn't clarified it yet.)

It's really difficult to provide some context without providing all of it.  Writing out the gory details is tricky, so your little uncertainty is hard to resolve (it's both, but the former has greater potential for mischief).  I've resorted to forward references here rather than explaining this further at this stage of the document.

> disambiguate fine, I think.)

This turns out to be subtly wrong.  The "except" is in two parts now:

Third-party call control (3PCC) [RFC3725] is a technique where a signaling
peer establishes a call that is terminated by a different entity.  This attack
is very similar in effect to some 3PCC practices, so use of 3PCC could appear to
be an attack.  However, 3PCC that follows RFC 3725 guidance is unaffected, and
peers that are aware of changes made by a 3PCC controller can correctly
distinguish actions of a 3PCC controller from attack.

> requires text to be UTF-8 encoded.

As I said in response to Roman's email, I don't think that's true.  RFC 8259 says that you need to encode JSON texts using UTF-8 if you aren't in a closed system:

   JSON text exchanged between systems that are not part of a closed
   ecosystem MUST be encoded using UTF-8 [RFC3629].

But this has a comparison requirement that is stronger than "not part of a closed system", so I think that  the encoding rule bears repeating.

> note about hash agility applies to the TLS extension as a whole, not

I have done that.  It required a tiny bit of glue, but it seems like it worked well.

> nit: and that it has validated the client's identity binding?

That isn't possible until the connection is established generally.  Maybe it indicates a willingness to validate an identity binding, but really all that is needed here is that it is willing to receive a binding.

> % zero-length extension body.

I went with "The absence of an identity binding
does not relax this requirement; if a peer provided no identity binding, a
zero-length extension MUST be present to be considered valid."

> Also, is the "not" intended?  I'm not entirely sure what "both" is

Reworded to:
Implementations written prior to the definition of the extensions in this
document will not support this extension for some time.  A peer that receives an
identity binding but does not receive an `external_id_hash` extension MAY accept
a TLS connection rather than fail a connection where the extension is absent.

> immediately hangs up?  Or is this entirely at the signalling layer?

reworded to:
"The attacker convinces the first endpoint that their session with the attacker has
been successfully established, but media is exchanged with the other honest
endpoint.  The attacker permits the session with the other honest endpoint to
complete only to the extent necessary to convince the other honest endpoint to
participate in the attacked session."

> Mallory lets them through"?

Yes.  "However, if Mallory allows DTLS packets to pass, it is
likely that Patsy will discard them as Patsy will already have a successful DTLS
connection established."

> This is the same certificate used by Norma in both sessions, right?

Ack.  Fixed.

> around as an active MITM?

Yes.  

RTP and RTCP are usually multiplexed onto the same path today, where they need to be fully distinguishable.  This is by far the most common deployment model, but that doesn't really matter for the purposes of analysis.

If RTP and RTCP use different DTLS connections, an attacker could switch port numbers (for instance) on the two and cause peers to believe that RTP is RTCP and vice versa.  That's highly unlikely to produce meaningful results SRTP (RFC 3711) provides key separation.  Key separation would be enough if it weren't for ludicrously short authentication tags in common use (we still see 32-bit tags; bits, not bytes!).  If key separation fails, the protocols are generally distinguishable, though not perfectly so where multiplexing is not chosen.

> the TLS extension contents that this document describes.

Yes, there are multiple validation steps.  This document assumes that the validation described in RFC 8122 (a=fingerprint) is always followed.  It also assumes that if you are using one of the identity assertions, you follow the respective validation processes.

Added to both sections (the bracket part only for the first):
Any validation performed of the `external_id_hash|external_session_id` extension is done in addition
to the validation required by {{!FINGERPRINT}} [and any identity assertion
definition].
 
> And in such a case the attacker has access to the media plaintext, too,

Yeah, but that doesn't expose the victims to the consequences of mistaken channel bindings.

> verification at the TLS layer, thereby achieving the validation "at both

WebRTC doesn't include validation at both layers in the way that this describes.

Imagine that Mallory pretends that they own fingerprints and assertions from Norma when communicating with Patsy and (vice versa) pretends that they own the Patsy's values when communicating with Norma.  They can then forward packets.  That puts Mallory on the signaling path, but not the media path (aside from having to forward encrypted packets).  If Norma and Patsy were to assume that signaling and media peers were the same, that leads to problems.

Neither Norma nor Patsy are under misapprehension of each other's identity, but they might not realize that they have an on-path attacker that can see (and modify) signaling.  If they were to send something derived from signaling in DTLS, that might fail.  If they were to send something derived from DTLS in signaling, that might leak to Mallory.

> How do or could the mitigations specified in this document address these

The point of this text is to make it clear that no guarantees transfer from media to signaling or between different connections.

With the mitigations described AND identity assertions, you can be confident that the peer at the media layer is the peer that the identity assertion reports, even in the presence of attackers on the signaling path.

With only fingerprints, the signaling system needs to have integrity, including safeguards on replacement of fingerprints by endpoints.

I've added those notes as security considerations.

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 6 Aug 2019 14:32:10 -0500

>    https://github.com/martinthomson/sdp-uks/pull/11

Thanks; I left a few comments there.

> > disclaimer in addition to the problematic sentence.)

(I'm not sure we quite got all of these in the linked pull request.)

>    responsible for producing, namely their own offers and answers.

yes, thanks

> It's really difficult to provide some context without providing all of it.  Writing out the gory details is tricky, so your little uncertainty is hard to resolve (it's both, but the former has grea…

That should work well.

> distinguish actions of a 3PCC controller from attack.

Ah, thank you.

> But this has a comparison requirement that is stronger than "not part of a closed system", so I think that  the encoding rule bears repeating.

I guess in my head once you are doing webrtc/etc. you are out of a closed
system and the loophole disappears, but there's a reason this was only a
non-blocking comment.

> I have done that.  It required a tiny bit of glue, but it seems like it worked well.

It does look like it (just from the markdown; hopefully also when
rendered).

> That isn't possible until the connection is established generally.  Maybe it indicates a willingness to validate an identity binding, but really all that is needed here is that it is willing to rec…

Ah, good point.

> a TLS connection rather than fail a connection where the extension is absent.

Thanks!

> participate in the attacked session."

Much better; thank you

> If RTP and RTCP use different DTLS connections, an attacker could switch port numbers (for instance) on the two and cause peers to believe that RTP is RTCP and vice versa.  That's highly unlikely t…

I'm going to insert "since" in "since SRTP provides key separation".
Thanks for clarifying/confirming, and I don't think we need any changes to
the text here.

> to the validation required by {{!FINGERPRINT}} [and any identity assertion

Thanks!

> Yeah, but that doesn't expose the victims to the consequences of mistaken channel bindings.

True.

> Neither Norma nor Patsy are under misapprehension of each other's identity, but they might not realize that they have an on-path attacker that can see (and modify) signaling.  If they were to send…

Thanks for helping flesh this out.  Looking back at the text I quoted from
the document, there are two things that I might want to change (still not
100% confident): (1) "unless it is assumed", which is potentially vague
about who/what is doing the assuming.  Perhaps "unless there is additional
data exchanged under the assumption that" or similar.  (2) "unless they also
validate the identity of peers at both layers".  This document describes
how to validate the identity of the peer at the TLS layer, but I wonder if
we want some extra reference/guidance on validating the peer's identity at
the signalling layer.

> I've added those notes as security considerations.

Thanks!

-Ben

---

From: "Martin Thomson" <[email-masked]>
Date: Wed, 07 Aug 2019 13:09:39 +1000

Pruning aggressively.  PR updated for the easy stuff that didn't require commentary.

> (I'm not sure we quite got all of these in the linked pull request.)

I think that I caught these now.  It seems obvious to me - to the point it doesn't warrant mention - that a server can't send an extension that a client didn't send...  Hence the omission.

> we want some extra reference/guidance on validating the peer's identity at

I've done part of that.  But the latter requires an existence proof.  I don't believe that any signaling system provides the necessary bindings.

In thinking about this, you need channel bindings.  That is, the signaling need to be able to assert not that the peer is the same as the peer at the media layer, but also that the media layer is in the correct place.  So I'm going to say that this is out of scope:

"Such a signaling system, while out of scope for this document, requires that the
signaling layer is authenticated and bound to the TLS connections."

I'm not aware of any such system, as much as it might be desirable.  SIP is still wrestling with the concept of caller identity.  Proper channel bindings seems like a bit much to expect.

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 6 Aug 2019 22:31:09 -0500

> I think that I caught these now.  It seems obvious to me - to the point it doesn't warrant mention - that a server can't send an extension that a client didn't send...  Hence the omission.

(I quote liberally from
https://github.com/martinthomson/sdp-uks/blob/cf6084c44581c794e93bda174e2bc6883fada26e/draft-ietf-mmusic-sdp-uks.md
in the following.)

It's quite obvious to you and me, but it's probably poor form to produce
statements of the form "an implementation MUST do X" with implied "except
in cases that are forbidden by the base protocol".  So, a standalone
paragraph of "In TLS 1.3, the <foo> extension MUST be sent in
the EncryptedExtensions message." still sounds like anything sending EE
that implements this document must include the <foo> extension, even though
you and I know that the intent is really "In TLS 1.3, the server's
responses to ClientHello extensions can either be in the ServerHello or
EncryptedExtensions, and this extension doesn't go in the ServerHello."
(To some extent, even this is redundant given the column in the registry,
though I don't mind the extra emphasis.)

On the other hand, "An endpoint that does not produce an identity binding
MUST generate an empty external_id_hash extension in its ClientHello or -
if a client provides the extension - in ServerHello or EncryptedExtensions"
is better about qualifying when it's allowed by the underlying protocol to
"do X", though a truly adversarial reader might claim that it is saying
that "all endpoints producing an identity binding MUST generate a
ClientHello".

> signaling layer is authenticated and bound to the TLS connections."

I'm fine leaving this out of scope, though I'd wordsmith your proposed text
to clarify that "such a signaling system" is one that can defend against
the attack being described.

> I'm not aware of any such system, as much as it might be desirable.  SIP is still wrestling with the concept of caller identity.  Proper channel bindings seems like a bit much to expect.

Oh, sure -- SIP is going to take some time to evolve, and I'm not trying to
force everything overnight.

-Ben

---

From: "Martin Thomson" <[email-masked]>
Date: Wed, 07 Aug 2019 14:27:06 +1000

> that "all endpoints producing an identity binding MUST generate a

Thanks for narrowing it down.  Again, I completely overlooked that bit.  My bad.  Changed in both cases to:

In TLS 1.3, an `xxx` extension sent by a server MUST be sent in the EncryptedExtensions message.