[Gen-art] Gen-ART review of IETF LC draft-kelly-ipsec-ciph-sha2-01.txt

[Miguel Garcia <[e-mail]>]

I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-kelly-ipsec-ciph-sha2-01.txt
Reviewer: Miguel Garcia <[e-mail]>
Review Date: 2007-01-21
IETF LC End Date: 2007-02-08.

Summary: This draft is basically ready for publication, but has nits 
that should be fixed before publication.

Comments:

1) In Section 1, second paragraph, there are references to 
HAMC-SHA-PRF-256, HMAC-SHA-PRF-384, and HMAC-SHA-PRF-512. The same 
references appear in the first paragraph in Section 2.4. However, in the 
table in Section 2.6 and the test vectors in Section 2.7.1, there are 
references to HMAC-SHA-256-PRF, HMAC-SHA-384-PRF, AND HMAC-SHA-512-PRF. 
And in the IANA considerations section the references are to 
PRF_HMAC_SHA2_256, PRF_HMAC_SHA2_384, and PRF_HMAC_SHA2_512. The 
question is: are these three groups of tokens the same thing? Notice the 
reversal of PRF-nnn and nnn-PRF, and the different naming scheme in the 
IANA section.

If they are the same thing, there should be only one way to refer to 
them. If they are different things, then the document has substantial 
misleading information as for to guarantee failures in implementations.

2) IANA considerations section. The document does not specify:

a) The registry IANA has to operate.
b) The subregistry within that registry IANA has to operate
c) A differentiation between instructions to IANA and background 
information to the reader about already assigned values.

Also, it is not clear the relation of the tokens written in this section 
with those written elsewhere in the draft. Common sense indicates that 
the digits are the key to map them, but I believe the draft should be 
more clear on this.

For example: is AUTH_HMAC_SHA2_256_128 a token that refers to what is 
known elsewhere as HMAC-SHA-256-128? Since the token name is different, 
one ask the question of whether they are the same thing or not. This 
applies to all tokens defined in the IANA section.

If I were to write this section, I will write something along these 
lines (please check if I got it right, it is not so easy):


------
4. IANA Considerations

4.1 New assignments

This memo instructs IANA to assign new values to the IKEv2 Transform 
Attributes Types registry, according to the following data:

      For Transform Type 2 (Pseudo-random Function),
      defined Transform IDs are:
      Number    Name                                Reference
      ------    ---------------------------------   ---------
                HMAC-SHA-256-PRF                    [RFCXXXX]
                HMAC-SHA-384-PRF                    [RFCXXXX]
                HMAC-SHA-512-PRF                    [RFCXXXX]


      For Transform Type 3 (Integrity Algorithm),
      defined Transform IDs are:
      Number    Name                                Reference
      ------    ---------------------------------   ---------
                HMAC-SHA-256-128                    [RFCXXXX]
                HMAC-SHA-384-192                    [RFCXXXX]
                HMAC-SHA-512-256                    [RFCXXXX]

4.2 Existing assignments

This memo uses the existing Authentication Algorithms HMAC-SHA2-256, 
HMAC-SHA2-384, and HMAC-SHA2-512 defined in the IPSEC Security 
Association Attributes subregistry of the ISAKMP registry. Please 
consult the online IANA ISAKMP registry for details of those values.

Note: This memo does not specify the conventions for using SHA-256+ for 
IKE Phase 1 negotiation.

---------


3) The document does not separate the references between Normative and 
Informative. This is a requirement for RFCs.

Some other minor comments:

4) You may want to expand acronyms before its first usage. For example, 
in Section 2.1.2 you may want to expand "IKE_SA".

5) As a personal matter of taste, I don't like references that do not 
include the RFC number. For example, I don't like text that reads "... 
is specified in [HMAC]". I would like to see "... is specified in RFC 
2104 [HMAC]".

6) You may want to add a reference to HMAC-SHA1-96 in the first 
paragraph of Section 3.1

7) The idnits tools report a number of errors that should be fixed. I 
attach those errors here for your convenience:

idnits 1.124

tmp/draft-kelly-ipsec-ciph-sha2-01.txt:

