Subject: Re: [Gen-art] [tram] Genart last call review of draft-ietf-tram-turnbis-25

From: "Konda, Tirumaleswar Reddy" <[email-masked]>
Date: Tue, 4 Jun 2019 09:46:09 +0000

Hi Vijay,

Thanks for the review, Please see inline.

> -----Original Message-----
> From: tram <[email-masked]> On Behalf Of Vijay Gurbani via
> Datatracker
> Sent: Friday, May 31, 2019 8:19 PM
> To: [email-masked]
> Cc: [email-masked]; [email-masked]; [email-masked]
> Subject: [tram] Genart last call review of draft-ietf-tram-turnbis-25
> 
> This email originated from outside of the organization. Do not click links or
> open attachments unless you recognize the sender and know the content is
> safe.
> 
> Reviewer: Vijay Gurbani
> Review result: Ready with Issues
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area Review
> Team (Gen-ART) reviews all IETF documents being processed by the IESG for
> the IETF Chair.  Please treat these comments just like any other last call
> comments.
> 
> For more information, please see the FAQ at
> 
> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
> 
> Document: draft-ietf-tram-turnbis-25
> Reviewer: Vijay K. Gurbani
> Review Date: 2019-05-31
> IETF LC End Date: 2019-05-28
> IESG Telechat date: Not scheduled for a telechat
> 
> Summary: Ready but has some issues that need to be looked at as described
> below.
> 
> Major:
> 
> - My advice would be to put S3 (Terminology) before S2.  There are a lot of
> terms defined in S3 that S2 simply uses; it will serve the reader well if they
> already knew these terms by the time the encountered them.

Works for me, moved Terminology Section.

> 
> Minor:
> - S1, paragraph 2: "...NATs that are well behaved."  Here, what is the
> definition of a "well behaved" NAT?  Is there a RFC that encodes expected
> behaviour?  If so, please provide a reference.  If not, then some resource
> should be made available (a paragraph or some reference) where the
> expected  behaviour of NATs is codified to some extent.

