Subject: Re: [MMUSIC] WGLC for draft-ietf-mmusic-ice-tcp-13

From: Flemming Andreasen <[email-masked]>
Date: Mon, 09 May 2011 22:06:46 -0400

I have not seen any comments on this document since the WGLC was issued. 
If anybody reviewed the document, but did not have any comments on it, 
please let us know.

In any case, I have a few comments on the document:

Non-Editorial:
===========
- Section 7.1, third paragraph
<quote>
    If the TCP connection cannot be established, the check is considered
    to have failed, and a full-mode agent MUST update the pair state to
    Failed in the check list.
</quote>
a) How long do you wait before determining it cannot be established ? 
Need some considerations around this.
b) Also, how fast do you attempt to establish additional TCP connections 
? Timer Ta considerations would seem to apply to the TCP connection 
establishment here.

- Section 11.1
The section seems to be written under the assumption that both sides are 
full ICE Implementations. Need to consider lite implementations as well.

- Section 12
I believe there is a TCP amplification attack similar to the STUN 
amplification attack that requires consideration, since the TCP 
connection is established prior to any STUN/ICE checks and involves TCP 
state creation on the part of the attacked party.

- Section 13
I was surprised to see no IANA considerations since "TCP" is defined as 
a new transport-extension value here, however upon checking RFC 5245, 
there apparently is no registry defined for this. RFC 5245 Section 14 
appears to indicate this was not an oversight at the time, however with 
ice-tcp, I think we should define a registry.

Editorial/Nits
==========
-  Section 1: s/in as a broad/in as broad

- Section 3, third paragraph
<quote> server reflexive, NAT-assisted, tunneled, or relayed candidates 
of these three types</quote>
Add reference to Section 5 for further explanation.

- Section 3, Figure 1
It would be helpful to either expand the figure to cover the non-(D)TLS 
case as well or have another figure showing that for comparison

- Section 4.2
s/local-preference/local preference/   for consistency with RFC 5245 
(multiple occurrences)

- Section 4.2
<quote>
The direction-pref MUST be between 0 and 7, with 7 being the most
    preferred.  The other-pref MUST be between 0 and 8191, with 8191
    being the most preferred.
</quote>
Rephrase to make clear that 0 and 8191 (presumably) are both valid 
values in these ranges.

- Section 4.3
<quote>
    If a media stream is inherently TCP-based, the agent MUST select an
    active candidate as the default.  This ensures proper directionality
    of connection establishment for NAT traversal with non-ICE
    implementations.
</quote>
It doesn't ensure it, however under the assumption that the ICE endpoint 
is behind a NAT, it enhances the likelihood of the connection 
establishment succeeding.

- Section 4.5
<quote>
    The connection-address and port encoded into the candidate attribute
    for active candidates MUST be set to the IP address that will be used
    for the attempt, but the port(s) MUST be set to 9 (i.e., Discard).
</quote>
- Delete the "and port" part.

- Section 5, third paragraph
<quote>
If the local network or host
    does not support IPv6 addressing, then clients SHOULD make use of
    other techniques, e.g., Teredo [RFC4380] or SOCKS IPv4-IPv6
    gatewaying [RFC3089], for obtaining IPv6 candidates.
</quote>
Add "TURN-IPV6 [RFC6156]" to the beginning of the example list.

- Section 5, 4th paragraph
<quote>
For
    example, using multiple NAT-assisted techniques with the same NAT
    usually results only in redundant candidates and similarly out of
    multiple different UDP tunneling or relaying techniques with similar
    features using just one is often enough.
