Re: [MMUSIC] Benjamin Kaduk's Discuss on draft-ietf-mmusic-ice-sip-sdp-37: (with DISCUSS and COMMENT)
Benjamin Kaduk <kaduk@mit.edu> Tue, 06 August 2019 20:00 UTC
Return-Path: <kaduk@mit.edu>
X-Original-To: mmusic@ietfa.amsl.com
Delivered-To: mmusic@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id B168A12003F; Tue, 6 Aug 2019 13:00:32 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.2
X-Spam-Level:
X-Spam-Status: No, score=-4.2 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id LRu3DGEtaWB7; Tue, 6 Aug 2019 13:00:29 -0700 (PDT)
Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id C809F1200D8; Tue, 6 Aug 2019 13:00:28 -0700 (PDT)
Received: from kduck.mit.edu ([24.16.140.251]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id x76K0JYW032715 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 6 Aug 2019 16:00:21 -0400
Date: Tue, 06 Aug 2019 15:00:19 -0500
From: Benjamin Kaduk <kaduk@mit.edu>
To: Christer Holmberg <christer.holmberg@ericsson.com>
Cc: The IESG <iesg@ietf.org>, "draft-ietf-mmusic-ice-sip-sdp@ietf.org" <draft-ietf-mmusic-ice-sip-sdp@ietf.org>, "mmusic-chairs@ietf.org" <mmusic-chairs@ietf.org>, "fandreas@cisco.com" <fandreas@cisco.com>, "mmusic@ietf.org" <mmusic@ietf.org>
Message-ID: <20190806200018.GL59807@kduck.mit.edu>
References: <156505044722.2011.681165665144624888.idtracker@ietfa.amsl.com> <HE1PR07MB3161ED365902E5643690CA4493D50@HE1PR07MB3161.eurprd07.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <HE1PR07MB3161ED365902E5643690CA4493D50@HE1PR07MB3161.eurprd07.prod.outlook.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Archived-At: <https://mailarchive.ietf.org/arch/msg/mmusic/Wg2bkWglx420PoeEiOweB9SNqyQ>
Subject: Re: [MMUSIC] Benjamin Kaduk's Discuss on draft-ietf-mmusic-ice-sip-sdp-37: (with DISCUSS and COMMENT)
X-BeenThere: mmusic@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Multiparty Multimedia Session Control Working Group <mmusic.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/mmusic>, <mailto:mmusic-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/mmusic/>
List-Post: <mailto:mmusic@ietf.org>
List-Help: <mailto:mmusic-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/mmusic>, <mailto:mmusic-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 06 Aug 2019 20:00:33 -0000
On Tue, Aug 06, 2019 at 12:42:28PM +0000, Christer Holmberg wrote: > Hi Benjamin, > > Thank You for the review! Please see inline. > > ---------------------------------------------------------------------- > DISCUSS: > ---------------------------------------------------------------------- > > > A fairly minor point, but the example in Section 4.6 is not compliant with the rest of the document. > > Specifically, any implementation *of this document* must include the "ice2" ice-option in addition > > to any other option being used, per Section 3.2.1.5. > > Good catch. We'll add the 'ice2' ice-option to the example. > > --- > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > > Do we have anywhere a definition of what it means to "indicate ICE support in an SDP offer/answer"? > > (As distinct from ice2 support.) We refer to the concept in several places but there are many protocol > > fields that might be interpreted as such; is any one of them sufficient? > > I don't there is a definition. It basically means when the offer or answer contains ICE-related attributes. > > I guess we could add some text about that. > > --- > > Section 2 > > > The suggested text in RFC 8174 includes a full BCP 14 citation with both RFCs; please consider using that form. > > Ok. > > --- > > Section 3.2.1.5 > > > An agent compliant to this specification MUST include an SDP ice- > > options attribute with an "ice2" attribute value [RFC8445]. If an > > agent receives an SDP offer or answer that does not contain an SDP > > ice-options attribute with an "ice2" attribute value, the agent can > > assume that the peer is compliant to [RFC5245]. > > > > I think this can only be assumed if there is some other indication of ICE support -- stock SDP O/A does not mandate ICE, IIRC. > > Correct. > > --- > > Section 3.2.2 > > > Aren't "rtcp attribute SHOULD be included" and "rtcp attribute MAY be omitted" just duplicating existing normative requirements > > from previous specifications (which thus would not need new normative language here)? > > Correct. The SHOULD and MAY shall not be normative. The MUST will remain, though. > > --- > > Section 3.2.5 > > > implementation dependent. Informally, the responding agent MAY > > consider the mismatched transport address information as a > > > > Perhaps the capitalized "MAY" is not needed for an informatl description? > > I agree. Will fix that. > > > 2. The transport address from the peer for the default destination > > correspond to IPv4/IPv6 address values "0.0.0.0"/"::" and port > > > > What does "correspond to" mean here (and later)? > > Perhaps "corresponds" is the wrong wording. It basically means that the default destination IP address is set to 0.0.0.0 (IPv4) or :: (IPv6). > > So, perhaps using "set" or "represents" instead: That was my guess, so thank you for confirming. > "The transport address from the peer for the default destination > is set to/represents address value "0.0.0.0"/"::" and port value "9"." > > --- > > Section 3.3.1 > > > If the initial offer SHOULD contain an ice-pacing attribute, why do we not include one in the > > examples (both in Section 3.2.6 and Appendix A)? > > We will add the attribute to the examples. > > --- > > > Section 3.3.2 > > > > (ice-pacing in examples could be good for answers, too) > > Will fix. > > > To check my understanding, the requirement for transport protocol match beween m= offer/answer applies > > just to the *default* destination, i.e., the address from the c= line and the port from the m= line, and thus I > > can have a=candidate entries for both IPv4 and IPv6 for the same m= section? > > Correct. > > > Or does "In each "m=" line, the answerer MUST use the same transport protocol as was used in the offer "m=" line." > > also restrict the a=candidate attributes? (As Éric notes, IPv6 examples would go a long way.) > > No, it only applies to the "m=" line. > > (Unfortunately, for backward compatibility, we have to deal with some legacy offer/answer restrictions, including those related to the m= line values. If we would know that every device (including intermediaries) support ICE we wouldn't need the m= line for anything) > > I think Adam earlier addressed Éric's comment on having IPv6 examples. However, if including IPv6 examples would make the spec more easy to understand then I think we should include them. Ah, Adam's response had slipped my memory with the intervening IETF week :) If there's not going to be a reason in practice to do something, we may not need to provide an example of it -- do what you think is best. > --- > > Section 3.3.4 > > > If there are one or more check lists with the state set to Failed, > > the controlling agent MUST generate a subsequent offer in order to > > remove the associated data streams by setting the port value of the > > data streams to zero (Section 3.4.1.1.2), even if the peer did > > indicate support for the 'ice2' ice-option. If needed, such offer > > can also be used to align the connection address, port and transport > > protocol, as described above. > > > > It feels a little weird to me that we say "can also be used" instead of "is used", since it seems to be a MUST-level > > requirement for the next offer to normalize the address/port/protocol in the offer with those discovered via ICE. > > I agree. Will modify to "is used". > > --- > > Section 3.4.1.1 > > > The rules governing the ICE restart imply that setting the connection > > address in the "c=" line to 0.0.0.0 (for IPv4)/ :: (for IPv6) will > > cause an ICE restart. Consequently, ICE implementations MUST NOT > > utilize this mechanism for call hold, and instead MUST use > > "a=inactive" and "a=sendonly" as described in [RFC3264]. > > > > Is this really "ICE implementations" or "SDP O/A implementations supporting ICE"? > > "ICE implementations" is more common, and the draft defines the procedures for ICE implementations using SDP O/A. > > --- > > Section 3.4.1.2.2 > > > line associated with that data stream MUST match the data associated > > with the nominated pair for that data stream. In addition, the > > offerer only includes SDP candidates representing the local > > candidates of the nominated candidate pair. The offerer MUST NOT > > include any other SDP candidate attributes in the subsequent offer. > > > > Does this mean that exactly one a=candidate line must appear in the corresponding m= section? > > Correct. Do you think that needs to be more clear? It's probably okay as-is; this was mostly to double-check my understanding. > --- > > Section 3.4.1.3 > > > A lite implementation MUST NOT add additional host candidates in a > > subsequent offer. If an agent needs to offer additional candidates, > > it MUST restart ICE. Similarly, the username fragments and passwords > > MUST remain the same as used previously. If an agent needs to change > > one of these, it MUST restart ICE for that data stream. > > > > nit: This "MUST remain the same" is worded oddly, as the next sentence effectively contradicts it. > > I agree. Maybe something like this: > > "A lite implementation MUST NOT add additional host candidates in a > subsequent offer, and it MUST NOT modify the username fragments > and passwords. If an agent needs to offer additional candidates, or > modify the username fragments and password, it MUST restart ICE > for that data stream." Works for me, thanks. > --- > > >Section 3.4.3.1 > > > > o If ICE state is completed and the SDP answer conforms to > > Section 3.4.2, the agent MUST remain in the ICE completed state. > > > > It's not entirely clear what "conforms to Section 3.4.2" means, given that some situations in Section 3.4.2 effectuate ICE restart. > > Section 3.4.2.1. is about ICE restart. Indeed, and it looks like I had confused myself a bit when I wrote this. (3.4.2 toplevel does have "The agent SHOULD generate an answer for this data stream as if the remote- candidates attribute had not been present, and then restart ICE for this stream.", which is not quite the same.) > Perhaps the following would be more clear: > > "If ICE state is completed and the SDP answer conforms to > Section 3.4.2, and the answer is not associated with an > ICE restart (3.4.2.1), the agent MUST remain in the ICE completed state." I'm definitely not insisting on this change, but it might still be worth doing. > --- > > Section 3.4.3.2 > > > there as described in section 12 of [RFC8445]. The state of ICE > > processing for each data stream MUST change to Running, and the state > > of ICE processing MUST change to Running > > > > Did this sentence get cut off prematurely? > > I think the sentence is correct (perhaps it should say "ICE state" instead of "state of ICE processing" - will look into that), but there needs to be a "." at the end. Thanks for checking. It seemed to make sense if I just added the '.', but double-checking seemed prudent. > -- > > Section 4.1 > > > I appreciate that IP address privacy is mentioned here. (It might be good in the security considerations, too.) > > Will look into that. > > --- > > Section 4.2 > > > I don't really understand why there can be more than one remote-candidate for a given component. Isn't there only > > going to be one nominated pair, and thus the single remote part of the pair? > > Where do you read that there can be more than one remote-candidate for a given component? > > Note that component is not the same as m= line: a given m= line can contain 2 components - one for RTP and one for RTCP. This was user error, sorry -- I mentally put the "component-ID" element in remote-candidate-att and not remote-candidate. > --- > > Section 4.5 > > > If absent in an offer and answer the default value of the attribute > > is 50 ms, which is the recommended value specified in [RFC8445]. > > > > nit: is this "offer and answer" or "offer or answer"? > > Correct. Will fix that. > > --- > > Section 6 > > > Note that ICE is not intended for NAT traversal for SIP, which is > > assumed to be provided via another mechanism [RFC5626]. > > > > This sentence reads a bit oddly when one recalls that the full title of RFC 8445 is "Interactive Connectivity Establishment (ICE): > > A Protocol for Network Address Translator (NAT) Traversal". Perhaps the intended sentiment is more that the scheme described > > in this document for using SDP to provide an ICE usage is not the primary mechanism for NAT traversal for SIP, though if one chooses > > to use it as such, the procedures in the rest of the section are needed. > > I guess the statement is misleading. What it means that ICE is not intended for NAT traversal of the *SIP signalling*. The procedures in section 6 still apply when using SIP to negotiate ICE for data plane NAT traversal. Ah, that's a subtlety that I missed on first read. It makes sense now :) > --- > > Section 6.1.1 > > > described in [RFC3262]. Such retransmissions MUST cease on receipt > > of a STUN Binding request with transport address matching candidate > > address for one of the data streams signaled in that SDP or on > > transmission of the answer in a 2xx response. If no Binding request > > > > nit: I think "candidate address" needs an article. > > I guess "transport address" also needs one? It can take one, yes, though my native-speaker filter would apparently be willing to accept it without one, too. > Something like: > > "with a transport address matching the candidate address for..." > > > the session terminated. For the ICE lite peers, the agent MUST cease > > retransmitting the 18x after sending it four times since there will > > be no Binding request sent and the number four is arbitrarily chosen > > to limit the number of 18x retransmits (poor man's version of > > [RFC3262] basically). (ICE will actually work even if the peer never > > > > nit: the tone of the parenthetical is rather distinct from conventional RFC style. > > Unless the other authors want to keep/modify it, I suggest to remove all text after "...limit the number of 18x retransmits. I don't remember if we imply anywhere else that the 18x helps middleboxes, but no real objection from here. > The relation to RFC 3262 is already described earlier in the section. > > --- > > Section 6.4 > > > Indeed, an agent SHOULD NOT indicate that QoS preconditions have been > > met until the checks have completed and selected the candidate pairs > > to be used for media. > > > > Does this include the updated offer/answer exchange having completed? > > I am not sure I understand what you mean by "offer/answer exchange having completed". The selection of candidate pairs is an ICE-level operation, and we can start using them for media once ICE has completed. There's also a partial requirement in this document to do an updated offer/answer exchange that records the result of ICE in the m= and c= lines (basically, for the benefit of middleboxes). With the current text, I don't know if there's any interaction between this offer/answer update and the indicated checks. (Reading it again, I'm not sure there actually is such an interaction -- sorry if I was just confused!) > --- > > Section 8 > > > I think this top-level section would be a great place to reiterate that the SDP and ICE security considerations apply, since > > we are using both of them in combination. Specifically, the IP Address Privacy concerns are only briefly mentioned > > elsewhere in the document, and could be worth reiterating. > > I am not sure whether we need to mention the SDP security considerations, because they are implicit via the SDP O/A security considerations that we reference in section 8.1. But, they also apply to other sub sections, so I agree we can mention them on the top-level. > > But, maybe something like: > > "The generic ICE security considerations are defined in (RFC8445), and the generic SDP offer/answer security considerations are defined in (RFC3264). The security considerations also apply to implementations of this document." That works for me; thanks! -Ben > --- > > Section 11.2 > > > draft-ietf-ice-pac has to be normative, since it is an OPTIONAL protocol feature (per https://www6.ietf.org/iesg/statement/normative-informative.html). > > We will make it normative. > > (Previously ice-pac was only mentioned in a note, so I guess that is the reason why the reference was only informative.) > > --- > > Regards, > > Christer >
- [MMUSIC] Benjamin Kaduk's Discuss on draft-ietf-m… Benjamin Kaduk via Datatracker
- Re: [MMUSIC] Benjamin Kaduk's Discuss on draft-ie… Christer Holmberg
- Re: [MMUSIC] Benjamin Kaduk's Discuss on draft-ie… Roman Shpount
- Re: [MMUSIC] Benjamin Kaduk's Discuss on draft-ie… Benjamin Kaduk
- Re: [MMUSIC] Benjamin Kaduk's Discuss on draft-ie… Benjamin Kaduk
- Re: [MMUSIC] Benjamin Kaduk's Discuss on draft-ie… Christer Holmberg