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