tmp/draft-kelly-ipsec-ciph-sha2-01.txt(353): Line is too long: the 
offending characters are '-+'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(354): Line is too long: the 
offending characters are ' |'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(355): Line is too long: the 
offending characters are ' |'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(356): Line is too long: the 
offending characters are '=+'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(357): Line is too long: the 
offending characters are ' |'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(358): Line is too long: the 
offending characters are '-+'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(359): Line is too long: the 
offending characters are ' |'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(360): Line is too long: the 
offending characters are '-+'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(361): Line is too long: the 
offending characters are ' |'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(362): Line is too long: the 
offending characters are '-+'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(363): Line is too long: the 
offending characters are ' |'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(364): Line is too long: the 
offending characters are '-+'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(365): Line is too long: the 
offending characters are ' |'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(366): Line is too long: the 
offending characters are '-+'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(367): Line is too long: the 
offending characters are ' |'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(368): Line is too long: the 
offending characters are '-+'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(634): Line is too long: the 
offending characters are ')'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(635): Line is too long: the 
offending characters are ')'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(709): Line is too long: the 
offending characters are ')'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(710): Line is too long: the 
offending characters are ')'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(711): Line is too long: the 
offending characters are ')'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(807): Line is too long: the 
offending characters are ')'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(808): Line is too long: the 
offending characters are ')'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(809): Line is too long: the 
offending characters are ')'
tmp/draft-kelly-ipsec-ciph-sha2-01.txt(810): Line is too long: the 
offending characters are ')'


   Checking nits according to http://www.ietf.org/ID-Checklist.html:

     Checking conformance with RFC 3978/3979 boilerplate...

   - This document has ISOC Copyright according to RFC 3978, instead of the
     newer IETF Trust Copyright according to RFC 4748.  You should consider
     updating it; the new Copyright statement will be required from February
     1st, 2007
   - This document has an original RFC 3978 Section 5.5 Disclaimer, 
instead of
     the newer disclaimer which includes the IETF Trust according to RFC 
4748.
     You should consider updating it; the new disclaimer will be 
required from
     February 1st, 2007
   * The document seems to lack a both a reference to RFC 2119 and the
     recommended RFC 2119 boilerplate, even if it appears to use RFC 2119
     keywords.

     RFC 2119 paragraph 2 text:
     "The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
     "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
     document are to be interpreted as described in RFC 2119."

     RFC 2119 keyword, line 152: '...e hash functions MUST be supported, 
and ...'
     RFC 2119 keyword, line 154: '...   MUST NOT be supported....'
     RFC 2119 keyword, line 196: '...   function MUST be used to 
generate the...'
     RFC 2119 keyword, line 248: '...m output size -- MUST be supported. 
  No ...'
     RFC 2119 keyword, line 785: '...              2001 MAY,...'

   Checking nits according to http://www.ietf.org/ietf/1id-guidelines.txt:
     Nothing found here (but these checks do not cover all of
     1id-guidelines.txt yet).

   Miscellaneous warnings:
     None.

   Experimental warnings:
   - Missing Reference: 'TBA-1' is mentioned on line 725, but not defined
     'PRF_HMAC_SHA2_256  [TBA-1]...'

   - Missing Reference: 'TBA-2' is mentioned on line 727, but not defined
     'PRF_HMAC_SHA2_384  [TBA-2]...'

   - Missing Reference: 'TBA-3' is mentioned on line 729, but not defined
     'PRF_HMAC_SHA2_512  [TBA-3]...'

   - Missing Reference: 'TBA-4' is mentioned on line 736, but not defined
     'AUTH_HMAC_SHA2_256_128  [TBA-4]...'

   - Missing Reference: 'TBA-5' is mentioned on line 738, but not defined
     'AUTH_HMAC_SHA2_384_192  [TBA-5]...'

   - Missing Reference: 'TBA-6' is mentioned on line 740, but not defined
     'AUTH_HMAC_SHA2_512_256  [TBA-6]...'







That's from my side.

BR,

      Miguel



-- 
Miguel A. Garcia           tel:[phone]
sip:[e-mail]
Nokia Research Center      Helsinki, Finland

_______________________________________________
Gen-art mailing list
[e-mail]
https://www1.ietf.org/mailman/listinfo/gen-art