Hi Tom,
I believe that I have addressed all of your concerns in the following
revision:
https://datatracker.ietf.org/doc/html/draft-ietf-i2nsf-capability-data-model-22

tcp-flags is named flags because their base identity is tcp and the
redundancy of naming is removed.

I will make sure that all of the five I2NSF YANG data model drafts are
well-synchronized.

If you have something to improve, please let me know.

Thanks.

Best Regards,
Paul

On Wed, Jan 12, 2022 at 9:55 PM t petch <ie...@btconnect.com> wrote:

> 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 <secdir-boun...@ietf.org> 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
> I2nsf@ietf.org
> https://www.ietf.org/mailman/listinfo/i2nsf
> .
>
> _______________________________________________
> I2nsf mailing list
> I2nsf@ietf.org
> https://www.ietf.org/mailman/listinfo/i2nsf
>
_______________________________________________
I2nsf mailing list
I2nsf@ietf.org
https://www.ietf.org/mailman/listinfo/i2nsf

Reply via email to