[Ice] Adam Roach's Discuss on draft-ietf-ice-rfc5245bis-17: (with DISCUSS and COMMENT)

[Adam Roach <[e-mail]>]

Adam Roach has entered the following ballot position for
draft-ietf-ice-rfc5245bis-17: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-ice-rfc5245bis/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Thanks to everyone who has contributed to this document over the many years of
its development.

The document currently appears to contain normative statements that, while not
literally contradictory, certainly point implementors in conflicting directions.
§5.1.1.1 says:

>  o  Host candidates corresponding to IPv6 link-local addresses MUST
>     NOT be gathered.

Further down, §6.1.2.2 says:

>  To keep the amount of resulting candidate pairs
>  reasonable and to avoid candidate pairs that are highly unlikely to
>  work, IPv6 link-local addresses [RFC4291] MUST NOT be paired with
>  other than link-local addresses.

This second passage is nonsensical if IPv6 link local addresses MUST NOT be
gathered. Please remove or amend one of these normative statements. If the first
statement is retained, please add rationale.

(As an aside, it makes more sense to cite 4291 the first time you mention IPv6
link local address rather than the second)


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I have a number of comments on technical content, followed by some editorial
nits.

---------------------------------------------------------------------------

§5.1.1.3:

>  o  For reflexive and relayed candidates, the STUN or TURN servers
>     used to obtain them have the same IP address.

This is ambiguous: it is unclear whether "the same IP address" refers to the
address that the ICE agent used to reach the TURN server, or whether the IP
address allocated by the TURN server on the client's behalf. In large-scale
TURN deployments, these addresses will frequently differ, since one IP address
can only support ~64k ports at once (and therefore TURN servers may need to
make use of multiple at once). Please clarify which IP address is to be used for
this comparison.

>  Similarly, two candidates have different foundations if their types
>  are different, their bases have different IP addresses, the STUN or
>  TURN servers used to obtain them have different IP addresses, or
>  their transport protocols are different.

The same clarification is required to this paragraph as well.

---------------------------------------------------------------------------

§5.2:

>  This default
>  SHOULD be chosen such that it is the candidate most likely to be used
>  with a peer.  For IPv6-only hosts, this would typically be a globally
>  scoped IPv6 address.  For dual-stack hosts, the IPv4 address is
>  RECOMMENDED.


I support Suresh's DISCUSS. I would propose that you recommend that dual-stack
hosts SHOULD allow configuration of whether IPv4 or IPv6 is used for the
default, and that the configuration needs to be based on which one its
administrator believes has a higher chance of success in the current network
environment. I am of the understanding, for example, that in certain Japanese
carriers, the selection of IPv6 as the default would maximize the chance of
success, even today.

---------------------------------------------------------------------------

§5.3:

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

The term "Related Address and Port" is not defined anywhere. Is it the same as
the base address? Something else? Please add a definition to §4. (I'll note that
this term is also used in Figure 6.) It may also be useful to call forward to
§B.3 here, so that implementors aren't confused by the apparent uselessness of
including this information.

---------------------------------------------------------------------------

§8.1.1:

>  NOTE: A controlling agent that does not support this specification
>  (i.e. it is implemented according to RFC 5245) might nominate more
>  than one candidate pair.  This was referred to as aggressive
>  nomination in RFC 5245.

Please here or elsewhere (probably Appendix B) indicate why aggressive
nomination was removed (i.e., it's redundant with the ability to send data on
any valid pair even prior to nomination).

Also (editorial nit) please put quotation marks around the term "aggressive
nomination."

---------------------------------------------------------------------------

§8.1.2:

>     *  The agent MAY begin transmitting data for this data stream as
>        described in Section 12.1.

This seems redundant with the fact that the data could be sent even before
nomination, and runs the risk that implementors might interpret it to mean that
they must not send data *prior* to nomination. I think it would be safest to
remove this, and possibly reiterate right here that sending data is not blocked
on successful pair nomination.

---------------------------------------------------------------------------

§9:

>  To restart ICE, an agent MUST change both the password and the
>  username fragment for the data stream(s) being restarted.  The agent
>  MUST verify that it has not used the new values in recent ICE
>  sessions, as packets from those sessions might still be present in
>  the network.

To be clear: the password and ufrag, taken together, provide 152 bits of
randomness, and need to be selected randomly on restart. This passage appears to
be guarding against random collisions between 152-bit values in a small set.
Do I have that correct?

If so: I couldn't find a birthday paradox calculator that could deal with
5,708,990,770,823,839,524,233,143,877,797,980,545,530,986,496 potential values.
The only one that didn't generate an error outright gave a probability of 0,
which is as close as correct as to make no difference.

Unless I'm misunderstanding what is being asked of implementors, this second
MUST is superfluous and should be removed.

---------------------------------------------------------------------------

§15:

