Hi Christer,
On 03/26/2017 02:34 AM, Christer Holmberg wrote:
Hi,
As co-author for the ICEbis draft, I was asked to review
draft-hip-native-nat-traversal-19.
I have not had time to review the whole document. However, many of my
comments are generic, and apply to the whole document.
thanks for the feedback! Your (editorial) comments should be addressed
in this version:
https://tools.ietf.org/html/draft-ietf-hip-native-nat-traversal-20
The diff is here:
https://www.ietf.org/rfcdiff?url2=draft-ietf-hip-native-nat-traversal-20
I have no knowledge of HIP, so I will not comment on HIP issues like
messages, parameters etc used.
If one implements this, some digging into the HIP documents is needed.
From a design perspective, the document should be readable as it is (I
hope).
General: =======
QG0: Throughout the document, you use RFC 2119 terminology (SHOULD,
MUST etc with capital letters) when you refer to procedures and rules
defined elsewhere. I think that is wrong, and it also makes it very
difficult what exactly is defined in this document, and what is
defined in some other specification.
I am keeping the SHOULD/MUST/etc for HIP documents (since this is an
extension of HIP) but ICE references are now with small letters (there
actually weren't too many).
QG1: You say that the mechanism in the draft is based on ICE. I think
it would be good to give a name to the mechanism. "HIP-ICE", or
something similar.
Done:
ICE:
Interactive Connectivity Establishment (ICE) protocol as specified
in [I-D.ietf-ice-rfc5245bis]
Legacy ICE-HIP:
Refers to the "Basic Host Identity Protocol (HIP) Extensions for
Traversal of Network Address Translators" as specified in
[RFC5770]. The protocol specified in this document offers an
alternative to Legacy ICE-HIP.
Native ICE-HIP:
The protocol specified in this document (Native NAT Traversal Mode
for HIP).
QG2: I would also like to have a dedicated section which on a
high-level describes the differences/restrictions between legacy ICE
and HIP-ICE. It helps very much when later reading the details in
section 4. That section should at least list the different types of
functions (HIP relays etc) are used for gathering candidates, what
protocol (HIP messages) is used instead of STUN, what types of
candidates are used and how they are retrieved.
The ICE comparison has been already in the previous version in Appendix
B ("Differences with respect to ICE") and now the intro also references
this section ("Appendix B explains the differences to ICE."). I think
it's not good to start the document with a diff to ICE. We have to
assume that the reader is familiar with HIP since this is an extension
to it.
It would also be good to give a short overview of the HIP messages
used for the connectivity checks. It is very useful when later
reading the details.
The intro now includes a really short intro to HIP, but I don't think we
should introduce the messaging formats here again. This is an extension
to HIP, so some familiarity with HIP is required.
(Btw, the ICE specification does not describe STUN/TURN messaging
formats either, so the situation is the same for that document)
QG3: You should use consistent terminology when you talk about
endpoints and relays. Sometimes the text says "host", sometimes "HIP
relay server client", sometimes "relay client", sometimes
"end-host". Sometimes you say "HIP relay", sometimes "HIP server
relay", etc. Sometimes you say "non-relay host", which suggests that
the relay is also a host.
the document has over 300 occurrences of host, could you be a bit more
specific where this is a problem? "End-host" means "non-relay host", and
yes, a relay is a host too.
I agree the client/server relay terminology was a bit sloppily used. In
general, I think the terms were a bit asymmetrical:
* HIP vs. data relay, why not just "control" and "data"
* HIP relay vs HIP relay client (why not HIP relay *server*)
So I came up with a better relay terminology that is now applied
consistently throughout the document:
Control Relay Client:
A requester host that registers to a Control Relay Server
requesting it to forward control-plane traffic (i.e. HIP control
messages). In the Legacy ICE-HIP specification, this is denoted
as "HIP Relay Client".
Data Relay Server:
A registrar host that forwards HIP related data plane packets,
such as Encapsulating Security Payload (ESP) [RFC7402], between
two hosts. This host implements similar functionality as TURN
servers.
Data Relay Client:
A requester host that registers to a Data Relay Server requesting
it to forward data-plane traffic (e.g. ESP traffic).
Section 3: ========
Q30: The text says:
"The hosts may use HIP relay servers (or even STUN or TURN servers)
for gathering the candidates."
This is confusing, as you have earlier said that HIP-ICE doesn't use
STUN.
(Implementations may of course provide both STUN- and HIP
functionality, but that is outside the scope of the document.)
I hope this is now better:
Gathering of candidates MAY also be performed by other means than
described in this section. For example, the candidates could be
gathered as specified in Section 4.2 of [RFC5770] if STUN servers are
available, or if the host has just a single interface and no STUN or
Data Relay Server are available.
How the candidates are gathered does not really affect interoperability.
Q31: The text says:
"To be contacted from behind a NAT, the Responder must be registered
with a HIP relay server reachable on the public Internet, and we
assume, as a starting point, that the Initiator knows both the
Responder's Host Identity Tag (HIT) and the address of one of its
relay servers”
First, when you say "its relay servers", I assume you mean the relay
servers of the Responder?
yes
Second, doesn't the Initiator need to know the address of the relay
to which the Responder is actually registered, in case there are
multiple relays but the Responder is not registered with all of
them? Maybe someone thinks it's obvious, but I think it should be
make clear.
Could you simply say:
"To be contacted from behind a NAT, the Responder must be registered
with a HIP relay server reachable on the public Internet. It is
assumed that
the Initiator knows the address of the relay server(s) to which the
Responder
is registered."
The text is now a bit rearranged, but this is now changed as follows:
To be contacted from behind a NAT,
at least the Responder must be registered with a Control Relay Server
reachable on the public Internet. [..]
We assume, as a starting point, that the Initiator knows both the
Responder's Host Identity Tag (HIT) and the address(es) of the
Responder's Control Relay Server(s) (how the Initiator learns of the
Responder's Control Relay Server is outside of the scope of this
document, but may be through DNS or another name service).
Q32: The section introduces the "base exchange" concept, but it is
not clearly defined. Is it something defined in HIP, is it HIP-ICE
specific? I think you should add a description/reference somewhere.
Defined in HIP and briefly now introduced and referenced in the
introduction.
Q33: The text says:
"At the end of the procedure, if successful, the hosts will have
established a
UDP-based tunnel that traverses both NATs, with the data flowing
directly from NAT to NAT or via a HIP data relay server."
What if the responder has only registered to a HIP server relay? The
text in the section seems to suggest that it is optional to register
to a data relay.
The text needs to be more clear on whether the endpoints need to
register to a server relay and/or data relay.
I think this should be clear now:
The Responder may have also
registered to a Data Relay Server that can forward the data plane in
case NAT penetration fails. [..]
To be contacted from behind a NAT,
*at least* the Responder must be registered with a Control Relay
Server reachable on the public Internet. [..]
Relayed candidates SHOULD be gathered in
order to guarantee successful NAT traversal, and implementations
SHOULD support this functionality even if it will not be used in
deployments in order to enable it by software configuration update if
needed at some point.
Section 4.2: ==========
Q421: In general, I think it would be useful to split the section
into HIP client procedures and HIP relay server procedures.
Actually now the section just includes client procedures, so I changed
the name.
Q422: I think the following text should be in the beginning of the
section:
"ICE guidelines for candidate gathering MUST be followed as described
in section 4.1.1 in [I-D.ietf-ice-rfc5245bis]. A number of host
candidates (loopback, anycast and others) should excluded as
described in section 4.1.1.1 of the ICE specification
[I-D.ietf-ice-rfc5245bis]."
I think moving this text does not improve readability of the section, so
I didn't do this. Currently, there is some intro text before this.
Q423: The text says:
"However, if no data relay is used, and the host has only a single
local IP address to use, the host MAY use the local address as the
only host candidate"
I am not sure what is meant by "as the only host candidate".
So if the host has no clue on the server reflexive/relayed addresses, it
should use the address as seen from the network interface as the (only)
host candidate. I change this if needed if some other wording works better?
Q424: The text says:
"Relayed candidates SHOULD be gathered in order to guarantee
successful NAT traversal. It is RECOMMENDED for implementations to
support this functionality"
If you say SHOULD use, why do you then only say RECOMMEND support?
Doesn't the support need to be stronger than the usage?
Fixed, both are now SHOULD.
Q425: The text says:
"Unlike ICE, this protocol only creates a single UDP flow between
the two communicating hosts, so only a single component exists."
This is confusing. What is meant by "unlike ICE"? You are using ICE
:) I assume you are referring to ICE usage for non-multiplexed
RTP/RTCP?
ICE is now defined in the terminology (I-D.ietf-ice-rfc5245bis)
I think you simply need to say that HIP only uses a single UDP flow,
using a single component, with a value of 1.
I can remove the ICE comparison if this is still confusing...
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.
Section 4.3: ==========
Q431: The text says:
"This section describes the usage of a new non-critical parameter
type."
Shouldn't it say that the section defines a new parameter type?
No, just the usage. The actual format/definitions are in section 5.
Q432: If I understand correctly, the NAT mode negotiation between an
endpoint and a relay takes place during registration.
yes. Registration = base exchange with some registration parameters.
But, when does the NAT mode negotiation between two endpoints (shown
in the call flow) take place? During connectivity checks? I think
you should clarify that.
During base exchange again, in steps 3-6 as defined in section:
4.5. Base Exchange via Control Relay Server
I am not sure what is unclear.
Section 4.6: ==========
Q460:The text says:
"but UDP encapsulated HIP control messages are used instead of ICE
messages."
What do you mean by "ICE messages"? STUN?
ICE refers to I-D.ietf-ice-rfc5245bis, so yes. I can change this if needed.
Section 4.6.2: ============
Q4620: The text says:
"The HITs of the two communicating hosts MUST be used as credentials
in this protocol (in contrast to ICE which employs username-password
fragments)."
Again, "contrast to ICE" sounds confusing, because you are using ICE.
Maybe you should say "legacy ICE", or something similar, when
referring to 5245bis procedures.
ICE refers to I-D.ietf-ice-rfc5245bis in the terminology, so let me know
if this is still unclear...
Section 4.7: ==========
Q470: The name of the section ("NAT traversal alternatives") is
confusing, because one may think it talks about something else than
HIP-ICE.
I would suggest to call it something like "Optimizations".
Done.
Section 4.9: ==========
Q490: I think Mobiliy should be described in a separate main
section.
The document is structured the same way as the other HIP related RFCs,
so all protocol interaction is described in "Protocol Description".
_______________________________________________
Hipsec mailing list
Hipsec@ietf.org
https://www.ietf.org/mailman/listinfo/hipsec
