[Last-Call] Secdir last call review of draft-ietf-i2nsf-nsf-facing-interface-dm-16

[Kyle Rose via Datatracker <[e-mail]>]

Reviewer: Kyle Rose
Review result: Has Issues

I have reviewed this document as part of the security directorate's ongoing
effort to review all IETF documents being processed by the IESG.  These
comments were written primarily for the benefit of the security area directors.
 Document editors and WG chairs should treat these comments just like any other
last call comments.

This document Has Issues.

I don't actually have a lot to say about this document from a security
perspective: its purpose is to describe, using YANG, a data model intended as
the basis for configuration schemas developed for implementations of the
Interface to Network Security Functions (I2NSF) framework. Security
considerations for the most part should be addressed in documents describing
system architecture or normatively detailing how implementations are to make
use of the data model described here. I'm not going to relitigate any such
issues here.

The main issues I found in this document are ones that I, as someone not
terribly familiar with YANG, found troubling from a general engineering
perspective. These are best illustrated by example:

 * Why are `ipvX-prefix`, `-range`, and (the misleadingly-named) `-address`
 defined here? These concepts are generic enough that they should be in modules
 of their own to minimize variation among data models and the errors that will
 inevitably result from capturing the same concept in slightly different ways
 that are not obvious to the user. * Overall, I have to imagine that much of
 the `nsfintf` data model is generic enough that it should be captured
 externally. For instance, `tcp-flags`, `port-range`, `flow-label`, `dscp`,
 etc. are generally useful elements of an abstract transport data model that
 they shouldn't be defined here, but rather incorporated from an external data
 model that is maintained by those in (for example) the transport area.

Am I just commenting on the YANG ecosystem in general? If these are standard
practices, then the overall ecosystem has major latent problems. Ideally, a
particular YANG module seems like it should describe only those elements
defined at a particular layer, in this case rules and actions, and use
reference external modules for elements that are defined at lower layers.

Also some nits:

 * `ipvX-address` describes a subspace of the global IPvX address space, not a
 single address. The name is going to cause problems. * The descriptions given
 are often (usually?) just restatements of the entity name. Example is
 `identity priority-by-order` described as "Identity for priority by order".
 The description should provide some value for the user beyond simply restating
 the name. * The headings in section 5 should be clearly labeled with the word
 "example", such as "Example Security Requirement 1". * IPv6 addresses in text
 MUST be represented in lowercase, according to RFC 5952 section 4.3.