Subject: [Hipsec] Eric Rescorla's Discuss on draft-ietf-hip-native-nat-traversal-28: (with DISCUSS and COMMENT)

From: Eric Rescorla <[email-masked]>
Date: Fri, 04 May 2018 12:34:27 -0700

Eric Rescorla has entered the following ballot position for
draft-ietf-hip-native-nat-traversal-28: 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-hip-native-nat-traversal/

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

Rich version of this review at:
https://mozphab-ietf.devsvcdev.mozaws.net/D3099

I am very familiar with ICE and yet I found this document extremely
hard to follow. The problem is that it cherry-picks pieces of ICE and
I'm just not sure that it's a complete specification when put all
together. I have noted a number of places where I actually am not sure
how to implement something, and fixing those will resolve this
DISCUSS, but IMO you really should totally rewrite this document
either (a) as a variant of ICE or (b) as an entirely new document not
with a pile of new text and then references out to ICE sections.

DETAIL
S 4.2.
>      request type SHOULD NOT create any state at the Control Relay Server.
>   
>      ICE guidelines [I-D.ietf-ice-rfc5245bis] for candidate gathering are
>      followed here.  A number of host candidates (loopback, anycast and
>      others) should be excluded as described in the ICE specification
>      [I-D.ietf-ice-rfc5245bis].  Relayed candidates SHOULD be gathered in

If you're going to normatively cherry-pick ICE, you need to note
specific sections, I think.

S 4.6.2.
>   
>      A host may receive a connectivity check before it has received the
>      candidates from its peer.  In such a case, the host MUST immediately
>      generate a response, and then continue waiting for the candidates.  A
>      host MUST NOT select a candidate pair until it has verified the pair
>      using a connectivity check as defined in Section 4.6.1.

Are you supposed to put this on a TODO check list as with ICE?

S 5.8.
>   
>   5.8.  RELAY_HMAC Parameter
>   
>      As specified in Legacy ICE-HIP [RFC5770], the RELAY_HMAC parameter
>      value has the TLV type 65520.  It has the same semantics as RVS_HMAC
>      [RFC8004].

What key is used for the HMAC?

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

S 2.
>      Server reflexive candidate:
>         A translated transport address of a host as observed by a Control
>         or Data Relay Server.
>   
>      Peer reflexive candidate:
>         A translated transport address of a host as observed by its peer.

You should indicate which definitions are the same as ICE/STUN

S 3.
>      connected to the public Internet.  To be contacted from behind a NAT,
>      at least the Responder must be registered with a Control Relay Server
>      reachable on the public Internet.  The Responder may have also
>      registered to a Data Relay Server that can forward the data plane in
>      case NAT traversal fails.  While, strictly speaking, the Initiator
>      does not need any Relay Servers, it may act in the other role with

Why doesn't it need Relay Servers? This isn't true for ordinary ICE.

S 4.1.
>      for that host and close the corresponding UDP port (unless other Data
>      Relay Clients are still using it).
>   
>      The Data Relay Server SHOULD offer a different relayed address and
>      port for each Data Relay Client because this can cause problems with
>      stateful firewalls (see Section 6.5).

by "this" I think you mean "not doing so"

S 4.2.
>      parameter discovered during the Control Relay Server registration as
>      a server reflexive candidate.  In this case, no further candidate
>      gathering is needed.
>   
>      A Data Relay Client MAY register only a single relayed candidate to
>      be used with multiple other peers.  However, it is RECOMMENDED that a

Nit: this would be clearer if you said "that it uses with"

S 4.2.
>      gathering is needed.
>   
>      A Data Relay Client MAY register only a single relayed candidate to
>      be used with multiple other peers.  However, it is RECOMMENDED that a
>      Data Relay Client registers a new server reflexive candidate for each
>      of its peer for the reasons described in Section 4.12.3.  The

This sentence feels like a bit of a non-sequiter. How do you
"register" a srflx? Or should this say "relayed"

S 4.2.
>      deployments in order to enable it by software configuration update if
>      needed at some point.  A host SHOULD employ only a single server for
>      gathering the candidates for a single HIP association; either one
>      server providing both Control and Data Relay Server functionality, or
>      one Control Relay Server and also Data Relay Server if the
>      functionality is offered by another server.  When the relay service

How does this interact with mult-layered NAT?>

S 4.3.
>      Servers (e.g. in the case of a single server, it would be 1).  A
>      smaller value than 500 ms for the RTO MUST NOT be used.
>   
>   4.3.  NAT Traversal Mode Negotiation
>   
>      This section describes the usage of a new non-critical parameter

Can you name the type here please?

S 5.6.
>        Protocol   IANA assigned, Internet Protocol number.
>                   17 for UDP, 0 for plain IP
>        Reserved   reserved for future use; zero when sent, ignored
>                   when received
>        Address    an IPv6 address or an IPv4 address in "IPv4-Mapped
>                   IPv6 address" format

Is there a concern about NATs rewriting these, as we had for XOR-
MAPPED-ADDRESS

S 5.7.
>      | Reserved  | 0        | Reserved for future extensions             |
>      | Preferred | 0 or 1   | Set to 1 for a Locator in R1 if the        |
>      | (P) bit   |          | Responder can use it for the rest of the   |
>      |           |          | base exchange, otherwise set to zero       |
>      | Locator   | Variable | Locator lifetime in seconds                |
>      | Lifetime  |          |                                            |

