Re: [Hipsec] Benjamin Kaduk's No Objection on draft-ietf-hip-native-nat-traversal-28: (with COMMENT)

[Miika Komu <[e-mail]>]

Hi Benjamin,

thanks for the very detailed comments and apologies for my extremely
slow reaction! My corrections are below. If you think I haven't
addressed your concerns, please let me know.

ke, 2018-05-09 kello 08:02 -0700, Benjamin Kaduk kirjoitti:
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-hip-native-nat-traversal-28: No Objection
> 
> 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/
> 
> 
> 
> -------------------------------------------------------------------
> ---
> COMMENT:
> -------------------------------------------------------------------
> ---
> 
> This document does a poor job at convincing me that there
> is a need to re-specify ICE for use in HIP contexts as opposed to
> just using ICE directly, up until Appendix B.  I'd suggest moving
> some of the key points into at least the Introduction and arguably
> the Abstract as well, to make it clear that this is not just
> needless duplication for ideological purity.

as discussed in earlier reviews, I have already added some text to the
introduction:

HIP poses a unique challenge to using standard ICE, due not
only to its kernel-space implementation, but also due to its
close integration with kernel-space IPSec; and, that while RFC5770"
provides a technically workable path, it incurs
unacceptable performance drawbacks for kernel-space
implementations. Also, implementing and integrating a full
ICE/STUN/TURN protocol stack as specified
in Legacy ICE-HIP results in a considerable amount of effort and
code which could be avoided by re-using and extending HIP
messages and state machines for the same purpose. [...]
The differences to the Legacy ICE-HIP are further elaborated in Section
[...]

As requested by your, I updated the last sentence of the abstract as
follows:

The main difference from the previously specified modes is the use of
HIP messages instead of ICE for all NAT traversal procedures due to its
kernel-space dependencies.

> I think there's some lingering terminology confusion (as a result of
> needing to align the new bits in Native HIP-ICE with those retained
> from RFC 5077, along with the move from 5245 to 5245bis.
> Specifically, while it's fine for this document to refer to "ICE
> offer" and "ICE answer", 5245bis itself does not talk of "offer" and
> "answer", which are now concepts only in SDP, IIUC.

RFC8445	mentions "offer/answer" as an example but I agree it is more of
an SDP terminology. The offer/answer + nat traversal procedures are
coupled into a single control plane (=HIP), hence we are combining
terminology from both the specifications. I updated the terminology a
bit to reflect your comments better:

   HIP offer:
      Before two end-hosts can establish a communication channel using
      the NAT traversal procedures defined in this document, they need
      exchange their locators (i.e. candidates) with each other.  In
      ICE, this procedure is called Candidate Exchange and it does
      specify how the candidates are exchanged but Session Description
      Protocol (SDP) "offer/answer" is mentioned as an example.  In
      contrast, the Candidate Exchange in HIP is the base exchange
      itself or a subsequent UPDATE prodecure occurring after a
      handover.  Following [RFC5770] and Session Description Protocol
      (SDP) [RFC3264] naming conventions, "HIP offer" is the the
      Initiator's LOCATOR_SET parameter in a HIP I2 or in an UPDATE
      control packet.

   HIP answer:
      The Responder's LOCATOR_SET parameter in a HIP R2 control packet.
      Corresponds to the SDP answer parameter [RFC3264], but is HIP
      specific.  Please refer also to the longer description of the 
      "HIP offer" term above.

> Things also
> seem a bit hazy about server reflexive vs. server relay candidates
> (though maybe here the confusion is just on my end) -- in regular
> ICE, a server reflexive candidate is obtained by a STUN client
> making a one-shot request of a STUN server to find out what address
> is being used on the other side of a NAT, and doesn't require any
> state on the STUN server.  But in this document we have a
> SERVER_REFLEXIVE_CANDIDATE_ALLOCATION_FAILED Notify/error packet
> that implies that state is needed on the Data Relay Server for a
> reflexive candidate, text about "when the relay service is split
> between hosts, the server reflexive candidate [from Control SHOULD
> be used over the one from Data]", and also other discussion about
> needing to register new reflexive candidates to avoid collisions or
> in potential multihoming future work.

you have a valid point, I have added the following text to Appendix B
to the STUN discussion:

It is worth noting that a drawback of not employing STUN is that
discovery of the address candidates requires creating (using HIP base
exchange) and maintaining (using HIP UPDATE procedures) state at the
Control Relay Client and Control Relay Server. Future extensions to
this document may define a stateless, HIP-specific mechanism for a end-
host to discover its address candidates.

