Subject: Re: [Ice] WGLC for draft-ietf-ice-rfc5245bis-12

From: Peter Saint-Andre <[email-masked]>
Date: Wed, 4 Oct 2017 20:00:13 -0600

On 10/1/17 12:02 AM, Peter Thatcher wrote:
> (in case you ignore the last once because of the erroneous title)
> 
> 
> We are starting a 3-week Working Group Last Call for ICE bis:
> 
>  
> 
> _https://tools.ietf.org/html/draft-ietf-ice-rfc5245bis-12_
> 
> 
>  Please review the draft and provide any comments you may have on the
> document by Oct 21, 2017.

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?

§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)?

§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?

Peter

---

From: Taylor Brandstetter <[email-masked]>
Date: Sat, 14 Oct 2017 15:14:55 -0700

I reviewed, and mostly only found minor editorial issues:

   - Section 4.1 is called "f Implementation"; is this meant to say "Full
   Implementation Procedures", like the corresponding "Lite Implementation
   Procedures" section?
   - The full/lite sections have some duplication; for example, defining
   "component ID" and "foundation". Maybe the duplicated parts could be moved
   to their own section.
   - "Foundation" is defined as "a sequence of up to 32 characters". Do we
   define what characters are allowed (or is that not necessary)?
   - Would be nice to elaborate a bit more on the 3PCC scenarios that
   involve role conflict. "For example, both endpoints may believe themselves
   to be the initiator, because the 3PCC entity acts as the responding agent,
   relaying ICE parameters between the endpoints."
   - "a check list with at least one pair in the Waiting state is called
   "active", while a check list with all pairs in the frozen state is called
   "Frozen"." The rest of the document never refers to check lists being
   "active" or "Frozen", so this sentence should be removed. I think I've
   pointed this out in the past.

The only somewhat significant issue I found (which is what Christer's
recent email is about) is the definition of "selected pair":

   The selected pair for a component of a media stream is:
>
>    o  empty if the state of the CHECK LIST for that media stream is
>       Running, and there is no previous selected pair for that component
>       due to an ICE restart
>
>    o  equal to the previous selected pair for a component of a media
>       stream if the state of the CHECK LIST for that media stream is
>
>       Running, and there was a previous selected pair for that component
>       due to an ICE restart
>
>
This defines when the selected pair is empty, but not how it ends up
non-empty. I think the intention is "if check list is Completed, the
selected pairs are the nominated pairs," but this should be stated
explicitly.

Lastly, one issue that was brought up in the past and never resolved (to my
knowledge): what happens if there's a role conflict, and the tiebreakers
are the same? Right now it looks like both endpoints end up stuck in the
controlling role. I'd suggest just saying "pick a new tiebreaker value". It
may be a corner case, but it doesn't seem like it would take more than one
sentence to fix it.

On Sat, Sep 30, 2017 at 11:02 PM, Peter Thatcher <[email-masked]>
wrote:

> *https://tools.ietf.org/html/draft-ietf-ice-rfc5245bis-12
> <https://tools.ietf.org/html/draft-ietf-ice-rfc5245bis-12>*
>
>
> Comments should be sent to the document authors and to the ICE WG list. If
> you review the document but do not have any comments, please send a note to
> that effect as well.
>
> _______________________________________________
> Ice mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/ice
>
>