Comments of draft-ietf-mobike-protocol-04.txt

1)

In section 3.2. the second example should probably clarify that the
exchange in step 3 is successful, i.e. the responder do respond to
that in this example protocol run (as can be seen from the return
packet sent by the responder, but the text below it gives feeling that
it failed ("initiator gives up")).

Perhaps there should also be example where that exchange is not
successful.
----------------------------------------------------------------------
2)

In section 4.2 we should always take the IP-addresses from the
IKE_AUTH request, as there might be multiple IKE_SA_INIT request, and
some of those might not create state at all (cookie,
invalid_ke_payload). I think it would be simplier to say that the
IP-addresses are always taken from the IP header of the IKE_AUTH
packet having SA payload (regardless of the NAT-T or no NAT-T).

In normal IKEv2 this does not matter, as if no NAT-T is used then the IP
address must stay same, and if NAT-T is used then we take the last
packet seen and use the IP-addresses from there. In Mobike we do need
to define the exact time what IP-address is used, and I think the
packet having the ADDITIONAL_*_ADDRESSES would be the most logical
choise (i.e. the IKE_AUTH packet having the SA payload).
----------------------------------------------------------------------
3)

I think that section 4.5 should clarify you use either
ADDITIONAL_*_ADDRESSES or NO_ADDITIONAL_ADDRESSES, not both (but you
always have either exactly one NO_ADDITIONAL_ADDRESSES or one or more
ADDITIONAL_*_ADDRESSES), and also explain more when to use
NO_ADDITIONAL_ADDRESSES.

The actual usage of the NO_ADDITIONAL_ADDRESSES is not really
explained anywhere (though it is quite obvious from the name if people
understand that the ADDITIONAL_*_ADDRESSES are only used for the
ADDITIONAL addresses beside the ones in the IP header of the exchange
packet). Actually, I think even that text is missing, i.e. text saying
that the ADDITIONAL_*_ADDRESSES are used in addition to the IP address
of the current exchange packet.
----------------------------------------------------------------------
4)

The last sentence in the section 4.5 should say

	Similarly, a simple "VPN gateway" that has only a single
	address, and is not going to change it, does not need to send
	or process ADDITIONAL_*_ADDRESS notifications.

It needs to understand them so much it can ignore them.
----------------------------------------------------------------------
5)

In section 4.6 the paragraph:

   Any INFORMATIONAL exchange can be used for return routability
   purposes, with one exception: when a valid response is received, we
   know the other party can receive packets at the claimed address.

should probably tell what is the one exception... Or more exactly what
that paragraph is trying to tell us?
----------------------------------------------------------------------
6)

Perhaps the section 4.7 should also mention that DPD packets can also
include UPDATE_SA_ADDRESSES in case the initiator suspects that the
NAT mapping has changed, and that will save one round trip.
----------------------------------------------------------------------
7)

The section 4.8 says that NO_NATS_ALLOWED should be put to the
IKE_SA_INIT (among other things), but we do want to keep IKE_SA_INIT
as small as possible, so I think IKE_AUTH is much better place for
that notify. That would also be in sync with my proposed change to the
4.2 where I proposed to say that use the IKE_AUTH packet having SA
payload to get the IP address tobe used. NO_NATS_ALLOWED is not needed
in the IKE_SA_INIT like NAT_DETECTION_*_IP notifies are, as they do
not cause changing of the port numbers for the IKE_AUTH.

Also moving that NOFITY to the encrypted IKE_AUTH packet will prevent
passive attackers of getting the information about the IP addresses
and ports used in case host behind NAT is configured to try first with
NO_NATS_ALLOWED before falling back and enabling NAT-T. It will also
make the attacker acting as a NAT between two hosts bit harder, as if
it is sent inside the IKE_SA_INIT, attacker can simply flip a bit
inside the NO_NATS_ALLOWED payload to cause DoS, otherwise he need to
flip bit in the IP-address or ports, which requires it to catch the
return packet and forward it to the original requestor to cause
attack. It is not much, but offers again a bit more protection against
attacks.
----------------------------------------------------------------------
8)

Perhaps the section 5.1 should say "There is no data associated with
this Notify type, and the when processing Notify received from the
other end the notify data MUST be ignored." Then we can later add some
data there and old version will ignore it. That will allow us
possibility to extend this payload later. With the current text
implementations can simply fail the whole IKE SA negotiation if there
is data in the this notify. (or if you consider this to be too close
to the already covered no-version-numbers case, then feel free to
ignore this comment).
----------------------------------------------------------------------
9)

I agree with the previous proposal that we should split those notifies
to error and status notifies, and have separate section for each of
those.
----------------------------------------------------------------------
10)

The section 5.7 of the NO_NATS_ALLOWED could have a bit more text
explaining the exact data put in, i.e. something like

	  Data = src-ip (4 or 16 bytes) | src-port (2 bytes) |
	         dst-ip (4 or 16 bytes) | dst-port (2 bytes)

	All data is stored in the network byte order without any
	padding. The lenght of the data is 12 bytes for IPv4, and 36
	bytes for the IPv6.
----------------------------------------------------------------------
11)

The section 6.1 should probably also mention that NO_NATS_ALLOWED
might be used with IPv6 in general, where NATs are not (yet) used.
----------------------------------------------------------------------
12)

The section 6.5 should mention that those information is generally
available only for the other peer, not to the passive listeners (it is
encrypted). There is two (or one if we move the NO_NATS_ALLOWED)
exceptions to the rule: NAT_DETECTION_*_IP packets of the IKE_SA_INIT
and the NO_NATS_ALLOWED (if sent inside the IKE_SA_INIT). Those
payloads having IP-address information is sent in clear.
-- 
[email protected]