Document:   draft-ietf-mip4-rfc3012bis-04.txt
Reviewer: Elwyn Davies [elwynd@dial.pipex.com]
Review Date:  Tuesday 9/27/2005 8:19 AM CST
LC Date:  9/26/2005

Summary: This is not one of the IETF's better productions.  
Given the last call comments from the mdir directorate, 
I expect that this document will get significantly revised.  
Accordingly, I have not done a very thorough 
review on the document.  I would concur that this draft 
needs considerably more work to be a useful PS.  There has to 
be some doubt, given the use of CHAP, whether it is *worth* 
devoting too much effort to it, but that is a decision for 
others.  The overall impression that comes over from the 
document is a hurried attempt to patch up a crumbling edifice 
that maybe ought to be demolished and rebuilt differently.  
The convoluted logic described and the fact that it is extensions to, combinations of or realignments of about three other protocols 
makes it extremely difficult to see whether the whole thing could be correct - the additions since RFC3012 appear to be attempts to 
fix something that was pretty broken!

Review:
One comment from the mdir was that this document describes two protocols 
(the CHAP stuff and the AAA stuff).  The usage of phrases such as 
'updates RFC3344 by including new authentication extension' in a 
paragraph added to the abstract gives the impression that an extra 
protocol has been added: in fact, this isn't the case.  No new protocol 
or extensions have been added since 3012 - just clarifications of 
operation.  So I would go through the document and remove all the 'new' 
words.  They weren't new before this document and they certainly aren't 
new now.  As regards separating out the two parts, this might simplify 
some parts: The wg would have to determine if the work needed is 
justified for either part.

A few points which jumped out at me:

Abstract: too long.

s2 and s2.1 may conflict: s2 says the challenge must be previously 
unused but s2.1 says a multicast advert should reuse the latest 
challenge with no quailifications on status.  There is no motivation for 
why the challenge should be reused... It is possible that this is on 
account of the extra piece in the security considerations about DoS 
attacks but something should be said here.

s2.1: .  CHALLENGE WINDOW hasn't been defined when it is used here

s3.1: First para has been added in update: why do we now go straight to 
retransmission? we don't know what is being transmitted yet!!

ss3.1-3.4: The way in which the logic is written down here made my brain 
hurt.  Surely there must be a better way to express all this!  The added 
pieces in s6 are just as bad.

s8: The end of the last paragraph may or may not make sense, but there 
is *surely* a better way of saying it:
  Finally, the least significant 237 bytes of the challenge
   are concatenated.  If the challenge has fewer than 238 bytes, this
   algorithm includes the high-order byte in the computation twice, but
   ensures that the challenge is used exactly as is.  Additional padding
   is never used to increase the length of the challenge; the input data
   is allowed to be shorter than 237 bytes long.
The last two sentences were added in the update and I really don't think 
they helped!