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

[Benjamin Kaduk <[e-mail]>]

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.

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

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?


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.


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?

   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.


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.

   [...] 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).

   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?


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?

   [...] 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?)

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


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

   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.


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


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.

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


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?


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?


Section 4.9

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


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


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

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.


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.


Section 6.6

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


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.