[secdir] Secdir last call review of draft-ietf-ice-rfc5245bis-16

Stephen Farrell <stephen.farrell@cs.tcd.ie> Fri, 26 January 2018 16:49 UTC

Return-Path: <stephen.farrell@cs.tcd.ie>
X-Original-To: secdir@ietf.org
Delivered-To: secdir@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 45A4A126BFD; Fri, 26 Jan 2018 08:49:00 -0800 (PST)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Stephen Farrell <stephen.farrell@cs.tcd.ie>
To: secdir@ietf.org
Cc: draft-ietf-ice-rfc5245bis.all@ietf.org, ietf@ietf.org, ice@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 6.70.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <151698534014.492.17944720668568287169@ietfa.amsl.com>
Date: Fri, 26 Jan 2018 08:49:00 -0800
Archived-At: <https://mailarchive.ietf.org/arch/msg/secdir/CEZKKQQanOdjwCJn4P3-pi1GOV4>
Subject: [secdir] Secdir last call review of draft-ietf-ice-rfc5245bis-16
X-BeenThere: secdir@ietf.org
X-Mailman-Version: 2.1.22
List-Id: Security Area Directorate <secdir.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/secdir>, <mailto:secdir-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/secdir/>
List-Post: <mailto:secdir@ietf.org>
List-Help: <mailto:secdir-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/secdir>, <mailto:secdir-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 26 Jan 2018 16:49:00 -0000

Reviewer: Stephen Farrell
Review result: Has Issues


(1) I think a bit more work on the security and privacy issues around
ICE would improve the document, and hopefully, ICE implementations.

- There has been (IMO somewhat justified) concern [1] about exposing
all possible addresses, for example where one is from a VPN intended
to preserve privacy, say if a user is aiming to circumvent local
censorship or surveillance.  5.1.1.1 has a SHOULD that, as I read it,
says to always include such addresses.  

   [1] https://www.w3.org/wiki/Privacy/IPAddresses

(Note: I'm not claiming [1] is authoritative, it's just a page I found
that describes the issue reasonably well.)

There is a bit of text about this (1st para in section 19), but IMO
it's a bit weak. 

- Did the WG consider REQUIRING or RECOMMENDING agent implementations
  provide some local interface that allows the host or an application
to say "Don't use <this> interface to generate any candidates until I
tell you otherwise"?  That might be better than having VPN operators
recommending to turn off WebRTC entirely for example. (Note: this is
my main comment, the rest are mostly suggested things to think about,
if you've not already.)

- Even if the text isn't changed, a reference to [2] would I think be
  useful. (Assuming [2] is still being worked on in rtcweb, though
mind you, I don't like how [2] refers to "consent" as I doubt a random
person can really provide meaningful consent for things like this.)

   [2] https://tools.ietf.org/html/draft-ietf-rtcweb-ip-handling-04

- Separately [1] also describes another potential privacy issue with
  STUN urls. I think it'd be worthwhile noting such issues that have
been found with deployments of ICE here, as any protocol using ICE and
accepting non locally configured STUN/TURN URLs may have issues like
that. For example, a recent paper [3] also describes some attacks (in
section 4.2 mostly) that could (I guess) be mounted against any ICE
agent and not only WebRTC clients. That'd be worth a reference and
maybe thinking about which attacks are generic ICE issues and don't
only affect WebRTC.

   [3] https://doi.org/10.1145/3019612.3019844

- I also wondered if I can fingerprint a host based on how it does ICE?  
  I  suspect that might work, but haven't tried to figure out details.

- Could I probe an internal network based on feeding it candidates to
  check? E.g. checking timing of reactions.  If I could that seems
noteworthy.

(2) 5.3: "...MUST contain ... <foo> bits of randomness" Don't you need
to say when these values MUST be different and when they're ok to be
re-used? I forget if STUN/TURN do that. (Also - the phrasing isn't
that great - how do I "contain" randomness? But that's just a nit.)

Nits:

I took at look at the diff [4] between this and 5245, but that wasn't
really that useful, so this review is based on a read of the draft
without comparing it to 5245. Apologies if that causes me to comment
on text that's unchanged - in such cases, I think it's fine to not
heed those particular comments. 

   [4] https://tools.ietf.org/rfcdiff?url1=rfc5245&url2=draft-ietf-ice-rfc5245bis-16.txt

- 2.1: Why "only UDP specified here"? Does that include QUIC which is
  UDP-based, but not UDP? I assume so.

- 2.3: how the controlling/controlled stuff works isn't clear from
  this, nor even (as I was reading it) in section 7.3.1.1 - I wasn't
clear how the tie-breaker value is initialised until I got to 16.1.
In any case I think a bit more explanatory text in 2.3 might be good.
(But if implementers haven't had a problem with this, maybe it's just
me.)

- 5.2: "The procedures in this section is common across the initiating
  and responding agents." s/is/are/ I guess?

- 6.1.1: What is "3PCC"? That note could maybe also do with a
  reference, as I at least don't get what you're trying to tell me. Ah
- it's 3rd party call control (mentioned in 7.3.1.1).

- 16.1: For a given ICE session, are the random numbers in the
  ICE-CONTROLLED and ICE-CONTROLLING attributes sent by the same agent
supposed to have the same value? I wasn't clear. If they are the same,
I understand how tie-breaking happens. If they can differ, I'm not
sure. (But I didn't go back and re-read 7.3.1.1, so it may be ok
either way:-) In any case saying "referred to as the tie-breaker
value" twice seems odd. 

- 16.1: (mega-mega-nit:-) the fact that tie-breaker is split over a
  line-breaker here is why I didn't find this when I was reading
2.3/7.3.1.1 - maybe tweak the words so a grep will find it?)

- 17: "looking to deploy ICE" is a bit confusing - do you mean
  "looking to be nice to ICE"? My assumption is that endpoints deploy
ICE agents but network operators don't, though they might deploy
STUN/TURN servers I guess. But maybe I'm thinking too much about
WebRTC there.

- 18: I wondered if this is still needed, given that 5245 already said
  it (I didn't check if the text has been updated). Saying this once
would seem sufficient to me anyway. I guess leaving it in is the
easier path.

- 19: s/DNS-SEC/DNSSEC/ would be more common

- 19.4.1: 1st sentence if missing a "T" - s/he/The/