[Int-area] Tsvart early review of draft-ietf-intarea-gue-07

David Black via Datatracker <noreply@ietf.org> Sat, 09 March 2019 02:24 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: int-area@ietf.org
Delivered-To: int-area@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 5C38A128B01; Fri, 8 Mar 2019 18:24:59 -0800 (PST)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: David Black via Datatracker <noreply@ietf.org>
To: tsv-art@ietf.org
Cc: draft-ietf-intarea-gue.all@ietf.org, int-area@ietf.org, ietf@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 6.93.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <155209829934.26739.17273081538632335266@ietfa.amsl.com>
Date: Fri, 08 Mar 2019 18:24:59 -0800
Archived-At: <https://mailarchive.ietf.org/arch/msg/int-area/Z67X78mJyzijDmt4GlCktndInzI>
Subject: [Int-area] Tsvart early review of draft-ietf-intarea-gue-07
X-BeenThere: int-area@ietf.org
X-Mailman-Version: 2.1.29
List-Id: IETF Internet Area Mailing List <int-area.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/int-area>, <mailto:int-area-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/int-area/>
List-Post: <mailto:int-area@ietf.org>
List-Help: <mailto:int-area-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/int-area>, <mailto:int-area-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sat, 09 Mar 2019 02:24:59 -0000

Reviewer: David Black
Review result: On the Right Track

This document has been reviewed as part of the transport area review team's ongoing
effort to review key IETF documents. These comments were written primarily for
the transport area directors, but are copied to the document's authors and WG to 
allow them to address any issues raised and also to the IETF discussion list for 
information. 

When done at the time of IETF Last Call, the authors should consider this
review as part of the last-call comments they receive. Please
always CC tsv-art@ietf.org if you reply to or forward this review.

GUE is a new generic UDP encapsulation that is intended to provide a common
widely applicable encapsulation mechanism that can displace a variety of
more specific encapsulation mechanisms.  The draft is well written and has
a number of clever ideas, e.g., use of the first two bits of the IP version number
to define GUE variant 1 without including a GUE header.

I found three major issues and a number of minor issues.  I apologize for the delay
in this review courtesy of a day-job crisis and being one of the recipients of the
second round of the flu in the eastern US.

--- Major ---

[1] IPv6 zero checksum

5.7.3:

   In IPv6, there is no checksum in the IPv6 header that protects
   against mis-delivery due to address corruption. Therefore, when GUE
   is used over IPv6, either the UDP checksum or the GUE header checksum
   SHOULD be used unless there are alternative mechanisms in use that
   protect against misdelivery. The UDP checksum and GUE header checksum
   SHOULD NOT be used at the same time since that would be mostly
   redundant.

   If neither the UDP checksum nor the GUE header checksum is used, then
   the requirements for using zero IPv6 UDP checksums in [RFC6935] and
   [RFC6936] MUST be met.

I don't think the second paragraph works, because it imposes design
requirements on the encapsulated protocol to protect the GUE header when
one cannot in general expect design of that protocol to anticipate GUE
encapsulation.  My initial suggestion is to change the first "SHOULD"
in the first paragraph to a "MUST" but even that may not meet RFC 6936's
requirements.

In any case, please read RFC 6936 in detail, and explain how GUE meets
RFC 6936's requirements - in general it is not sufficient to require to just
state that they have to be met because some of the requirements are
protocol design requirements that GUE has to meet.

[2] Tunnels

Section 5.8 needs to normatively reference draft-ietf-intarea-tunnels
and be revised accordingly, e.g., as that draft will update RFC 4459.

[3] Congestion control

Section 5.9:

   In the case that the encapsulated traffic does not implement any or
   sufficient control, or it is not known whether a transmitter will
   consistently implement proper congestion control, then congestion
   control at the encapsulation layer MUST be provided per [RFC8085].

Ok, that text has the right overall view, but I question whether this
"MUST" requirement is implementable in practice based on the brief
discussion in this section.


--- Minor ---

[A] 3.2.2. Ctype field:

   This document does not specify any standard control message types
   other than type 0. Type 0 does not define a format of the control
   message. Instead, it indicates that the GUE payload is a control
   message, or part of a control message (as might be the case in GUE
   fragmentation), that cannot be correctly parsed or interpreted
   without additional context.

Hmm - seems to be a lot of "trust us" in this text.  What does this
text add over and above the first paragraph in this section?  The quoted
paragraph does not by itself appear to specify anything that interoperates.

[B] 3.3.1. (Flags and extension fields) Requirements:

   Extension fields are placed in order of the flags. New flags are to
   be allocated from high to low order bit contiguously without holes.

That is not mentioned in the IANA considerations. How will that be
enforced?

[C] 3.4. Private data:

   If a decapsulator receives a GUE packet with private data, it MUST
   validate the private data appropriately.

What does "appropriately" mean? In other words, how a protocol designer
or implementer determine whether the "MUST" requirement has been complied
with?

[C] 5.3. Encapsulator operation:

   For instance, if an IP packet is being
   encapsulated in GUE then diffserv interaction [RFC2983] and ECN
   propagation for tunnels [RFC6040] SHOULD be followed.

That's close, but not what needs to be said.  RFC 2983 is informative
- it should be referenced as a useful source of design guidance without
using an RFC 2119 keyword.  The quoted text requires that RFC 2983 be
normatively referenced, which is unlikely to be what was wanted (NB:
I'm the author of RFC 2983).

In contrast, the RFC 6040 requirement ought to be a "MUST" requirement,
not a "SHOULD" requirement.

[D] 5.11.1. Flow classification:

This discussion of IPsec headers (AH and ESP) needs to reference the
relevant IPsec RFCs.

In addition, some more discussion on how AH transport mode works is
needed, as the GUE receiver does some header processing *before* the
IPsec AH transport mode processing that includes header checks.  That
appears to merit mention in security considerations.

[E] Section B.1 on privileged ports appears to contain a security
consideration that should be included in the security considerations
section

--- Editorial ---

Section 5.1:

   Network tunneling can be achieved by encapsulating layer 2 or layer 3
   packets. 

Should explain what layer 2 and layer 3 mean.  GUE encapsulation of
layer 3 packets is directly provided by GUE variant 1, but how is GUE
intended to provide (or how could GUE provide) encapsulation of layer 2
packets?  Adding an example will suffice.

Section 5.2:

   When encapsulating layer 4 packets,

Say a few words on how this is done, e.g., protocol nubmer for UDP or TCP
in GUE variant 0.

Section 5.4:

   Note that set flags in a GUE
   header that are unknown to a decapsulator MUST NOT be ignored. If a
   GUE packet is received by a decapsulator with unknown flags, the
   packet MUST be dropped.

This should be stated in Section 3.3.1 also as part of explaining how
GUE flags work.