> Some section-by-section comments follow.
> 
> Section 2
> 
>    Responder:
>       The Responder is the host that receives the I1 packet from the
>       Initiator.
> 
> Does this still hold if the message is misdelivered or an attacker
> is in the network?

If misdelivered, the HITs don't match and the receiver host drops the
packet as documented in the base exchange specification in RFC7401
(which is the basis for the NAT traversal extensions). Also the case
when both ends initiate with each other at the same time is documented
in RFC7401. The attacker scenarios are also addressed in RFC7401.

Some potential new threats arising from the extensions are addressed in
section 6.3, 6.5, 6.6. and 6.7.

> Section 3
> 
>    [...] At this point, also the HIP signaling can be sent over the
>    same address/port pair, and is demultiplexed from IPsec as
> described
>    in the UDP encapsulation standard for IPsec [RFC3948].
> 
> "demultiplex" does not appear anywhere in RFC 3948; it might be
> worth using a few more words here to clarify what is going on.

demultiplexed has been used, e.g., here:

https://tools.ietf.org/html/rfc5761

But you're correct, RFC3948 does not use this word, so I changed the
text as follows:

...demultiplexed (or, in other words, separated) from IPsec as
described..

> Section 4.1
> 
>    A Control Relay Server MUST silently drop packets to a Control
> Relay
>    Client that has not previously registered with the HIP relay.
> 
> How does the relay know where they are targetted without the
> registration information?

you right, this is a bit of a no-op. I removed the sentence.

>    This applies both renewals of service registration but also to
> host
>    movement, where especially the latter requires the Control Relay
>    Client to learn its new server reflexive address candidate.
> 
> This is kind of awkward wording; maybe:
> 
>    This applies to both renewals of service registration and to host
>    movement.  It is especially important for the case of host
>    movement, as this is the mechanism that allows the Control Relay
>    Client to learn its new server reflexive address candidate.

thanks, changed as you suggested.

> Section 4.2
> 
>    [...] 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.
> 
> The second half of this sentence seems to contradict the SHOULD, so
> probably some rewording is in order.

good catch. Also Eric Rescolarla commented this:

"Well, with multi-layered NAT, you actually want a STUN server at each
level so that you minimize hairpinning. But you recommend against that
here."

So I took the unnecessary constraint out and removed the following two
sentences:

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 is split between two hosts, the
server reflexive candidate from the Control Relay Server SHOULD 
be used instead of the one provided by the Data Relay Server.

>    [...] If a relayed candidate is identical to a host
>    candidate, the relayed candidate must be discarded.  NAT64
>    considerations in [I-D.ietf-ice-rfc5245bis] apply as well.
> 
> I don't think this has enough detail of reference to ICE -- a
> relayed candidate being identical to a host candidate is, IIUC,
> quite unexpected.  This is even noted in 5245bis, at the bottom of
> page 20 (of the -20).

thanks, I adapted the text from the ICE specification to fit here, and
changed the text as follows:

Similarly as explained in section 5.1.1.2 of the ICE specification
[RFC8445], if an IPv6-only host is in a network that utilizes NAT64
[RFC6146] and DNS64 [RFC6147] technologies, it may also gather IPv4
server- reflexive and/or relayed candidates from IPv4-only Control or
Data Relay Servers.  IPv6-only hosts SHOULD also utilize IPv6 prefix
discovery [RFC7050] to discover the IPv6 prefix used by NAT64 (if any)
and generate server-reflexive candidates for each IPv6-only interface,
accordingly.  The NAT64 server-reflexive candidates are prioritized
like IPv4 server-reflexive candidates.

>    Unlike ICE, this protocol only creates a single UDP flow between
> the
>    two communicating hosts, so only a single component
> exists.  Hence,
>    the component ID value MUST always be set to 1.
> 
> ICE or SDP?

I changed as follows:

Unlike with SDP used in conjunction with ICE...

> Section 4.5
> 
>    In step 2, the Control Relay Server receives the I1 packet.  If
> the
>    destination HIT belongs to a registered Responder, the Control
> Relay
>    Server processes the packet.
> 
> Do HIP participants register specifically as Responder/Initiator, or
> is this just that the entity that is Responder in this exchange, has
> registered at the Control Relay Server?

I think this is common confusion stemming from RFC8003 and  RFC8004 :)

Initiator just means the participant sending the I1 of the base
exchange, and responder is the participant receiving the I1 and
responding with an R1. And there are two base exchanges involved: one
during registration and the other one where the actual NAT traversal
procedures take place. Perhaps this clarifies the situation:

