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

[t petch <[e-mail]>]

Hi Paul!
(adding I2NSF and document alias like an official response to a 
directorate review)

Thanks for this review.  A response below and the authors/WG can correct me.

<tp>

Chipping in as a WG member, my words of 2021 (2022, 2023...) are 
coherence and silos.

This I-D is one of a set of five, or more, which have a lot in common. 
'Improve' one and the risk is that the set as a whole becomes 
incoherent.  I put some effort in in 2021 to reduce the incoherence so I 
am twitchy about the nature of the IETF, operating in silos, the 
different parts bringing incoherence back in again.

Thus there is a comment about POP3 +/- POP3S. POP3 occurs in three I-D 
so change one and IMHO the others have to be changed.

url-capability appears in several I-D.  Change one and the other uses of 
it need changing.

tcp-flags already appears in another I-D so here I think that changing 
tcp to tcp-flags would be beneficial.

And so on.

Ideally, as I said about a Genart review, reviewers should be required 
to review the whole set, especially YANG Doctors, but I won't hold my 
breath.

Digging down, there is so much that ought to be in a common I-D, be that 
terminogy, YANG module or whatever but the time for that was several 
years ago, such as 6July 2019 when the WG produced a Terminology I-D.

Tom Petch

>> -----Original Message-----
>> From: secdir <[e-mail]> On Behalf Of Paul Wouters
>> Sent: Monday, November 29, 2021 4:06 PM
>>
>> 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 modeling choice was made because ICMPv4 and ICMPv6 are not the same 
protocol.  TCP and UDP are the same protocol regardless of whether they 
are using IPv4 or v6.

>> This term seems to be rather generic:
>>
>>          |  |  +--rw url-capability*    identityref
>>
>> Perhaps what is meant is url-filtering-capability or url-protection-capability ?

I'll leave it up to the WG to decide if they want to scope it as such.

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

There is a history in the naming convention of advanced vs. 
generic-nsf-capabilities.  Advanced capabilities were initially 
extension points discussed in other documents.  After refinement, the 
work didn't evolve this way.  The WG has discussed and modified this 
convention, and arrived at roughly the explanation documented in Section 
5.1:

==[ snip ]==
    In this
    document, two kinds of condition capabilities are used to classify
    different capabilities of NSFs such as generic-nsf-capabilities and
    advanced-nsf-capabilities.  First, the generic-nsf-capabilities
    define NSFs that operate on packet header for layer 2 (i.e., Ethernet
    capability), layer 3 (i.e., IPv4 capability, IPv6 capability, ICMPv4
    capability, and ICMPv6 capability.), and layer 4 (i.e., TCP
    capability, UDP capability, SCTP capability, and DCCP capability).
    Second, the advanced-nsf-capabilities define NSFs that operate on
    features different from the generic-nsf-capabilities, e.g., the
    payload, cross flow state, application layer, traffic statistics,
    network behavior, etc.  This document defines the advanced-nsf into
    two categories such as content-security-control and attack-
    mitigation-control.
==[ snip ]==


>> 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 can see your point.  "target" is used in place of "destination" in a 
few places.  This seems editorial and I'd leave it up to the WG to decide.
>> 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's the latter.  Consider Example 5 of Section A.5 which depicts the 
interrelationship between <anti-ddos-capability> and the 
<action-capabilities>.

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

For your antivirus configuration there might be something like the 
following:
<condition-capabilities>
    <advanced-nsf-capabilities>
    <anti-virus-capability>detect</anti-virus-capability>
...
<action-capabilities>
...
      <egress-action-capability>drop</ingress-action-capability>

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

See explanation above on generic vs. advanced-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 ?

Not question this contact information will age.  However, it seems to be 
common convention to include all of the document authors in the YANG 
contact information.
  >> 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?

Thoughts from WG?

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

Seems like there should be consistency or an approach either way as to 
whether the protocol name is a prefix to a field name.  I'll leave it to 
the authors to decide on the approach.

>> 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 think the intent is for two identities: POP3+POP3S and IMAP+IMAPS, but 
I'll let the WG make the right change.

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

Paul: Makes sense to me to add it in with your explanation.  WG: What do 
you think?  I believe "reject" was in -05, but removed in -06 after the 
first AD review 
(https://mailarchive.ietf.org/arch/msg/i2nsf/Qkup2tKpVyAcelxy3QooLf7P1KI/) 
pointed out that all of these identities were undocumented.

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

This text is a derivative of the standard YANG boiler plate text 
included in most YANG document recently in recent years.  The working 
source is kept at 
https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines.
>> 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 ?

It's a web-filter capability because of "<url-capability>".

"user-defined" is a specific type of URL-filter whose list is generated 
by the operator:

      identity user-defined {
        base url-filtering;
        description
          "Identity for user-defined URL Database condition capability.
           that allows a users manual addition of URLs for URL
           filtering.";
      }


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

The semantics are definitely reused from packet focused behavior.  For 
security mitigation devices that operate on streams 
pass/drop/mirror/log/etc are common actions though.
  >> And should these actions not be inside the capability 
<url-capability> ?
The YANG module design is modeled on the premise that each part of the 
E-C-A rule is a separate top-level container per Section 3.1.  That 
certainly does remove flexibility but was a design choice.

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

WG thoughts?

>> NITS

[...]

Thanks.  Leaving to the authors.

Regards,
Roman

_______________________________________________
I2nsf mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/i2nsf
.