Subject: [rtcweb] AD Review: draft-ietf-rtcweb-security

From: Adam Roach <[email-masked]>
Date: Thu, 1 Nov 2018 19:36:57 -0500

This is my AD review for draft-ietf-rtcweb-security. I think this 
document is
ready to go into IETF last call, modulo the ICE citation issue. I plan to
issue last call for this document at the same time as
draft-ietf-rtcweb-security-arch and draft-ietf-rtcweb-ip-handling, so it 
will
wait for those to become ready.

I'm marking the document as "revised ID needed" pending the ICE reference
update.

I also have a number of non-blocking comments that should be treated the
same as IETF last-call comments.

/a

---------------------------------------------------------------------------

ID Nits reports:

   == The document seems to contain a disclaimer for pre-RFC5378 work, 
but was
      first submitted on or after 10 November 2008.  The disclaimer is 
usually
      necessary only for documents that revise or obsolete older RFCs, 
and that
      take significant amounts of text from those RFCs.  If you can 
contact all
      authors of the source material and they are willing to grant the BCP78
      rights to the IETF Trust, you can and should remove the disclaimer.
      Otherwise, the disclaimer is needed and you can ignore this comment.
      (See the Legal Provisions document at
      https://trustee.ietf.org/license-info for more information.)

   -- Obsolete informational reference (is this intentional?): RFC 6222
      (Obsoleted by RFC 7022)

---------------------------------------------------------------------------

[DISCUSS]

Per the discussion around Cluster 238 dependencies, please reference RFC 
8445
instead of RFC 5245.

---------------------------------------------------------------------------

§1:

 >  The Real-Time Communications on the Web (RTCWEB) working group is
 >  tasked with standardizing protocols for real-time communications
 >  between Web browsers, generally called "WebRTC"

I plan to close RTCWEB as soon as Cluster 238 is published, at which point
this text will be out of date. Consider rephrasing in a way that will 
survive
the passage of time better.

---------------------------------------------------------------------------

§1:

 >  In the system shown in Figure 1, Alice and Bob both have WebRTC
 >  enabled browsers...

Please add Alice and Bob to Figure 1.

Nit: "WebRTC-emabled"

---------------------------------------------------------------------------

§2:

Please update to match the RFC 8174 boilerplate.

Regardless of whether this update is made, there seem to be some ambiguous
uses of RFC 2119 language (e.g. §4.3: "This service must be provided for 
both
data and voice/video") that need a bit of auditing.

---------------------------------------------------------------------------

§3.1:

 >  While the browser has access to local resources such as keying
 >  material, files, the camera and the microphone,

Consider adding an Oxford comma.

---------------------------------------------------------------------------

§3.1:

 >  [Note: in many cases browsers are explicitly designed to avoid
 >  dialogs with the semantics of "click here to screw yourself", as

I hate to be a wet blanket, but this phrasing seems out of character for an
RFC. Consider something less colloquial (and perhaps with less rough
connotations) than "screw" here.

---------------------------------------------------------------------------

 > 3.2.  Same Origin Policy

Nit: "Same-Origin"

---------------------------------------------------------------------------

§3.2:

 >  Many other resources are accessible but isolated.  For instance,
 >  while scripts are allowed to make HTTP requests via the
 >  XMLHttpRequest() API

Consider informatively citing https://xhr.spec.whatwg.org/ (or something
better if you can find it).

---------------------------------------------------------------------------

§4.1.2.2:

 >  to avoid the users just clicking through.  Note also that the user
 >  interface chrome must clearly display elements showing that the call

Consider defining the term "chrome" for those readers who may not be 
familiar
with it.

---------------------------------------------------------------------------

§4.1.3:

 >  target.  Callee-oriented consent provided by the calling site not
 >  work well because a malicious site can claim that the user is calling

Nit: "...does not work well..." (or "...would not work well...")

 >  cryptographically established identity.  While not suitable for all

Nit: "cryptographically-established"

---------------------------------------------------------------------------

§4.1.4:

 >  Even if calls are only possible from HTTPS [RFC2818] sites, if the
 >  site embeds active content (e.g., JavaScript) that is fetched over
 >  HTTP or from an untrusted site, because that JavaScript is executed
 >  in the security context of the page [finer-grained].

I can't parse this sentence. Consider reworking.

---------------------------------------------------------------------------

§4.2.3:

 >  o  Use or RTCP as an implicit reachability check.

Nit: "Use of..."

---------------------------------------------------------------------------

§4.2.4:

 >  In addition, either side may wish to hide their location entirely by
 >  forcing all traffic through a TURN server.

Suggested improvement: "...hide their location from the other..." (to avoid
implying that this hides their location from either the web server or 
the STUN
server)

---------------------------------------------------------------------------

§4.3:

 >  However, we must examine this
 >  technology to the WebRTC context, where the threat model is somewhat
 >  different.

Nit: "...in the WebRTC context..."

---------------------------------------------------------------------------

§4.3:

 >  MITM attack or by diverting them entirely.  (Note that this attack

Please expand "MITM" on first use.

---------------------------------------------------------------------------

§4.3.2.1:

 >  All the calling service needs to do to avoid
 >  triggering a key continuity warning is to tell the browser that "this
 >  is a call to user Y" where Y is close to X.

I read the meaning of the term "close" here to mean "confusable with X,"
although it took some work to arrive at that conclusion. If "confusable" is
the intention, I would suggest phrasing it in that way.

---------------------------------------------------------------------------

§4.3.2.2:

 >  simply ignore such indicators even in the rather more dire case of
 >  mixed content warnings.

Nit: "mixed-content"

---------------------------------------------------------------------------

§4.3.2.3:

 >  However, a new generation of Web-based identity
 >  providers (BrowserID, Federated Google Login, Facebook Connect,
 >  OAuth, OpenID, WebFinger), has recently been developed

Consider adding informative citations for at least BrowserID, OAuth, OpenID,
and Webfinger [RFC7033], if not the other systems.

---------------------------------------------------------------------------

§4.3.2.4:

 >  I.e., I must be able to verify that
 >  the person I am calling has engaged a secure media mode.

Is this possible in the general case? For non-browser endpoints (or for
modified browswers), this verification seems to be impossible (unless I've
missed some mechanism in the system that can guarantee this property).

Clearly I'm not asking for a change in design, but it seems that this
statement needs to be caveated to indicate that it requires trust in the
remote endpoint to enforce the indicated policy, and that this trust 
cannot be
verified; at least, not as WebRTC is designed today.

A simple forward citation to §4.3.3 might serve the purpose.

---------------------------------------------------------------------------

§4.4.1:

 >  While persistent endpoint identifiers can be a useful security
 >  feature (see Section 4.3.2.1 they can also represent a privacy threat

Nit: missing a closing paren.

---------------------------------------------------------------------------

§4.2.2:

Consider citing https://www.w3.org/TR/fingerprinting-guidance/ for further
information.

---

From: Sean Turner <[email-masked]>
Date: Sat, 17 Nov 2018 00:59:50 -0500

> On Nov 1, 2018, at 20:36, Adam Roach <[email-masked]> wrote:
> 
> This is my AD review for draft-ietf-rtcweb-security. I think this document is
> draft-ietf-rtcweb-security-arch and draft-ietf-rtcweb-ip-handling, so it will
>   == The document seems to contain a disclaimer for pre-RFC5378 work, but was
>      first submitted on or after 10 November 2008.  The disclaimer is usually
>      necessary only for documents that revise or obsolete older RFCs, and that
>      take significant amounts of text from those RFCs.  If you can contact all

I suspect the answer here is the same as for -security-arch, i.e., we can’t really get away with removing the disclaimer.

>   -- Obsolete informational reference (is this intentional?): RFC 6222

It is there intentionally.  The paragraph talks about both 6222 and 7022.

> Per the discussion around Cluster 238 dependencies, please reference RFC 8445
> instead of RFC 5245.

PR:
https://github.com/rtcweb-wg/security/pull/8

> this text will be out of date. Consider rephrasing in a way that will survive

PR (the nits PR):
https://github.com/rtcweb-wg/security/pull/10

> Nit: "WebRTC-emabled”

See nits PR.

See nits PR.

> uses of RFC 2119 language (e.g. §4.3: "This service must be provided for both

See nits PR.

I changed it to:
"click here to bypass security checks”
though I think the previous wording was about right ;)

See nits PR.

> Nit: "Same-Origin”

See nits PR.

I just pointed to the WhatWG doc.
See nits PR.

> Consider defining the term "chrome" for those readers who may not be familiar
> with it.

I stole something from a W3C doc.
See nits PR.

> Nit: "...does not work well..." (or "...would not work well…")

I went with “would”.

> Nit: "cryptographically-established”

See nits PR.

I was not sure what to do with this one so I submitted an issue:
https://github.com/rtcweb-wg/security/issues/9

> Nit: "Use of…"

See nits PR.

> implying that this hides their location from either the web server or the STUN
> server)

See nits PR.

> Nit: "...in the WebRTC context…"

See nits PR.

MITM is in the RFC abbreviations list:
https://www.rfc-editor.org/materials/abbrev.expansion.txt
not incorporated ;)

See nits PR.

> Nit: "mixed-content”

See nits PR.

I hate XML.

I added informative references for OAuth, OpenID, and Webfinger.

See nits PR.

> remote endpoint to enforce the indicated policy, and that this trust cannot be

I added the forward reference.

See nits PR.

See nits PR.

See nits PR.