Subject: Re: [Ice] WGLC for draft-ietf-ice-rfc5245bis-12 - Peter S-A technical comments

From: Christer Holmberg <[email-masked]>
Date: Thu, 5 Oct 2017 07:35:01 +0000

Hi Peter,

Thanks for your comments! Please see inline.

>I've sent a number of editorial suggestions to the authors. I have only
>a few technical comments.
>
>§4.3 says:
>
>   The exchange of parameters is symmetric; both agents need to send the
>   same set of attributes as defined above.
>
>What happens if the agents don't follow that advice? Do they need to
>terminate ICE processing or return an error?

I am not even sure the text above is true. Why would both agents need to
send the same set? An ICE lite implementation for sure won¹t do it.

----

>§4.4 says:
>
>   Certain middleboxes, such as ALGs, may alter the ICE candidate
>   information that breaks ICE.  If the using protocol is vulnerable to
>   this kind of changes, called ICE mismatch, the responding agent needs
>   to detect this and signal this back to the initiating agent.  The
>   details on whether this is needed and how it is done is defined by
>   the usage specifications.  One exception to the above is that an
>   initiating agent would never indicate ICE mismatch.
>
>In general, how can the responding agent determine that such breakage
>has occurred if it doesn't know what the initiating agent sent? And why
>wouldn't the initiating agent also have some responsibility for
>detecting ICE mismatch in the other direction (i.e., from the responding
>agent to the initiating agent)?

First, I think the section title is misleading. It¹s not about verifying
ICE support.

Could we simply remove the whole section?

Then, if we do that, we should also remove the 3(?) other occurrences of
³ICE mismatch².

----

>§6.1.3 says:
>
>   The controlling ICE agent MUST include the ICE-CONTROLLED attribute
>   in a Binding request.  The controlled ICE agent MUST include the ICE-
>   CONTROLLING attribute in a Binding request.
>
>However, §6.2.5.1 says:
>
>   If the Binding request generates a 487 (Role Conflict) error
>   response, and if the ICE agent included an ICE-CONTROLLED attribute
>   in the request, the agent MUST switch to the controlling role.  If
>   the ICE agent included an ICE-CONTROLLING attribute in the request,
>   the agent MUST switch to the controlled role.
>
>The combination of these two paragraphs seems to say that the
>controlling agent would have included an ICE-CONTROLLED attribute in its
>Binding request, and if there is a role conflict then the controlling
>agent must switch to the controlling role [sic] - likewise, that the
>controlled agent would have included an ICE-CONTROLLING attribute in its
>Binding request, and if there is a role conflict then the controlled
>agent must switch to the controlled role [sic]. Am I missing something
>obvious? Is there an error in §6.1.3?

Not sure I follow. Need to read more.

Regards,

Christer

---

From: Peter Saint-Andre <[email-masked]>
Date: Thu, 5 Oct 2017 08:34:28 -0600

> send the same set? An ICE lite implementation for sure won¹t do it.

Good point. In fact, the text also says:

      Related Address and Port:  The related IP address and port of the
         candidate.  These MAY be omitted or set to invalid values if
         the agent does not want to reveal them, e.g., for privacy
         reasons.

So I'd agree that we can remove the symmetric requirement.

> Could we simply remove the whole section?

That seems fine.

> Not sure I follow. Need to read more.

Unless I'm missing something obvious, I think that ICE-CONTROLLED and
ICE-CONTROLLING need to be swapped in §6.1.3. That is, it should say:

   The controlling ICE agent MUST include the ICE-CONTROLLING attribute
   in a Binding request.  The controlled ICE agent MUST include the ICE-
   CONTROLLED attribute in a Binding request.

Peter

---

From: Christer Holmberg <[email-masked]>
Date: Fri, 6 Oct 2017 07:08:42 +0000

Hi,

Š

>   CONTROLLED attribute in a Binding request.

Aaah, now I see it. Of course :)

Regards,

Christer

---

From: Christer Holmberg <[email-masked]>
Date: Mon, 16 Oct 2017 18:17:28 +0000

Hi,

...

>>> has occurred if it doesn't know what the initiating agent sent? And 
>>> why wouldn't the initiating agent also have some responsibility for 
>>> detecting ICE mismatch in the other direction (i.e., from the 
>>> responding agent to the initiating agent)?
>> 
>> First, I think the section title is misleading. It¹s not about 
>> verifying ICE support.
>> 

I had a chat with Ari today, and he does not think we should remove it. Perhaps we should change the title or something, but we should keep the text.

However, then your question would still apply. 