</quote>
Hard to read. Suggest breaking sentence in two ("...candidates.  
Similarly, out of..."). Regardless, I'm not sure exaclty what "similar 
features" implies here. An example would be good.

- Section 5.3, last paragraph
Should reference the PCP WG and ongoing work there as well.

- Section 5.5, last paragraph
<quote>
It is also possible to use, e.g., an SSH [RFC4250] tunnel
</quote>
RFC 4251 would seem to be a better reference here.

- Section 12
s/sips/SIPS [RFC3261]/

- Section 15.2
RFC 3261, 5389, 5766, and 6062 should all be normative rather informative.

- Appendix A
s/hand shake/handshake
s/SYNACK/SYN-ACK

- Please make sure to run id-nits check before submitting the update.

Thanks

-- Flemming

On 4/15/11 11:54 AM, Flemming Andreasen wrote:
> This is to announce a 2 week Working Group Last Call for
>
>     draft-ietf-mmusic-ice-tcp-13
>
> as Proposed Standard. Please review and provide any comments you may 
> have on the document before Monday, May 2nd, 2011. Comments should be 
> sent to the document authors and the MMUSIC WG list.
>
>
> Thanks
>
>     Flemming & Miguel
>
>
>
> Draft Info:
>
> 	Title           : TCP Candidates with Interactive Connectivity Establishment (ICE)
> 	Author(s)       : J. Rosenberg, et al.
> 	Filename        : draft-ietf-mmusic-ice-tcp-13.txt
> 	Pages           : 29
> 	Date            : 2011-04-11
>
> Interactive Connectivity Establishment (ICE) defines a mechanism for
> NAT traversal for multimedia communication protocols based on the
> offer/answer model of session negotiation.  ICE works by providing a
> set of candidate transport addresses for each media stream, which are
> then validated with peer-to-peer connectivity checks based on Session
> Traversal Utilities for NAT (STUN).  ICE provides a general framework
> for describing candidates, but only defines UDP-based transport
> protocols.  This specification extends ICE to TCP-based media,
> including the ability to offer a mix of TCP and UDP-based candidates
> for a single stream.
>
> A URL for this Internet-Draft is:
> http://www.ietf.org/internet-drafts/draft-ietf-mmusic-ice-tcp-13.txt
>    
>

---

From: Simon Perreault <[email-masked]>
Date: Tue, 10 May 2011 09:07:39 -0400

Flemming Andreasen wrote, on 05/09/2011 10:06 PM:
> I have not seen any comments on this document since the WGLC was issued. If
> anybody reviewed the document, but did not have any comments on it, please let
> us know.

I have reviewed the document multiple times in its lifetime. All my previous
comments have been addressed.

ICE TCP is needed not only for NAT traversal, but also for the transition to
IPv6. This work is important and overdue.

Simon
-- 
DTN made easy, lean, and smart --> http://postellation.viagenie.ca
NAT64/DNS64 open-source        --> http://ecdysis.viagenie.ca
STUN/TURN server               --> http://numb.viagenie.ca

---

From: Ari Keranen <[email-masked]>
Date: Wed, 11 May 2011 18:55:49 +0300

Hi Flemming,

Thanks for the thorough review! See inline.

On May 10, 2011, at 5:06 AM, Flemming Andreasen wrote:
[...]
> a) How long do you wait before determining it cannot be established ? Need some considerations around this

I have considered this as an implementation/platform detail: you do connect() and wait until it succeeds or fails. Or do you have suggestions what additional to recommend here?

> b) Also, how fast do you attempt to establish additional TCP connections ? Timer Ta considerations would seem to apply to the TCP connection establishment here. 

Yes, the agents pace the start of the checks like for UDP. The end of section 4.1 says "Like its UDP counterparts, TCP-based STUN transactions are paced out at one every Ta seconds." It could be a bit confusing that the comment is under "Gathering candidates" section, but it refers to all STUN transactions.

> - Section 11.1
> The section seems to be written under the assumption that both sides are full ICE Implementations. Need to consider lite implementations as well. 

This seemed OK to me. What would you change here to accommodate also lite agents?

> - Section 12
> I believe there is a TCP amplification attack similar to the STUN amplification attack that requires consideration, since the TCP connection is established prior to any STUN/ICE checks and involves TCP state creation on the part of the attacked party.  

I guess you're right. We should document that, but any ideas for mitigation?

> - Section 13
> I was surprised to see no IANA considerations since "TCP" is defined as a new transport-extension value here, however upon checking RFC 5245, there apparently is no registry defined for this. RFC 5245 Section 14 appears to indicate this was not an oversight at the time, however with ice-tcp, I think we should define a registry.  

Good point; we'll add one in the next revision.

