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

[Robert Sparks <[e-mail]>]

Hi Magnus -

Thanks for your careful treatment of these comments - I think you're on 
a good path with all the suggestions below.
Responses to questions and a few comments inline:

On 6/18/13 6:42 AM, Magnus Westerlund wrote:
> Hi Robert,
>
> Thanks for the massive review. A number of really good comments in here.
> Below you will find my answers, comments, questions and in most cases
> clarifications what I see us doing with your comment. These covers your
> major and minor comments. The nits we will simply implement as we see
> fit, if we have questions we will comeback with those.
>
> WG, there are a number of issues in here that would be greatly helped by
> your input!
>
> On 2013-06-05 23:56, Robert Sparks wrote:
>> 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
>>
>>
>> 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.
> Agree, it would be differently structure if we wrote it from scratch
> today. But it is an document that is 10 years in the making with dual
> heritage in RFC 2326 and RFC 2068.
>
>> 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?
> Yes, using A or AAAA DNS records are assumed. No problem making that
> explicit.
>
> Should this protocol
>> use NAPTR/SRV?
> Potentially, but as everyone have been using A records for all these
> years, I think it is not worth defining it at this stage.
>
> 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?
> This draft says in Section 19.2 the following regarding this:
>
>     RTSP MUST follow the same guidelines with
>     regards to TLS [RFC5246] usage as specified for HTTP, see [RFC2818].
>
> Where section 3.1 (Server Identity) of RFC 2818 contains the following:
>
>     If a subjectAltName extension of type dNSName is present, that MUST
>     be used as the identity. Otherwise, the (most specific) Common Name
>     field in the Subject field of the certificate MUST be used.
>
> Isn't that clear enough on that matter?
Yes - I was worried that I might have missed it before.
>
>> 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.
> I want to check my understanding of the issue you are seeing here. So a
> client looks ups foo.example.org, gets an ip address A and establishes
> an TCP/TLS connection and verifies that there are a SubjectAltName that
> matches foo.example.org works. Then client is going to open
> bar.example.com and looks up that address and gets the same IP address A.
> Client matches A against existing connections and simply sends a request to
> bar.example.com. Thus bypassing the certificate verification.
>
> If we clarify that in the process of opening rtsps://bar.example.com the
> client MUST verify that the server certificate for the TLS connection it
> considers re-using has an SubjectAltName of bar.example.com, does that
> resolve your concerns. If not, please explain the concern further.
You understood what I intended, and your resolution looks sufficient.
>
>
>> 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?
>
> Okay, this security issue I have totally missed but it is clearly
> serious. I don't see any easy way of fixing this. Thus I would suggest
> that we simply make it clear that a server MUST NOT queue the S->C RTSP
> message, independent if they are requests PLAY_NOTIFY and TEARDOWN or
> responses to client requests.
Works for me. Does this mean you are removing cross-connection queueing 
altogether?
>
>
>> 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.
> Agreed, lets mandate that they are 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?
> Very good question. It is far from obvious what the best choice is. And
> I think even talking only about URI related scope may not be the best.
>
> The case I have in mind is if a proxy that has several already
> established RTSP sessions over a persistent connection. When a client
> connected to the proxy requests to establish a new session the server
> may have insufficient resource to handle the SETUP request and send a
> 503 answer. However if another client already having an RTSP session for
> the same resource want to terminate that session it would be strange to
> refuse to process the message as it will actually free up resources.
>
> This makes me believe that you need to have a RTSP Session scope for any
> request carrying a Session-ID and a general server, or host name scope
> for non-associated that doesn't prevent established sessions to try.
>
> Thoughts on this?
That seems like it might work - exploring the idea should get some
WG cycles.
>
>
>> * 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.
> If we us a host and session scope then this issue would not be present.
> However, if the proxy is actually overloaded that still needs its own
> 552 Proxy Unavailable response with the same scope rules as for the 503,
> but where unassociated requests concerns sending to the proxy at all.
If you build one of these, please be sure to highlight the impact of the
other sessions already established, and call out the potential for
problems if you forward one as a Proxy, as opposed to generating your
own based on the situation.
>
>> 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.
> Yes, clearly an inconsistency issue that needs to be resolved.
>
>> 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.
> I would like to characterize this differently. I think you will have a
> working protocol of RTSP, but you can't use it for anything. This as you
> will have no media transport (C), no media description protocol (D), and
> no parameters format (F). But, these appendixes are replaceable with
> other protocol solutions.
>
> The Introduction section (1) says:
>
>                                   The appendixes describe and define some
>     functionalities that are not part of the core RTSP specification, but
>     which are still important to enable some usages.  Among them, the RTP
>     usage is defined in Appendix C and the SDP usage with RTSP is defined
>     in Appendix D, which are two mandatory appendixes.
>
> I note that we should add F to this list.
>
> I think we should be clear in each of this appendices that if one
> support Foo for the purpose of bar then this appendix defines
> normatively how one does it.
Makes my head swim, but maybe it's must me.

