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

Christer Holmberg <christer.holmberg@ericsson.com> Wed, 31 January 2018 14:42 UTC

Return-Path: <christer.holmberg@ericsson.com>
X-Original-To: secdir@ietfa.amsl.com
Delivered-To: secdir@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 6DBEA131B22 for <secdir@ietfa.amsl.com>; Wed, 31 Jan 2018 06:42:39 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.32
X-Spam-Level:
X-Spam-Status: No, score=-4.32 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=ericsson.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id XT5Cv05rcgX4 for <secdir@ietfa.amsl.com>; Wed, 31 Jan 2018 06:42:36 -0800 (PST)
Received: from sesbmg22.ericsson.net (sesbmg22.ericsson.net [193.180.251.48]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 1844412EACF for <secdir@ietf.org>; Wed, 31 Jan 2018 06:39:15 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; d=ericsson.com; s=mailgw201801; c=relaxed/simple; q=dns/txt; i=@ericsson.com; t=1517409554; h=From:Sender:Reply-To:Subject:Date:Message-ID:To:CC:MIME-Version:Content-Type: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=MypaGRKItGlda/LgU6E5riFrwV9GkkVawrq5ijuohMQ=; b=G3pDIVFMw5txAMeT4+U+TZZeuVpxyBQVMc2f0Oh02ZpWad5rhGmbqttP6tepAx/r g4+wRxbt/asR1gAQ03pZk8y+qpXLqS5ZKIKk6abwUyKTvuoBGufJgXZ5Mj4Tcgnv UtTXXGvGcJGAUpEIlTfG0kW1bLp8EbyJo+suLlRiJbk=;
X-AuditID: c1b4fb30-11d5e9c000006bc7-76-5a71d5124cb4
Received: from ESESSHC019.ericsson.se (Unknown_Domain [153.88.183.75]) by sesbmg22.ericsson.net (Symantec Mail Security) with SMTP id D1.14.27591.215D17A5; Wed, 31 Jan 2018 15:39:14 +0100 (CET)
Received: from ESESSMB109.ericsson.se ([169.254.9.195]) by ESESSHC019.ericsson.se ([153.88.183.75]) with mapi id 14.03.0352.000; Wed, 31 Jan 2018 15:39:13 +0100
From: Christer Holmberg <christer.holmberg@ericsson.com>
To: Stephen Farrell <stephen.farrell@cs.tcd.ie>, "secdir@ietf.org" <secdir@ietf.org>
CC: "draft-ietf-ice-rfc5245bis.all@ietf.org" <draft-ietf-ice-rfc5245bis.all@ietf.org>, "ietf@ietf.org" <ietf@ietf.org>, "ice@ietf.org" <ice@ietf.org>
Thread-Topic: Secdir last call review of draft-ietf-ice-rfc5245bis-16
Thread-Index: AQHTlsWSdlXqvNmVDEWzQ6CHG1vNgKOG2QlggAVgroCAAFA6gP//5hCAgAG49wA=
Date: Wed, 31 Jan 2018 14:39:12 +0000
Message-ID: <D697A3EC.2A2BC%christer.holmberg@ericsson.com>
References: <151698534014.492.17944720668568287169@ietfa.amsl.com> <7594FB04B1934943A5C02806D1A2204B6C143E76@ESESSMB109.ericsson.se> <f3a610a1-14da-2cd5-c794-f54cc4b8be97@cs.tcd.ie> <D696400E.2A0CD%christer.holmberg@ericsson.com> <dc80f650-c20a-a531-5a90-50d10c633df6@cs.tcd.ie>
In-Reply-To: <dc80f650-c20a-a531-5a90-50d10c633df6@cs.tcd.ie>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
user-agent: Microsoft-MacOutlook/14.7.7.170905
x-originating-ip: [153.88.183.17]
Content-Type: text/plain; charset="euc-kr"
Content-ID: <CD3F1DE2D2A77E468BE8BBB1A39BDC2C@ericsson.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrHIsWRmVeSWpSXmKPExsUyM2K7t67Q1cIogwcXxC2O//jDbvHtQq3F s43zWSw+LHzIYjF97zV2B1aPtd1X2TyWLPnJFMAUxWWTkpqTWZZapG+XwJXxovMqc8GD1YwV C/sjGxjXLGfsYuTkkBAwkWh++JsZxBYSOMwosf43dxcjF5C9hFFi7v4FLF2MHBxsAhYS3f+0 QWpEBMIk7r8/xApSwywwk1Hi9c+XYIOEBVwkXj3+wwRR5CqxctMPKNtP4sTqTrAFLAKqEveb /7OD2LwC1hJts09DLZ7HJHH5ujGIzSlgK7Hv3wewOKOAmMT3U2vA5jALiEvcejKfCeJoAYkl e84zQ9iiEi8f/2MFsUUF9CQ2nLjNDnKzhICixPJ+OYhWLYkvP/axQdjWEoc2NLJD2IoSU7of Qp0jKHFy5hOWCYzis5Bsm4WkfRaS9llI2mchaV/AyLqKUbQ4tTgpN93ISC+1KDO5uDg/Ty8v tWQTIzAWD275bbCD8eVzx0OMAhyMSjy8Mw4VRgmxJpYVV+YeYpTgYFYS4d1zESjEm5JYWZVa lB9fVJqTWnyIUZqDRUmc96Qnb5SQQHpiSWp2ampBahFMlomDU6qBcfHLb1d+u/xp27nR/pPG VtFZPn7N7o5H1dd23tHKuZx/ZllJ0pVtX03fP2l81easFOR70tnITtXd71rrcXvu3Vrs86pW 6Oy8veP7RLnuk1KLXsYZn1D0No87w7AtWo3j4CKJF9r8ezg/zf2Q/OdyzQev2pyU4g2iO76f so34VvxojydD27bJz5VYijMSDbWYi4oTAZv9qrXBAgAA
Archived-At: <https://mailarchive.ietf.org/arch/msg/secdir/P_TlBZ1utnYTgQXJyuzBbHlIed4>
Subject: Re: [secdir] Secdir last call review of draft-ietf-ice-rfc5245bis-16
X-BeenThere: secdir@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
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: Wed, 31 Jan 2018 14:42:39 -0000

Hi,

I’ve updated the pull request with changes based on Stephen’s sec-dir
review.

I am still looking whether there is something that could be added
regarding WebRTC IP leakage.

https://github.com/ice-wg/rfc5245bis/pull/57


I have not yet committed any changes based on Magnus’ trv-dir review.

Regards,

Christer




On 30/01/18 16:32, "Stephen Farrell" <stephen.farrell@cs.tcd.ie>; wrote:

>
>Hiya,
>
>Those look close-enough or fine to me. For the
>recent WebRTC thread, if you find something, that'll
>be a good thing. If not, and nobody suggests some
>text, then I guess at least we did try:-) And FWIW I'm
>fine with that, as I'm not up to speed on non-WebRTC
>uses of ICE, so don't know if the same leakage issues
>can arise in reality, outside of WebRTC.
>
>Thanks,
>S.
>
>On 30/01/18 13:53, Christer Holmberg wrote:
>> Hi,
>> 
>>>>> 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
>>>>
>>>> I would not want to REQUIRE implementing a local interface for
>>>> controlling the candidates, as ICE is also used by different types of
>>>> network entities (gateways etc). But, I am ok to RECOMMEND.
>>>
>>> Fair enough.
>>>
>>>>
>>>> What about:
>>>>
>>>> OLD:
>>>>
>>>> "Individual implementations may also have implementation-specific
>>>> rules for controlling which addresses are revealed."
>>>>
>>>> NEW:
>>>>
>>>> "Individual implementations may also have implementation-specific
>>>> rules for controlling which addresses are revealed. It is RECOMMENDED
>>>> that applications interacting with human users provide a user
>>>> interfaces that allows the users to control which interfaces are used
>>>> to generate candidates, or when to use candidates generated for the
>>>> interfaces.
>>>>
>>>> [REF-to- draft-ietf-rtcweb-ip-handling] provide additional
>>>> information and Requirements regarding the handing of IP addresses
>>>> for WebRTC applications."
>>>
>>> That's a fine improvement from my POV, esp if folks will
>>> really provide such interfaces. I'm not sure "interacting
>>> with humans" is quite right though, maybe it'd be better
>>> to add a reference to the problem (via [1] or [rtcweb-ip])
>>> and to then say "Implementations where such issues can
>>> arise are RECOMMENDED to..." I'd also suggest maybe
>>> s/user interface/interface/ there, as e.g. a VPN client
>>> might want to use an API provided by an ICE implementation
>>> so the UI might be part of the VPN client and not the ICE
>>> implementation. So, overall I'd suggest something like
>>> this:
>>>
>>> "Individual implementations may also have implementation-specific
>>> rules for controlling which addresses are revealed. For example,
>>> [REF-to- draft-ietf-rtcweb-ip-handling] provides additional
>>> information about the privacy aspects of revealing IP addresses
>>> via ICE for WebRTC applications. ICE implementations where such
>>> issues can arise are RECOMMENDED to provide a programmatic or
>>> user interface that provides control over which network interfaces
>>> are used to generate candidates."
>> 
>> Looks good :)
>> 
>> ---
>> 
>>>>
>>>>> - 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 am struggling to figure out exactly what to do here... I really
>>>> hope we don't have to perfom a study on [3], and see what is general,
>>>> and what is WebRTC-specific. Are we even allowed to reference [3], as
>>>> it's not freely available?
>>>
>>> Sure, we can reference academic papers behind paywalls, and
>>> there's often another version available that's not. (Not sure
>>> in this case.)
>>>
>>>>
>>>> Some help and guidance would be appreciated :)
>>>
>>> I guess about as much as can be done with this -bis effort is
>>> to note where any leakages have been found. There's been some
>>> recent traffic on the W3C WebRTC list about that I think, (in
>>> the thread about CSP iirc) so maybe a trawl of that thread
>>> will suggest some text?
>> 
>> I did follow that discussion. I will take a second look.
>> 
>> ---
>> 
>>>>> - 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.
>>>>
>>>> Something like?
>>>>
>>>> "Based on the types of candidates provided by the peer, and the
>>>> results of the connectivity tests performed Against those candidates,
>>>> an agent might be able to determine characteristics of the peer
>>>> network."
>>>
>>> Maybe:
>>>
>>> "Based on the types of candidates provided by the peer, and the
>>> results of the connectivity tests performed against those candidates,
>>> the peer might be able to determine characteristics of the local
>>> network, e.g. if different timings are apparent to the peer. In
>>> the limit the peer might be able to probe the local network."
>>>
>>> That said - I'm not claiming this is possible for-sure, I just
>>> bet it would be:-)
>> 
>> The text looks good.
>> 
>> ---
>> 
>>>>> (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.
>>>>
>>>> Section 7.2.2 says:
>>>>
>>>> "A connectivity check Binding request MUST utilize the STUN
>>>> short-term credential mechanism."
>>>>
>>>> RFC 5389 (STUN) says:
>>>>
>>>> "Short-Term Credential:  A temporary username and associated
>>>> password that represent a shared secret between client and server.
>>>> Short- term credentials are obtained through some kind of protocol
>>>> mechanism between the client and server, preceding the STUN exchange.
>>>> A short-term credential has an explicit temporal scope, which may be
>>>> based on a specific amount of time (such as 5 minutes) or on an event
>>>> (such as termination of a SIP dialog). The specific scope of a
>>>> short-term credential is defined by the application usage."
>>>>
>>>> If anything extra is needed, I think that should be done as an update
>>>> to STUN.
>>>
>>> Not sure I agree - maybe I wasn't clear in my comment though,
>>> so let me try rephrase it.
>>>
>>> STUN doesn't define the duration for which short-term creds
>>> are OK to be used. ICE does define a session concept therefore
>>> ICE ought to say if short term STUN creds MUST/MUST-NOT/MAY
>>> (or whatever) be re-used in different ICE sessions. Even
>>> within an ICE session I guess in theory one could require
>>> different short-term STUN credentials every single time, or
>>> not. So I do think there's something to be said here.
>> 
>> Section 9 says that an agent must create a new username fragment and
>> password for every new ICE session:
>> 
>>      "To restart ICE, an agent MUST change both the password and the
>>       username fragment for the data stream(s) being restarted.²
>> 
>> 
>> Since I also uses the username to identity the ICE session, the values
>> cannot be changed within a session.
>> 
>> There is no text on when a value can be re-used - apart from the fact
>>that
>> they cannot be re-used when an ICE restart takes place.
>> 
>> My assumption has been that the randomness criteria are assumed to
>>ensure
>> that two agents don¹t choose the same values for the same session, and
>> that values are unlikely to be re-used within a short period of time.
>> 
>> We could say that agents shall verify that the same value is not re-used
>> as long as packets from a previous ICE session may still be present in
>>the
>> network. But, that is not really security related, but more to make sure
>> ICE works.
>> 
>> ---
>> 
>>>>
>>>>> (Also - the phrasing isn't that great - how do I "contain"
>>>>> randomness? But that's just a nit.)
>>>>
>>>> Do you have a suggestion for a better word?
>>>
>>> I think what you want is to say "unguessable, with at
>>> least 128 bits of random number generator output used
>>> to generate each" or something like that? It's a hard
>>> one to get right, but even what you have isn't that
>>> bad (I did say this was a nit:-)
>> 
>> What about?
>> 
>>      "Username Fragment and Password:  Values used to perform
>>connectivity
>>       checks. The values MUST be unguessable, with at least 128 bits of
>> random 
>>       number generator output used to generate the password, and at
>>least
>> 24 
>>       bits output to generate the username fragment."
>> 
>> 
>> Thanks! :)
>> 
>> Regards,
>> 
>> Christer
>> 
>> 
>>>>
>>>> ========================
>>>>
>>>> Nits:
>>>>
>>>>> I took a 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-rfc524
>>>>>5b
>>>>> is-16.txt
>>>>>
>>>>>
>>>>>
>>> - 2.1: Why "only UDP specified here"? Does that include QUIC which is
>>>>> UDP-based, but not UDP? I assume so.
>>>>
>>>> There is a separate specification, RFC 6544, for TCP based
>>>> candidates.
>>>>
>>>> Regarding QUIC, I don't know. QUIC wasn't discussed during the work,
>>>> but how/if the procedures also apply to QUIC without any
>>>> modifications etc I don't know.
>>>>
>>>>> - 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.)
>>>>
>>>> In general, one of the tasks of the bis work was to *simplify*
>>>> section 2, because the previous version was too detailed for
>>>> non-implementers who just want to figure out what ICE is all about
>>>> (in order to  actually implement ICE, reading section 2 is obviously
>>>> not enough). For that reason, my opinion was that section 2 doesn't
>>>> really need to talk about role conflicts and tie-breaker values, as
>>>> it's not essential for a high-level description of ICE.
>>>>
>>>> ---
>>>>
>>>>> - 5.2: "The procedures in this section is common across the
>>>>> initiating and responding agents." s/is/are/ I guess?
>>>>
>>>> Yes. I will fix as suggested.
>>>>
>>>> ---
>>>>
>>>>> - 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).
>>>>
>>>> I can use "3PCC (3rd Party Call Control)" in section 6.1.1.
>>>>
>>>> As the spec is protocol independent, I don't think there are any
>>>> references that can be added.
>>>>
>>>> ---
>>>>
>>>>> - 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:-)
>>>>
>>>> It is covered in the 2nd paragraph of section 7.2.5.1, which says
>>>> that the agent MAY change the value when it switches role.
>>>>
>>>>> In any case saying "referred to as the tie-breaker value" twice
>>>>> seems odd.
>>>>
>>>> I'll remove it from the ICE-CONTROLLING definition.
>>>>
>>>> ---
>>>>
>>>>> - 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?)
>>>>
>>>> Good catch. I'll fix it.
>>>>
>>>> It would be really cool if search functions could discard line-breaks
>>>> when searching for strings.
>>>>
>>>> ---
>>>>
>>>>> - 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.
>>>>
>>>> Perhaps something like:
>>>>
>>>> "This section discusses issues relevant to operators operating
>>>> networks where ICE will be used by endpoints."
>>>>
>>>> ---
>>>>
>>>>> - 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.
>>>>
>>>> I'll leave it :)
>>>>
>>>> ---
>>>>
>>>>> - 19: s/DNS-SEC/DNSSEC/ would be more common
>>>>
>>>> I will fix as suggested.
>>>>
>>>> ---
>>>>
>>>>> - 19.4.1: 1st sentence if missing a "T" - s/he/The/
>>>>
>>>> I will fix as suggested.
>>>>
>>>> ---
>>>>
>>>> Regards,
>>>>
>>>> Christer
>>>>
>>>
>>> -- 
>>> PGP key change time for me.
>>> New-ID 7B172BEA; old-ID 805F8DA2 expires Jan 24 2018.
>>> NewWithOld sigs in keyservers.
>>> Sorry if that mucks something up;-)
>> 
>> 
>
>-- 
>PGP key change time for me.
>New-ID 7B172BEA; old-ID 805F8DA2 expires Jan 24 2018.
>NewWithOld sigs in keyservers.
>Sorry if that mucks something up;-)