Draft:  draft-ietf-mobike-protocol-07.txt
Reviewer: Elwyn Davies [elwynd@dial.pipex.com]
Review Date: Friday 1/6/2006 8:10 PM
LC Date: 3 january 2006

Summary:
This document appears to be an excellent piece of work which I found 
highly comprehensible, given that I am not a deep security expert.  
There are a small number of minor quibbles and some editorial nits 
(particularly the explanation of terminology in s2.2) which ought to be 
fixed but it is essentially ready for the IESG. [I note that it has been 
scheduled for the next IESG telechat - this review is a couple of days 
late for IETF Last Call]

Detail:

General: There is no discussion of how the Critical bit should be set in 
the various Notifications.

s1.1, para 1: IKEv2 doesn't just create SAs for use with tunnel mode.

s3.5: Might be good to say explicitly before this point that additional 
info is needed in the SA (presumably in the SAD) including 
pending_update flag and the additional addresses.

s3.9:
  'If the exchange initiator receives an UNEXPECTED_NAT_DETECTED
   notification in response to its INFORMATIONAL request, it SHOULD
   retry the operation several times using new INFORMATIONAL requests.
   Similarly, if the initiator receives UNEXPECTED_NAT_DETECTED in the
   IKE_AUTH exchange, it SHOULD retry IKE_SA establishment several
   times, starting from a new IKE_SA_INIT request.'
Do we have any advice on the value of several here?

s3.12: Would it not be the case that the IPsec packets would fail 
validation against the SA database (i.e. the  SPI wouldn't match with 
the addresses and verification would fail)?  Or are there cases (null 
encryption/auth maybe) when more verification is needed? I wonder if 
this can be made more specific?

Editorial:
s1.1: Maybe should explain why only IKEv2 is considered.
s1.1, para 1: Need a reference for IKEv2 here (now RFC4306).

s2.1, para 5: s/common with the use/common to the use/

s2.2, title: ? s/Runs/Exchanges/

s2.2: This needs an explanation of the notation including a copy of the 
notation table from RFC4306 s1.2 and what is the meaning of N(...), plus 
the meaning of (IPx:4500 -> IPy:4500) etc.
s2.2: I don't think the meaning of the braces pair {} in the packet 
exchanges is defined.

s3.4: Maybe explicitly say ADDITIONAL_*_ADDRESS ::= 
ADDITIONAL_IPv4_ADDRESS | ADDITIONAL_IPv6_ADDRESS

s3.6, Next to last para: Might be good to add the reason why the 
additional addresses are not needed, i.e., the same one as in 
parentheses at the end of the next para.