> Editorial/Nits

fixed

> <quote> server reflexive, NAT-assisted, tunneled, or relayed candidates of these three types</quote>

OK, fixed

> - Section 3, Figure 1
> It would be helpful to either expand the figure to cover the non-(D)TLS case as well or have another figure showing that for comparison

Do we really need one, considering the non-(D)TLS case is fairly trivial, right? The "strangeness" in the (D)TLS case comes from the fact that STUN is used outside of the (D)TLS connection. 

Although, the figure caption should be updated to reflect the fact that this is the (D)TLS case; now it says "ICE TCP Stack with (D)TLS".

> - Section 4.2
> s/local-preference/local preference/   for consistency with RFC 5245 (multiple occurrences)

Good catch! Fixed.

> Rephrase to make clear that 0 and 8191 (presumably) are both valid values in these ranges. 

OK, now it says: 

   The direction-pref MUST be between 0 and 7 (inclusive), with 7 being
   the most preferred.  The other-pref MUST be between 0 and 8191
   (inclusive), with 8191 being the most preferred.

> It doesn't ensure it, however under the assumption that the ICE endpoint is behind a NAT, it enhances the likelihood of the connection establishment succeeding. 

I guess you're right. I'll have a closer look at this.

> - Delete the "and port" part. 

Why? The "port" refers to the "port" and "rel-port" parts in the candidate-attribute.

> Add "TURN-IPV6 [RFC6156]" to the beginning of the example list. 

OK, fixed

> Hard to read. Suggest breaking sentence in two ("...candidates.  Similarly, out of..."). Regardless, I'm not sure exaclty what "similar features" implies here. An example would be good. 

OK, I removed the "similar features" part (it was a bit redundant anyway). Now it says:

   For
   example, using multiple NAT-assisted techniques with the same NAT
   usually results only in redundant candidates.  Similarly, out of
   multiple different UDP tunneling or relaying techniques using just
   one is often enough.

> Should reference the PCP WG and ongoing work there as well. 

OK. I added PCP base to the beginning of the list of "other known protocols".

> RFC 4251 would seem to be a better reference here. 

Good point, fixed.

> s/sips/SIPS [RFC3261]/

fixed

> RFC 3261, 5389, 5766, and 6062 should all be normative rather informative. 

Fixed the first two, but 5776 (TURN) and 6062 (TURN TCP) are intentionally informative since TURN is just one of the possible techniques for relaying. 

> s/SYNACK/SYN-AC

fixed

> - Please make sure to run id-nits check before submitting the update. 

Will do. Although, at least all the current idnits complaints seem to be false positives.

Thanks,
Ari

>> as Proposed Standard. Please review and provide any comments you may have on the document before Monday, May 2nd, 2011. Comments should be sent to the document authors and the MMUSIC WG list. 
>> 
>> 
>> Thanks 
>> 
>>     Flemming & Miguel 
>> 
>> 
>> 
>> Draft Info:
>> 
>>

---

From: Flemming Andreasen <[email-masked]>
Date: Thu, 12 May 2011 17:05:16 -0400

On 5/11/11 11:55 AM, Ari Keranen wrote:
> I have considered this as an implementation/platform detail: you do 
> connect() and wait until it succeeds or fails. Or do you have 
> suggestions what additional to recommend here?
>
Does RFC 5389 Section 7.2.2 simply apply directly here and hence we 
should just reference that ?

> > b) Also, how fast do you attempt to establish additional TCP 
> connections ? Timer Ta considerations would seem to apply to the TCP 
> connection establishment here.
>
> Yes, the agents pace the start of the checks like for UDP. The end of 
> section 4.1 says "Like its UDP counterparts, TCP-based STUN 
> transactions are paced out at one every Ta seconds." It could be a bit 
> confusing that the comment is under "Gathering candidates" section, 
> but it refers to all STUN transactions.
>
I think we should mention it in Section 7.1, however looking at it 
again, I assume we simply following Section 5.8 from RFC 5245 
(Scheduling Checks), and hence a reference to that effect would suffice.

