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