Address-dependent or Address-and port-dependent mappings are not well behaved NATs, this paragraph refers to RFC4787 (see https://tools.ietf.org/html/rfc4787#section-4.1 ) and says hole-punching technique will not work
with NATs having Address-dependent or Address-and port-dependent mapping behavior. 

NEW:
   As described in [RFC5128] and [RFC4787], hole punching techniques
   will fail if both hosts are behind NATs that are not well behaved.
   For example, if both hosts are behind NATs that have a mapping
   behavior of "address-dependent mapping" or "address- and port-
   dependent mapping" (Section 4.1 in RFC4787), then hole punching techniques generally fail.

> 
> - S 2.4, Figure 3: "To partly mitigate this attack ...", just to be explicit,  this is
> partial mitigation because an attacker, observing the CreatePermission
> request and response can masquerade as the client and immediately send a
> Send  indication to the peer address observed in the CreatePermission
> request.  Is  this correct?  
>If so, I think it is worth documenting in the draft.  If
> not,  some idea on the origins of the "partial mitigation" will be good to know
> for  the implementors of the specification for the sake of completeness.

If the attacker spoofs the TURN client IP address and sends bogus Send indication to the server to a peer based on the permission installed by the TURN client to the peer, this attack cannot be mitigated the server unless (D)TLS is used. However, if the attacker spoofs the TURN client IP address and sends bogus Send indication to the server to peer but the client has not installed permission to the peer, this attack can be mitigated by the server.  The above points are covered in https://tools.ietf.org/html/draft-ietf-tram-turnbis-25#section-20.1.4. 
> 
> - S2.7, first paragraph: s/try hard to avoid/avoid/  (What does it for a program
> compliant to this specification to "try hard"?
>  Either the author of the program knows fragmentation is not desirable or
> they don't and the packet is fragmented.)

Done.

> 
> - S5, second to last paragraph: "When TCP transport is used ..."  Here,
> wouldn't  TCP detect the bit flip?  What am I missing here?  TCP is
> transporting TURN  packets, if one of the bits in the TURN packet flips, won't
> the TCP checksum  become invalid?

Please see the discussion in BEHAVE WG mailing list https://mailarchive.ietf.org/arch/msg/behave/O7A7vTihIBJ-uKmJT_bSea13wIA 

> 
> - S5, second to last paragraph: "...a long sequence of invalid..."  Here, how
>   long is "long"?  2 messages?  3 messages?  4 messages?  I think an explicit
>   guideline may help make the error handling more robust.

The threshold for long sequence of invalid TURN messages looks implementation specific to me, it was not specified in the base TURN spec (see https://tools.ietf.org/html/rfc5766#section-4).  

> 
> - S7.3, third paragraph: "...before trying to request any more ..."  Here, how
>   many times should a client retry the request upon the receipt of a 508?
>   Again, an explicit guideline will help interoperability; otherwise, some
>   clients will retry only once, other more than once, and so on.

The number of attempts the client would retry is implementation specific. For example, if other TURN servers are available, the client can try to get the relayed transport address from the other TURN servers and need not retry the request (see https://tools.ietf.org/html/rfc8445#section-5.1.1.2). 

> 
> - S12: Please label the figure that appears in this section, especially since it
>   will be nice to refer to it explicitly, as is required in S12.6.  (There, the
>   text simply says "...as described above.", where "above" probably implies
> the
>   table in S12.)

Sure, added label and referred to it explicitly.
, 

> 
> - S13: s/runs in userland/runs in user space/  ("userland" is colloquial usage,
> better to use "user space".)

Updated.

> 
> Nits:
> 
> - Abstract: s/from some other/from other/

Fixed.

Cheers,
-Tiru

> 
> That's it!  Thanks.
> 
> _______________________________________________
> tram mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tram

---

From: Vijay Gurbani <[email-masked]>
Date: Tue, 4 Jun 2019 09:15:27 -0500

Tiru: Thank you for attending to my comments. Please see inline for some
more questions based on your comments.  Thank you for your time!

> Works for me, moved Terminology Section.

Great; thanks.

> Address-dependent or Address-and port-dependent mappings are not well
> behaved NATs, this paragraph refers to RFC4787 (see
> https://tools.ietf.org/html/rfc4787#section-4.1 ) and says hole-punching
> technique will not work
>    As described in [RFC5128] and [RFC4787], hole punching techniques [...]
>

The new text is a great addition.  Thanks.

> > - S 2.4, Figure 3: "To partly mitigate this attack ...", just to be
> explicit,  [...]
>
> If the attacker spoofs the TURN client IP address and sends bogus Send
> indication to the server to a peer based on the permission installed by the
> TURN client to the peer, this attack cannot be mitigated the server unless
> (D)TLS is used. However, if the attacker spoofs the TURN client IP address
> and sends bogus Send indication to the server to peer but the client has
> not installed permission to the peer, this attack can be mitigated by the
> server.  The above points are covered in
> https://tools.ietf.org/html/draft-ietf-tram-turnbis-25#section-20.1.4.
>

I think a forward reference to Section 20.1.4 can easily be made in Section
2.4 so the reader is aware of the what mitigation properties are being
provided by S2.4 and which section is providing the remaining properties.
It will make your document much more complete.

> [...]
> Please see the discussion in BEHAVE WG mailing list
> https://mailarchive.ietf.org/arch/msg/behave/O7A7vTihIBJ-uKmJT_bSea13wIA
>

Hmm ... so according to the above link, the bit flip has to get through the
TCP checksum *AND* hit the length field of the TURN packet.  Given today's
networks, such an event has a very low probability, but in the interest of
specifying protocols to handle all kinds of errors, it makes sense to
address this.  Of course, this is not a problem endemic to TURN, it is a
problem inherent any time TCP is used.  I do not know what mitigation
strategies are used across IETF for an event like this.  Perhaps the TSV
area directorate can help.  In any case, I am fine with leaving the text
as-is, although it may make sense to talk to some of the folks in TSV to
see how other protocols that ride in TCP handle such cases.  (For
curiosity's sake, ff not for anything else.)

> The threshold for long sequence of invalid TURN messages looks
> implementation specific to me, it was not specified in the base TURN spec
> (see https://tools.ietf.org/html/rfc5766#section-4).
>

If it was not specified in the base TURN specification, then it may make
sense to tighten up the behaviour in the bis specification, no?  This would
be our chance to add a behaviour that can be measured and test cases
created against it for protocol resiliency, no?  Or am I missing something
on why the bis specification does not want to tighten this behaviour?

> The number of attempts the client would retry is implementation specific.
> For example, if other TURN servers are available, the client can try to get
> the relayed transport address from the other TURN servers and need not
> retry the request (see https://tools.ietf.org/html/rfc8445#section-5.1.1.2).
>
>

Same feedback as above; if the number of attempts were not specified in
rfc5766, could we not do better now?  Any reason not to?

Thanks.

- vijay

---

From: "Konda, Tirumaleswar Reddy" <[email-masked]>
Date: Wed, 5 Jun 2019 12:21:26 +0000

Hi Vijay,

Please see inline [TR]

From: Vijay Gurbani <[email-masked]>
Sent: Tuesday, June 4, 2019 7:45 PM
To: Konda, Tirumaleswar Reddy <[email-masked]>
Cc: [email-masked]; [email-masked]; [email-masked]; [email-masked]
Subject: Re: [tram] Genart last call review of draft-ietf-tram-turnbis-25

CAUTION: External email. Do not click links or open attachments unless you recognize the sender and know the content is safe.

________________________________
Tiru: Thank you for attending to my comments. Please see inline for some more questions based on your comments.  Thank you for your time!

Great; thanks.

NEW:

> - S 2.4, Figure 3: "To partly mitigate this attack ...", just to be explicit,  [...]

I think a forward reference to Section 20.1.4 can easily be made in Section 2.4 so the reader is aware of the what mitigation properties are being provided by S2.4 and which section is providing the remaining properties.  It will make your document much more complete.

[TR] Okay, added the following line:
The techniques to fully mitigate the attack are discussed in Section 20.1.4.

> - S5, second to last paragraph: "When TCP transport is used ..."  Here, [...]

Hmm ... so according to the above link, the bit flip has to get through the TCP checksum AND hit the length field of the TURN packet.  Given today's networks, such an event has a very low probability, but in the interest of specifying protocols to handle all kinds of errors, it makes sense to address this.  Of course, this is not a problem endemic to TURN, it is a problem inherent any time TCP is used.

[TR] Agreed.

I do not know what mitigation strategies are used across IETF for an event like this.  Perhaps the TSV area directorate can help.  In any case, I am fine with leaving the text as-is, although it may make sense to talk to some of the folks in TSV to see how other protocols that ride in TCP handle such cases.  (For curiosity's sake, ff not for anything else.)

If it was not specified in the base TURN specification, then it may make sense to tighten up the behaviour in the bis specification, no?  This would be our chance to add a behaviour that can be measured and test cases created against it for protocol resiliency, no?  Or am I missing something on why the bis specification does not want to tighten this behaviour?

[TR] The threshold values for long sequence of invalid TURN messages is not discussed in the TRAM WG, and I don’t have information on the typical values used in the field. Anyways, not documenting the threshold values would not cause any interoperable issues for a really low probability scenario.

Same feedback as above; if the number of attempts were not specified in rfc5766, could we not do better now?  Any reason not to?

[TR] The number of attempts depends on the context, For instance (1) If other TURN servers are available, and the client gets the relayed transport address from at least one other TURN server, the client need not retry the request (2) if the client is using ICE (https://tools.ietf.org/html/rfc8445), the client can keep retrying till the candidate gathering phase expires (please note ICE specification does not specify the candidate gathering phase timeout).

Cheers,
-Tiru

Thanks.

- vijay

---

From: Vijay Gurbani <[email-masked]>
Date: Thu, 6 Jun 2019 07:39:20 -0500

Dear Tiru, thank you for your response.  Please see inline.

On Wed, Jun 5, 2019 at 7:21 AM Konda, Tirumaleswar Reddy <
[email-masked]> wrote:

> I think a forward reference to Section 20.1.4 can easily be made in
> Section 2.4 so the reader is aware of the what mitigation properties are
> being provided by S2.4 and which section is providing the remaining
> properties.  It will make your document much more complete.
>
>
>
Great, thanks.

> [TR] The threshold values for long sequence of invalid TURN messages is
> not discussed in the TRAM WG, and I don’t have information on the typical
> values used in the field. Anyways, not documenting the threshold values
> would not cause any interoperable issues for a really low probability
> scenario.
>
>
Since the WG has not addressed a threshold for invalid TURN messages, I
suspect that there is nothing more to be done here from a process point of
view.  However, despite this being a low probability scenario, it will be
encountered and I suspect that it has been already but just not
documented.  So from a completeness point of view, it may make sense to
bring this to the attention of the TRAM WG and if they have decided, for
good reasons, to punt on this, then the issue is closed.  I can send an
email to the TRAM WG chairs, just to close the loop.

> [TR] The number of attempts depends on the context, For instance (1) If
> other TURN servers are available, and the client gets the relayed transport
> address from at least one other TURN server, the client need not retry the
> request (2) if the client is using ICE (
> https://tools.ietf.org/html/rfc8445), the client can keep retrying till
> the candidate gathering phase expires (please note ICE specification does
> not specify the candidate gathering phase timeout).
>

Okay, sounds good.

Thank you for attending to my comments.  I appreciate it.

- vijay

---

From: "Konda, Tirumaleswar Reddy" <[email-masked]>
Date: Thu, 6 Jun 2019 13:16:29 +0000

Hi Vijay,

Please see inline [TR2]

Sent: Thursday, June 6, 2019 6:09 PM

Great, thanks.

Since the WG has not addressed a threshold for invalid TURN messages, I suspect that there is nothing more to be done here from a process point of view.  However, despite this being a low probability scenario, it will be encountered and I suspect that it has been already but just not documented.  So from a completeness point of view, it may make sense to bring this to the attention of the TRAM WG and if they have decided, for good reasons, to punt on this, then the issue is closed.  I can send an email to the TRAM WG chairs, just to close the loop.

[TR2] Sure, go ahead. If members in the WG respond, I will update the draft accordingly.

Cheers,
-Tiru

Okay, sounds good.

- vijay