Re: [Gen-art] Genart last call review of draft-ietf-ipsecme-iptfs-12

Lars Eggert <lars@eggert.org> Tue, 23 August 2022 15:07 UTC

Return-Path: <lars@eggert.org>
X-Original-To: gen-art@ietfa.amsl.com
Delivered-To: gen-art@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id CA6BEC1526EF; Tue, 23 Aug 2022 08:07:13 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -7.107
X-Spam-Level:
X-Spam-Status: No, score=-7.107 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_HI=-5, RCVD_IN_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01, URIBL_BLOCKED=0.001, URIBL_ZEN_BLOCKED_OPENDNS=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=eggert.org
Received: from mail.ietf.org ([50.223.129.194]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id WJeD-_wTRRdd; Tue, 23 Aug 2022 08:07:10 -0700 (PDT)
Received: from mail.eggert.org (mail.eggert.org [IPv6:2a00:ac00:4000:400:211:32ff:fe22:186f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 88BACC14F74D; Tue, 23 Aug 2022 08:07:09 -0700 (PDT)
Received: from smtpclient.apple (unknown [217.149.135.167]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.eggert.org (Postfix) with ESMTPSA id 4C4CF273D9F; Tue, 23 Aug 2022 18:07:00 +0300 (EEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=eggert.org; s=dkim; t=1661267220; bh=q57BGmGodnBG2x2tGhA5U6//mGJSCjBucmloqOD0s/g=; h=Subject:From:In-Reply-To:Date:Cc:References:To; b=Rr9ibJQe2LSJKdMnB6dEgnqJJmu0vqw38oiN/ZwkwWkhwlHFOhaSlhS4o7Y2MUY9I dynFEa2Z2u0GOfeMPsPYat5YVDQ4WvYZPn4u7AMiEL5YZCqJ4KoY0ARkYto6IDMTix s2VIJa7mmHQip6snMcd6De5LqZmSmd9/bWWLh6hE=
Content-Type: multipart/signed; boundary="Apple-Mail=_6FE68825-C0FD-4BC9-813A-7B710E9D519C"; protocol="application/pgp-signature"; micalg="pgp-sha512"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.1\))
From: Lars Eggert <lars@eggert.org>
In-Reply-To: <165377514782.5823.17082725207808127680@ietfa.amsl.com>
Date: Tue, 23 Aug 2022 17:06:59 +0200
Cc: General Area Review Team <gen-art@ietf.org>, draft-ietf-ipsecme-iptfs.all@ietf.org, ipsec@ietf.org, last-call@ietf.org
Message-Id: <9FA3F559-42E1-4895-9630-A320795B444E@eggert.org>
References: <165377514782.5823.17082725207808127680@ietfa.amsl.com>
To: Peter Yee <peter@akayla.com>
X-MailScanner-ID: 4C4CF273D9F.A5C5C
X-MailScanner: Not scanned: please contact your Internet E-Mail Service Provider for details
X-MailScanner-From: lars@eggert.org
Archived-At: <https://mailarchive.ietf.org/arch/msg/gen-art/Gl3xaoEBfbMybekqnG8M1KODFyY>
Subject: Re: [Gen-art] Genart last call review of draft-ietf-ipsecme-iptfs-12
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.39
Precedence: list
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 23 Aug 2022 15:07:13 -0000

Peter, thank you for your review. I have entered a Discuss ballot for this document based on my own review.

Lars


> On 2022-5-28, at 23:59, Peter Yee via Datatracker <noreply@ietf.org> wrote:
> 
> Reviewer: Peter Yee
> Review result: Ready with Issues
> 
> 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.
> 
> For more information, please see the FAQ at
> 
> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
> 
> Document: draft-ietf-ipsecme-iptfs-12
> Reviewer: Peter Yee
> Review Date: 2022-05-27
> IETF LC End Date: 2022-05-18
> IESG Telechat date: Not scheduled for a telechat
> 
> Summary: This draft specifies an improved method for countering traffic
> analysis of IPsec tunnels. There are some nits and minor issues that should be
> addressed. I did not evaluate the appendices for correctness. [Ready with
> issues.]
> 
> Major issues: None
> 
> Minor issues:
> 
> Page 7, 3rd paragraph, 1st sentence (and elsewhere in the document): You make
> reference to the “user” and what the “user” is supposed to do. I can’t begin to
> imagine an ordinary user coming up with an optimal window size or do some of
> the other things that are being required. Do you really want to put this
> requirement on a user, or should it be a different entity, such as the IP-TFS
> implementation?
> 
> Page 13, 1st partial paragraph: How would the referenced AGGFRAG_PAYLOAD empty
> payload be recognized? The ESP Next Header won’t indicate that the contents is
> an AGGFRAG_PAYLOAD because the SA isn’t an AGGFRAG_PAYLOAD SA.
> 
> Page 13, 2nd full paragraph: the unnumbered figure from page 17 would be really
> helpful here given how many disparate header fields are referenced in this and
> the following paragraphs. Page 15, section 6.1: RFC 4303 says, “The Next Header
> is a mandatory, 8-bit field that identifies the type of data contained in the
> Payload Data field, e.g., an IPv4 or IPv6 packet, or a next layer header and
> data.  The value of this field is chosen from the set of IP Protocol Numbers
> defined on the web page of the IANA, e.g., a value of 4 indicates IPv4, a value
> of 41 indicates IPv6, and a value of 6 indicates TCP.” Thus, I don’t believe
> you can arbitrarily choose 0x5. See the registry at
> https://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml.
> 
> Nits/editorial comments:
> 
> General:
> 
> Insert a hyphen between “congestion” and “controlled” throughout the document.
> This includes the “non-“ cases as well.
> 
> Insert a hyphen between “AGGFRAG_PAYLOAD” and “enabled” throughout the
> document. This includes the “non-“ cases as well.
> 
> Change “inner-packet” to “inner packet”. The latter already predominates
> throughout the document, noting also that “outer packet” never appears in
> hyphenated form.
> 
> Ensure that all the figures have proper captions with numbers. For example, the
> figures on pages 16, 17, 18, and 19 aren’t labeled. The figure on page 17 could
> really use a number so that there can be a pointer from page 13 if the figure
> isn’t moved to page 13, as suggested above.
> 
> Specific:
> 
> Page 1, Abstract, 1st sentence: change “payload” to “payloads”. Or change “ESP
> payload” to “an ESP payload”. I can see arguments either way, but the sentence
> needs one or the other.
> 
> Page 5, 1st paragraph, 2nd sentence: delete a duplicated “the” before “tunnel
> packets”.
> 
> Page 5, Figure 1: change “subtype” to “sub-type” to match usage in the rest of
> the document.
> 
> Page 6, 3rd paragraph: append a comma after “outer”.
> 
> Page 7, 4th paragraph, last sentence: “one” who? What entity is supposed to be
> making this choice?
> 
> Please 7, 4th paragraph, 1st sentence: append a comma after “note”.
> 
> Page 7, 4th paragraph, 3rd sentence: delete “amount of”.
> 
> Page 7, 5th paragraph, 1st sentence: consider changing “with no gaps” to
> “consecutively”.
> 
> Page 8, section 2.2.3.1, 1st paragraph, last sentence: delete the comma after
> “researching”.
> 
> Page 9, section 2.2.5.3, 1st sentence: append a comma after “default”.
> 
> Page 10, section 2.3, 1st sentence: insert a hyphen between “AGGFRAG_PAYLOAD”
> and “enabled”.
> 
> Page 10, section 2.4.1, 2nd paragraph, 3rd sentence: append a comma after
> “case”. Append a period after “etc”.
> 
> Page 10, section 2.4.2, 1st paragraph, 2nd sentence: append a comma after
> “transport”.
> 
> Page 11, 1st partial paragraph: change “packet” to “packets”. Append a closing
> parenthesis after “congestion”.
> 
> Page 11, 1st full paragraph, 1st sentence: insert a hyphen between “TCP” and
> “friendly”.
> 
> Page 11, 3rd paragraph, 1st sentence: insert a hyphen between “IP-TFS” and
> “enabled”.
> 
> Page 11, 4th paragraph: append a comma after the closing parenthesis.
> 
> Page 12, 1st partial paragraph, 1st full sentence: delete this sentence as it
> doesn’t really add anything. But if you are unwilling to delete the sentence,
> then change “are” to “is”.
> 
> Page 12, section 2.5, 1st paragraph: insert a hyphen between “AGGFRAG” and
> “enabled”.
> 
> Page 12, section 2.5, 2nd paragraph, 3rd sentence: append a comma after “For
> partial packets”. Delete “the” before “they”.
> 
> Page 12, section 2.5, 2nd paragraph, 4th sentence: insert “the” before
> “AGGFRAG_PAYLOAD”.
> 
> Page 12, section 2.5, 3rd paragraph, 1st sentence: insert “an” before
> “in-order”.
> 
> Page 12, section 2.5 3rd paragraph, 2nd sentence: change “make sure” to
> “ensure”, if you care. “Tastes light” vs. “less filling”, I suppose. Change
> “in-order” to “in order”. Insert “a” between “when” and “lost”. Also consider
> breaking up the sentence into multiple sentences because of its sheer length.
> For example, the final parenthetical potion could be a whole sentence on its
> own.
> 
> Page 12, section 3, 2nd sentence: change “it’s” to “its”.
> 
> Page 13, 2nd full paragraph, 2nd sentence: change “locally, subsequent” to
> “locally. Subsequent”.
> 
> Page 13, 3rd paragraph, 1st sentence: expand the initialism “CC”. I’m assuming
> “Congestion Control”. It’s not in the RFC Editor’s list of abbreviations.
> 
> Page 13, 4th paragraph, 3rd sentence: change “senders” to “sender’s”.
> 
> Page 16, section 6.1.1, 1st paragraph: change “4 octet” to 4-octet”.
> 
> Page 16, section 6.1.1, “Reserved” definition: delete the comma after
> “generation”.
> 
> Page 17, section 6.1.2, “Reserved” definition: delete the comma after
> “generation”.
> 
> Page 17, section 6.1.2, “P” and “E” definitions: insert “that” before “if”.
> 
> Page 18, “Echo Delay” and “Transmit Delay” definitions, 2nd sentence: change
> “value” to “delay” because by definition, the value cannot be larger than
> 0x1FFFFF, while the delay can be. Change “it” to “the value”.
> 
> Page 18, “Datablocks” definition: 2nd sentence: change “an” to “a”. Insert a
> hyphen between “non-IP-TFS” and “enabled”. Consider changing “value” to “field”
> because DataBlocks isn’t really a value.
> 
> Page 19, section 6.1.3.1, figure: shouldn’t the “TypeOfService” field be the
> “DiffServ” field instead?
> 
> Page 20, section 6.1.4, “0” definition: delete the comma.
> 
> Page 21, 1st paragraph, last sentence: change “it’s” to “its”.
> 
> Page 22, section 8, 1st paragraph, 1st sentence: change “it” to “its”.
> 
> Page 22, section 8, 3rd paragraph: append a comma after “maintained” and after
> “would be”.
> 
> Page 24, Appendix A, title: change “Of” to “of”.
> 
> Page 24, Appendix A, 1st paragraph, 1st sentence: append a comma after “Below”.
> 
> Page 25, Figure 3: Explain what the 1500 means.
> 
> Page 25, 1st paragraph: change “800 octet” to “800-octet” twice. Make a similar
> change for “60”, “240”, and “4000”.
> 
> Page 25, 2nd paragraph, 2nd sentence: place “ESP1” in parentheses.
> 
> Page 25, 2nd paragraph, 3rd sentence: change “packet ESP2s” to “packet’s
> (ESP2)”. Change “60 octet” to “60-octet”.
> 
> Page 25, 2nd paragraph, 4th sentence: place “ESP3” in parentheses. Change “4000
> octet” to “4000-octet”. Change “forth” to “fourth”.
> 
> Page 25, 2nd paragraph, 5th sentence: change “packet ESP4s” to “packet’s
> (ESP4)”. Append a comma after “1400”. Change “4000 octet” to “4000-octet”.
> 
> Page 25, Appendix B, 1st sentence: change “TCP friendly” to “TCP-friendly”.
> 
> Page 25, Appendix B, 2nd sentence: change “TCP friendly” to “TCP-friendly”.
> 
> Page 25, Appendix B, 3rd sentence: append a comma after “[RFC4342])”.
> 
> Page 25, Appendix B, 3rd paragraph: append a comma after “addition”.
> 
> Page 26, 1st paragraph, 2nd sentence: append a comma after “[RFC5348]”.
> 
> Page 26, section C.1, 1st paragraph, 1st sentence: append a comma after
> “overhead”.
> 
> Page 26, section C.1.1, 1st sentence: append a comma after “For comparison”.
> Insert “an” before “AGGFRAG”.
> 
> Page 26 section C.1.1, 2nd sentence: append a comma after “Therefore”. Change
> “fractional” to “fractions”.
> 
> Page 27, 1st formula: change “Paylaod” to “Payload”.
> 
> Page 28, section C.3, 3rd sentence: insert a hyphen between “well” and
> “understood”.
> 
> Page 28, section C.3.1, 2nd sentence: change the second “and” to “an”. Append a
> comma after the closing parenthesis.
> 
> Page 28, section C.3.1, 3rd sentence: append a comma after “Additionally”.
> 
> Page 30, 1st paragraph, 1st sentence: append a hyphen after “small”. Insert a
> hyphen between “medium” and “sized”.
> 
> 
> _______________________________________________
> Gen-art mailing list
> Gen-art@ietf.org
> https://www.ietf.org/mailman/listinfo/gen-art