Subject: [Hipsec] Benjamin Kaduk's Discuss on draft-ietf-hip-native-nat-traversal-30: (with DISCUSS and COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Tue, 03 Mar 2020 20:09:00 -0800

Benjamin Kaduk has entered the following ballot position for
draft-ietf-hip-native-nat-traversal-30: 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:
----------------------------------------------------------------------

One fairly minor point to start: Section 4.3 says that we define a new
mode (ICE-HIP-UDP) for the NAT_TRAVERSAL_MODE parameter type, but then
goes on to say that "the presence of the parameter in a HIP base
exchange means that the end-host supports NAT traversal extensions
defined in this document".  If I undrestand correctly, only the specific
presence of the ICE-HIP-UDP mode of the NAT_TRAVERSAL_MODE parameter
does so, and so to say that the presence of "the [NAT_TRAVERSAL_MODE]
parameter" indicates support for this document would be backwards
incompatible with RFC 5770.

I'd also like to delve a little further into the potential
"cross-protocol" attack (same protocol, really, but the same attack)
that Ekr raised, between RVS_HMAC and RELAY_HMAC.  This is probably a
"discuss discuss", so let's see where it leads...

The semantics for either type of HMAC is that it is an HMAC over the HIP
packet excluding itself and subsequent parameters.  Pulling up the HIP
packet format from RFC 7401, that looks like:

    0                   1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   | Next Header   | Header Length |0| Packet Type |Version| RES.|1|
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |          Checksum             |           Controls            |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                Sender's Host Identity Tag (HIT)               |
   |                                                               |
   |                                                               |
   |                                                               |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |               Receiver's Host Identity Tag (HIT)              |
   |                                                               |
   |                                                               |
   |                                                               |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                                                               |
   /                        HIP Parameters                         /
   /                                                               /
   |                                                               |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

The HMAC key is the integrity key for that direction of traffic between
HITs, so the "cross-protocol" part can only come in by confusing the
packet recipient into confusion as to whether it is processing an
RVS_HMAC or a RELAY_HMAC (but any other entity will reject the packet by
virtue of it using the wrong key).  Modern best practices are to go
through a key derivation step that incorporates as much information as
possible about what the derived key will be used for, which would in
this case include the TLV type of the HMAC parameter and presumably the
HITs in question as well.

In particular, the TLV type of the HMAC parameter is *not* input into
the HMAC calculation (at least for RVS_HMAC), so the trivial
discriminator is not present.  The "packet type" in the header in the
header is potentially going to differ across usages, so I think that's a
good place to focus discussion.  Unfortunately, Section 4.2.1 of RFC
8004 suggests that RVS_HMAC is going to be present a lot of the time, so
it's not really clear to me what packet types either RVS_HMAC and/or
REPLAY_HMAC are expected to occur in.  Could a HIP expert please jump in
and help clarify?

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

[My latest reply to the ballot thread for my previous ballot position,
https://mailarchive.ietf.org/arch/msg/hipsec/iP_of3P2RfxJIbeVDz2qGonO8nY/
, seems to have not gotten a response]

Why did some rfc5245bis references get converted to RFC5245 and others
to RFC8445?

The phrase "Control Data Relay" occurs just five times in the document
(vs. over a hundred for "Control Relay") and seems to needlessly
conflate "Control Relay" and "Data Relay".  I'd suggest rewording to
avoid this phrase.

Abstract

   and UDP encapsulation of data and signaling traffic.  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.

It might be worth disambiguating what "its" refers to.

Section 1

   messages tunneled over a single UDP flow.  The benefit of using ICE
   and its STUN/TURN messaging formats is that one can re-use the NAT
   traversal infrastructure already available in the Internet, such as
   STUN and TURN servers.  Also, some middleboxes may be STUN-aware and
   may be able to do something "smart" when they see STUN being used for
   NAT traversal.

Don't we go on to say that we can't actually use stock TURN
infrastructure since we'd need extra encapsulation for the HIP bits?

Section 2

   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

nit: s/does specify/does not specify/

      handover.  Following [RFC5770] and Session Description Protocol
      (SDP) [RFC3264] naming conventions, "HIP offer" is the the

nit: we just expanded SDP a few lines previously; we probably don't need
to do it again.

