Subject: [Ice] Benjamin Kaduk's practice ballot on draft-ietf-ice-rfc5245bis-17: (with COMMENT)

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 20 Feb 2018 16:00:03 -0600

[resending with fixed Subject and recipients list]

[incoming AD ramping up on pre-telechat document review]

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

Given that this is a -bis document, there are some areas that may
have aged poorly and could benefit from reconsideration:

In Section 5.2, we say that for the Lite implementation, preferring
IPv4 is RECOMMENDED for dual-stack hosts.  Is that still the advice
we want to give?

Section 17 is very focused on voice traffic; is that still the main
use of ICE?

Appendix B.8 has:

   Additionally, using a Binding Indication allows integrity to be
   disabled, allowing for better performance.  This is useful for large-
   scale endpoints, such as PSTN gateways and SBCs.

which talks of disabling STUN's MESSAGE-INTEGRITY as a potential
benefit.  Do we really want to advocate for increased levels of
unauthenticated plaintext on the internet?

Appendix C has a table of bandwidth consumption in various
scenarios, most of which are measured in kilobits per second.  Is
this still relevant to modern deployments?

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

Some other general comments/questions.

I was also a little confused in Section 6.1.2.2, where we say

   for each interface.  To keep the amount of resulting candidate pairs
   reasonable and to avoid candidate pairs that are highly unlikely to
   work, IPv6 link-local addresses [RFC4291] MUST NOT be paired with
   other than link-local addresses.

but in section 5.1.1.1 we say that IPv6 link-local addresses MUST
NOT be gathered.  Is this just to handle the chance that we might
get a reflexive candiate for a link-local address?

Section 13 has:

   Consequently, any future ICE enhancements MUST
   preserve triggered checks.

which does not seem like an enforceable requirement, given that
future updates to this document could just remove it.  Perhaps it
should be worded differently to give the motivation for the need to
keep triggered checks.

In section 14.3:

   For connectivity checks, agents SHOULD calculate the RTO value using
   the following formula:

     RTO = MAX (500ms, Ta*N * (Num-Waiting + Num-In-Progress))

What is 'N'?

In Section 15:

   will succeed.  Consequently, agent R constructs a new candidate pair
   using the mapped address from the response as the local candidate (R-
   PUB-1) and the destination of the request (NAT-PUB-1) as the remote
   candidate.  This pair is added to the valid list for that data
   stream.

which leaves me confused, perhaps just about which response is "the
response" that we're taking the mapped address from.  The previous
text is about a STUN Binding request from L to R that causes R to
generate a triggered check, so is this the response that R generates
to the Binding request from L?  IIUC the mapped address therein
would be where R sees traffic from L as originating from, and would
not be appropriate to use as a local address for R, etc..  So
presumably I am picking the wrong response to use as the source of
data.

In Section 18.3:

   [...] The process of determining
   connectivity is very robust.

This seems to contrast with section 14.1, where

   The loss of the first single packet for any
   connectivity check is likely to cause that pair to take a long time
   to be validated, and instead, a lower-priority check (but one for
   which there was no packet loss) is much more likely to complete
   first.

Something of a side note, but is HMAC-SHA1 still the state of the
art for STUN MESSAGE-INTEGRITY?  I know that there are no known
breaks against HMAC-SHA1, but I would be a little surprised if no
one was interested in producing an update.

In Section 19.3:

   However, TURN servers are susceptible to DNS attacks for purposes of
   turning it into a zombie or rogue server.  These attacks can be
   mitigated by DNS-SEC and through good box and software security on
   TURN servers.

I don't understand this.  Is the ICE agent or the TURN server
"turned into" a zombie or rogue server by DNS attacks?  If the TURN
server, how would a bogus DNS response turn it into a rogue server?

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

On an editorial note, there is a general feel to the document that
it started off life as only covering ICE for RTP and the application
profiling was later split off into separtae documents.  I don't know
that it's worth the effort to try to change that, though.  I might
give some thought to factoring out some of the duplicated content,
though, such as discussion of how RTP gets component ID 1 and RTCP
id 2, and I think Ekr noted some other areas of duplication.

Other purely editorial nits follow (IESG feel free to ignore).

Section 18.2:

   Therefore, the servers get used less and
   less, and can eventually be remove when their usage goes to zero.

*removed

Section 19.4:

   In addition to attacks where the attacker is a third party trying to
   insert fake candidate information or stun messages,

s/stun/STUN/

Section 21:

   o  Make technical changes, due to discovered flows in RFC 5245 and
      based on feedback from the community that has implemented and
      deployed ICE applications based on RFC 5245.

flows or flaws?

-Benjamin

---

From: Peter Saint-Andre <[email-masked]>
Date: Tue, 20 Feb 2018 15:42:49 -0800

On 2/20/18 2:00 PM, Benjamin Kaduk wrote:

> [incoming AD 

Welcome, and good luck! :-)

> ramping up on pre-telechat document review]

The status of a "practice ballot" is unclear in IETF process. Is this
feedback that the authors and WG need to address?

Peter

---