Please either update the example to use IPv6 in addition to IPv4, or provide
an additional example that uses IPv6 addresses.  See
https://www.iab.org/2016/11/07/iab-statement-on-ipv6/ for guidance on the use of
IPv6 in IETF specifications, including their examples.

---------------------------------------------------------------------------

§15:

>            |              |(9) Bind Req  |              |Begin
>            |              |S=$R-PUB-1    |              |Connectivity
>            |              |D=L-PRIV-1    |              |Checks
>            |              |<----------------------------|
>            |              |Dropped       |              |

This is quite misleading. If a host on the public Internet (as R is in this
example) sent a Binding Request to $L-PRIV-1 (10.0.1.1 in this example), it
would *not* be routed to the NAT, as is shown in this example. If it didn't get
blocked before reaching the default free zone, I believe it would be blackholed
once it did so. I would propose keeping the arrow, but having it end prior to
reaching the NAT line.

(Minor nit: please replace "L-PRIV-1" with "$L-PRIV-1")

---------------------------------------------------------------------------

§15:

>  Since the check was generated in the reverse direction of a
>  check that contained the USE-CANDIDATE attribute...

It was? I don't see that in the ladder, can't find it in the prose, and believe
it to be actually incorrect. By my understanding, the example would need an
additional messages 18 through 21, originating from L (as the controlling agent)
that nominates this pair.

I believe what you've shown is an aggressive nomination call flow, but this
document no longer supports aggressive nomination.

If I'm simply confused here, then please add the "USE-CANDIDATE" attribute to
the ladder diagram and to the corresponding text where it belongs.

---------------------------------------------------------------------------

§20.1 and §20.2:

Please add instructions to IANA to update the registry to point to this document
instead of RFC 5245.


===========================================================================

All of the following are editorial nits.

> J. Rosenberg
> jdrosen.net

You might want to double-check with the author that this is the intended
affiliation for this document.

---------------------------------------------------------------------------

§1:
>  sinks.  However this poses challenges when operated through Network

"However, this..."

>  (RTCP) [RFC3605].  Unfortunately, these techniques all have pros and
>  cons which, make each one optimal in some network topologies, but a

"...cons that make..."

>  to-peer connectivity checks.  The IP addresses and ports are
>  exchanged exchanged using ICE usage-specific mechanisms (for example,

Remove duplicate "exchanged"

>  including in a offer/answer exchange) and the connectivity checks are
>  performed using Session Traversal Utilities for NAT (STUN)

"STUN" has already been expanded in this section.

>  specification [RFC5389].  ICE also makes use of Traversal Using
>  Relays around NAT (TURN) [RFC5766], an extension to STUN.  Because
>  ICE exchanges a multiplicity of IP addresses and ports for each media
>  stream, it also allows for address selection for multihomed and dual-
>  stack hosts.  For this reason RFC 5245 [RFC5245] deprecated RFC 4091

"For this reaason, RFC..."

"...deprecated the solutions previously defined in..."

---------------------------------------------------------------------------

§2:

>  Figure 1 shows a typical ICE deployment.  The agents are labelled L
>  and R.  Both L and R are behind their own respective NATs though they
>  may not be aware of it.  The type of NAT and its properties are also
>  unknown.  L and R are capable of engaging in an candidate exchange

"...in a candidate..."

>  In addition to the agents, a signaling server and NATs, ICE is

"...signaling server, and NATs,..."

---------------------------------------------------------------------------

§2.4:

>  Once ICE is concluded, it can be restarted at any time for one or all
>  of the data streams by either ICE agent.  This is done by sending an
>  updated candidate information indicating a restart.

"...sending updated..."

---------------------------------------------------------------------------

§3:

>  This document specifies generic use of ICE with protocols that
>  provide means to exchange candidate information between the ICE
>  agents.  The specific details of (i.e how to encode candidate

"...specific details (i.e., how.."

(remove "of", include period and comma after "i.e")

>  information and the actual candidate exchange process) for different
>  protocols using ICE (referred to as using protocol) are described in

'...(referred to as "using protocol")...'

>  separate usage documents.
>
>  One mechanism for agents to exchange the candidate information by
>  using [RFC3264] based Offer/Answer semantics as part of the SIP

"...information is using..."

---------------------------------------------------------------------------

§4:

>  Responding Peer, Responding Agent, Responder:  A receiving agent is

"A responding agent..."

>  Component:  A component is a piece of a data stream.  A data stream
>     may require multiple components, each of which has to work in
>     order for the data stream as a whole to work.  For RTP/RTCP data
>     streams, unless RTP and RTCP are multiplexed in the same port,
>     there are two components per data stream -- one for RTP, and one
>     for RTCP.  A component has a candidate pair, which cannot be used
>     by other components

Add a period to the end of this sentence.

