[MMUSIC] Gen-art last call review: draft-ietf-mmusic-rfc2326bis-34

[Robert Sparks <[e-mail]>]

I am the assigned Gen-ART reviewer for this draft. For background on 
Gen-ART, please see the FAQ at

<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments 
you may receive.

Document: draft-ietf-mmusic-rfc2326bis-34
Reviewer: Robert Sparks
Review Date: 05-Jun-2013
IETF LC End Date: 04-Jun-2013
IESG Telechat date: not yet on a telechat

Summary: This draft is on the right track but has open issues, described 
in the review.

I have not reviewed this document at the level of detail I prefer for 
Gen-art reviews, but have tried to be thorough in the sections I have 
reviewed.

In particular,
I didn't verify the tables and the text agree
I didn't carefully check the ABNF
I haven't thought about possible edge cases and race conditions as much 
as I would have liked
I didn't closely review the security mechanisms in section 19 or the 
specialized MIKEY keying mechanism in the appendix - both need careful 
review from security experts.

One observation I would like to make before calling out issues and nits:

The document is very long, and the structure is unusual - much of the 
definition of the protocol itself is in the appendices. You are missing 
an opportunity to make this document much shorter (thereby likely 
increasing its effectiveness). Much of the jump in from RFC2326 was 
importing the description of headers and responses from HTTP, tailoring 
them to RTSP. That was a good exercise in that it highlights some issues 
that would otherwise be non-obvious (and raises a few questions below). 
But you followed the style from HTTP perhaps too closely - much shorter 
descriptions without examples might have done the job better. Overall, 
separating exposition and examples from the protocol definition would 
make it much easier for an implementer to find the definition of the 
protocol, and use the document as a reference when diagnosing a failure 
to interoperate.

Major Issues

I'm not seeing what instructs an RTSP element on how to find the server 
it would try to open a connection to given an rtsp or rtsps URI? Are you 
assuming it would be doing A or AAAA DNS lookups? Should this protocol 
use NAPTR/SRV? The document should be explicit. On a related note, (and 
maybe I missed this), but where do you talk about what an element should 
check in the server's certificate when connecting over TLS? Are you 
assuming a common name match? Or are you expecting some SubjectAltName 
processing?

The document should say more about when connection reuse is appropriate, 
particularly when the connections happened because of an rtsps uri. Two 
different names might resolve to the same IP address - reusing a 
connection formed when looking at the first name (and verifying the 
server's cert) is dangerous. A new connection needs to be formed (and 
verified) instead.

The text talks about the option to queue S->C requests if there isn't a 
connection to the client available. Ostensibly, this means the request 
can go down some future connection. It's not clear how the server can 
tell the right client connected. In particular, when using rtsps, how do 
you prevent a malicious client from getting such a queued message that 
wasn't meant for him?

Given that the text talks about queuing S-C requests, it should 
explicitly call out whether a server can queue a response if the 
connection the associated request arrived on is no longer available. I 
think what you want to say is that such a response must not be queued, 
and must be dropped.

There are several places in the text that talk about using a 503 
response with a Retry-After header to push back on traffic from an 
element (the first is section 10.7).
* It's not clear what the subject of a 503 is. Is it intended to be 
scoped to requests to the resource in the RURI of the associated 
request? Is it intended to be scoped to the domain in that resource? Or 
is it, like in SIP, intended to be scoped to the server issuing the 
response, independent of what the RURI contained?
* If the intent is that the 503 talks about the server, then you should 
clarify that a proxy doesn't simply forward 503 responses (after 
rewriting CSeqs), and should probably have a response of its own. 
Otherwise, clients that might be talking to two different servers 
through one proxy would lose access to both of them when one of the 
servers 503ed.

Section 4.9.1 lists values the Seek-Style header can take, but 18.45 
lists a completely different set (which most of the rest of the document 
uses). Should 4.9.1 be changed to use the values in 18.45? Are the right 
strings being used in sections 13.4.4 through 13.4.6? Those appear to be 
using strings from 4.9.1.

The document will not stand if you delete some of the appendices. They 
aren't supplementary material. Please consider restructuring the 
normative sections back into the main document, or clearly identifying 
which ones define protocol and which are background information.

