Subject: [I2nsf] Tsvart early review of draft-ietf-i2nsf-capability-data-model-13

From: Michael Scharf via Datatracker <[email-masked]>
Date: Fri, 06 Nov 2020 14:49:26 -0800

Reviewer: Michael Scharf
Review result: Almost Ready

This is a TSV-ART review of draft-ietf-i2nsf-capability-data-model-13. This
review focuses *only* on transport protocol topics. The document may have
other, more general problems. Apparently this is already discussed in the IESG.

Most YANG model entries related to Internet transport protocols are poorly
defined. The definition, description, and the references need to be rewritten
to actually reflect the current use of transport protocols in the Internet.
Precise definitions and/or better references are needed in many places.

In particular the selection of "capabilities" related to TCP is not well
motivated in the document. The definitions in the YANG data model may be pretty
disconnected from actual capabilities of TCP middleboxes. It might be quite
challenging to comprehensively describe in a YANG model all possible TCP
processing in sophisticated TCP middleboxes. But even if the objective of the
document was to focus on basic state-less processing of TCP, the document does
not describe all capabilites that can be found e.g. in ACL lists in
state-of-the-art routers or firewalls. In contrast, the document lists some TCP
capabilities that may have no or only very limited real-world benefit, at least
to the best of the knowledge of the reviewer. At minimum, the document would
have to better reason its selection of capabilities, in particular regarding
processing of TCP.

Detailed comments regarding the YANG Model:

1. identity ipv4-tos {
     base ipv4-capability;
     description
       "Identity for IPv4 Type-Of-Service (TOS)
       condition capability";
     reference
       "RFC 791: Internet Protocol - Type of Service";
   }

According to RFC 3168, this field is subdivided into a DSCP and an ECN field.
Thus, it is unclear what this identity refers to. Does it refer only to the
DSCP part (for instance, RFC 8329 lists only DSCP)? Or does it also include the
two ECN bits? If so, what capability regarding the two ECN fields would this
refer to? Given the ongoing discussion on use of ECN bits, it would be a
surprise if there was industry consensus on what capabilities an NSF should
have regarding ECN bits.

Instead of a reference to the protocol specification, it would be useful to
have a reference to a precise definition of this capability. This comment also
applies in many other places.

Formally, the reference to RFC 791 seems insufficient, as the ToS definition in
that document has been updated by RFC 2474, RFC 3168, etc.

2. identity ipv6-traffic-class {
     base ipv6-capability;
     description
       "Identity for IPv6 traffic class
       condition capability";
     reference
       "RFC 8200: Internet Protocol, Version 6 (IPv6)
       Specification - Traffic Class";
   }

The previous comment also applies here. RFC 8200 explains the subdivisions of
the "traffic class" field. At least regarding the two ECN bits it is entirely
unclear what capability this identity would refer to.

3. identity tcp-capability {
     base condition;
     description
       "Base identity for TCP condition capabilities";
     reference
       "RFC 793: Transmission Control Protocol";
   }

State-of-the-art TCP is not only defined by RFC 793. For instance, RFC 1122
updates and clarifies RFC 793. Regarding the TCP header, a better reference
would be draft-ietf-tcpm-rfc793bis, which is in WGLC in TCPM and will obsolete
RFC 793. A modern TCP stack implements many features beyond RFC 793. RFC 7414
provides a roadmap.

This comment about reference RFC 793 also applies to the following identities.

Independent of the question where the TCP header is defined, listing the
standard for the protocol may not be a precise definition of a capability
inside the network. TCP standards describe the protocol in the connection
endpoints. That can be *very* different to the handling in a middlebox, which
may not even be specified in any IETF document (sometimes for good reasons).

4. identity exact-tcp-window-size {
     base tcp-capability;
     description
       "Identity for exact-match TCP window size condition capability";
     reference
       "RFC 793: Transmission Control Protocol - Window Size";
   }

   identity range-tcp-window-size {
     base tcp-capability;
     description
       "Identity for range-match TCP window size condition capability";
     reference
       "RFC 793: Transmission Control Protocol - Window Size";
   }

>From this document it is entirely unclear why a match on the window size would
matter. It is well-known that many firewalls process quite a number of TCP
header fields. But the absolute value of the (receive) window alone is actually
not very useful. A well-known middlebox capability would e.g. be some checks
whether sequence numbers are valid. This may require logic for processing the
window and other parameters. As far as I can tell, that logic does not need
(only) an exact or range match for the window value.