From: Peter Saint-Andre <[email-masked]>
Date: Tue, 20 Feb 2018 15:46:00 -0800

> [resending with fixed Subject and recipients list]

[same]

Peter

---

From: Eric Rescorla <[email-masked]>
Date: Tue, 20 Feb 2018 15:47:20 -0800

I would suggest treating it as a Comment.

Ben, if there is anything you think is DISCUSS-worthy, please call it to my
attention and we can talk

-Ekr

---

From: Peter Saint-Andre <[email-masked]>
Date: Tue, 20 Feb 2018 15:54:44 -0800

That seems like a fine approach!

On 2/20/18 3:47 PM, Eric Rescorla wrote:
> Ben, if there is anything you think is DISCUSS-worthy, please call it to
> my attention and we can talk
> 
> -Ekr
> 
> 
> On Tue, Feb 20, 2018 at 3:46 PM, Peter Saint-Andre <[email-masked]
> <mailto:[email-masked]>> wrote:
>

---

From: Christer Holmberg <[email-masked]>
Date: Wed, 21 Feb 2018 19:30:50 +0000

Hi Ben,

Thank You for the review and comments! Please see inline.

>Given that this is a -bis document, there are some areas that may have aged poorly and could benefit from reconsideration:

Yes. The focus of the bis work was to fix bugs, and clarify procedures. But, if outdated information is found we can of course fix that.

> IPv4 is RECOMMENDED for dual-stack hosts.  Is that still the advice we want to give?

Suresh had the same comment. I suggest to remove the sentence.

> Section 17 is very focused on voice traffic; is that still the main use of ICE?

In my opinion it is.

---

> which talks of disabling STUN's MESSAGE-INTEGRITY as a potential benefit.  Do we really want to advocate for 
> increased levels of unauthenticated plaintext on the internet?

I wouldn't say that we are advocating. The truth is that people don't see it feasibly to integrity protect keep-alives.

---

>Appendix C has a table of bandwidth consumption in various scenarios, most of which are measured in kilobits per second.  Is this still relevant to modern deployments?

Appendix C was added to the bis document, based on real measurements (brought by Google, as far as I remember), so they are relevant.

---

> but in section 5.1.1.1 we say that IPv6 link-local addresses MUST NOT be gathered.  
> Is this just to handle the chance that we might get a reflexive candiate for a link-local address?

Well, even if that happens, it shouldn't be used in my opinion.

I suggest to remove the text in Section 6.1.2.2, to avoid confusion.

---

> which does not seem like an enforceable requirement, given that future updates to this document could just remove it.  
> Perhaps it should be worded differently to give the motivation for the need to keep triggered checks.

Perhaps we could remove the sentence. There are many things that extensions need to keep, and I don't see any reason to just mention one of them. And, triggered checks isn't even needed for ICE to work - it's just an optimization.

---

>     RTO = MAX (500ms, Ta*N * (Num-Waiting + Num-In-Progress))

The number of checks to be performed. See section 14.1.

---

> which leaves me confused, perhaps just about which response is "the response" that we're taking the mapped address from.  
> The previous text is about a STUN Binding request from L to R that causes R to generate a triggered check, so is this the response 
> that R generates to the Binding request from L?

Correct.

>  IIUC the mapped address therein would be where R sees traffic from L as originating 
> from, and would not be appropriate to use as a local address for R, etc..  So presumably I am picking the wrong response to use as the 
> source of data.

I think you are picking the right response :)

---

>   which there was no packet loss) is much more likely to complete

What is meant in 18.3 is that ICE is very robust when it comes to finding *A* connectivity, as there is no single point of failure in the network (you can use multiple STUN servers etc).

---

> Something of a side note, but is HMAC-SHA1 still the state of the art for STUN MESSAGE-INTEGRITY?  
> I know that there are no known breaks against HMAC-SHA1, but I would be a little surprised if no one was interested in producing an update.

RFC 5389 (STUN) uses HMAC-SHA1.

Anyway, I don't think it is mentioned in the 5245bis document, is it?

---

> I don't understand this.  Is the ICE agent or the TURN server "turned into" a zombie or rogue server by DNS attacks?  
> If the TURN server, how would a bogus DNS response turn it into a rogue server?

I will have to come back on the security related comments, because I need input from others.

---

>On an editorial note, there is a general feel to the document that it started off life as only covering ICE for RTP and the application 
>profiling was later split off into separtae documents.  I don't know that it's worth the effort to try to change that, though.  I might 
>give some thought to factoring out some of the duplicated content, though, such as discussion of how RTP gets component ID 1 and 
>RTCP id 2, and I think Ekr noted some other areas of duplication.
>

---

>   less, and can eventually be remove when their usage goes to zero.

I will fix as suggested.

---

>   insert fake candidate information or stun messages,

I will fix as suggested.

---

>      deployed ICE applications based on RFC 5245.

Flaws :) I will fix it.

---

Regards,

Christer

---

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 22 Feb 2018 13:50:36 -0600

> Suresh had the same comment. I suggest to remove the sentence.

Ok.