What is the purpose of this? It's not an ICE parameter.

S 5.7.
>      | Port      |          |                                            |
>      | Transport | Variable | IANA assigned, transport layer Internet    |
>      | Protocol  |          | Protocol number.  Currently only UDP (17)  |
>      |           |          | is supported.                              |
>      | Kind      | Variable | 0 for host, 1 for server reflexive, 2 for  |
>      |           |          | peer reflexive or 3 for relayed address    |

Why do you need to send prflx candidates?

S 10.2.
>      o  If the agent is using Diffserv Codepoint markings [RFC2475] in its
>         media packets, it SHOULD apply those same markings to its
>         connectivity checks.
>   
>      o  Unlike in ICE, the addresses are not XOR-ed in Native ICE-HIP
>         protocol in order to avoid middlebox tampering.

I noted this earlier. Why?

---

From: Christer Holmberg <[email-masked]>
Date: Sun, 6 May 2018 17:19:58 +0000

Hi,

> I am very familiar with ICE and yet I found this document extremely hard to follow. The problem is that it cherry-picks pieces 
> of ICE and I'm just not sure that it's a complete specification when put all together. I have noted a number of places where I 
> actually am not sure how to implement something, and fixing those will resolve this DISCUSS, but IMO you really should totally 
> rewrite this document either (a) as a variant of ICE or (b) as an entirely new document not with a pile of new text and then 
> references out to ICE sections.

I haven't been involved in the work on this draft, so I may be wrong, but I did review the document and my understanding is that RFC 5770 is the "variant of ICE", and this document is a modification/extension to RFC 5770.

Regards,

Christer

---

From: Eric Rescorla <[email-masked]>
Date: Sun, 6 May 2018 12:01:01 -0700

On Sun, May 6, 2018 at 10:19 AM, Christer Holmberg <
[email-masked]> wrote:

> Hi,
>
> > I am very familiar with ICE and yet I found this document extremely hard
> to follow. The problem is that it cherry-picks pieces
> > of ICE and I'm just not sure that it's a complete specification when put
> all together. I have noted a number of places where I
> > actually am not sure how to implement something, and fixing those will
> resolve this DISCUSS, but IMO you really should totally
> > rewrite this document either (a) as a variant of ICE or (b) as an
> entirely new document not with a pile of new text and then
> I haven't been involved in the work on this draft, so I may be wrong, but
> I did review the document and my understanding is that RFC 5770 is the
> "variant of ICE", and this document is a modification/extension to RFC 5770.
>

This document is a variant of ICE in the sense that it is ICE-like and
explicitly depends on quite a bit of ICE.

-Ekr

---

From: Christer Holmberg <[email-masked]>
Date: Sun, 6 May 2018 19:05:42 +0000

Hi,

The question is whether this document should re-define the HIP variations to ICE that RFC 5770 already does.

Regards,

Christer

Sent from my iPhone

On 6 May 2018, at 22.01, Eric Rescorla <[email-masked]<mailto:[email-masked]>> wrote:

On Sun, May 6, 2018 at 10:19 AM, Christer Holmberg <[email-masked]<mailto:[email-masked]>> wrote:
Hi,

This document is a variant of ICE in the sense that it is ICE-like and explicitly depends on quite a bit of ICE.

-Ekr

Regards,

Christer

---

From: Eric Rescorla <[email-masked]>
Date: Sun, 6 May 2018 12:09:40 -0700

> to ICE that RFC 5770 already does.

That may be your question, but it's not my question. My question is that
I'm not sure this document is sufficiently clear and unambigious to
implement, given its current structure.

-Ekr

> Regards,
>
> Christer
>
> Sent from my iPhone
>
> On 6 May 2018, at 22.01, Eric Rescorla <[email-masked]> wrote:
>
>
>
>> hard to follow. The problem is that it cherry-picks pieces
>> > of ICE and I'm just not sure that it's a complete specification when
>> put all together. I have noted a number of places where I

---

From: Christer Holmberg <[email-masked]>
Date: Sun, 6 May 2018 19:23:18 +0000

Hi,

> That may be your question, but it's not my question. My question is that I'm not sure this document is 
> sufficiently clear and unambigious to implement, given its current structure.

Sure, the may be editorial work to do, but I still think it is important to clarify whether the reader of this document is expected to be familiar with RFC 5770, or whether this document is supposed to be an "ICE variant" on its own.

Regards,

Christer

On Sun, May 6, 2018 at 10:19 AM, Christer Holmberg <[email-masked]> wrote:
Hi,

-Ekr

Regards,

Christer

---

From: Miika Komu <[email-masked]>
Date: Thu, 10 May 2018 00:21:48 +0300

Hi,

> Sure, the may be editorial work to do, but I still think it is important to clarify whether the reader of this document is expected to be familiar with RFC 5770, or whether this document is suppose…

we wanted to keep the same document structure as RFC5770 because the 
developers were already familiar with it (and has two interoperable 
implementations). While we tried to duplicate some RFC5770 material to 
make the specification a bit more standalone, the document is still 
aimed for people who developed RFC5770.

As you noted, the document is missing the exact section references 
because the ICE spec has been a moving target but I guess the references 
would safe to be fixed now.

Thanks Eric for the insightful technical comments. I'll try to get you 
answers as soon as possible.