A YANG Model for Transmission Control Protocol (TCP) Configuration and State
draft-ietf-tcpm-yang-tcp-09

[Ballot comment]
old DISCUSSes:

[ballot text incomplete as cloudflare triggers a weird refusal if I include a certain sentence, see Cloudflare Ray ID: 722aa6477e01a1db]

Thanks for the -07 changes in response to the various area reviews!

I still have two small discuss questions left which might be caused by a limited
understanding of the underlying yang model inclusions.


              1. For an application willing to accept both IPv4 and
                IPv6 datagrams, the value of this object must be
                ''h (a zero-length octet-string), with the value
                of the corresponding 'type' object being
                unknown (0).

              2. For an application willing to accept only IPv4 or
                IPv6 datagrams, the value of this object must be
                '0.0.0.0' or '::' respectively, with
                'type' representing the appropriate address type.

#D1

To me, it seems very counter-intuitive that '' means (0.0.0.0 or ::). For
LISTEN, I find it concerning because it means that '' as a default means to
listen to any IP, rather than the more secure default of '' meaning 'none'.

Is it possible to have some kind of "None" default? (or is that achieved by
omission of this option?

#D2

Is it intended that this cannot be specified using names



#C1

It is unclear to me if '0.0.0.0' denotes the type inet:ip-address or
the type string with text value "0.0.0.0". I also worry that if this is
represented in C with a struct with union, that than it is unclear what
a zero'ed out struct is set to? The string or the v4 ANY or the v6 ANY ?
(I personally like enums to start from value 1 for that reason)

#C2

How is this option set when fully disabled (eg not listening on anything, only
willing to make an outgoing TCP connection). Is it by being omitted?

NITS:

listner -> listener

for TCP connection that -> for a TCP connection that

[Ballot comment]
Hi,

Thanks for another YANG module!

I have some non-blocking comments, I suspect that many of these have already been raised/discussed so really just want to check that they had been proactively considered.

1)
      TCP connection list: Access to status information for all TCP
      connections.  Note, the connection table is modeled as a list that
      is read-writeable, even though a connection cannot be created by
      adding entries to the table.  Similarly, deletion of connections
      from this list is implementation-specific.

I think that it would be helpful to further describe and perhaps constrain the behaviour here:
- I would suggest clarifying that the list is read/writable to allow configuration to be associated with existing connections, or connections that may be subsequently made.
- It would be helpful to understand what happens if the configuration for a connection changes for an existing connection.  Does it force the connection to be closed and reopened?  Or does it depend on the configuration change?
- I don't think that deleting the configuration properties should cause a connection to be closed (although it might cause the connection to flap, as per my comment above.)  E.g., for BGP, isn't it the BGP neighbour configuration that should control whether a connection logically exists?

2) With the ambiguity regarding YANG IP address and zones, can I clarify that in all cases, the intention is that zoned ip-addresses are allowed where the inet:ip-address type is used?

3) leaf enable-ao {
I presume that there was a conscious decision not to use a presence container here (and hence avoid the need for the must statements)?

4) leaf enable-md5 {
I presume that implementations won't need to add extra MD5 specific parameters?  I.e., it doesn't make sense for this to be a presence container?

5) Connection is closed. Connections in this state
                    may not appear in this list.";

In the operational state tree then what does the tcp/connections list represent?  Does it contain all connections that either exist or have some configuration associated with them?

6) I think that leaf "state" (describing the connection state) should be marked as "config false".  A client should not be able to configure the current protocol "state".

7)      leaf address {
          type union {
            type string;
            type inet:ip-address;
          }

Given the described behaviour, then I think that you should probably:
(i) List the inet:ip-address type first.
(ii) Add a length "0" restriction to the string type.

8)              For an application willing to accept both IPv4 and
                IPv6 datagrams, the value of this object must be
                ''h (a zero-length octet-string), with the value
                of the corresponding 'type' object being
                unknown (0).

What does the h mean?  I've got a nagging feeling that this is a known convention ...  I presumed that the WG considered and dismissed the option of having two separate listeners listed, one for v4 and one for v6 rather than the union with the string type?

9)
list tcp-listeners {
        key "type address port";
This allows multiple listeners for the same port.  I just wanted to check that is allowed/expected?

10) container statistics

These counters are global level, so there are not many of them.  Some of these counters are 32 bits, I presume that there is no reasonable risk that they would overflow over a reasonable time frame?

Nits:

