Subject: [Hipsec] Comments on draft-ietf-hip-nat-base-06 May 6 version

From: Samu Varjonen <[email-masked]>
Date: Wed, 06 May 2009 13:05:31 +0300

Hi,

Just read the new version of the NAT Traversal draft and found few 
points that need more clarifications.

In Fig.1 I was wondering the need for TURN and STUN servers because in 
the Fig. they are not part of the signaling. Also in the text only the 
TURN server is referred to. STUN server seems to be only mentioned in 
the glossary.

At least some titles, section 4.9 and Appendix C. uses "packets" and 
most of the text talks about "messages", when discussed of HIP control 
packets...

In section 4.1 in paragraph "in step2, ..." in the end it says:
"The support for HIP-over-UDP relaying is denoted by the Registration 
Type [RFC5203] value RELAY_UDP_HIP." This is a bit confusing 5203 
defines the registration extensions but the RELAY_UDP_HIP is defined in 
this document but it is hidden into section 5.9. Maybe a clarification 
like "(see section 5.9)" should be added here.

Section 4.2, in my opinion should be named "ICE candidate Gathering in 
BEX" because it covers only BEX and not mobility related gathering. I 
had a brief discussion with Miika on the topic and he said that mobility 
is out of scope of this document. But I did not find it clearly written 
anywhere. So maybe a clarification to the scope should be added.

In section 4.3 "("critical" parameters are defined in[RFC5201])", why 
the quotation characters around critical. Does this point to RFC 5201 
section 5.2.1 and field C and its meaning or does it mean that all the 
critical parameters are defined in 5201 or ... some pseudo critical 
stuff :) ...

Table 4.3 introduces the NAT_TRAVERSAL_MODES and section 5.4 contains 
their numerical values. Just before the table it says they are defined 
in this document. We know where so why not mention the section number.

Fig. 4 step 6 in my opinion the NAT_TM should be indented to the same 
column as LOCATOR in the same step. It would look nicer...

Section 4.5 "in step2, the HIP..." if says RELAY_HMAC as described in 
[RFC5204]" RFC 5204 defines RVS_HMAC. Section 5.8 in this document 
defines the RELAY_HMAC to be similar than RVS_HMAC.

In section 4.3 "Host MUST include ..." says "if they intend to use such 
servers ..." The need to use such servers may come after the BEX. This 
is probably also out of scope as stated earlier about mobility.

In section 4.9 about the wait times. Its a local policy also on how long 
to wait for a timeout when waiting for R1s. consider the case "If I send 
UDP encapsulated I1 and plain I1 to the responder and receive UDP one 
first should I start to using it or how long should I wait for the plain 
one that might work". Now the policy contains only the interval in which 
the packets are sent out and the order but nothing on the inbound R1s 
and their timeouts. Should I actually wait for the both to arrive or 
timeout before I continue.

Next paragraph "The I1 packet without UDP encapsulation may arrive 
directly at the Responder.  When the recipient is the Responder, the 
procedures in [RFC5201] are followed for the rest of the base exchange." 
   The part where it says "When the recipient is the Responder" seems to 
be useless. Recipient of the I1 packet is always Responder OR does this 
have something to do with the relay/RVS?

Section 5.4 last paragraph, it should be added that "Conversely, a 
recipient MUST be prepared to handle received transport parameters that 
contain more than six Mode IDs by accepting the first six Mode IDs and 
dropping the rest." Just to make it uniform with HIP_TRANSFORM and 
ESP_TRANSFORM.

Fig. 8 should the address be locator as defined in glossary and to make 
it uniform with LOCATOR in the next section?

In section 6.1 the privacy reasons should be stated or referred to. For 
example RFC 4555 section 5.5 has discussion on the topic.

Hopefully these are helpful :)

BR,
Samu Varjonen

---

From: Ari Keranen <[email-masked]>
Date: Wed, 06 May 2009 15:14:19 +0300

Hi Samu,

Thanks for the review and good comments! My comments inline.

> TURN server is referred to. STUN server seems to be only mentioned in 

I think they are useful in the figure to show where they are in network 
topology (i.e., in the public Internet as the HIP relay server). Also it 
nicely shows how the architecture aligns with the ICE draft (there's a 
similar figure in the overview section of the ICE draft).

There's no text about the usage of STUN server since it's assumed that 
one uses it as defined in the ICE draft. Section 4.2 (ICE Candidate 
Gathering) tells you to do so.

> most of the text talks about "messages", when discussed of HIP control 

Yes, they are used as synonyms, and both seem to be used quite 
frequently. Do you think they should all be "packets" (or "messages")?

> like "(see section 5.9)" should be added here.

The reference to 5203 was there to clarify what is meant by 
"Registration Type", but you're right that a pointer to sec 5.9 would be 
more helpful. Fixed that.

> anywhere. So maybe a clarification to the scope should be added.

Actually I see no reason why you couldn't use those candidates for 
mobility too -- but, as you said, that is out of scope of the document. 
And the candidates of sec 4.2 are not really for (or usually gathered 
during) the BeX, so I think the current title is more appropriate.

> critical parameters are defined in 5201 or ... some pseudo critical 