In draft-ice-sip-sdp, the determination is done by comparing the c/m-line address:port with the candidate attributes, and if there is no match then it is assumed that an intermediary has modified the c/m-line without doing the corresponding attribute changes.

Regards,

Christer

---

From: Peter Saint-Andre <[email-masked]>
Date: Mon, 16 Oct 2017 12:48:20 -0600

> In draft-ice-sip-sdp, the determination is done by comparing the c/m-line address:port with the candidate attributes, and if there is no match then it is assumed that an intermediary has modified t…

OK. So I guess we should at least say that mechanisms for determining
ICE mismatch are the responsibility of the using protocol.

Peter

---

From: Christer Holmberg <[email-masked]>
Date: Mon, 16 Oct 2017 18:55:20 +0000

Hi,

>>>>> In general, how can the responding agent determine that such 
>>>>> breakage has occurred if it doesn't know what the initiating agent 
>>>>> sent? And why wouldn't the initiating agent also have some 
>>>>> responsibility for detecting ICE mismatch in the other direction 
>>>>> (i.e., from the responding agent to the initiating agent)?
>>>>
>> I had a chat with Ari today, and he does not think we should remove it. Perhaps we should change the title 
>> or something, but we should keep the text.
>> 
>> In draft-ice-sip-sdp, the determination is done by comparing the c/m-line address:port with the candidate 
>> attributes, and if there is no match then it is assumed that an intermediary has modified the c/m-line without doing the corresponding attribute changes.
>
> OK. So I guess we should at least say that mechanisms for determining ICE mismatch are the responsibility of the using protocol.

Yes.

Also, instead of saying "may alter the ICE candidate information that breaks ICE " I would say something more generic, e.g., "may alter information that breaks ICE".

Because, in the case of ice-sip-sdp, the problem is that the ALG does NOT alter the candidate information :)

Regards,

Christer

---

From: Christer Holmberg <[email-masked]>
Date: Tue, 17 Oct 2017 12:11:27 +0000

Hi,

Is there a reason why we couldn’t simply re-name the section to "ICE
mismatch"? Because, that’s what it’s all about, and that’s also the
wording used elsewhere in the document to reference it.

Regards,

Christer

On 16/10/17 21:55, "Ice on behalf of Christer Holmberg"
<[email-masked] on behalf of [email-masked]> wrote:

>>> attributes, and if there is no match then it is assumed that an
>>>intermediary has modified the c/m-line without doing the corresponding
>>>attribute changes.
>>
>Also, instead of saying "may alter the ICE candidate information that
>breaks ICE " I would say something more generic, e.g., "may alter
>information that breaks ICE".
>
>_______________________________________________
>Ice mailing list
>[email-masked]
>https://www.ietf.org/mailman/listinfo/ice

---

From: Christer Holmberg <[email-masked]>
Date: Tue, 17 Oct 2017 12:34:44 +0000

New PR (I messed up the previous one :)

https://github.com/ice-wg/rfc5245bis/pull/47

Regards,

Christer

---

From: Christer Holmberg <[email-masked]>
Date: Wed, 18 Oct 2017 11:00:52 +0000

The PR has been merged.

Regards,

Christer

On 17/10/17 15:34, "Ice on behalf of Christer Holmberg"
<[email-masked] on behalf of [email-masked]> wrote:

>>>>>>>>   information that breaks ICE.  If the using protocol is
>>>>>>>>vulnerable
>>>>>>>>to
>>>Because, in the case of ice-sip-sdp, the problem is that the ALG does
>>>NOT
>>>alter the candidate information :)
>>>
>>>Regards,
>>>
>>>Christer
>>>

---

From: Peter Thatcher <[email-masked]>
Date: Sat, 28 Oct 2017 18:17:52 +0000

Looking through Peter's suggestions and reviewing the PR and email thread,
it appears to me that most, if not all, of Peter's suggestions have been
resolved.

Do the two of you agree, or is there something still missing that we need
to do?

---

From: Peter Saint-Andre <[email-masked]>
Date: Sat, 28 Oct 2017 12:45:04 -0600

I think we're good!

Peter

> On Oct 28, 2017, at 12:17 PM, Peter Thatcher <[email-masked]> wrote:
> 
> Looking through Peter's suggestions and reviewing the PR and email thread, it appears to me that most, if not all, of Peter's suggestions have been resolved.
> 
> Do the two of you agree, or is there something still missing that we need to do?  
> 
>> On Wed, Oct 18, 2017 at 4:01 AM Christer Holmberg <[email-masked]> wrote:
>> The PR has been merged.
>> 
>> Regards,
>> 
>> Christer
>> 
>>