Review of draft-ietf-ipsecme-rfc4307bis-15

Pete Resnick <presnick@qti.qualcomm.com> Thu, 02 February 2017 16:06 UTC

Return-Path: <presnick@qti.qualcomm.com>
X-Original-To: ietf@ietf.org
Delivered-To: ietf@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 694AC12947A; Thu, 2 Feb 2017 08:06:05 -0800 (PST)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Pete Resnick <presnick@qti.qualcomm.com>
To: <gen-art@ietf.org>
Subject: Review of draft-ietf-ipsecme-rfc4307bis-15
X-Test-IDTracker: no
X-IETF-IDTracker: 6.42.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <148605156539.13830.6416316067398939320.idtracker@ietfa.amsl.com>
Date: Thu, 02 Feb 2017 08:06:05 -0800
Archived-At: <https://mailarchive.ietf.org/arch/msg/ietf/ct-K5Kl1Y5yi7vw7xH7j3uprD5I>
Cc: ipsec@ietf.org, draft-ietf-ipsecme-rfc4307bis.all@ietf.org, ietf@ietf.org
X-BeenThere: ietf@ietf.org
X-Mailman-Version: 2.1.17
List-Id: IETF-Discussion <ietf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ietf>, <mailto:ietf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/ietf/>
List-Post: <mailto:ietf@ietf.org>
List-Help: <mailto:ietf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ietf>, <mailto:ietf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 02 Feb 2017 16:06:05 -0000

Reviewer: Pete Resnick
Review result: Ready with Nits

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments. (Apologies for the late
submission.)

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>;.

Document: draft-ietf-ipsecme-rfc4307bis-??
Reviewer: Pete Resnick
Review Date: 2017-02-02
IETF LC End Date: 2017-01-31
IESG Telechat date: 2017-02-16

Summary:

This document is basically ready, but there are a large number of
nits, so many that I would think additional review would be desired.

Major issues: None

Minor issues: None

Nits/editorial comments: 

[Note: I do not see any particular technical problems with the
document, but I don't have expertise in this area. I have listed below
many clarifications and fixes to typographical and grammatical errors.
(I did not note all of the punctuation errors as none of them left
ambiguity.) Normally, I would simply list these as nits/editorial
comments and leave it at that. But there are so many of these that I'm
a bit concerned that the document did not receive sufficient technical
review, given that it went through 15 versions and nobody fixed all of
these editorial issues. That said, that is an issue between the AD and
the WG.]

It seems like section 2 should be moved up above section 1 since these
specialized conventions are used in section 1.

1: The second to last sentence is ungrammatical and hard to read. Try
this:

OLD
   This document describes the parameters of the IKE protocol and
   updates the IKEv2 specification because it changes the mandatory
to
   implement authentication algorithms of the section 4 of the
RFC7296
   by saying RSA key lengths of less than 2048 are SHOULD NOT.
NEW
   This document describes the parameters of the IKE protocol. It
also
   updates the IKEv2 specification because it changes the mandatory
to
   implement authentication algorithms of section 4 of RFC 7296
   by saying RSA key lengths of less than 2048 SHOULD NOT be used.
END

1.1, first sentence: s/then/than

1.1, last sentence: s/in a separate document/in this separate
document.

1.2: The last sentence of the third paragraph repeats what is in the
last paragraph of 1.2 and therefore redundant. You can strike it. 

2: I suggest adding a sentence after the first paragraph for
clarification: "When used in the tables in this document, these terms
indicate that the listed algorithm MUST, MUST NOT, SHOULD, SHOULD NOT
or MAY be implemented as part of an IKEv2 implementation." 4307 never
said that specifically, and I've always found it weird.

3.1, first paragraph: s/an integrity algorithms in Section 3.3/one of
the integrity algorithms in Section 3.3

3.1, third paragraph: s/CRFG/the Crypto Forum Research Group (CFRG) of
the IRTF

(You not only didn't define it, you got the acronym backwards.)

3.1, third from last paragraph: s/on those cases/in those cases

3.1, second from last paragraph: s/implementation/implementations

3.1, last paragraph: s/of-the-shelves/off-the-shelf ;
s/therefor/therefore

3.3, last paragraph: s/status ware/statuses were

3.4, third paragraph: s/were/was

3.4, fourth paragraph: s/vulnerable to be broken/vulnerable to being
broken

3.4, last paragraph: s/thater/that; s/academia have/academia has

4: s/concerned on/concerned with

4.1, first paragraph: 

OLD
                                           RSA Digital Signature is
not
   recommended for keys smaller then 2048, but since these signatures
   only have value in real-time, and need no future protection,
smaller
   keys was kept at SHOULD NOT instead of MUST NOT.

NEW
	See section 4.1.1 for a discussion of key length recommendations for
	use in RSA Digital Signature.
END

(If you want, include some of the original text in 4.1.1.

4.1, third paragraph: s/it does not/they do not

4.2, paragraphs 1, 2, & 3: s/When Digital Signature
authentication/When a Digital Signature authentication ; also, strike
the word "then" in the first two paragraphs.