Subject: Genart last call review of draft-ietf-tram-turnbis-25

From: Vijay Gurbani via Datatracker <[email-masked]>
Date: Fri, 31 May 2019 07:48:39 -0700

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

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

- 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.)

- 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?

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

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

- 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.)

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

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

That's it!  Thanks.