Last Call Review of draft-ietf-i2nsf-capability-data-model-21
review-ietf-i2nsf-capability-data-model-21-secdir-lc-wouters-2021-11-29-00

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)