included in TCP MIB => included in the TCP MIB
TCP-AO TCP-AO [RFC5925] => Duplicate TCP-AO.

Thanks,
Rob

[Ballot comment]
Just some editing suggestions:

In Section 1:

  ... This is conscious decision as these parameters
  hardly matter in a state-of-the-art TCP implementation.  It would
  also be possible also to translate a MIB into a YANG module, ...

Need "a" before "conscious", and drop one instance of "also".

Section 2.1 should probably indicate that there are lots of places where "I-D.ietf-tcpm-rfc793bis" in the YANG module will need to be replaced by its final RFC number.

Section 5.2:

OLD:

  This document registers a YANG module in the "YANG Module Names"
  registry YANG - A Data Modeling Language [RFC6020].  Following the
  format in YANG - A Data Modeling Language [RFC6020], the following
  registration is requested:

NEW:

  The following entry is requested to be added to the "YANG Module Names"
  registry created by [RFC6020]:

[Ballot comment]
Thanks for this document. I have two minor comments.

1. In §1,

  *  TCP-AO and TCP MD5 configuration for Layer 3 VPNs is modeled in A
      Layer 3 VPN Network YANG Model [RFC9182].  This model assumes that
      TCP-AO specific parameters are preconfigured in addition to the
      keychain parameters.

When you say "this model" are you referring to RFC 9182, or the present document? You could clarify by saying "that model" if you're talking about 9182 (which I think is the case?), or "the present document" if you mean your own spec.

