Document:    draft-maino-fcsp-02.txt
Reviewer: Elwyn Davies [elwynd@dial.pipex.com]
Review Date:  Tuesday 9/27/2005 3:29 AM
Telechat Date:  Thursday 9/29/2005

Summary: An excellent document IMO - easy to read and with 
excellent balance  between motivation, explanation and technical detail! 
Kudos to the authors.  Given this document is informing the IETF 
about what Fibre Channel is doing, there is no obligation to fix 
up the couple of issues relating to extensibility/future proofing 
but given IETF experience, it might be appropriate to consider remedying them.  There are also some editorial nits.

Review:
Generally an excellent document.  Given recent IETF experience and the 
general degradation over time of the value of security algorithms, I 
think it would be appropriate to provide very explicitly for algorithm 
replacement, especially as regards the integrity algrithms which are (as 
I read the document) currently fixed.  The first two issues give the 
details:

s4.1, last para: It might be good to cite 
RFC2402RFC2406/draft-ietf-ipsec-esp-ah-algorithms-02.txt to cover all 
the 'standard' algorithms rather than one specific algorithm (RFC3602).  
Also it would probably be good to make it crystal clear that any future 
transforms that might be invented to go with ESP would be available for 
use for Fibre Channel.

s4.2, last para: Nothing is said here about alternative future integrity 
algorithms.  Given recent discussion about attacks on MD5 and SHA1, and 
general views about the need for security algorithms to be replaceable 
limiting integrity protection to just two current algorithms is not a 
good idea.

s8.1: I would consider refs FC-FS, FC-GS and FC-SP as normative.

s8.2: I think RFCs 2625, 3643 and 3821 are informative as the various 
payloads are not IP encapsulated.

Editorial nits:
s4, para 3: s/Preambol/Preamble/

s4, last para: s/Security Association for/Security Associations for/

s4.1: Fields are 'normalized before computation': presumably this is 
clear to somebody skilled in the Fibre Channel arts but a ref to the 
appropriate piece of specification or an inline description would help 
for the unenlightened.

s4.1, Figure 1: Technically the 'Auth' coverage should be 'Integrity' 
coverage (and this would match with the corresponding figure in 
draft-ietf-ipsec-esp-v3-10.txt).

 s5.2, para 2: s/protocol ID/protocol IDs/

s5.4, para 5 (next to last): s/he function/the function/

s5.4, last para: s/Associaton/Association/

s6, para 2: s/then there are no theoretical limitations/so that there 
are no a priori limitations/ (the previous phrase gives the theoretical 
limit of 4GB!)

s8.2: Should be entitled Normative References