That parenthetical part was added there because of a comment that the 
meaning of "critical" parameters might not be that obvious for everyone. 
And yes, we're referring to the C field -- not to any pseudo critical 
things of our own :)

I changed the last sentence into:

    As the parameter is non-critical (as defined in Section
    5.2.1 of [RFC5201]), it can be ignored by an end-host which means
    that the host does not support or is not willing to use these
    extensions.

That's better?

> in this document. We know where so why not mention the section number.

Good point. Fixed it.

> column as LOCATOR in the same step. It would look nicer...

True. Fixed that too.

> defines the RELAY_HMAC to be similar than RVS_HMAC.

The whole sentence is:
    The relay protects the I1 packet with
    RELAY_HMAC as described in [RFC5204], except that the parameter type
    is different (see Section 5.8).

Do you think that needs further clarification? I think the latter part 
of the sentence makes it clear how RVS_HMAC and RELAY_HMAC relate to 
each other.

> is probably also out of scope as stated earlier about mobility.

Yes, that should be defined by the mobility draft. I added "immediately" 
to the sentence to be more clear that you can signal more relay servers 
after the BeX. The first sentence of that paragraph says now:

    Hosts MUST include the address of one or more HIP relay servers
    (including the one that is being used for the initial signaling) in
    the LOCATOR parameter in I2/R2 if they intend to use such servers for
    relaying HIP signaling immediately after the base exchange completes.

> timeout before I continue.

I'd say it's totally up to you (i.e., local policy) and we shouldn't 
restrict that in the draft.

> have something to do with the relay/RVS?

Yes, it refers to the fact that you receive I1 *without* relaying (i.e., 
not via RVS or relay). I changed that into:

    The I1 packet without UDP encapsulation may arrive directly, without
    any relays, at the Responder.  When this happens, the procedures in
    [RFC5201] are followed for the rest of the base exchange.

Hopefully this version is more clear on that.

> dropping the rest." Just to make it uniform with HIP_TRANSFORM and 

Why not. Here's the new text:

    The sender of a NAT_TRAVERSAL_MODE parameter MUST make sure that
    there are no more than six (6) Mode IDs in one NAT_TRAVERSAL_MODE
    parameter.  Conversely, a recipient MUST be prepared to handle
    received NAT traversal mode parameters that contain more than six
    Mode IDs by accepting the first six Mode IDs and dropping the rest.

(BTW, I guess there's a typo in 5202: s/transport/transform/)

> it uniform with LOCATOR in the next section?

Actually the "Locator" from glossary is more generic than the one we're 
referring to here and also the LOCATOR parameter contains a lot more 
stuff (traffic type, lifetime, priority, etc) than what is needed here. 
I think the current format is good for the purpose.

> example RFC 4555 section 5.5 has discussion on the topic.

OK, we'll look into this and see if there's room for improvement.

> Hopefully these are helpful :)

Yes they were -- thank you!

Cheers,
Ari

---

From: Varjonen Samu <[email-masked]>
Date: Fri, 22 May 2009 20:15:53 +0300

> similar figure in the overview section of the ICE draft).

OK

> Gathering) tells you to do so.

Thats 2 pages forward... but its mentioned. Maybe addition to the text 
like "as depicted in Fig. 1".

Noticed one more thing in Section 4.2
"
    However, if no TURN server
    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 and the
    address from the REG_FROM parameter discovered during the relay
    registration as a server reflexive candidate
"

Should this be "However, if no TURN or STUN server..."?

> frequently. Do you think they should all be "packets" (or "messages")?

I think they should be "packets" as they are in RFC 5201.

>> defines the registration extensions but the RELAY_UDP_HIP is defined 
>> in this document but it is hidden into section 5.9. Maybe a 
>> clarification like "(see section 5.9)" should be added here.
> 

OK

>> had a brief discussion with Miika on the topic and he said that 
>> mobility is out of scope of this document. But I did not find it 
>> clearly written anywhere. So maybe a clarification to the scope should 
>> be added.
> 

OK

>    that the host does not support or is not willing to use these
At least I am happy with this

>> column as LOCATOR in the same step. It would look nicer...

OK

> of the sentence makes it clear how RVS_HMAC and RELAY_HMAC relate to 

Must have somehow missed the rest of the sentence... :) Its fine as it is.

>> In section 4.3 "Host MUST include ..." says "if they intend to use 
>> such servers ..." The need to use such servers may come after the BEX. 
>> This is probably also out of scope as stated earlier about mobility.
> 

OK

>> In section 4.9 about the wait times. Its a local policy also on how 
>> long to wait for a timeout when waiting for R1s. consider the case "If 
>> I send UDP encapsulated I1 and plain I1 to the responder and receive 
>> UDP one first should I start to using it or how long should I wait for 
>> the plain one that might work". Now the policy contains only the 
>> interval in which the packets are sent out and the order but nothing 
>> on the inbound R1s and their timeouts. Should I actually wait for the 
>> both to arrive or timeout before I continue.
> 

OK

