Subject: [secdir] (updated) review of draft-ietf-i2nsf-capability-data-model-21

From: Paul Wouters <[email-masked]>
Date: Mon, 29 Nov 2021 16:05:30 -0500 (EST)

Note to tools team: I was assigned draft-ietf-i2nsf-capability-data-model,
although I had already reviewed the -16 version. I did a review now of
the -21 version but did not see a way within datatracker to update the
review. So I opted to use the secdir mailing list for now.

Paul

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.

The summary of the review is Has Issues

I have reviewed the document. I don't have any particular security concerns, and the
Security Considerations section is fine. I do have some questions/issues from reading
the document.

I am a bit confused about this part:

         |  |  +--rw ipv4-capability*       identityref
         |  |  +--rw ipv6-capability*       identityref
         |  |  +--rw icmpv4-capability*     identityref
         |  |  +--rw icmpv6-capability*     identityref
         |  |  +--rw tcp-capability*        identityref
         |  |  +--rw udp-capability*        identityref

There is an item for v4 and v6 support. Why is there a split of icmpv4 and icmpv6?
Why isn't that done similarly to tcp and udp that don't have v4/v6 versions?

This term seems to be rather generic:

         |  |  +--rw url-capability*                    identityref

Perhaps what is meant is url-filtering-capability or url-protection-capability ?

It also seems rw advanced-nsf-capabilities is really either "rw protection-nsf-capabilities" or
"rw filtering-nsf-capabilities" ? It seems "advanced" is a very generic term? It could be
useful to allow for further non-filter/non-protective options, but it does seem right now
this "advanced" category really means some kind of "client protection" category.

Similarly, "rw target-capabilities" might be better descriped as "rw destination-capabilities"
to avoid confusing about this being a "targetting system" or the client being "targetted".

I also find "rw action-capabilities" confusing. Isn't "anti-virus" and "anti-ddos" also an
action capability ? Or should I read this as a condition of "anti-virus" kind activate
an action capability (filter in, filter out, log). It also seems the selector (eg "anti-virus")
is coupled to an action (eg "block") so I'm a bit confused on why there is no capability link
between these. Eg having "filtering" as a capability seems related to some conditionals, but
perhaps not all. So I am not sure if the current model could describe that. Eg lets say there
is a packet filter, not but no filter based on anti-virus but it can detect anti-virus. How
would one know from these capabilities that anti-virus has "filter" and not only "log" ?

And "rw generic-nsf-capabilities" seems to be more like "rw transport-nsf-capabilities"

There are many email contacts listed in Section 6. These will not stand the passing of time.
Why are they needed? Should there be an IANA registry/contact instead ?

the identity targets include base target-device which only has a description field. So all
these identity targets _only_ have a description. Is the presense of an empty identity entry
enough to indicate this support, or is some kind of boolean field needed?

identity flags is only about TCP. Should it be called tcp-flags (like tcp-options) ?
Similar issue with identity total-length, verification-tag, identity chunk-type and service-code.

I see identity for pop3 and imap. Does that include pop3s and imaps (version of those protocols over
TLS). If so, should it be clarified in the description text? If not, do we need seperate identity
types for these?

I see identities for pass, drop, mirror and rate-limit, but not for reject (eg send an ICMP back)

Security Considerations Section:

 	The lowest layer of RESTCONF protocol layers
 	MUST use HTTP over Transport Layer Security (TLS), that is, HTTPS
 	[RFC7230][RFC8446] as a secure transport layer.

This excludes QUIC? Perhaps it is better to say use an encrypted and authenticated transport layer, such
as TLS or QUIC using HTTPS.

I am a little confused at Example 3. It shows:

It's only capability is "user-defined". It's only actions are "ingress/egree options that do
pass/drop/mirror. Where does it state this is a web filter capability ? And does it really
mean the web URI and content can be passed/dropped/mirrored? It feels like these pass/drop/mirror
targets are for packets, not web-uri streams ?

And should these actions not be inside the capability <url-capability> ? What if you define an NSF
that has url-capability and a packet filter capability, how would one know the pass/mirror/drop
targets are for the url-capability ot the packet filter capability ?

Maybe, one of the examples can include an NSF with multiple conditions and actions that don't fully overlap?

NITS

 	Every NSF SHOULD be described with the set of capabilities it offers

This is a directive for humans at the manufacturor which we can't impose RFC2119 words on :)
I would just lowercase the SHOULD.

Similar:

 	Capabilities MUST be defined in a vendor- and technology- independent manner

I would say "Capabilities are defined ...."

 	Humans can refer to categories of security controls and understand each other.

I find the use of the word "humans" a little amusing here :) More seriously though, I
think a lot of this is for automated processing by tools, not humans.

 	As a further example, network security experts unequivocally refer to
 	"packet filters" as stateless devices

I am not sure this is true. Some "packet filters" are statefull firewalls.

I find the long sentence using "one can do this, one can do that and one can do something else" very
hard to read. Perhaps split this up and just repeat "an attack can " ....

I struggled when reading:

 	Some of the features that this document defines capability indicators for are

(although it is perfectly fine. For some reason it took me a little while to figure out how to read it)