Re: WGLC of msrp-acm and msrp-sessionmatch - Adam's comments
Christer Holmberg <christer.holmberg <at> ericsson.com>
2010-01-11 13:48:34 GMT
Hi Adam,
My appologies also to you for the delay of the reply, due to my xmas vacation.
Comments inline.
>Just for clarification: the fact that we're last calling
>these at the same time doesn't mean that we intend them to
>progress as a cluster
>(http://www.rfc-editor.org/cluster_def.html), does it? There
>is no normative dependency between them, and I'd hate to see
>them both be delayed due to issues related to only one of
>them (e.g., in IETF last call, or IESG review).
See Ben's reply.
----------
> ==========
> draft-ietf-simple-msrp-acm
>
> 1. NIT: The abstract contains several acronyms that need to be
> expanded (MSRP, UA, COMEDIA) -- see section 3.6 of
> http://www.rfc-editor.org/rfc-style-guide/rfc-style
I'll fix that.
----------
> 2. NIT: In the introduction, the sentence "[RFC4975] also allows, but
> does not define, MSRP UAs to use other mechanisms in order to
> create the MSRP transport connection" lacks parallelism. Try
> "[RFC4975] also allows, but does not define, alternate
> mechanisms for MSRP UAs to create MSRP transport connections."
I'll fix that.
----------
> 3. NIT: Also in the introduction: "[RFC4145] defines a mechanism,
> COMEDIA, which endpoints can use..." should read "[RFC4145]
> defines a mechanism, COMEDIA, that endpoints can use..." (the
> phrase at the end of the sentence isn't incidental to its meaning
> -- it is the purpose of the sentence).
I'll fix that.
----------
> 4. SUBSTANTIVE: I agree with earlier comments that the first
> paragraph of section 3 is a bit confusing, as it appears to impose
> a normative requirement on all MSRP UAs. Removal seems a bit of
> overkill, though, as it leaves implementors no information about
> when ACM should be employed.
I suggested the following text in my reply to Ben:
"Support of this specification is optional for MSRP user agents in general. User Agents that are likely to
be deployed in networks where User Agents need to establish the TCP connections in order to traverse NATs
SHOULD support this specification in order to improve the odds of being able to communicate across NATs."
----------
> The second paragraph of section 3 talks about NAT traversal
> failures under certain circumstances, but provides implementors no
> guidance about how to avoid these failures. I think you need to
> add text that basically recommends that UA implementations that
> can be configured to use ACM as their NAT traversal strategy also
> MUST have the ability to use MSRP relays. (In other words:
> mandatory to implement, optional to use). Otherwise, you're
> encouraging deployment of clients that can perform NAT traversal,
> but only under fairly limited network circumstances.
Are you sure you mean section 3? There is only one paragraph.
I am not sure I understand the comment about MUST implement ability to use MSRP relays. I don't think we
should mandate ACM clients to always implement support of MSRP relays. An ACM client may not even be
located behind a NAT (they support ACM just in order to allow other clients behind NATs to be active), or
they are only used in networks where MSRP relays are not used.
We could say that clients which can be located behind MSRP relays should support MSRP relays, but doesn't
RFC 4976 already say that? It's true even if the client doesn't support ACM.
I don't think I encourage deployment of clients that can perform NAT traversal only in limited network
circumstances. The draft is about how clients can use COMEDIA, it is not a generic implementation guide or
profile for MSRP clients behind NATs :)
Or, did I missunderstand your comment?
----------
> 5. NIT: in section 4.2, "holdcon" should be spelled "holdconn" (two
> 'n's).
I'll fix that.
----------
> 6. SUBSTANTIVE: In section 4.2, I think we need an explanation in the
> document regarding why "holdconn" is normatively prohibited. It's
> not immediately clear to me why we prevent this, and I can't find
> any discussion on the SIMPLE mailing list on the topic.
I suggest to reomve the sentence, and not say anything about holdconn.
I have some difficulties in understanding how holdconn is related to TCP setup direction, but I don't think
there is anything MSRP specific about it.
----------
> 7. SUBSTANTIVE: A natural consequence of the rules in section 4.2 is
> that an _offer_ for an MSRP session must never be "passive." To
> avoid misinterpretation, this should be stated as a normative
> requirement.
You are probably right.
----------
> 8. SUBSTANTIVE: Unlike SIP, I don't think MSRP has rules that require
> recipients to ignore CRLF sequences in a TCP connection. I'm
> pretty sure that the CRLF mechanism described in section 5 for
> pinhole preservation is NOT backwards compatible. However, RFC
> 4975 does talk about NAT binding keepalives in RFC 4975, section
> 7.1: "Bodiless requests may also be used in certain applications
> to keep Network Address Translation (NAT) bindings alive, etc."
Good point.
I can re-write to:
"If an MSRP UA is located behind a NAT the UA MUST keep the NAT binding alive, in accordance with section 10 of [I.D.ietf-mmusic-
ice]. The UA SHOULD use MSRP requests without bodies as NAT keepalive messages, as defined in section 7.1 of
[ref to RFC4975]."
----------
> 9. SUBSTANTIVE: Also related to section 5: I'm certain that nothing
> in draft-ietf-mmusic-ice provides guidance here: that document
> deals with UDP, while MSRP is exclusively a TCP protocol. Perhaps
> you intended to cite draft-ietf-mmusic-ice-tcp?
Yes. I can change the reference.
> ==========
> draft-ietf-simple-msrp-sessmatch
>
> 1. NIT: The abstract contains several acronyms that need to be
> expanded (MSRP, UA, ALG) -- see section 3.6 of
> http://www.rfc-editor.org/rfc-style-guide/rfc-style
I'll fix that.
----------
> 2. NIT: The unexpanded acronym issue continues through the
> introduction: SLA, B2BUA, NAPT, ALG.
I'll fix that.
----------
> 3. SUBSTANTIVE: This document updates RFC 4975, section 7.1, which
> talks about ongoing sessions. However, to succeed, we also need to
> modify the matching procedure that takes place during session
> establishment. This is described in RFC 4975, section 5.4.
Good point.
I guess the following text:
"When the first request arrives, its To-Path header field should contain a URI that the listening element
provided in the SDP for a session. The element that accepted the connection looks up the URI in the received
request, and determines which session it matches."
...should be modified to:
"When the first request arrives, its To-Path header field should contain a URI with a session-id part that
the listening element provided in the SDP for a session. The element that accepted the connection looks up
the session-id part of the URI in the received request, and determines which session it matches."
----------
> 4. SUBSTANTIVE: In the security section, we need to talk through the
> implications of this mechanism on TLS connections. After some
> cursory thought on the topic, I think it works correctly: outbound
> connections to MSRP relays will be okay, since the cert presented
> by the relay will match the hostname the client used to connect to
> it; and, for peer-to-peer mode (as described in RFC 4975, section
> 14.4), the hostname isn't part of the information used to match
> the certificate. However, just because I think it works doesn't
> mean that it actually does. :) I'd be a lot more comfortable if we
> have a brief treatment of this issue in the security section so
> that it gets some explicit security review.
I suggest we try to get the rest of the stuff fixed first, and then we can concentrate on the security stuff.
----------
> 5. NIT: Somewhere in the document, it would probably be good to cite
> the text from RFC 4975, section 14.1, regarding MSRP security
> properties: "Most components of the URI, such as the scheme and
> the authority components, are common knowledge. The secrecy is
> entirely provided by the session-id component." This goes a long
> way towards allaying any concerns that the host portion was
> intended to provide any kind of protection when matching sessions.
I think that could be added to the security considerations sections.
Regards,
Christer
RSS Feed