Base exchange 1: the registration (=base exchange with some extra
parameters) you have the following roles:
* Control Relay Client = Initiator1
* Control Relay Server = Responder1

Base exchange 2: during a base exchange via a HIP relay, the roles are
as follows:
* Initiator2 = (for example a web browser connecting to a HIT of
Responder2)
* Control Relay Server = same as above (i.e. HIP message forwarder)
* Responder2 = (for example a web server)

(Note that the numbers 1-1 and 2-2 are coupled with each other)

After the second base echange, Initiator2 and Responder2 initiate the
NAT traversal procedures with each other.

We have chosen to name the end-hosts just as "Initiator" and
"Responder" in this section because the registration is documented in a
separate section. The terminology is aligned with RFC7401 but I can
understand your confusion :)

>    [...] The
>    RELAY_TO parameter is not integrity protected by the signature of
> the
>    R1 to allow pre-created R1 packets at the Responder.
> 
> This seems to allow an attacker to replay R1 packets and have the
> Relay transmit them to the Initiator.  Are there cases where this
> presents an increase in attacker capabilities (e.g., when the Relay
> is allowed to send to the Initiator but the attacker is not)?
> (When would such pre-created R1 packets be useful?)

this feature is inherited from RFC7401 and benefits are documented
there:

https://tools.ietf.org/html/rfc7401#section-4.1.1

the replay protection is also documented there:

https://tools.ietf.org/html/rfc7401#section-4.1.4

> Should step 7 explicitly duplicate the "validate REPLAY_HMAC" part
> of step 3?

yes, good catch. I added to step 7:

The Responder validates the RELAY_HMAC
   according to [RFC8004] and silently drops the packet if the
   validation fails. 

> Section 4.6
> 
> The situation with the (non-)existence of aggressive nomination
> between
> 5245bis and MMUSIC-ICE probably merits further investigation.
> (That is, it may not be necessary to mention explicitly that regular
> nomination is used.)

this was a remainder from an earlier ICE draft version. Also noted by
other reviewers, and this statement has been removed.

>    The Initiator MUST take the role of controlling host and the
>    Responder acts as the controlled host.  The roles MUST persist
>    throughout the HIP associate lifetime (to be reused in the
> possibly
>    mobility UPDATE procedures).  In the case both communicating nodes
>    are initiating the communications to each other using an I1
> packet,
>    the conflict is resolved as defined in section 6.7 in [RFC7401]:
> the
>    host with the "larger" HIT changes to its Role to Responder.  In
> such
>    a case, the host changing its role to Responder MUST also switch
> to
>    controlling role.
> 
> The last clause seems to not match the earlier text about the
> Initiator being the controlling role.

thanks, good catch. Changed to:

In such a case, the host changing its role to Responder MUST also
switch to controlled
role.                                                   