fwiw - I'm still worried that there are information in Appendix B that's 
not captured in the main text, but I can't find easy things to point to. 
Elwyn seems more comfortable that everything is covered.
>
>
>> 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.
> Not really, there is no definition of transparent proxies currently.
> There is one security concern that make require some word smithing if we
> remove transparent proxies.
>
> >From my perspective a transparent proxy is something that hijacks a TCP
> connection towards an RTSP server to be a proxy between client and
> server. Further is not detectable from the client's or the servers
> perspective, but still may modify RTSP messages within the bounds of the
> RTSP protocol.
>
> But, in reality as we have proxies that are visible we should focus on
> those and forget this bastard version.
>
>> 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?
> I think you are considering the methods for keying SRTP that are
> Offer/Answer dependent. The only RTSP SDP using method that exist is the
> one defined in RFC 4567, which we are explicit on not using.
>
> 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"?
> Yes, that is appears better, but we will have to make it clear that is
> is regarding RTSP 2.0. Must also think if RFC 4567 needs to be worked
> into that sentence.
>
> Could you provide an example in the
>> document of an RTSP message carrying SDP reflecting the use of SRTP as
>> defined in this document?
> The point is that the only difference is that he RTP profile is SAVP/RTP
> or SAVPF/RTP in the SDP. Thus making such an SDP isn't difficult.
> Question is if it provides a benefit?
I think it would be worth the extra space it would take up.
>
>
>
>> 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 "control connection" is the connection established for sending RTSP
> messages on it. I agree it is undefined and could likely be replaced by
> the "RTSP connection" or "connection for RTSP messages"
>
>> 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.
> I have gotten these comment elsewhere and have prepared a text update
> for section 4.2:
>
>     The details of the syntax of "rtsp" and "rtsps" URIs has been changed
>     from RTSP 1.0.  These changes are:
>
>     o  Support for IPV6 literal in host part and future IP literals
>        through RFC 3986 defined mechanism.
>
>     o  A new relative format to use in the RTSP protocol elements that is
>        not required to start with "/".
>
>     Neither should have any significant impact on interoperability.  If
>     one is required to use IPv6 literals to reach an RTSP server, then
>     that RTSP server must be IPv6 capable, and RTSP 1.0 is not a fully
>     IPv6 capable protocol.  If an RTSP 1.0 client attempts to process the
>     URI it will not match the allowed syntax and be considered invalid
>     and processing will be stopped.  This is clearly a failure to reach
>     the resource, however it is not a signification issue as RTSP 2.0
>     support was needed anyway in both server and client.  Thus failure
>     will only occur in a later step when there is a RTSP version
>     mismatch between client and server.  The second change will only
>     occur in RTSP messages, that are explicitly identified as being RTSP
>     2.0 messages, thus a RTSP 1.0 only agent will not be required to
>     parse this variant.
>
> Clear enough?
Maybe. In the last sentence, are you saying you would never see a relative
URL in a request URI? If you're not already saying that explicitly in 
the document
(you might be) please consider doing so. Saying it that way in this text 
would
also help.
>
> The above is Edited In.
>
>   (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 talked to him in person about this just after the discussion to better
> understand his comments. If we want a confirmation I think we need to
> loop him into a discussion regarding this particular issue.
I suggest the shepherd/AD double-check with him if he doesn't jump into 
this thread before.
>
> I provided a clarification to the designated URI reviewer which accepted
> the request.
>
>>
>> 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.
> Yes, it is clearly sufficient to ignore the body, and likely much better
> from an error handling consideration.
>
> We will propose a text change.
>
>
>> 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.
> This is likely a good suggestion, and works fine if the client simply
> wants to abort the request and is anyway abandoning the RTSP session.
> However, the question is what to do if just the request becomes
> irrelevant and instead want to do something else with the RTSP session.
> A single client may abandon the connection, open a new one and issue a
> request, to a potentially undetermined session state. Yet another
> complexity is what a proxy does that has multiple RTSP sessions on the
> same server. Here the rules for processing in order the different
> requests and their relation to answers in the same order.
>
> This makes me want to weaken the recommendation on re-using an
> connection for multiple RTSP sessions.
Are there also considerations here wrt pipelining?
>
>
>> 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.
> Probably, as a clarification to what is already present in section 18.19
> (CSeq header)
>
>     Proxies that aggregate several sessions on the same transport will
>     have to ensure that the requests sent towards a particular server
>     have a joint sequence number space, i.e., they will regularly need to
>     renumber the CSeq header field in requests (from proxy to server) and
>     responses (from server to proxy) to fulfill the rules for the header.
>     The proxy MUST increase the CSeq by one for each request it
>     transmits, without regard of different sessions.
>
>
>> 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?
> Yes, required is Required to implement in the server. The GET_PARAMETER
> is not required as only one method for keep-alive is necessary. Thus,
> the ones that needs to use GET/SET_PARAMETER for keep-alive will have to
> choose SET_PARAMETER.
>
> It is only required at the server, as it is in the client implementation
> choice of how to ensure keep-alive works. As this is also dependent on
> the media transport support one has, the client may not need a method
> based keep-alive.
>
> Clarifying "Required" should be straightforward.
>
>> 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?
> Some arguments are probably lost in the passage of time. However, the
> headers specified to be suitable for inclusion in a GET_PARAMETER
> requests are used for providing these information also in other methods,
> thus we have not wanted to duplicate the representation of this
> information in both a body and a header form.
>
>> Section 13.10: "That should not provide any benefit." is not clear - can
>> you expand it a little?
>     If the REDIRECT request times out following the rules in Section 10.4
>     the server MAY terminate the session or transport connection that
>     would be redirected by the request.  This is a safeguard against
>     misbehaving clients that refuse to respond to a REDIRECT request.
>     That should not provide any benefit.
>
> The intention here is to make clear that there shouldn't be an incentive
> in the protocol to not acknowledge a REDIRECT request and instead simply
> go on as it wasn't received or seen. Thus, we want to make clear that
> the server may simply terminate the session in those cases.
>
> I propose we change this sentence to:
>
> Thus, removing any incentive to not acknowledge the reception of a
> REDIRECT request.
>
> Edited In
>
>> 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?
> You mean this sentence:
>
>     After a REDIRECT request has been processed, a client that wants to
>     continue to send or receive media for the resource identified by the
>     Request-URI will have to establish a new session with the designated
>     host.
>
> The mentioning of send media is likely a RECORD reference. In this case
> I don't see any issue with removing it.
>
> Edited In
>
>> 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.
> There are text in Section 18.19 CSeq also. But I do think I agree that
> this should be clarified.
>
>> 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?
> Sorry, I don't quite get this comment. Section 17.3 says:
>
>     If a 3rr response is received for a request in relation to
>     an established session, the client SHOULD send a TEARDOWN request for
>     the session, and MAY reestablish the session using the resource
>     indicated by the Location.
>
>     If the Location header is used in a response it MUST contain an
>     absolute URI pointing out the media resource the client is redirected
>     to, the URI MUST NOT only contain the host name.
>
> As explained above unknown 3XX codes will be 3rr codes, as 304 isn't
> unknown.
>
> If you don't believe this is sufficient can you please be more explicit
> about the issue?
Sure. It might be quickest if you look at the table of contents.
Note that you define 100 in 17.1.1, 200 in 17.2.1, 400 in 17.4.1, 500 in 
17.5.1, but you do not define 300 anywhere - you jump straight to 301.
Please add a section defining it, or declaring that you will never see a 
literal 300 on the wire, but if you did you would do (what you quote 
above) and nothing else.

>
>> 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.
> Are you referring to Section 19.1 here?
Yes
>
> Yes, I would not have any issue with requiring Basic to be combined with
> TLS. I think that is appropriate.
>
> When it comes to DIGEST, are you referring to section 22.4 in RFC 3261?
Yes
>
> That section indicates that some clarifications are likely needed.
>
>>
>> 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?
> It really should occur for malformed messages, which we could use 400 as
> response. So yes, 411 could be removed.
>
>>
>> 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?
> I agree that for TCP or other reliable transports the receiver
> SHOULD/SHALL ignore gaps. For any future specification of an unreliable
> transport of RTSP messages this would be needed. As noted in Appendix G,
> CSeq is retained for future support of unreliable transport.
>
>> 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?
> Good question. I think it is a case we must ensure that you can
> negotiate and configure your desired way, without really considering if
> it should be allowed at all. I think mandating usage of RTCP with RTP
> for RTSP 2.0 would be good. But, I think that needs to be verified with
> the WG.
Ack
>
> cheers
>
>
> Magnus Westerlund
>
> ----------------------------------------------------------------------
> Multimedia Technologies, Ericsson Research EAB/TVM
> ----------------------------------------------------------------------
> Ericsson AB                | Phone  [phone]
> Färögatan 6                | Mobile [phone]
> SE-164 80 Stockholm, Sweden| mailto: [e-mail]
> ----------------------------------------------------------------------
>