>   Default Destination/Candidate:  The default destination for a
>     component of a data stream is the transport address that would be
>     used by an ICE agent that is not ICE-aware.

Users are likely to be confused by the phrase "ICE agent that is not ICE-aware."
It certainly threw me for a loop. Perhaps "...used by a peer that is not
ICE-aware" or something like that.

>  Nomination, Regular Nomination:  The process of the controlling agent
>     indicating to the controlled agent which candidate pair the ICE
>     agents will use for sending and receiving data.

With the exception of its use in §13 (which I think should be removed), the term
"Regular Nomination" (which was used to distinguish from the now-removed
aggressive nomination) does not appear in this document. I suggest it be removed
from this definition.

>  Timer HTO:  The timeout timer for a given STUN or TURN transaction.

It would be nice if the document indicated some mnemonic for "HTO" (that is:
expand or explain this acronym, please).

---------------------------------------------------------------------------

§5.1.1.1:

>  For other than RTP/RTCP streams, use of multiple components is

"For uses other than..."

>  discouraged since using them increases the complexity of ICE

"...discouraged, since..."

---------------------------------------------------------------------------

§5.3:

>  The using protocol may (or may not) need to deal with backwards
>  compatibility with older implementations that do not support ICE.  If
>  the fallback mechanism is being used...

It's not clear what "the fallback mechanism" refers to in this sentence.

---------------------------------------------------------------------------

§5.4:

>   Certain middleboxes, such as ALGs, can alter signaling information in
>   ways that break ICE.

"...break ICE (for example, by rewriting IP addresses in SDP)."

---------------------------------------------------------------------------

§6.1.1:

>  Both agents are full:  The initiating agent which started the ICE

"...agent that started..."

>  Both lite:  The initiating agent which started the ICE processing

"...agent that started..."

>  NOTE: There are certain 3PCC (third party call control) scenarios
>  where an ICE restart might cause a role conflict.

Please add an informative citation to RFC 3725.

---------------------------------------------------------------------------

§6.1.2:

>  There is one check list for each data stream.  To form a check list,
>  initiating and responding ICE agents form candidate pairs, computes
>  pair priorities, orders pairs by priority, prunes pairs, removes
>  lower-priority pairs, and sets check list states.

Please match up your number tense here. Either:

   There is one check list for each data stream.  To form a check list,
   initiating and responding ICE agents form candidate pairs, compute
   pair priorities, order pairs by priority, prune pairs, remove
   lower-priority pairs, and set check list states.

...or...

   There is one check list for each data stream.  To form a check list,
   each initiating and responding ICE agent forms candidate pairs, computes
   pair priorities, orders pairs by priority, prunes pairs, removes
   lower-priority pairs, and sets check list states.

---------------------------------------------------------------------------

§6.1.2.6:

>                      Figure 8: Initial Pair State

I think calling this a figure is quite a stretch. I was really thrown for a loop
after reading four paragraphs to find this non-sequitur label just hanging out
at the end of the section.

---------------------------------------------------------------------------

§6.1.4.2:

>  Once the agent has picked a candidate pair, for which a connectivity
>  check is to be performed, the agent performs the check by sending a

"...a candidate pair for which..."

---------------------------------------------------------------------------

§6.2:

>  Lite implementations skips most of the steps in Section 6 except for

"Lite implementations skip..."

---------------------------------------------------------------------------

§7.2.5.2.2:

   An ICE agent MAY support processing of ICMP errors for connectivity
   checks.  If the agent supports processing of ICMP errors, and if a
   Binging request generates a hard ICMP error, the agent SHOULD set the

"...Binding request..."

---------------------------------------------------------------------------

§9:

>  An ICE agent MAY restart ICE for existing data streams.  An ICE
>  restart causes all previous state of the data streams, excluding the
>  roles of the agents to be flushed.

"...excluding the roles of the agents, to be flushed."

(insert comma)

---------------------------------------------------------------------------

§12.1:

>  Once selected pairs have been produced for a data stream, an agent
>  MUST send data on those pairs.

"...on those pairs only."

---------------------------------------------------------------------------

§12.2:

>  Even though ICE agents are only allowed to send data using valid
>  candidate pairs (and, once selected pairs have been produced, only on
>  the selected pairs) ICE implementations SHOULD by default be prepared
>  to receive data on any of the candidates provided in the most recent
>  candidate exchange with the peer.  ICE usages MAY define rules that
>  differs from this, e.g., by defining that data will not be sent until

"...rules that differ..."

---------------------------------------------------------------------------

§13:

>  One of the complications in achieving interoperability is that ICE
>  relies on a distributed algorithm running on both agents to converge
>  on an agreed set of candidate pairs.  If the two agents run different
>  algorithms, it can be difficult to guarantee convergence on the same
>  candidate pairs.  The regular nomination procedure described in

