A Network Address Translator (NAT) Traversal Mechanism for Media Controlled by the Real-Time Streaming Protocol (RTSP)
draft-ietf-mmusic-rtsp-nat-22

Nice, clear document, and what looks to be good technology.  Thanks for this.
I have a number of  non-blocking comments that I'd like you to consider.  I'm happy to chat about any of them, if you like (particularly the ones that are not marked as "Editorial").  There's no need to respond to any you agree with, and thanks in advance for considering them.

-- Section 4.2 --

I like the way you show the source for the ABNF items that are defined elsewhere.  I think I'll remember that, and use it myself.

Editorial:
      Note, the syntax
      allows multicast addresses, but they SHALL NOT be used in this
      context.

Entirely your option here, but I think it would be better to word this positively, as "This context MUST have a unicast address for this parameter, even though a muticast address would be syntactically valid."

      An IP address
      SHOULD be used, but an FQDN MAY be used in place of an IP address.

(I think I'm going to give this comment a number, so I can say "Comment #27".)  MAY can't be used this way, as an alternative to SHOULD: MAY says that something is completely optional, and that's not what you mean here.

What I'd like to see is something that explains why you might not use an IP address, despite the SHOULD... and then says that the alternative for those cases is an FQDN, but without the 2119 key word.  Something like this:

      An IP address
      SHOULD be used, but [in situations such as X or Y], it could be
      necessary to use an FQDN instead.

-- Section 4.3 --

Editorial:
   The ICE password and username for each agent needs to be transported
   using RTSP.

The subject is plural, so the verb should be too: "need".

-- Section 4.5.1 --
I see that ICE doesn't itself specify any sort of keep-alive responses for the connectivity checks.  Here, without advice to the client, I worry about clients that are too strict about checking the timing of the responses.  A server sending a response 200 ms after receiving a request does not translate into a client receiving the response within 200 ms of having sent the request.  If network delays mean that the response gets to the client after 250 ms, or 300, or whetever, and the client has set a 200 ms timer based on this section, there'll be a problem, no?  Similarly for the 150 ms time for the subsequent keep-alives.  Should this say something about that?

-- Section 6.10 --

Editorial:
   Both server and client MAY free its non selected candidates

Another case of a plural subject: make it "their non-selected candidates" (with hyphen).

In the second paragraph, "respectively" needs commas around it (one before, one after). "Client's" should have the apostrophe removed.  "Thus" needs a comma after it.

-- Section 6.11 --

Editorial:
   This is important as normally RTSP play mode sessions
   only contain traffic from the server to the client so the bindings in
   the NAT need to be refreshed by the client to server traffic provided
   by the STUN keep-alive.

Please add a comma after "important", and hyphens in "client-to-server".

-- Section 6.12 --

   ICE restarts may be triggered due to changes of client or server
   attachment to the network, i.e., changes to the media streams
   destination or source address or port.

Is "changes to the media streams destination or source address or port" an exhaustive list?  If not, you should change "i.e.," to "such as".

   Most RTSP parameter changes
   would not require an ICE restart; instead existing mechanisms in RTSP
   for indicating from where in the RTP stream they apply is used.

I can't parse this; it looks like there's a word missing, or maybe it's just that the structure is awkward.  Maybe this?:

NEW
   Most RTSP parameter changes
   would not require an ICE restart, but would use existing mechanisms
   in RTSP to indicate from what point in the RTP stream they apply.
END

OLD
   Even if otherwise not supporting SETUP during PLAY state for other
   purposes, the server SHALL support SETUP requests in PLAY state, as
   long as the SETUP changes only the ICE parameters, which are: ICE-
   Password, ICE-ufrag and the content of ICE candidates.
NEW
   Even if the server does not normally support SETUP during PLAY state,
   it SHALL support SETUP requests in PLAY state for the purpose of
   changing only the ICE parameters, which are ICE-Password, ICE-ufrag,
   and the content of ICE candidates.
END

-- Section 7.1 --
A "media-handling proxy" needs a hyphen.

-- Section 7.2 --
Editorial:
"Signalling-only proxy" should have that hyphen in there.  Please add it in the three places that "signalling only" appears.  Also, the first paragraph has the two words not capitalized, and the second paragraph has them both capitalized.  Please pick one way or the other.  (The section title should leave both words capitalized, regardless, because that's what we do in section titles.)

-- Section 7.3 --
Editorial:
A "media-handling proxy" needs a hyphen; "non-media-handling proxy" needs two hyphens.  There are several instances of both in this section.

-- Section 11.1 --

   To protect against the attacks in ICE-based on signalling information
   RTSP signalling SHOULD be protected using TLS to prevent
   eavesdropping and modification of information.

This doesn't parse.  I think you mean this (but tweak it differently if I didn't get it quite right):

NEW
   To protect against attacks on ICE based on signalling information,
   RTSP signalling SHOULD be protected using TLS to prevent
   eavesdropping and modification of information.
END

Yet more variation on ice that said it appears to be suitable to task.

If someone wanted to write a draft that stated that the IETF is sufficiently bad at nat traversal that it should not be pursued here I would totally sponsor that.

Thank you for adding the text on logging!

Two issues in the IANA section, otherwise a fine document:

   150:  The 150 response code indicates that ICE connectivity checks
      are still in progress and haven't concluded.  This response SHALL
      be sent within 200 milliseconds of receiving a PLAY request that
      currently can't be fulfilled because ICE connectivity checks are
      still running.  Subsequently, every 3 seconds after the previous
      sent one, a 150 reply SHALL be sent until the ICE connectivity

I am not sure why the IANA section contains normative language that is important for the implementer, e.g., the above 200 milliseconds.  Further the status codes of RTSP 2.0 have usually a short text description called "Directionalty" (which is odd by the way in the registry). 


   480:  The 480 response code is used in cases when the request can't
      be fulfilled due to a failure in the ICE processing, such as all
      the connectivity checks have timed out.  This error message can
      appear either in response to a SETUP request to indicate that no
      candidate pair can be constructed or to a PLAY request that the
      server's connectivity checks resulted in failure.

Can't this be shortened to a truly meaningful error description? And also saying somewhere in the text when this error code should be used, instead of having this in the IANA considerations?

- I slightly disagree with Kathleen's discuss. NAT logs within
enterprises are useful for n/w management reasons, but I'm not
sure that extends to public Internet cases at all, where such
logs will be huge and potentially quite privacy invasive.
(Yes, they maybe required by local legislation but that's
different and not really our concern.) I'd hope that text
resolving the discuss properly takes both aspects into
account. Privacy and log flushing does get a mention but I'd
emphasise it more and maybe consider that difference between
enterprise and public Internet use-cases.

- I don't recall RTSP 2.0 security well enough, but was a bit
surprised to not see any mention of (D)TLS for media content
here. Can you say why that's not needed?