>> procedures in [RFC5201] are followed for the rest of the base 
>> exchange."   The part where it says "When the recipient is the 
>> Responder" seems to be useless. Recipient of the I1 packet is always 
>> Responder OR does this have something to do with the relay/RVS?
> 

Yes its better

>> recipient MUST be prepared to handle received transport parameters 
>> that contain more than six Mode IDs by accepting the first six Mode 
>> IDs and dropping the rest." Just to make it uniform with HIP_TRANSFORM 
>> and ESP_TRANSFORM.
> 

OK

>> Fig. 8 should the address be locator as defined in glossary and to 
>> make it uniform with LOCATOR in the next section?
> 

Then I think it should be clarified in Section 5.6 that how it differs 
from the more generic address ...?

>> In section 6.1 the privacy reasons should be stated or referred to. 
>> For example RFC 4555 section 5.5 has discussion on the topic.
> 

OK

> Yes they were -- thank you!
BR,
Samu

---

From: Ari Keranen <[email-masked]>
Date: Wed, 27 May 2009 18:44:29 +0300

Hi,

>> Samu Varjonen wrote:

[removed all the parts that were already OK]

>>> In Fig.1 I was wondering the need for TURN and STUN servers because 
>>> in the Fig. they are not part of the signaling. Also in the text only 
>>> the TURN server is referred to. STUN server seems to be only 
>>> mentioned in the glossary.
>>
>> I think they are useful in the figure to show where they are in 
>> network topology (i.e., in the public Internet as the HIP relay 
>> server). Also it nicely shows how the architecture aligns with the ICE 
>> draft (there's a similar figure in the overview section of the ICE 
>> draft).
>>
> 
> OK
> 

You have point. I added a comment about the role of TURN and STUN 
servers to the introduction part (where the Figure 1 is). Now the second 
paragraph says:

     The first steps are for both the Initiator and Responder to register
     with a relay server (need not be the same one) and gather a set of
     address candidates.  The hosts may use TURN and STUN servers for
     gathering the candidates. [...]

> Should this be "However, if no TURN or STUN server..."?

Actually no. If you have only a single local IP address, you don't need 
STUN servers at all since you already get the server reflexive address 
from the HIP relay server.

Of course there's a chance that due to some strange routing arrangements 
you may get different server reflexive addresses from different servers, 
but I'd say that's out of scope for this document.

>>> most of the text talks about "messages", when discussed of HIP 
>>> control packets...
>>

Actually RFC 5201 uses both, but you're right that "packets" are used 
much more often. I changed all the HIP "messages" to "packets".

>> Actually the "Locator" from glossary is more generic than the one 
>> we're referring to here and also the LOCATOR parameter contains a lot 
>> more stuff (traffic type, lifetime, priority, etc) than what is needed 
>> here. I think the current format is good for the purpose.
>>
> 

I think the explanation of "Address" in Section 5.6, Figure 8 ('an IPv6 
address or an IPv4 address in "IPv4-Mapped IPv6 address" format') does 
this already. I.e., it is just "plain IP address". Or can you propose 
some better way to clarify this?

Cheers,
Ari

---

From: Varjonen Samu <[email-masked]>
Date: Thu, 28 May 2009 11:09:56 +0300

Ari Keranen wrote:
>>>> in the Fig. they are not part of the signaling. Also in the text 
>>>> only the TURN server is referred to. STUN server seems to be only 
>>> server). Also it nicely shows how the architecture aligns with the 
>>> ICE draft (there's a similar figure in the overview section of the 
>>> ICE draft).
>>>
>>
>> OK
>>
>>> There's no text about the usage of STUN server since it's assumed 
>>> that one uses it as defined in the ICE draft. Section 4.2 (ICE 
>>> Candidate Gathering) tells you to do so.
>>>
>>

OK

OK

OK

>>> more stuff (traffic type, lifetime, priority, etc) than what is 
>>> needed here. I think the current format is good for the purpose.
>>>
>>

OK, but if I follow this correctly we can say that the "locator" in the 
LOCATOR parameter should actually be "address" because the locator is 
address + the extra information like priority etc.

This has been fun.
Waiting for the next revision,
Samu

---

From: Ari Keranen <[email-masked]>
Date: Fri, 29 May 2009 10:49:58 +0300

Hi,

Varjonen Samu wrote:
>>>> we're referring to here and also the LOCATOR parameter contains a 
>>>> lot more stuff (traffic type, lifetime, priority, etc) than what is 
>>> Then I think it should be clarified in Section 5.6 that how it 
>>> differs from the more generic address ...?
>>
>> I think the explanation of "Address" in Section 5.6, Figure 8 ('an 
>> IPv6 address or an IPv4 address in "IPv4-Mapped IPv6 address" format') 
>> does this already. I.e., it is just "plain IP address". Or can you 
>> propose some better way to clarify this?
> 

You're right. So, actually the problem was not in Figure 8, but rather 
in Figure 9 (depicting the LOCATOR parameter) and Table 2 (explaining 
the Figure 9). I'll change the "locator" there into "address". Good catch!

> This has been fun.

I'm glad you've enjoyed reviewing the draft :)

> Waiting for the next revision,

We'll do some final brushing for the draft and publish the next version 
soon.

Cheers,
Ari