>
> > - Section 11.1
> > The section seems to be written under the assumption that both sides 
> are full ICE Implementations. Need to consider lite implementations as 
> well.
>

You have the following text for example
<quote>

If the TCP connection is established, the framing of RFC 4571 is

utilized. If the agent opened the connection, it MUST send a STUN

connectivity check.
</quote>

That latter sentence would presumably only apply if the agent is a full 
implementation.

Similarly
<quote>

If an agent receives a connectivity check after re-

establishment of the connection, it MUST generate a triggered check

over that connection in response if it has not already sent a check.

Once an agent has sent a check and received a successful response,

the connection is considered Valid and media can be sent (which

includes a TLS or DTLS-SRTP session resumption or restart)
</quote>
seems to assume full ICE implementations.

> connection is established prior to any STUN/ICE checks and involves 
> TCP state creation on the part of the attacked party.
>

I'm not sure - maybe we can limit the number of outstanding TCP 
connection attempts to the same IP-address ? It would probably be 
worthwhile asking some of the security experts here.

>
> > - Section 13
> > I was surprised to see no IANA considerations since "TCP" is defined 
> as a new transport-extension value here, however upon checking RFC 
> 5245, there apparently is no registry defined for this. RFC 5245 
> Section 14 appears to indicate this was not an oversight at the time, 
> however with ice-tcp, I think we should define a registry.
>
Ok.

> > Editorial/Nits
snipped the ones we're in sync on

> > - Section 3, Figure 1
> > It would be helpful to either expand the figure to cover the 
> non-(D)TLS case as well or have another figure showing that for comparison
>
> Do we really need one, considering the non-(D)TLS case is fairly 
> trivial, right? The "strangeness" in the (D)TLS case comes from the 
> fact that STUN is used outside of the (D)TLS connection.
>
> Although, the figure caption should be updated to reflect the fact 
> that this is the (D)TLS case; now it says "ICE TCP Stack with (D)TLS".
>
You don't *need* it, but it would have been helpful to me the first time 
I read it at least.

>    (inclusive), with 8191 being the most preferred.
I'd say "both inclusive".

> Why? The "port" refers to the "port" and "rel-port" parts in the 

The sentence reads
<quote>
    for the attempt,
</quote>

You can only set the connection-address to the IP address, not the port. 
It's a nit.

> Fixed the first two, but 5776 (TURN) and 6062 (TURN TCP) are 
> intentionally informative since TURN is just one of the possible 
> techniques for relaying.
>
Except from http://www.rfc-editor.org/rfc-editor/instructions2authors.txt

<quote>
       Within an RFC, references to other documents fall into two general
       categories: "normative" and "informative".  Normative references
       specify documents that must be read to understand or implement the
       technology in the new RFC, or whose technology must be present for
       the technology in the new RFC to work.  An informative reference
       is not normative; rather, it provides only additional information.
       For example, an informative reference might provide background or
       historical information.  Material in an informative reference is
       not required to implement the technology in the RFC.
</quote>