Section 3

   connected to the public Internet.  To be contacted from behind a NAT,
   at least the Responder must be registered with is own Control Relay
   Server reachable on the public Internet.  The Responder may have also

nit: "is own" is not right; I'm not sure whether "his own" or "its own"
was intended (but the "a Control Relay Server in the -28 didn't seem
wrong to me either).

Section 4.1

   In step 3, the Relay Client selects the services for which it
   registers and lists them in the REG_REQ parameter.  The Relay Client
   registers for the Control Data Relay service by listing the
   RELAY_UDP_HIP value in the request parameter.  If the Relay Client
   requires also ESP relaying over UDP, it lists also RELAY_UDP_ESP.

(This is one of the five "Control Data Relay"s I mention previously.)
I'd suggest using "Data Relay Service" in combination with RELAY_UDP_ESP
to reinforce the link between the two terms.  The current "mix and
match" of "Control [...] Relay" and "RELAY_UDP_ESP" requires more effort
to understand than using a uniform set of terminology would.

   ECHO_REQUEST_SIGNED . In case the Data Relay Server admitted a new
   relayed UDP port for the Data Relay Client, the REG_RES parameter
   MUST list RELAY_UDP_ESP as a service and the UPDATE message MUST also
   include a RELAYED_ADDRESS parameter describing the relayed UDP port.

nit: "admitted" is a surprising word choice, to me.

Section 4.2

   local preference value MUST be unique.  Dual-stack considerations for
   IPv6 apply also here as defined in [RFC8445] in section 5.1.2.2.

It looks like this should be 5.1.2.1, not 5.1.2.2.

   where Ta is the value used for the connectivity check pacing and Num-
   Of-Cands is the sum of server-reflexive and relay candidates.  A
   smaller value than 500 ms for the RTO MUST NOT be used.

nit: I don't think addition is defined on (set-of-server-reflexive
candidates, set-of-relay candidates); presumably there's some missing
"number of"s here.

Section 4.3

   In step 4, the Responder concludes the base exchange with an R2
   packet.  If the Initiator chose ICE NAT traversal mode, the Responder

nit(?): s/ICE NAT/ICE-HIP-UDP/?

Section 4.5

   repeated here.  Similarly, the NAT traversal mode negotiation process
   (denoted as NAT_TM in the example) was described in Section 4.3 and
   is neither repeated here.  If a Control Relay Server receives an R1

nit: AFAIK this is not a correct usage of "neither".

   It is RECOMMENDED that the Initiator send an I1 packet encapsulated
   in UDP when it is destined to an IPv4 address of the Responder.

   Respectively, the Responder MUST respond to such an I1 packet with a
   UDP-encapsulated R1 packet, and also the rest of the communication
   related to the HIP association MUST also use UDP encapsulation.

Is this Responder behavior also restricted to IPv4?

   Server.  The Control Relay Server protects the I1 packet with
   RELAY_HMAC as described in [RFC8004], except that the parameter type
   is different (see Section 5.8).  The Control Relay Server changes the

I suggest dropping the 8004 reference and just using Section 5.8 of this
document.

   In step 3, the Responder receives the I1 packet.  The Responder
   processes it according to the rules in [RFC7401].  In addition, the
   Responder validates the RELAY_HMAC according to [RFC8004] and
   silently drops the packet if the validation fails.  The Responder

[likewise here]

   In step 7, the Responder receives the I2 packet and processes it
   according to [RFC7401].  The Responder validates the RELAY_HMAC
   according to [RFC8004] and silently drops the packet if the
   validation fails.  It replies with an R2 packet and includes a

[and here]

   Hosts MUST include the address of one or more Control Relay Servers
   (including the one that is being used for the initial signaling) in
   the LOCATOR_SET parameter in I2 and R2 if they intend to use such
   servers for relaying HIP signaling immediately after the base
   exchange completes.  The traffic type of these addresses MUST be "HIP
   signaling" and they MUST NOT be used as HIP candidates.  If the

