Re: [Hipsec] WGLC: draft-ietf-hip-native-nat-traversal

[Miika Komu <[e-mail]>]

Hi Ari,

below is more detailed list of the nits and also some technical comments 
about the protocol.

On 02/16/2016 04:01 PM, Ari Keränen wrote:
> On 12/02/16 22:59, Miika Komu wrote:
>> Hi,
>>
>> On 01/29/2016 02:32 PM, Gonzalo Camarillo wrote:
>>> Hi,
>>>
>>> I would like to start a WGLC on the following draft. This WGLC will end
>>> on February 12th:
>>>
>>> https://datatracker.ietf.org/doc/draft-ietf-hip-native-nat-traversal/
>>>
>>> Please, send your comments to this list.
>>
>> in general, the draft should have a short intro to the NAT traversal
>> procedure and re-introduce some terms even though it all is specified in
>> RFC5770. This would make the draft a bit easier to read. I have also
>> some other nits which I'll send a bit later.
>
> Thanks for the review Miika! Also Petri commented along the same lines.
> We'll add some intro text to the draft to address this.

 > 2.  Terminology

I would repeat some of the terms used in RFC5770. Particularly these 
would be useful:

* relayed address
* server reflexive candidate
* relayed candidate
* mapped address

They are used the text and it would be nice to make the draft a bit more 
self-explanatory.

I would also suggest to explain the ICE term "permission" here.

 > 3.  Protocol Description

I would suggest to add a small intro here of the entire process 
(registration, discovery of relay, base exchange, hole punching, ESP). A 
picture similar to figure 1 in RFC5770 would be nice.

 > 3.1.  Relay Registration

 > Section 3.3 at [I-D.ietf-hip-rfc5203-bis]), and the relay has

at -> in

 > 3.2.  Forwarding Rules and Permissions
 >
 > Permissions are not required for the connectivity checks, but if a
 > relayed address is selected to be used for data, the registered host
 > MUST send an UPDATE message [RFC7401] with a PEER_PERMISSION
 > parameter (see Section 4.2) with the address of the peer and the
 > outbound and inbound SPI values the host is using with this peer.

PEER_PERMISSION is not a part of RFC5770, why is it introduced here?

The description is missing also the destination where this message is to 
be sent (it is the relay).

 > 3.3.  Relaying UDP Encapsulated Data and Control Packets

 > When a host wants to send a HIP control packet (such as a
 > connectivity check packet) to a peer via the data relay, it MUST add