Section 15 talks about a "transparent' proxy, but there is no 
description in this document of what protocol such a thing should 
follow. What bad thing happens if you remove all mention of 
"transparent" proxies from the document? As far as I can tell, that 
won't affect the protocol definition at all.

The list of steps for establishing an SRTP cryptographic context in 
C.1.4.1 says "This specification does not specify an explicit method for 
indicating this SRTP cryptographic context establishment method, but 
future specifications may." in the context of looking at the SDP. SDP 
_has_ methods of indicating what keying mechanism is used with SRTP - 
are you talking about something different? Why doesn't this section say 
something like "the use of SRTP with RTSP is only defined with MIKEY 
with keys established as defined in this section. Future documents may 
talk about how an RTSP implementation might treat SDP that indicates 
some other key mechanism be used"? Could you provide an example in the 
document of an RTSP message carrying SDP reflecting the use of SRTP as 
defined in this document?


Minor Issues

The document uses the notion of a "control connection" (it appears first 
in section 2.5) without defining what that is, or what kind of 
connections you might have that aren't control connections.

The update to the registration of the rtsp and rtsps schema call out 
that there are changes that can result in interoperability issues. It 
doesn't say what the issues are, or who might encounter them. I can 
infer that the point is that there might be problems if a URI following 
this updated registration were to be processed by an RTSP 1.0 
implementation (or any other application that relied on the previous 
definition). It would be better to say that explicitly, and talk about 
how a previous implementation is likely to act when presented with a URI 
that exercised these new changes. (It's not clear to me that all of the 
thread at www.ietf.org—msg01567.html 
<http://www.ietf.org/mail-archive/web/uri-review/current/msg01567.html> 
concluded - I see how the fragments question ended, but what about 
things like the addition of IPV6 literals? In particular, I'm not 
finding a response where Ted Hardie agreed that the potential for 
interoperability failure had been adequately addressed).


I think you've said something different than you mean in 5.4 item 1. I 
think you mean to say that an RTSP implementation would ignore any body 
that happened to come in message that MUST NOT contain one. What you've 
said is that the part of the parser that's doing framing has to stop 
framing at the end of the headers, and that such an errant body would be 
parsed as a separate message. As written this would keep someone from 
using an Internet Message Format parser since it would have to ignore 
any Content-Length that might have appeared, even though it wasn't 
supposed to.

In section 10.4, it looks like a server can keep an RTSP transaction 
pending forever if it sends 100 responses often enough. I assume the 
client's recourse if it doesn't want to remain involved is to close the 
connection. If that's right, it's worth calling it out explicitly in 
this section.

Somewhere, probably in section 15.2, you should be explicit that a proxy 
that is multiplexing requests MUST keep the requests in the same order 
as they are folded together. You can infer that, or things simply break, 
but saying it explicitly would be better.

Both GET_PARAMETER and SET_PARAMETER are listed as keep-alive methods in 
10.5, but the note in section 13 on table 7 only uses that as a reason 
for requiring SET_PARAMETER. Why doesn't it also apply to GET_PARAMETER? 
And why is this only required at servers? "Required" in this section 
means Mandatory to Implement, I think. If that's right, it should be 
made explicit. If that's not right, what does it mean?

Section 13.8 makes a good argument for always putting parameters to be 
retrieved in a body. Why are you keeping the complexity of also allowing 
them in headers?

Section 13.10: "That should not provide any benefit." is not clear - can 
you expand it a little?

In Section 13.10 you talk about clients _sending_ media (search for 
"send or receive media"). Do you mean anything more than possibly RTCP? 
Can you make that clear in the section?

Section 15.2's second paragraph appears to be the only place the proxy 
rewriting of CSeq in order to multiplex requests is defined, and it is 
very loose. It would be easy for a reader to fail to recognize this as 
an interoperability requirement. Please consider expanding how this is 
specified when addressing the comment about above about keeping requests 
in relative order when multiplexing.

You follow the principle of saying a client should treat a response with 
an unknown response code, the same as it would treat the x00 of that 
class (e.g. 400 for an unknown 4xx). However, you do not define what a 
response code of 300 means. How is a client supposed to handle an 
unknown 3xx response?