2. In §6, perhaps consider using a different term for an on-path attacker other than "MITM", which conventionally (to my knowledge) expands to "man in the middle". (Or I think you could just drop "e.g. MITM", which doesn't seem to add much.)

[Ballot discuss]
** B.2.  Thanks for providing an example of a TCP-AO configuration.  Unfortunately, this isn’t a valid example example and it seems to be making citations that don’t match.  Specifically:

-- “hmac-sha-256”

The algorithms usable by TCP-AO come from https://www.iana.org/assignments/tcp-parameters/tcp-parameters.xhtml#tcp-parameters-3.  Only SHA1 and AES128 are registered.  As far as I tell, the only work to add SHA256 is in an unadopted and expired draft (https://datatracker.ietf.org/doc/draft-nayak-tcp-sha2/).

As aside, I think the algorithm name would have been SHA-256 or SHA256 (no HMAC).

--    Per the link to RFC9235:

    The following example demonstrates how to model a TCP-AO [RFC5925]
  configuration for the example in TCP-AO Test Vectors [RFC9235].  The
  IP addresses and other parameters are taken from the test vectors.

RFC9235 doesn’t use SHA256; and uses KeyID 61 and 84.  Different KeyIDs are used in this example.  Which parameters are being used from the test-vector?

[Ballot comment]
Thank you to Hilarie Orman for the SECDIR review.

I concur with the SECDIR observation that “there are no statistics kept for authentication failures.  If a shared key falls out of synch, the statistics might help detect that.”  I appreciate the response (https://mailarchive.ietf.org/arch/msg/secdir/BnjVJ7_IgN7mSNaGi3H2ujVYvn0/) which recommends that this should be generically modeled.  I disagree that this should be deferred. There is a very specific TCP-level primitive being described here and it makes sense to me to provide a corresponding logging primitive.

[Ballot discuss]
[ballot text incomplete as cloudflare triggers a weird refusal if I include a certain sentence, see Cloudflare Ray ID: 722aa6477e01a1db]

Thanks for the -07 changes in response to the various area reviews!

I still have two small discuss questions left which might be caused by a limited
understanding of the underlying yang model inclusions.


              1. For an application willing to accept both IPv4 and
                IPv6 datagrams, the value of this object must be
                ''h (a zero-length octet-string), with the value
                of the corresponding 'type' object being
                unknown (0).

              2. For an application willing to accept only IPv4 or
                IPv6 datagrams, the value of this object must be
                '0.0.0.0' or '::' respectively, with
                'type' representing the appropriate address type.

#D1

To me, it seems very counter-intuitive that '' means (0.0.0.0 or ::). For
LISTEN, I find it concerning because it means that '' as a default means to
listen to any IP, rather than the more secure default of '' meaning 'none'.

Is it possible to have some kind of "None" default? (or is that achieved by
omission of this option?

#D2

Is it intended that this cannot be specified using names

[Ballot discuss]
[ballot text incomplete at cloudflare triggers a weird refusal if I include a certain sentence, see Cloudflare Ray ID: 722aa6477e01a1db]

Thanks for the -07 changes in response to the various area reviews!

I still have two small discuss questions left which might be caused by a limited
understanding of the underlying yang model inclusions.


              1. For an application willing to accept both IPv4 and
                IPv6 datagrams, the value of this object must be
                ''h (a zero-length octet-string), with the value
                of the corresponding 'type' object being
                unknown (0).

              2. For an application willing to accept only IPv4 or
                IPv6 datagrams, the value of this object must be
                '0.0.0.0' or '::' respectively, with
                'type' representing the appropriate address type.

#D1

To me, it seems very counter-intuitive that '' means (0.0.0.0 or ::). For
LISTEN, I find it concerning because it means that '' as a default means to
listen to any IP, rather than the more secure default of '' meaning 'none'.

Is it possible to have some kind of "None" default? (or is that achieved by
omission of this option?

#D2

Is it intended that this cannot be specified using names

[Ballot discuss]
Thanks for the -07 changes in response to the various area reviews!

I still have two small discuss questions left which might be caused by a limited
understanding of the underlying yang model inclusions.


              1. For an application willing to accept both IPv4 and
                IPv6 datagrams, the value of this object must be
                ''h (a zero-length octet-string), with the value
                of the corresponding 'type' object being
                unknown (0).

              2. For an application willing to accept only IPv4 or
                IPv6 datagrams, the value of this object must be
                '0.0.0.0' or '::' respectively, with
                'type' representing the appropriate address type.

#D1

To me, it seems very counter-intuitive that '' means (0.0.0.0 or ::). For
LISTEN, I find it concerning because it means that '' as a default means to
listen to any IP, rather than the more secure default of '' meaning 'none'.

Is it possible to have some kind of "None" default? (or is that achieved by
omission of this option?

#D2

Is it intended that this cannot be specified using names

[Ballot discuss]
Thanks for the -07 changes in response to the various area reviews!

I still have two small discuss questions left which might be caused by a limited
understanding of the underlying yang model inclusions.


              1. For an application willing to accept both IPv4 and
                IPv6 datagrams, the value of this object must be
                ''h (a zero-length octet-string), with the value
                of the corresponding 'type' object being
                unknown (0).

              2. For an application willing to accept only IPv4 or
                IPv6 datagrams, the value of this object must be
                '0.0.0.0' or '::' respectively, with
                'type' representing the appropriate address type.

#D1

To me, it seems very counter-intuitive that '' means (0.0.0.0 or ::). For
LISTEN, I find it concerning because it means that '' as a default means to
listen to any IP, rather than the more secure default of '' meaning 'none'.

Is it possible to have some kind of "None" default? (or is that achieved by
omission of this option?

#D2

[Ballot discuss]
Thanks for the -07 changes in response to the various area reviews!

I still have two small discuss questions left which might be caused by a limited
understanding of the underlying yang model inclusions.


              1. For an application willing to accept both IPv4 and
                IPv6 datagrams, the value of this object must be
                ''h (a zero-length octet-string), with the value
                of the corresponding 'type' object being
                unknown (0).

              2. For an application willing to accept only IPv4 or
                IPv6 datagrams, the value of this object must be
                '0.0.0.0' or '::' respectively, with
                'type' representing the appropriate address type.

#D1

To me, it seems very counter-intuitive that '' means (0.0.0.0 or ::). For
LISTEN, I find it concerning because it means that '' as a default means to
listen to any IP, rather than the more secure default of '' meaning 'none'.

Is it possible to have some kind of "None" default? (or is that achieved by
omission of this option?

[Ballot discuss]
Thanks for the -07 changes in response to the various area reviews!

I still have two small discuss questions left which might be caused by a limited
understanding of the underlying yang model inclusions.


              1. For an application willing to accept both IPv4 and
                IPv6 datagrams, the value of this object must be
                ''h (a zero-length octet-string), with the value
                of the corresponding 'type' object being
                unknown (0).

              2. For an application willing to accept only IPv4 or
                IPv6 datagrams, the value of this object must be
                '0.0.0.0' or '::' respectively, with
                'type' representing the appropriate address type.

[Ballot comment]
#C1

It is unclear to me if '0.0.0.0' denotes the type inet:ip-address or
the type string with text value "0.0.0.0". I also worry that if this is
represented in C with a struct with union, that than it is unclear what
a zero'ed out struct is set to? The string or the v4 ANY or the v6 ANY ?
(I personally like enums to start from value 1 for that reason)

#C2

How is this option set when fully disabled (eg not listening on anything, only
willing to make an outgoing TCP connection). Is it by being omitted?

NITS:

listner -> listener

for TCP connection that -> for a TCP connection that

[Ballot comment]
I'm happy to see the changes from the -06 version.  Thanks to Jeff Haas for his review!

[One more comment from Jeff, which I'm transcribing here as it wasn't wart of an archived discussion.]

It occurs to me that as the keychain extensions are defined it might be desirable to protect them via if-feature.  Basically, what happens if tcp-ao or tcp-md5 aren't supported by the implementation?

[Ballot discuss]
Thanks for working on this useful specification.

I have noted the following which I think needs cross checking to make the meaning clear, it might be simple oversight or intentional. I would like to which one is correct.

- Section 4 :
    - any reason why the leaf send_id and recv_id does not use normative "MUST" in the description?

    - how should we interpret strongly " RECOMMENDED"? is this a MUST or RECOMMENDED?

[Ballot comment]

I have some minor observations, which we might want to address -

- Section 3.1 : It is not clear to me what is the difference between a "Global configuration" and "Policies". To me policies can include global configurations that will valid for all the TCP sessions. So, it is not clear is policy is a special case of global configuration or vise verse.  Also the term "Global" here seems ambiguous. I kind of read that as a global variable definition, still the text need to be clear about the scope of "Global" is self, global to what context?

- What are we really following when we say "directly following from TCP standards."? a reference is needed here to understand what is meant here.

[Ballot comment]
# Éric Vyncke, INT AD, comments for draft-ietf-tcpm-yang-tcp-07
CC @evyncke

Thank you for the work put into this document.

Please find below some non-blocking COMMENT points (but replies would be appreciated even if only for my own education), and some nits.

Special thanks to Yoshifumi Nishidar for the shepherd's detailed write-up including the WG consensus even if it lacks the justification of the intended status and uses an unusual template.

I hope that this helps to improve the document,

Regards,

-éric

## COMMENTS

### Section 1

```
  As such, TCP is implemented on network
  elements that can be configured via network management protocols such
  as NETCONF [RFC6241] or RESTCONF [RFC8040].
```
AFAIK, NETCONF & RESTCONF can also be used to monitor/collect statistics. So "configured" seems rather limited.

### Section 1 YANG usefulness

I wonder whether the paragraph containing: `Moreover, many existing TCP/IP stacks do not use YANG data models.` really brings any value to this document.

### Section 1, orthogonal or not ?

```
  This specification is orthogonal to the Management Information Base
  (MIB) for the Transmission Control Protocol (TCP) [RFC4022].  The
  basic statistics defined in this document follow the model of the TCP
  MIB.
```
Is the model is based on the MIB, can it really be orthogonal to it ?


## NITS 


### listner ? 

Is there a typo in "listner" ?

### Section 3.1

Perhaps a rendering issue, but "TCP-AO TCP-AO" is probably wrong ;-)

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues.

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments

The document has been updated, but there has been no dialogue with last call reviewers and not all relevant conversations have been vetted by the WG. I am holding this until those actions are complete.

(Via drafts-lastcall@iana.org): IESG/Authors/WG Chairs:

The IANA Functions Operator has completed its review of draft-ietf-tcpm-yang-tcp-06. If any part of this review is inaccurate, please let us know.

The IANA Functions Operator understands that, upon approval of this document, there are two actions which we must complete.

First, in the ns registry on the IETF XML Registry page located at:

https://www.iana.org/assignments/xml-registry/

a new namespace will be registered as follows:

ID: yang:ietf-tcp
URI: urn:ietf:params:xml:ns:yang:ietf-tcp
Filename: [ TBD-at-Registration ]
Reference: [ RFC-to-be ]

As this document requests registrations in a Specification Required (see RFC 8126) registry, we will initiate the required Expert Review via a separate request. This review must be completed before the document's IANA state can be changed to "IANA OK."

Second, in the YANG Module Names registry on the YANG Parameters registry page located at:

https://www.iana.org/assignments/yang-parameters/

a new YANG module will be registered as follows:

Name: ietf-tcp
File: [ TBD-at-Registration ]
Maintained by IANA? N
Namespace: urn:ietf:params:xml:ns:yang:ietf-tcp
Prefix: tcp
Module:
Reference: [ RFC-to-be ]

While the YANG module name will be registered after the IESG approves the document, the YANG module file will be posted after the RFC Editor notifies us that the document has been published.

The IANA Functions Operator understands that these are the only actions required to be completed upon approval of this document.

Note:  The actions requested in this document will not be completed until the document has been approved for publication as an RFC. This message is meant only to confirm the list of actions that will be performed.

Thank you,

Sabrina Tanamal
Lead IANA Services Specialist

The following Last Call announcement was sent out (ends 2022-03-02):

From: The IESG
To: IETF-Announce
CC: draft-ietf-tcpm-yang-tcp@ietf.org, martin.h.duke@gmail.com, nsd.ietf@gmail.com, tcpm-chairs@ietf.org, tcpm@ietf.org
Reply-To: last-call@ietf.org
Sender:
Subject: Last Call:  (A YANG Model for Transmission Control Protocol (TCP) Configuration) to Proposed Standard


The IESG has received a request from the TCP Maintenance and Minor Extensions
WG (tcpm) to consider the following document: - 'A YANG Model for
Transmission Control Protocol (TCP) Configuration'
  as Proposed Standard

The IESG plans to make a decision in the next few weeks, and solicits final
comments on this action. Please send substantive comments to the
last-call@ietf.org mailing lists by 2022-03-02. Exceptionally, comments may
be sent to iesg@ietf.org instead. In either case, please retain the beginning
of the Subject line to allow automated sorting.

Abstract


  This document specifies a minimal YANG model for TCP on devices that
  are configured by network management protocols.  The YANG model
  defines a container for all TCP connections and groupings of
  authentication parameters that can be imported and used in TCP
  implementations or by other models that need to configure TCP
  parameters.  The model also includes basic TCP statistics.  The model
  is compliant with Network Management Datastore Architecture (NMDA)
  (RFC 8342).




The file can be obtained via
https://datatracker.ietf.org/doc/draft-ietf-tcpm-yang-tcp/



No IPR declarations have been submitted directly on this I-D.

1. Summary

The document shepherd is Yoshifumi Nishida
The responsible Area Director is Martin Duke

This document specifies a YANG Model that can be used to configure
TCP on YANG supported network elements. This is a model with narrow scope
which includes fundamental TCP functions, basic statistics for TCP and
configuration parameters for TCP-AO and TCP MD5. One expected use case for
the model is BGP YANG Model.

2. Review and Consensus

The draft has been reviewed and discussed by various participants in the
WG for long time. When the draft was presented for the first time, some
concerns had been raised in the WG that there are other TCP related
YANG models which might create conflicts or overwrap.
In order to address it, the focus of the draft has been limited to a
small sets of parameters when it was adopted.
The draft has been updated to address several comments since then,
however, there was no controversial point that required long arguments.

In addition, some prototypes implementations efforts have been made to check
the feasibility of the model.

I believe there is a solid consensus in the WG for publication.

3. Intellectual Property

Each author has confirmed that their direct, personal knowledge of any
IPR related to this document has already been disclosed.
None of the authors is aware of any IPR related to this document.

4. Other Points

This draft contains a YANG model. We have requested a YANG doctors review request after the WGLC.

1. Summary

The document shepherd is Yoshifumi Nishida
The responsible Area Director is Martin Duke

This document specifies a YANG Model that can be used to configure
TCP on YANG supported network elements. This is a model with narrow scope
which includes fundamental TCP functions, basic statistics for TCP and
configuration parameters for TCP-AO and TCP MD5. One expected use case for
the model is BGP YANG Model.

2. Review and Consensus

The draft has been reviewed and discussed by various participants in the
WG for long time. When the draft was presented for the first time, some
concerns had been raised in the WG that there are other TCP related
YANG models which might create conflicts or overwrap.
In order to address it, the focus of the draft has been limited to a
small sets of parameters when it was adopted.
The draft has been updated to address several comments since then,
however, there was no controversial point that required long arguments.

In addition, some prototypes implementations efforts have been made to check
the feasibility of the model.

I believe there is a solid consensus in the WG for publication.

3. Intellectual Property

Each author has confirmed that their direct, personal knowledge of any
IPR related to this document has already been disclosed.
None of the authors is aware of any IPR related to this document.

4. Other Points

This draft contains a YANG model. We have requested a YANG doctors review request after the WGLC.