Re: [MMUSIC] Benjamin Kaduk's Discuss on draft-ietf-mmusic-ice-sip-sdp-37: (with DISCUSS and COMMENT)

[Christer Holmberg <[e-mail]>]

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:

       "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.

---

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?

---

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."

---

>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.

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."

---

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.

--

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.

---

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.

---

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?

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.

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".

---

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."

---

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