> > Section 17 is very focused on voice traffic; is that still the main use of ICE?

Voice to the exclusion of video, or does that include voice+video?

> I wouldn't say that we are advocating. The truth is that people don't see it feasibly to integrity protect keep-alives.

I suppose I will have to defer to your experience, but that seems
surprising to me and not consistent with my viewport into the usage of
crypto in other areas.

> Appendix C was added to the bis document, based on real measurements (brought by Google, as far as I remember), so they are relevant.

Ok.  Sorry for not noticing it was new.

> I suggest to remove the text in Section 6.1.2.2, to avoid confusion.

Ok.

> Perhaps we could remove the sentence. There are many things that extensions need to keep, and I don't see any reason to just mention one of them. And, triggered checks isn't even needed for ICE to…

Okay.  I have no strong preference for any given change (or not
change) here, and trust your judgment.

> The number of checks to be performed. See section 14.1.

Given that the other terms have their own prose description after
the formula, I would suggest doing so for N as well, as a reminder.

> I think you are picking the right response :)

Forgive me for being dense.  Does this mean that a change in the
text is needed?

> What is meant in 18.3 is that ICE is very robust when it comes to finding *A* connectivity, as there is no single point of failure in the network (you can use multiple STUN servers etc).

Ok.

> RFC 5389 (STUN) uses HMAC-SHA1.

Right.  IIUC we use the STUN MESSAGE-INTEGRITY structure to protect
certain ICE messages, so it is at least somewhat relevant.

> Anyway, I don't think it is mentioned in the 5245bis document, is it?

I don't think so, which is why it's a side note and mostly for my
curiousity.

> I will have to come back on the security related comments, because I need input from others.

Sure.

Thanks again,

Benjamin

---

From: Christer Holmberg <[email-masked]>
Date: Thu, 22 Feb 2018 20:25:57 +0000

Hi,

...

> Voice to the exclusion of video, or does that include voice+video?

While video has become more common due to WebRTC, my opinion is still that voice-only traffic is still the main use.

Note, however, that this is just my personal observation - feel free to tell me I'm wrong :)

And, even if we would cover video, I don't think it would impact the text.

---

 ---
 
> Given that the other terms have their own prose description after the formula, I would suggest doing so for N as well, as a reminder.

Others also asked about N, so I will do that.
 
---
 
>>> The previous text is about a STUN Binding request from L to R that 
>>> causes R to generate a triggered check, so is this the response that R generates to the Binding request from L?
>> 
>> Correct.
>> 
>>>  IIUC the mapped address therein would be where R sees traffic from 
>>> L as originating from, and would not be appropriate to use as a 
>>> local address for R, etc..  So presumably I am picking the wrong response to use as the source of data.
>> 
> Forgive me for being dense.  Does this mean that a change in the text is needed?

I got some other comments on the example too, so I am going to take a closer look at it, and see what/if needs to be changed :)

---
 
>> I will have to come back on the security related comments, because I need input from others.

I looked into this, and I have to admit that I have no clue that the text means (text comes from RFC 5245). If seems like a virus could cause the TURN server to start doing bad things (or, to not do anything), but I am not sure that's something we would need to say.

So, but unless someone can explain what exactly it means, I suggest to remove the text.

Regards,

Christer

> ---
> 
> >On an editorial note, there is a general feel to the document that it 
> >started off life as only covering ICE for RTP and the application 
> >that it's worth the effort to try to change that, though.  I might give some thought to factoring out some of the duplicated content, though, such as discussion of how RTP gets component ID 1 and RTCP id 2, and I think Ekr noted some other areas of duplication.
> >

---

From: Peter Saint-Andre <[email-masked]>
Date: Thu, 22 Feb 2018 12:37:39 -0800

> And, even if we would cover video, I don't think it would impact the text.

We might want to add a sentence to the end of Section 17.2.1 to the
effect that these planning considerations are more significant in
deployments involving video (e.g., video conferencing). Also, the number
of participants in a session obviously has an impact as well, and it
seems that the current text considers only 1:1 calls, not 1:many?

Peter

---

From: Christer Holmberg <[email-masked]>
Date: Fri, 23 Feb 2018 14:04:53 +0000

Hi,

> We might want to add a sentence to the end of Section 17.2.1 to the effect that these planning considerations 
> are more significant in deployments involving video (e.g., video conferencing). Also, the number of participants in 
> a session obviously has an impact as well, and it seems that the current text considers only 1:1 calls, not 1:many?

I suggest the following:

"The planning considerations above become more significant in multi-media
scenarios (e.g., audio and video conferences), and when the numbers of
participants in a session grow."

Regards,

Christer

---

From: Peter Saint-Andre <[email-masked]>
Date: Fri, 23 Feb 2018 08:29:48 -0800

> participants in a session grow."

WFM. This probably isn't the spec in which to precisely define the
operational impact of running a videoconferencing service anyway...

Peter

---

From: Benjamin Kaduk <[email-masked]>
Date: Fri, 23 Feb 2018 15:22:29 -0600

> operational impact of running a videoconferencing service anyway...

WFM as well.

-Benjamin