Deterministic Networking (DetNet) YANG Model
draft-ietf-detnet-yang-20

# Internet AD comments for draft-ietf-detnet-yang-19
CC @ekline

* comment syntax:
  - https://github.com/mnot/ietf-comments/blob/main/format.md

* "Handling Ballot Positions":
  - https://ietf.org/about/groups/iesg/statements/handling-ballot-positions/

## Comments

### S8

* ipsec-spi is also defined in the same way in RFC 4302.  RFC 4301 says
  it's a 32-bit number, but doesn't seem to say anything about zero being
  reserved.

## Nits

### S4

* The section references for the sub-layers are all 3.x and look like they
  should 4.x

I have no comments, exept a note to Erik Kline's ballot comment :)

IPsec SPI is defined in RFC 4303:

   The set of SPI values in the range 1 through 255 are reserved by the
   Internet Assigned Numbers Authority (IANA) for future use; a reserved
   SPI value will not normally be assigned by IANA unless the use of the
   assigned SPI value is specified in an RFC.  The SPI value of zero (0)
   is reserved for local, implementation-specific use and MUST NOT be
   sent on the wire.

So the definition is probably right. While right now there is no valid SPI < 256,
and some implementations use it to refer to internal things, in theory these
could appear in the future.

** Section 10.  Per the write operations:

-- Editorial. Consider if this might be clear:

OLD
   Write operations (e.g., edit-config)
   to these data nodes without proper protection can break or
   incorrectly connect DetNet flows.
   Since this is a configured Data
   Plane any changes that are not coordinated with all devices along the
   path the whole DetNet module is considered vulnerable and should have
   authorized access only.

NEW
Unauthorized write operations (e.g., edit-config) to any elements of this module can break or incorrectly connect DetNet flows. Since DetNet is a configured Data Plane, any changes that are not coordinated with all devices along the path will create a denial of service.

-- Is it possible quantify the security impact of these changes beyond a DoS?  Is the following accurate, or is coordinate required in the path required?

NEW
Additional, arbitrary write operations could also enable an attacker to modify a network topology to enable select traffic to avoid inspection or treatment by security controls, or route traffic in a way that it would be subject to inspect/modification by an adversary node.

** Section 10.  Per the read operations:

These are the subtrees and data
   node and their sensitivity/vulnerability:

   /detnet/app-flows: This controls the application details so it could
   be considered sensitive.

   /detnet/traffic-profile/member-app: This links traffic profiles to
   applications, service sub-layers and/or and forwarding sub-layers so
   this also could be considered more sensitive.

   /detnet/service/sub-layer/incoming/app-flow: This links applications
   to services.

   /detnet/service/sub-layer/outgoing/app-flow: This links applications
   to services.

-- Please amend the text to explain why these nodes are sensitive (i.e., why revealing this information is a problem).

-- Is there confidence that only these nodes are read sensitive?  Wouldn’t almost every part of the module reveal internal topology which would inform the targeting of an attacker?

Thank you for this document -- usually I dread reading YANG documents, but this one was surprisingly clean, understandable, etc. 

After reading John's comment ("When reviewing, do *not* attempt to follow the examples in Appendix B without referring to one of the renderings that shows you the SVGs...") I was getting ready to DISCUSS, as I believe that all RFCs should render completely in ASCII -- but in this case the SVG is so clean and useful that I'm actually OK with it -- this has somewhat shaken my world view :-))

Thanks for working on this specification. I would just agree with my fellow transport AD and TSVART reviewer.

# Éric Vyncke, INT AD, comments for draft-ietf-detnet-yang-19

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).

Special thanks to János Farkas for the shepherd's detailed write-up (using the old template) including the WG consensus and a simple but present justification of the intended status.

Please note that Jean-Michel Combes is the Internet directorate reviewer (at my request) and you may want to consider this int-dir review as well when it will be available (no need to wait for it though):
https://datatracker.ietf.org/doc/draft-ietf-detnet-yang/reviewrequest/18768//

I hope that this review helps to improve the document,

Regards,

-éric


# COMMENTS (non-blocking)

## Section 7

The following points may be purist or even pedantic, but here they are:

### Circular references

'traffic-profile' includes leaves such as 'member-app' and 'app-flow' has a leaf 'traffic-profile'. This can lead to *inconsistency* if the values are not compatible. Do YANG/netconf have functions to retrieve which 'app-flow' use 'traffic-profile' ?

Or did I miss something ?

### Leaves names

Why the leaves under 'traffic-profile' are named 'member-app', 'member-fwd-sublayer' rather than 'member-app-flows' and 'member-forwarding' to match the naming used elsewhere ?

## IPv6 in the YANG module

I am uneasy with the model of an IPv6 flow

### inet:ip-address-no-zone

The use of inet:ip-address-no-zone is fine when purely matching on an IPv6 packet, but this also means that link-local addresses cannot be used probably. Is it on purpose ?

### protocol-next-header

```
      description
        "Internet Protocol number.  Refers to the protocol of the
         payload.  In IPv6, this field is known as 'next-header',
         and if extension headers are present, the protocol is
         present in the 'upper-layer' header.";
```

This description is rather vague... It should be really specific when using IPv4. For IPv6 with extension headers, there is no protocol in 'upper-layer' header.

## Section 13.2

Reference IEEE8021QCX is marked as 'superseded' since 2022 at https://ieeexplore.ieee.org/document/9212765 apparently by https://ieeexplore.ieee.org/document/10004498. 

## Appendix B

Thank you for using IPv6 examples and very nice SVG diagrams.

Albeit using flows 2001:db8::1/128 <-> 2001:db8::8/128 seems a little unrealistic as both nodes are probably in the same layer-2 link. But, DTN also work on a layer-2 link w/o actual layer-3 routing. Same for the IPv4 examples.

You may also want to refresh the dates to 2024 rather than 2020 ;-

Having some examples is really a good thing, I am just slighlty concerned by the large amount of them in this document. It is a matter of taste, so no need to reply.

Thanks to Joerg Ott for the TSVART review. I see no transport-layer issues