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

[Benjamin Kaduk <[e-mail]>]

On Tue, Feb 18, 2020 at 05:22:35PM +0000, Miika Komu wrote:
> 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

A well-behaved receiver host drops the packet; what if the host is under
the control of an attacker?

> (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 :)

I don't think I'm actually confused, but I'm saying that if we talk about a
"registered Responder", a reasonable reader might infer that there is some
entity that has registered to be a Responder, which is not exactly what's
going on -- there's an entity that has registered, and registration is
needed in order to be a responder when behind a NAT, and it happens to be the
responder this time.  But it is free to be an initiator for other exchanges
without additional registration.

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

The "fixed" version says "Control Relay" but I thought it was going to be
"Data Relay"; you didn't explicitly tell me I was wrong about it, so please
confirm one way or the other...

Thanks for the updates!

-Ben

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