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

[Benjamin Kaduk via Datatracker <[e-mail]>]

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

Thanks for addressing the potential "cross-message" attack on the HMAC
contents of RELAY_HMAC/RVS_HMAC by prohibiting the Control Relay Server
from offering the rendezvous services.  I think in order for the
protection against the attack to be complete, though, we need to say
that a HIP peer attempting to reach a Control Relay Server MUST reject
any messages appearing to originate from that server, that contain an
RVS_HMAC parameter.  That is, the current text will keep honest actors
from generating the bad situation, but we also want to protect ourselves
against accepting input from a bad actor attempting to cause the bad
situation.

Thank you as well for addressing all of my other comments on the -30.
They seem generally satisfactory, and my apologies for not responding to
them sooner.  I just have two remaining remarks:

Section 1

   tunneling overhead).  Another solution is specified in [RFC5770],
   which will be referred to "Legacy ICE-HIP" in this document.  The

nit: s/to/to as/

Section 4.6.2

   [RFC7401] section 5.3.5 states that UPDATE packets have to include
   either a SEQ or ACK parameter (but can include both).  According to
   the RFC, each SEQ parameter should be acknowledged separately.  In

I don't see anything to support "acknowledged separately"; on the
contrary, I see "A host MAY choose to acknowledge more than one UPDATE
packet at a time; e.g., the ACK parameter may contain the last two SEQ
values received, for resilience against packet loss."  Perhaps the
intent was "each SEQ parameter needs to be explicitly acknowledged"?