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

[Sean Turner <[e-mail]>]

> On Nov 1, 2018, at 20:36, Adam Roach <[e-mail]> wrote:
> 
> 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.)

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
>      (Obsoleted by RFC 7022)

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

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

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

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

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

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

See nits PR.

> ---------------------------------------------------------------------------
> 
> §2:
> 
> Please update to match the RFC 8174 boilerplate.

See nits PR.

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

See nits PR.

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

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

See nits PR.

> ---------------------------------------------------------------------------
> 
> > 3.2.  Same Origin Policy
> 
> Nit: "Same-Origin”

See nits PR.

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

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

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

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

> ---------------------------------------------------------------------------
> 
> §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…")

I went with “would”.

> >  cryptographically established identity.  While not suitable for all
> 
> Nit: "cryptographically-established”

See nits PR.

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

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


> ---------------------------------------------------------------------------
> 
> §4.2.3:
> 
> >  o  Use or RTCP as an implicit reachability check.
> 
> Nit: "Use of…"

See nits PR.

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

See nits PR.

> ---------------------------------------------------------------------------
> 
> §4.3:
> 
> >  However, we must examine this
> >  technology to the WebRTC context, where the threat model is somewhat
> >  different.
> 
> Nit: "...in the WebRTC context…"

See nits PR.

> ---------------------------------------------------------------------------
> 
> §4.3:
> 
> >  MITM attack or by diverting them entirely.  (Note that this attack
> 
> Please expand "MITM" on first use.

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

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

See nits PR.

> ---------------------------------------------------------------------------
> 
> §4.3.2.2:
> 
> >  simply ignore such indicators even in the rather more dire case of
> >  mixed content warnings.
> 
> Nit: "mixed-content”

See nits PR.

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

I hate XML.

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

See nits PR.

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

I added the forward reference.

See nits PR.

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

See nits PR.

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

See nits PR.