I'm not sure I understand how the recipient will know that a given "HIP
signaling" candidate is intended only for signaling and is not to be
used as a HIP candidate.  Is that inherent to the "HIP signaling"
traffic type (vs. ESP)?  (We don't seem to define "HIP candidate"
specifically", which probably contributes to my confusion.)

Section 4.6.2

   All of the connectivity check packets MUST be protected with HMACs
   and signatures (even though the illustrations in this specification
   omit them for simplicity).  Each connectivity check sent by a host

Are these the RELAY_HMACs defined by this document or the normal
HIP_MAC?

   [RFC7401] states that UPDATE packets have to include either a SEQ or
   ACK parameter (or both).  According to the RFC, each SEQ parameter
   should be acknowledged separately.  In the context of NATs, this

I'm not finding where RFC 7401 says that each SEQ parameter should be
"acknowledged separately".  (A few sentences later we say that an ACK
parameter may acknowledge multiple sequence identifiers, too...)

   Full ICE procedures for prioritizing candidates, eliminating
   redundant candidates, forming check lists (including pruning) and
   triggered check-queues MUST be followed as specified in section 6.1
   [RFC8445], with the exception that the foundation, frozen candidates

nit: missing "of"

Section 4.7.2

   traversal mode as one of the supported modes in the R1 packet.  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

Previous mentions of LOCATOR_SET have mandated an ENCRYPTED container.
If this instance is notable in its exception from that, I suggest
calling it out explicitly as not encrypted; otherwise, I think a
reminder might be in order.

   is able to receive UDP-encapsulated ESP and HIP packets.  This
   locator MUST be of type "Transport address", its Traffic type MUST be
   "both", and it MUST have the "Preferred bit" set (see Table 1).  If
   there is no such locator in R1, the source address of R1 is used as
   the Responder's preferred address.

Given the last sentence, it seems like the "MUST also include [...]
preferred address" is unenforceable by the Initiator.

Section 4.9

   responds with an UPDATE with ECHO_REQ_SIGN as described in step 3 in
   Figure 6.  Upon receiving the valid response from host M as described
   in step 6, the peer host MUST restart the connectivity checks with
   host M.  This way, both hosts start the connectivity checks roughly
   in a synchronized way.  It is also important that peer host does not
   restart the connectivity checks until it has received a valid "fresh"
   confirmation from host M because the UPDATE message carrying locators
   could be replayed by an attacker.

Can you remind me which step this "fresh" confirmation is in?  Step 6
doesn't really seem to match that description...

Section 5.4

   The format of NAT traversal mode parameter is borrowed from Legacy
   ICE-HIP [RFC5770].  The format of the NAT_TRAVERSAL_MODE parameter is

"Borrowed from"?  It had darn well better be *the same as* (i.e.,
"retained from") the RFC 5770 format, the way we're using the IANA
registry for NAT traversal modes...
(The "defined in [RFC5770]. but repeated for completeness" language we
use for TRANSACTION_PACING seems much better.)

Section 5.7

Should we say that the LOCATOR_SET must always appear within an
ENCRYPTED wrapper?

Section 6.1

It's hard to see the phrase "requires further experimentation" in a
proposed standard.  (Maybe it shouldn't be, given the "proposed" part,
but in practice it seems to be.)  Perhaps we could just say that which
host addresses to exclude from the LOCATOR_SET is a matter of local
policy?

   This scenario is referred to as the hairpin problem [RFC5128].  With
   such a legacy NAT, the only option left would be to use a relayed
   transport address from an Control Relay Server server.

Data Relay Server, no?  (Also, nit: "Server server".)

Section 6.7

   replay attacks that are difficult to prevent.  The connectivity
   checks in this protocol are immune against replay attacks because a
   connectivity request includes a random nonce that the recipient must
   sign and send back as a response.

If I was feeling particularly pedantic I'd request "effectively immune",
since there is the minor risk of collision in the random nonce values.

Section 10.1

RFC 5766 could probably be just an informative reference; RFCs 6146 and
6147 as well.

Section 10.2

I'm not sure that we still need to reference RFC 5245 (vs. 8445)?

The status of RFC 5770 is less clear; in Section 5 we note that "most of
the parameters are shown for the sake of completeness", implying that
not all are shown, which would in theory leave a normative dependency on
RFC 5770 for those parameters' interpretation.

Similarly, the status of RFC 3948 is also less clear; we do use a fair
bit of machinery from it and it's not trivially clear that we reproduce
everything that is needed.

Could we get "draft-rosenberg-mmusic-ice-nonsip" included in the
[MMUSIC-ICE] reference, assuming that's the right I-D?

Similarly, a draft name for [HIP-MIDDLE] would be helpful.

Appendix A

["further experimentation" here as well]