As written, a client and server can use HTTP Basic authentication over 
TCP that is not protected with TLS. Consider restricting it's use to TLS 
only. Are you sure you don't need to say anything RTSP specific about 
DIGEST computations (I don't think you need to go as far as SIP did 
talking about DIGEST, but I'm surprised you haven't run into issues 
relying only on 2617 literally.


How does 411 Length Required for this protocol make sense? Given that 
you've restricted the protocol to TCP and require the Content-Length 
header to be present if there is a body, in what circumstance would a 
server need to send a 411?


Section 18.19 requires senders to increment CSeq by 1. We have a 
reliable transport with no request retransmission, so there should never 
be gaps at the receiver. Should the receiver check and react some way if 
there are gaps? I think you should explicitly tell them not to even 
look. If you agree, why is incrementing by one important as long as you 
are always strictly increasing?

You call out several places where RTCP is essential to allow RTSP using 
RTP to work correctly, yet section C.1.2 sets up conditions where RTCP 
MUST NOT be sent. Why are you allowing those instead of treating the 
conditions you describe as errors?


Nits


The definition of Message still talks about connectionless transports

22.14 says "These URI schemes are defined in existing registries which 
are not created by RTSP." Should it say that they are _registered_ 
rather than defined, and "not created by this document" instead of by 
RTSP? In section 4.2 in the paragraph discussing ports 554 and 322, why 
is the language different for rtsp and rtsps? "MUST be used" vs "is 
registered and MUST be assumed"?

Consider a reference in 4.4 to where SMPTE 30 drop is defined, 
particularly for where the 1/100th frame measurement comes from.

Section 4.5 introduces NPT as indicating "the absolute position" and 
then defines something that can carry a point or a range. I suggest 
adjusting the first sentence to allow for a range.

Section 4.9.1's description of Conditional Random Access contains some 
ambiguity. It would help be more explicit. I think it's trying to say 
"would move the client's play point further away from the requested play 
point than it's play point currently is."?

Section 5 : what do you mean by "tricky" switching?

Section 7: consider octets instead of bytes

Section 10.3: what is "take appropriate action" trying to say?

The document mixes using "3rr" and "3xx" to talk about redirect 
responses. Why are there two ways to say the same thing?

(Total nit, but) The examples show dates in 1997, when there could be no 
RTSP/2.0. Are there other aspects of the examples that might not have 
been updated?

Is "will need to use" at the bottom of page 85 in section 13.5.2 trying 
to say something that should be said with 2119?

Consider saying that a client must not put a Terminate-Reason in a C->S 
Teardown request, or tweak the definition of Terminate-Reason to discuss 
client use and the possibility of C->S specific values.

These two sentences in 13.7.1 are particularly hard to parse together: 
"The time period is considered extended when it is 10 times the Session 
Timeout period. Consideration of the application of the server and its 
content should be performed when configuring what is considered as 
extended period of time." I suspect they were not written at the same 
time? Can you simplify what they are trying to say, noting that the 
suggested "10 times" is a default, and that if you want to change that 
default, you need to consider the context?

Section 14: "Interleaved binary data SHOULD only be used if RTSP is 
carried over TCP". Is this leftover from when UDP was an option?

(small nit) Section 14, you mention an ASCII dollar sign - maybe you 
should just say an octet with value 0x24?

It's not clear how far up to go with "upper-layer protocol headers" in 
section 14, given that you are trying to speak generally about carrying 
things in interleaved data. Your example for RTP may cover the only case 
that matters. Consider speaking more normatively (instead of with an 
example) for RTP and provide some guidelines about where to cut the 
prefixes for other things you might want to carry (lest you end up with 
the v6 headers too when tunneling some new foo protocol?)

In section 17, you say "Status codes that have the same meaning are not 
repeated here." But you did repeat them (see 200 OK). Should that 
sentence be deleted?

304 (section 17.3.4) probably should have been a different class 
response, maybe even a 2xx. The text does call it out as "unusual" for 
the 3xx, but it would be better to say why this ended up in this 
response class.

I might be confused, but I'm not sure the redirection semantics defined 
in this document lead to the "black hole" case discussed in 17.4.14. If 
I'm confused, could you send me an example of such a redirection black 
hole happening?

Section 18.11 first paragraph should be split into two sentences.

Is "MUST be transported over TCP" in C.2.1 stale now that no 
connectionless transport is defined?

The end of Appendix G still implies that CSeq was primarily for 
unreliable transports. There are other places that out and point to 
proxy multiplexing. All of them should acknowledge that CSeq is 
necessary for associating responses with requests.