Subject: Re: [Curdle] comments on draft-ietf-curdle-gss-keyex-sha2-03

From: Simo Sorce <[email-masked]>
Date: Mon, 22 Jan 2018 15:45:56 -0500

I just posted -04, hopefully it fixes all reported issues.
Some nits are "artwork" so I have no changed anything in there.

Simo.

On Tue, 2017-12-19 at 19:16 -0500, Daniel Migault wrote:
> Hi,
> 
> Please find my review for draft-ietf-curdle-gss-keyex-sha2-03 [1].
> 
> The corresponding shepherd write-up can be found here[2]. Feel free to
> comment as well.
> 
> 
> [1] https://tools.ietf.org/html/draft-ietf-curdle-gss-keyex-sha2-03
> [2]
> https://datatracker.ietf.org/doc/draft-ietf-curdle-gss-keyex-sha2/shepherdwriteup/
> 
> section 4, 5.2
> 
> I believe that "RECOMMENDED" and "OPTIONAL" can be removed and are
> redundant with SHOULD / MAY.
> 
> References:
> 
> [FIPS-180-4] is referenced, but not mentioned in the text.
> 
> 'NIST-SP-800-131Ar1' should be moved as informative references
> in my opinion. The reference is provided to justify the rational,
> not to describe the protocol.
> 
> ISO-IEC-8825-1 is a reference for ASN1. It seems to me that
> informational is the right place.
> 
> [I-D.ietf-curdle-ssh-modp-dh-sha2] is now an RFC, I believe it
> should be an informational document rather than a normative
> document as it is only cited as an example to move from SHA1 to sha2.
> 
> RFC6194]  Polk, T., Chen, L., Turner, S., and P. Hoffman, "Security
> Considerations for the SHA-0 and SHA-1 Message-Digest Algorithms"
> should be in my opinion an informational reference.
> 
> It would be good to add a link to the IANA in the IANA section
> registry and have it as an informational reference.
> 
> The draft mentions the SSH algorithm registry, but I am not
> sure that is the correct registry. instead, the Key Exchange
> Method Names registry might be more appropriated.
> 
> 
> here is the output of the nits:
> 
> idnits 2.15.00
> 
> tmp/draft-ietf-curdle-gss-keyex-sha2-03.txt:
> 
>   Checking boilerplate required by RFC 5378 and the IETF Trust (see
>   https://trustee.ietf.org/license-info):
>   ----------------------------------------------------------------------------
> 
>      No issues found here.
> 
>   Checking nits according to https://www.ietf.org/id-info/1id-guidelines.txt:
>   ----------------------------------------------------------------------------
> 
>      No issues found here.
> 
>   Checking nits according to https://www.ietf.org/id-info/checklist :
>   ----------------------------------------------------------------------------
> 
>   ** The abstract seems to contain references ([RFC4462]), which it
>      shouldn't.  Please replace those with straight textual mentions of the
>      documents in question.
> 
>   -- The draft header indicates that this document updates RFC4462, but the
>      abstract doesn't seem to directly say this.  It does mention RFC4462
>      though, so this could be OK.
> 
> 
>   Miscellaneous warnings:
>   ----------------------------------------------------------------------------
> 
>   == Line 412 has weird spacing: '... string    out...'
> 
>   == Line 418 has weird spacing: '... string    ser...'
> 
>   == Line 430 has weird spacing: '... string    out...'
> 
>   == Line 443 has weird spacing: '... string    out...'
> 
>   == Line 457 has weird spacing: '... string    mic...'
> 
>   == (2 more instances...)
> 
>      (Using the creation date from RFC4462, updated by this document, for
>      RFC5378 checks: 2005-08-23)
> 
>   -- The document seems to lack a disclaimer for pre-RFC5378 work, but may
>      have content which was first submitted before 10 November 2008.  If you
>      have contacted all the original authors and they are all willing to grant
>      the BCP78 rights to the IETF Trust, then this is fine, and you can ignore
>      this comment.  If not, you may need to add the pre-RFC5378 disclaimer.
>      (See the Legal Provisions document at
>      https://trustee.ietf.org/license-info for more information.)
> 
>   -- The document date (December 12, 2017) is 7 days in the past.  Is this
>      intentional?
> 
> 
>   Checking references for intended status: Proposed Standard
>   ----------------------------------------------------------------------------
> 
>      (See RFCs 3967 and 4897 for information about using normative references
>      to lower-maturity documents in RFCs)
> 
>   == Unused Reference: 'FIPS-180-4' is defined on line 637, but no explicit
>      reference was found in the text
> 
>   -- Possible downref: Non-RFC (?) normative reference: ref. 'ANSI-X9-62-2005'
> 
>   -- Possible downref: Non-RFC (?) normative reference: ref. 'FIPS-180-4'
> 
>   == Outdated reference: A later version (-06) exists of
>      draft-ietf-curdle-ssh-curves-04
> 
>   == Outdated reference: draft-ietf-curdle-ssh-modp-dh-sha2 has been
>      published as RFC 8268
> 
>   -- Possible downref: Non-RFC (?) normative reference: ref. 'ISO-IEC-8825-1'
> 
>   -- Possible downref: Non-RFC (?) normative reference: ref.
>      'NIST-SP-800-131Ar1'
> 
>   ** Downref: Normative reference to an Informational RFC: RFC 1321
> 
>   ** Downref: Normative reference to an Informational RFC: RFC 6194
> 
>   ** Downref: Normative reference to an Informational RFC: RFC 7546
> 
>   ** Downref: Normative reference to an Informational RFC: RFC 7748
> 
>   -- Possible downref: Non-RFC (?) normative reference: ref. 'SEC2v2'
> 
> 
>      Summary: 5 errors (**), 0 flaws (~~), 9 warnings (==), 8 comments (--).
> 
>      Run idnits with the --verbose option for more detailed information about
>      the items above.
> _______________________________________________
> Curdle mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/curdle

-- 
Simo Sorce
Sr. Principal Software Engineer
Red Hat, Inc