There is only one nomination procedure here; this makes more sense as "The
nomination procedure described in..."

>  Section 8 eliminates some of the tight coordination by delegating the
>  selection algorithm completely to the controlling agent.
>  Consequently, when a controlling agent is communicating with a peer
>  that supports options it doesn't know about, the agent MUST run a
>  regular nomination algorithm.  When regular nomination is used, ICE

"...MUST run the nomination algorithm described in Section 8. When this
algorithm is used,..."

---------------------------------------------------------------------------

§14.2:

>  ICE agents SHOULD use the default Ta value, 50 ms, but MAY use

"ICE agents SHOULD use a default Ta value of 50 ms, but MAY use..."
                       ^                  ^^

>     1.  Let MaxBytes be the maximum number of bytes allowed to be
>         outstanding in the network at start-up, which SHOULD be 14600
>         bytes per RFC 6928.

Please add a citation to RFC 6928 (rather than just mentioning its RFC number).

---------------------------------------------------------------------------

§14.3:

>    RTO = MAX (500ms, Ta*N * (Num-Waiting + Num-In-Progress))

I can't tell whether the "*N" in "Ta*N" is a typo, or if the document simply
doesn't define the meaning of "N" anywhere. Please either fix the formula, or
add a description of the meaning of "N."

---------------------------------------------------------------------------

§16.1:

>  This specification defines four new STUN attributes, PRIORITY, USE-
>  CANDIDATE, ICE-CONTROLLED, and ICE-CONTROLLING.

"...new STUN attributes: PRIORITY, ..."

---------------------------------------------------------------------------

§17.1:

>  Consequently, it is not necessary to replace or reconfigure existing
>  firewall and NAT equipment in order to facilitate deployment of ICE.
>  Indeed, ICE was developed to be deployed in environments where the
>  Voice over IP (VoIP) operator has no control over the IP network
>  infrastructure, including firewalls and NAT.

"...and NATs."

>  That said, ICE works best in environments where the NAT devices are
>  "behave" compliant, meeting the recommendations defined in [RFC4787]
>  and [RFC5382].  In networks with behave-compliant NAT, ICE will work

"...behave-compliant NATs, ..."

---------------------------------------------------------------------------

§17.2.1:

>  First and foremost, ICE makes use of TURN and STUN servers, which
>  would typically be located in the data centers.

"...in data centers."


---------------------------------------------------------------------------

§17.3:

>  Deployments utilizing a mix of ICE and ICE-lite interoperate
>  perfectly.  They have been explicitly designed to do so.

"Perfectly" seems a bit hubristic. Perhaps "...interoperate with each other."


---------------------------------------------------------------------------

§18:

>  The IAB has studied the problem of "Unilateral Self-Address Fixing",

Since you use the "UNSAF" acronym further down, please use this opportunity to
expand it.

>  and the difference has a significant impact on the issues raised by
>  IAB.  Indeed, ICE can be considered a B-SAF (Bilateral Self-Address

"...raised by the IAB."

---------------------------------------------------------------------------

§18.2

>  less, and can eventually be remove when their usage goes to zero.

"...be removed..."

---------------------------------------------------------------------------

§19:

The text in the top-level "Security Considerations" section is its own topic,
similar to those found in Section 19's subsections. Please give it its own title
(e.g., "19.1.   IP Address Privacy")

---------------------------------------------------------------------------

§19.1:

>  UDP header of the message triggering the error.  These fields also
>  needs to be validated.  The IP destination and UDP destination port

"...need..."

>  needs to match either the targeted candidate address and port, or the

"...need..."

>  candidates base address.  The source IP address and port can be any

"...candidate's..."

---------------------------------------------------------------------------

§19.4:

>  In addition to attacks where the attacker is a third party trying to
>  insert fake candidate information or stun messages, there are attacks

"...STUN..."

---------------------------------------------------------------------------

§20.3:

>       The ICE option indicates that the ICE agent using the ICE option
>       is compliant and implemented according to RFC XXXX.

Suggest removing "compliant and" from this.

---------------------------------------------------------------------------

§21:

>  o  Make technical changes, due to discovered flows in RFC 5245 and

"...discovered flaws..."

---------------------------------------------------------------------------

§22

>  Harald Alvestrand and Roman Shpount.  Ben Campbill did the AD review.

"Ben Campbell"

---------------------------------------------------------------------------

§B.1:

>  rate at which they will create new bindings.  Experiments have shown

I agree with EKR that a citation to these experiments is called for.

>  that once every 5 ms is well supported.  This is why Ta has a lower

"...well-supported..."

---------------------------------------------------------------------------

§B.8:

>  Additionally, using a Binding Indication allows integrity to be
>  disabled, allowing for better performance.  This is useful for large-
>  scale endpoints, such as PSTN gateways and SBCs.

Expand "PSTN" and "SBC".