* wants -> intends (machines don't have a will, at least yet :)
* it -> ambiguous, should be "the host"
* via the *peer's* data relay, right? I mean both hosts may have their 
own data relays.

 > send it to the data relay's address.  The data relay MUST send the

address of the data relay of the peer (right?)

 > When a host wants to send a UDP encapsulated ESP packet to a peer via
 > the data relay, it MUST have an active permission at the data relay
 > for the peer with the outbound SPI value it is using.

*peer* data relay

 > The host MUST send the UDP encapsulated ESP packet to the data 
relay's address.

What host? Whose data relay?

* wants -> intends
* peer's data relay (right? please correct twice)

The third ("If the data relay..."), fourth (When a host) and fifth 
("When the data relay...") paragraphs appear a bit of 
redundant/overlapping, perhaps it is better to merge them together.

Please state the owner of the data relay (i.e. registered host) in all 
cases. The data relay only relays data traffic to one way (to the 
registered host), right?

 > 3.4.  Candidate Gathering

 > Gathering of candidates MAY also be performed like specified in

like -> as

 > 3.7.  Connectivity Check Pacing Negotiation

 > the check pacing negotiation -> the connectivity check pacing negotiation

 > 3.8.  Connectivity Checks

 > [RFC5770] but instead of STUN packets, the connectivity checks are

..., but instead of STUN packets,,,

 > checklist and start check transactions every Ta milliseconds as long

..start *to* check..

 > The UPDATE packets that acknowledge a
 > connectivity check requests MUST be sent from the same address that
 > received the check and to the same address where the check was
 > received from.

it would be easier to read this in singular form rather than plural:

An/Any UPDATE packet that acknowledges a connectivity check request MUST 
originate from the same address that
was used to receive the check and destined to the same address where the 
check was
received from.

(please note that I changed the wording a bit)

 > The acknowledgment UPDATE packets MUST contain a MAPPED_ADDRESS
 > parameter with the port, protocol, and IP address of the address
 > where the connectivity check request was received from.

same here:

An/Any acknowledgment UPDATE packet MUST...

 > If the controlling host
 > does not have any data to send, it SHOULD send an ICMP echo request

ICMPv6 inside the tunnel - right?

 > using the nominated pair to signal to the controlled host that it can

... in order to signal ...

 > stop checks and start using the nominated pair.  When the controlled
 > host receives the first ESP packet, it MUST select that pair for use
 > and send back an ESP packet to acknowledge a working candidate pair.
 > If the controlled host does not have any data to send, it SHOULD send
 > an ICMP echo request.

ICMPv6 inside the tunnel?

 > If the connectivity checks failed the hosts SHOULD notify each other
 > about the failure with a CONNECTIVITY_CHECKS_FAILED Notify Message
 > Type [RFC5770].

... failed, the hosts SHOULD ...

It would also worthwhile to explain how the connectivity end in the case 
of success, maybe through an example.

 > 3.9.  NAT Keepalives

 > To keep the NAT bindings towards the HIP relay server and the HIP
 > data relay alive, if a registered host has not sent any data or
 > control messages to the relay for 15 seconds, it MUST send a HIP
 > NOTIFY packet to the relay.

When a registered host has not sent any data or control messages to the 
relay for 15 seconds,
it MUST send a HIP NOTIFY packet to the relay in order to keep the NAT 
bindings towards the HIP relay server and the HIP
data relay alive.

 > Likewise, if the host has not sent any
 > data to a host it has security association and has run connectivity

... to a *peer* host ...
... and with which it has run ...


 > checks with, it MUST send either a HIP NOTIFY packet or an ICMP echo
 > request using the same locators as the security association is using.

ICMPv6 inside the tunnel, right?

... security association is based on.

 > 3.10.  Handling Conflicting SPI Values

 > Since the HIP data relay determines from the SPI value to which peer
 > an ESP packet should be forwarded, the outbound SPI values need to be
 > unique for each relayed address registration.  Thus, if a registered
 > host detects that a peer would use an SPI value that is already used
 > with another peer via the relay, it MUST NOT select the relayed
 > address for use.

This is a bit confusing, do you mean inbound SPI values? Or from which 
viewpoint is this written?

 > However, a host with many peers MAY decrease the odds of a conflict
 > by registering more than one relayed address using different local
 > addresses.

local addresses? Do you mean in the case the host is multihomed? Or just 
by using different SPI values?

 > 4.1.  RELAYED_ADDRESS and MAPPED_ADDRESS Parameters

 > This document specifies only use of UDP relaying and...

... the use of ...

 > 4.2.  PEER_PERMISSION Parameter

 > The parameter is used for setting up and refreshing forwarding rules
 > and permissions at the data relay for data packets.

... and the permission for data packets at the data relay.

 > OSPI      the outbound SPI value the registered host is using for
 >           the peer with the Address and Port
 > ISPI      the inbound SPI value the registered host is using for
 >           the peer with the Address and Port

What happens if both of the end-host have their own data relays? Then I 
think the OSPI could be zero.

Why do you need to open both directions explicitly? I think the 
registered host should be allowed to send through the relay after 
successfuly data relay registration. So just opening the inbound 
direction should be sufficient and OSPI could be removed?

 > 4.3.  HIP Connectivity Check Packets

Why is the priority included separately in a new parameter since it was 
already exhanged in the locator?

 > 5.  Security Considerations

I didn't find the described issue from RFC5770, but I would add that 
you're talking about non-HIP aware firewalls. Also, the relay listens at 
a fixed port for registered clients, but it can decide about the port 
facing the peer host.