In other words, just because you don't mandata the use of TURN doesn't 
automatically turn it into an informative reference (as I've been told

I think the TURN text in e.g. Section 1, Section 4.1, Section 11.2 etc 
makes RFC 5766 a Normative reference per the above. RFC 6062 is perhaps 
more debatable.

Thanks

-- Flemming

---

From: Ari Keranen <[email-masked]>
Date: Thu, 19 May 2011 17:40:16 +0300

On May 13, 2011, at 12:05 AM, Flemming Andreasen wrote:
> Does RFC 5389 Section 7.2.2 simply apply directly here and hence we should just reference that ? 

That's probably the easiest fix. I'll add a reference to that (after the sentence you quoted): "See Section 7.2.2 in [RFC5389] for more details on STUN over TCP".

> I think we should mention it in Section 7.1, however looking at it again, I assume we simply following Section 5.8 from RFC 5245 (Scheduling Checks), and hence a reference to that effect would suffice. 

OK; that's actually true for the whole document (5245 procedures are followed unless said otherwise), but I guess it makes sense to make this more explicit here too. Right before 7.1 it says now (added the reference part between commas):

   [...] The TCP candidate pairs are in the same
   checklist with the UDP candidate pairs and they are scheduled for
   connectivity checks, as described in Section 5.8 in [RFC5245], based
   on the priority order.

> That latter sentence would presumably only apply if the agent is a full implementation. 

True, but a lite agent would not open (initiate) a connection if it has only passive candidates. The default candidate complicates this a bit though; I added "and is a full agent" clarification there to make it more clear.

> seems to assume full ICE implementations. 

True. I changed that into "If a full agent [...]".

> I'm not sure - maybe we can limit the number of outstanding TCP connection attempts to the same IP-address ? It would probably be worthwhile asking some of the security experts here.

That should work. Anyway, I sent a separate mail about this to the list.

[..]
> You don't *need* it, but it would have been helpful to me the first time I read it at least. 

OK, I'll see how combining those two in the same figure works out.

> I'd say "both inclusive". 

Fixed.

> You can only set the connection-address to the IP address, not the port. It's a nit. 

Ah, yes, good catch. Actually, it should say "[...] MUST be set to the IP address and port" (fixed).

> In other words, just because you don't mandata the use of TURN doesn't automatically turn it into an informative reference (as I've been told 
> 
> I think the TURN text in e.g. Section 1, Section 4.1, Section 11.2 etc makes RFC 5766 a Normative reference per the above. RFC 6062 is perhaps more debatable. 

OK. The idea was to have TURN "at the same level" as the other techniques and that's why it is informative, like the rest. That said, it's true that TURN has a bit of special attention in the text and I'm OK with moving 5766 to normative.

Cheers,
Ari

---

From: Flemming Andreasen <[email-masked]>
Date: Thu, 19 May 2011 17:09:11 -0400

Thanks Ari

This addresses all of my comments (modulo any additional suggestions for 
better ways of mitigating the TCP amplification attack).

-- Flemming

On 5/19/11 10:40 AM, Ari Keranen wrote:
> >> > a) How long do you wait before determining it cannot be 
> established ? Need some considerations around this
> >>
> That's probably the easiest fix. I'll add a reference to that (after 
> the sentence you quoted): "See Section 7.2.2 in [RFC5389] for more 
> details on STUN over TCP".
>
> >
> >> Yes, the agents pace the start of the checks like for UDP. The end 
> of section 4.1 says "Like its UDP counterparts, TCP-based STUN 
> OK; that's actually true for the whole document (5245 procedures are 
> followed unless said otherwise), but I guess it makes sense to make 
> this more explicit here too. Right before 7.1 it says now (added the 
> reference part between commas):
>
> >> > The section seems to be written under the assumption that both 
> sides are full ICE Implementations. Need to consider lite 
> implementations as well.
> >>
> True, but a lite agent would not open (initiate) a connection if it 
> has only passive candidates. The default candidate complicates this a 
> bit though; I added "and is a full agent" clarification there to make 
> it more clear.
>
> > Similarly
> > <quote>
> > Except from 
> http://www.rfc-editor.org/rfc-editor/instructions2authors.txt
> >
> > <quote>
> > In other words, just because you don't mandata the use of TURN 
> doesn't automatically turn it into an informative reference (as I've 
> been told
> >
> > I think the TURN text in e.g. Section 1, Section 4.1, Section 11.2 
> etc makes RFC 5766 a Normative reference per the above. RFC 6062 is 
> perhaps more debatable.
>
> OK. The idea was to have TURN "at the same level" as the other 
> techniques and that's why it is informative, like the rest. That said, 
> it's true that TURN has a bit of special attention in the text and I'm 
> OK with moving 5766 to normative.
>
>
> Cheers,
> Ari
>

---

From: Ari Keranen <[email-masked]>
Date: Fri, 20 May 2011 14:57:44 +0300

Thanks for your review Flemming! I'll wait for a while for comments on the TCP amplification attack and then submit a new version with the changes we have discussed.

Cheers,
Ari

On May 20, 2011, at 12:09 AM, Flemming Andreasen wrote:

> Thanks Ari
> 
> This addresses all of my comments (modulo any additional suggestions for better ways of mitigating the TCP amplification attack). 
> 
> -- Flemming 
> 
>