In addition, for this field it is clearly not appropriate to only reference RFC
793. RFC 7323 changes the semantics of this header field by scaling, which is
used by basically all modern TCP stacks.

As far as I know, the RFC 7323 scaling of the window can only correctly be
reverse-engineered by an NSF that can keep per-connection state (i.e., records
the scaling factor in the SYN). It is unclear how this model deals with an NSF
that decodes the wrong window sizes, i.e., an NSF that cannot correctly decode
the actual parameter in the endpoints due to lack of knowledge of the scale
factor.

5. identity tcp-flags {
     base tcp-capability;
     description
       "Identity for TCP flags condition capability";
     reference
       "RFC 793: Transmission Control Protocol - Flags";
   }

Further TCP flags are defined in RFC 3168. As already noted,
draft-ietf-tcpm-rfc793bis might be a better reference regarding the current
definition of the TCP flags.

The TCPM working group also currently standardizes further semantics for the
TCP flags in draft-ietf-tcpm-accurate-ecn. Internet transport protocols evolve
over time... In order to be meaningful, this document would have to be much
more precise what is meant by "tcp-flags".

6. Selection of parameters (and omissions) in the YANG model

As already noted, the selection of TCP-related capabilities in this YANG model
is not well motivated and actually surprising (at least to me). It is pretty
well-known in industry - and the IETF - what capabilities widely deployed TCP
middleboxes have. For instance, widely deployed TCP middleboxes are known to
process TCP options. There are also well-known studies in this field (e.g., "Is
it still possible to extend TCP?" by Michio Honda et al.). A YANG model that
focuses on some few fixed TCP header fields seems entirely disconnected from
running code in the Internet. For instance, why are TCP options omitted?

7. identity exact-udp-total-length {
     base udp-capability;
     description
       "Identity for exact-match UDP total-length condition capability";
     reference
       "RFC 768: User Datagram Protocol - Total Length";
   }

   identity range-udp-total-length {
     base udp-capability;
     description
       "Identity for range-match UDP total-length condition capability";
     reference
       "RFC 768: User Datagram Protocol - Total Length";
   }

These definitions need to take into account that the UDP total length can be
smaller than the IP transport length. See draft-ietf-tsvwg-udp-options for more
details.

7. identity sctp-capability {
    description
       "Identity for SCTP condition capabilities";
     reference
       "RFC 4960: Stream Control Transmission Protocol";
   }

In addition to SCTP, the IETF has also standardized e.g. DCCP. For instance,
RFC 8329 also mentions DCCP. Well, DCCP is to the best of the knowledge of the
reviewer not widely deployed, i.e., maybe it could indeed be omitted. But
https://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml lists
other parameters. Which criteria determines that SCTP must be included in this
document, but other protocols are not considered?

Comments regarding the Appendix:

- The examples in the appendix need to be checked against the YANG model and
they have to be updated. For instance, in example A1, there is an entry
"<tcp-capability>exact-fourth-layer-port-num</tcp-capability>". That apparently
contradicts the definition in the document.

- Example 1 seems unrealistic. For instance, I cannot think of any firewall
that can only process TCP port numbers, but not UDP port numbers.

- I suggest to avoid the terms "forth-layer"/"forth layer" in this document.
For instance, IETF documents use the term "transport layer" or "transport
protocol" to refer to TCP, UDP, etc.

---

From: "Mr. Jaehoon Paul Jeong" <[email-masked]>
Date: Sun, 8 Nov 2020 23:25:13 +0900

Hi Michael,
I sincerely appreciate your review and comments.

I will work for this draft by removing some unnecessary parts and improving
your commented ones
in the NSF Capability YANG Data Model.

Thanks.

Best Regards,
Paul

On Sat, Nov 7, 2020 at 7:49 AM Michael Scharf via Datatracker <
[email-masked]> wrote:

> _______________________________________________
> I2nsf mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/i2nsf
>

-- 
===========================
Mr. Jaehoon (Paul) Jeong, Ph.D.
Associate Professor
Department of Computer Science and Engineering
Sungkyunkwan University
Office: +82-31-299-4957
Email: [email-masked], [email-masked]
Personal Homepage: http://iotlab.skku.edu/people-jaehoon-jeong.php
<http://cpslab.skku.edu/people-jaehoon-jeong.php>