> Section 4.6.1
> 
>    In step 2, the Initiator sends a connectivity check, using the
> same
>    address pair candidate as in the previous step, and the message
>    traverses successfully the NAT boxes.  The message includes the
> same
>    parameters as in the previous step.  It should be noted that the
>    sequence identifier is locally assigned by the Responder, so it
> can
>    be different than in the previous step.
> 
> Step 2 is from Initiator to Responder, and the message in step 1 got
> dropped, so I'm quite confused at how the sequence identifier in
> step 2 could be assigned by the Responder as opposed to the
> Initiator.
> 
> Step 4 could say whether the sequence number from step 1 is reused
> or a new one is allocated for the retransmission (even though it is
> clarified later as "SHOULD be sent with the same sequence
> identifier").

thanks good catch, I corrected step 2 as follows:

It should be noted that the sequence identifier is locally
assigned by the *Initiator*, so it can be different than in
the previous step.

> Section 4.7.2
> 
>    [...] However, the
>    Responder SHOULD NOT send any ESP to the Initiator's address
> before
>    it has received data from the Initiator, as specified in Sections
>    4.4.3. and 6.9 of [RFC7401] and in Sections 3.2.9 and 5.4 of
>    [RFC8046].
> 
> I don't see a Section 3.2.9 in RFC 8046.

the sentence was coming from earlier iterations of the RFCs and was
actually somewhat accurate. Looking at the specs in detail again, the
SHOULD NOT statement is not in the final RFCs, so I have removed the
sentence, and instead modified the previous sentence:

Instead, if R2 and I2 are received and processed successfully, a
security association can be created and UDP-encapsulated ESP can be
exchanged between the hosts after the base exchange completes
*according to the rules in Section 4.4 in [RFC7401]*.

>    Since an I2 packet with UDP-ENCAPSULATION NAT traversal mode
> selected
>    MUST NOT be sent via a Control Relay Server, the Responder SHOULD
>    reject such I2 packets and reply with a
>    NO_VALID_NAT_TRAVERSAL_MODE_PARAMETER NOTIFY packet (see
>    Section 5.10).
> 
> How does this mesh with a few paragraphs back, when the Responder
> MUST include the address it got by registering at a Control Relay
> Server in its R1 (only when it is including UDP-ENCAPSULATION NAT as
> one of its supported modes)?

Some connecting dots missing... a Control Relay Server can be used only
with the ICE-HIP-UDP mode but with UDP-ENCAPSULATION mode is can still
be used for discovery of address candidates (i.e. registration only).

So clarified this "few paragraphs back" as follows:

If the Responder has registered to a Control Relay Server *in order to
discover its address candidates*, it MUST also include a LOCATOR_SET
parameter in R1 that contains a preferred address where the Responder
is able to receive UDP-encapsulated ESP and HIP packets. 

And I clarified the other sentence as follows (added the first extra
sentence):

The Control Relay Server can be used for discovering address candidates
but it is not intended to be used for relaying end-host packets using
the UDP-ENCAPSULATION NAT mode.  Since an I2 packet with UDP-
ENCAPSULATION NAT traversal mode selected MUST NOT be [...]

> Section 4.7.3
> 
>    [...] The Initiator may receive multiple R1 packets, with
>    and without UDP encapsulation, from the Responder.  However, after
>    receiving a valid R1 and answering it with an I2, further R1
> packets
>    that are not retransmissions of the original R1 message MUST be
>    ignored.
> 
> We just said there may be multiple, so which one is "the" original
> R1 message?

reworded this as follows:

However, after receiving a valid R1 and answering it with an I2,
further R1 packets that are not retransmissions of the R1 message
received first MUST be ignored.
 
> Should Section 4.8 repeat the earlier admonitions against a Relay
> Server forwarding traffic that does not involve a Client that has
> egistered with that Relay Server?

as I noted earlier, IMHO this is a no-op since the it would know where
to relay them anyways.

> Section 4.9
> 
> The relay still has to apply RELAY_HMAC; that's just not currently
> shown in the diagram, right?

yes, omitted in the figure but described in the text. It was also
omitted from the earlier figure number 4 in order to keep the figure
size more compact.

> Section 5.6
> 
>    The format of the REG_FROM, RELAY_FROM, and RELAY_TO parameters is
>    shown in Figure 10.  All parameters are identical except for the
>    type.  REG_FROM is the only parameter covered with the signature.
> 
> I suggest "Of the three, only REG_FROM is covered by the signature."

thanks, fixed as you suggested.

> Section 6.1
> 
> The RFC 5770 text talks about TURN servers, but that's no longer
> applicable in this document (instead, Data Relay Servers are used to
> relay data in a similar usage).

fixed:

With such a legacy NAT, the only option left would be to use a relayed
transport address from an *Control Relay Server* server.

> Also, the protection against DoS described in the last paragraph
> seems to only be partial protection, since incoming connections can
> still be affected by DoS, but outgoing ones are still possible.

true, changed as follows:

This *partially* protects the Responder against Denial-of-Service...

> Section 6.2
> 
> This is the first mention of Opportunistic Mode at all in the
> document; it might be nice to refer to the section of 7401 where
> it's documented.

added:

In opportunistic HIP mode (cf.  Section 4.1.8 in [RFC7401]), [...]

> Section 6.6
> 
> Is the invalid list of candidates sent *for* its peer or *to* its
> peer?

should be "to", this is corrected now.

> Appendix B
> 
>    o  Unlike in ICE, the addresses are not XOR-ed in Native ICE-HIP
>       protocol in order to avoid middlebox tampering.
> 
> This reads a bit oddly.  Just to check: we don't need to do the
> XORing in Native ICE-HIP because the addresses are covered by an
> HMAC and the "tampering" wouldn't work?
> If so I'd suggest:
> 
>    o  In ICE, addresses being conveyed across a NAT are XOR-ed to
>       prevent middlebox tampering.  Native ICE-HIP does not need to
>       use XOR because such tampering is prevented at the HIP layer.

based on feedback from Eric Rescorla, we changed this text earlier as
follows:

Unlike in ICE, the addresses are not XOR-ed in Native ICE-HIP
protocol but rather encrypted to avoid middlebox tampering.