RE: [tram] Genart last call review of draft-ietf-tram-turnbis-25

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Vijay,

Thanks for the review, Please see inline.

> -----Original Message-----
> From: tram <tram-bounces@xxxxxxxx> On Behalf Of Vijay Gurbani via
> Datatracker
> Sent: Friday, May 31, 2019 8:19 PM
> To: gen-art@xxxxxxxx
> Cc: draft-ietf-tram-turnbis.all@xxxxxxxx; ietf@xxxxxxxx; tram@xxxxxxxx
> 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
> tram@xxxxxxxx
> https://www.ietf.org/mailman/listinfo/tram





[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Mhonarc]     [Fedora Users]

  Powered by Linux