Re: [I2nsf] Yangdoctors last call review of draft-ietf-i2nsf-consumer-facing-interface-dm-07

"Mr. Jaehoon Paul Jeong" <jaehoon.paul@gmail.com> Fri, 17 July 2020 15:49 UTC

Return-Path: <jaehoon.paul@gmail.com>
X-Original-To: i2nsf@ietfa.amsl.com
Delivered-To: i2nsf@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 781473A07CB; Fri, 17 Jul 2020 08:49:16 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.085
X-Spam-Level:
X-Spam-Status: No, score=-2.085 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HK_NAME_FM_MR_MRS=0.001, HTML_MESSAGE=0.001, SPF_HELO_NONE=0.001, T_SPF_TEMPERROR=0.01, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id STNmO2cr_sgM; Fri, 17 Jul 2020 08:49:08 -0700 (PDT)
Received: from mail-lj1-x22f.google.com (mail-lj1-x22f.google.com [IPv6:2a00:1450:4864:20::22f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id C7D953A07BA; Fri, 17 Jul 2020 08:49:07 -0700 (PDT)
Received: by mail-lj1-x22f.google.com with SMTP id f5so13208422ljj.10; Fri, 17 Jul 2020 08:49:07 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=HpfzWFf38FxaiuJXE2geR/8VVT2E7Fgu4RXZyN4K4S0=; b=V7EXuMSk94tpEYyuvk3EIB8q6AWqTEHQUGtdvhQJUOjyzYTXqe7hWadLV+/wMtkBgB Y1ViF5MpizfhcgVT5x4J7yi44UpX6gXqxwNnV5KSayitdJgmbpcMUn6UeQtmA8J6X7Zn A5taeyThPXyp7L8mZG1Gdy1KccAKfNAbxho/FRJBrt4Zh1JGKgJdkmHjuP54YHz9L/t4 1pwEbrkXMyu4G+fTlqzeOF62WKf/rhiytnsGi41HDvud2S84CYKJ9XEJq35njAEO3ThO LPnAzyCcLqBGCyurVEkj1Ufi+va6aPZwgDm1RmvU8yz78FqpkQzKn8+mVfYm82SWhJqt Nmyw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=HpfzWFf38FxaiuJXE2geR/8VVT2E7Fgu4RXZyN4K4S0=; b=VMjRcP8ULJ/YOdwTmiWsQ5eo55bjCWgPyn75SqAYbOSVQ23oaI/bc8OVY+9jM1JNDs kzY+Jnfw+VcsLePsZNkshcVpxAJgLYzqGxkUMLmfZa5lOsNFE+epDWcs6EX79dbGWWmq aNvaLm0smD/MW10nkiv2GSkEVfiB28ciPiciNHS5qnV27RYwAR2/IVMja+qg67lPzW4Z wQvR21ZhMzYMMohZPu0NUInD7ZFnc6O37DnsJIs8NRXs9+iH3kLs+TESw+a3G/vddmgj q70M7NDablPFeqrCQwEhRzHBa4Rt2lW1mHBwkMnJ9QezBIVCIqOot07XnD2XqM6g7JN8 8Ojg==
X-Gm-Message-State: AOAM53116Zf5Y022SS40SLd4gijqkfr+g8jwVkMQ96ECbBshaThKJE6j +p7liI5ATWa3jVYFZCWA3mAk8NKOcO/Piv+Hhp0=
X-Google-Smtp-Source: ABdhPJzUueiovlrvnLctVcn7RVSRueQH3EdmNBU9M10Kj5tOPvVegrxQNAaE98XNUgbq8y7/En96Hs/1Q9hafa4HOSM=
X-Received: by 2002:a05:651c:1116:: with SMTP id d22mr5058925ljo.170.1595000945487; Fri, 17 Jul 2020 08:49:05 -0700 (PDT)
MIME-Version: 1.0
References: <157349122063.7571.1978842562243958252@ietfa.amsl.com> <CAPK2Dexgk81Saufei3z67E4XZg=LLra1HdTUWU-kU33Pj_o+eg@mail.gmail.com> <CAPK2DeyWEzR6Qy6HPURnKp481mH=y+3O2xpLBS9kLc1MPbcjBg@mail.gmail.com> <44A4E4A8-AF9A-47AF-A31A-8AAACAF0A6BA@tail-f.com> <CAPK2Dey7GzzAWh8AeKA8e5Ng8skxZBf1SYKGLyuatpZDJ+YPWQ@mail.gmail.com> <5F117489.6060709@btconnect.com> <5F118663.8010707@btconnect.com>
In-Reply-To: <5F118663.8010707@btconnect.com>
From: "Mr. Jaehoon Paul Jeong" <jaehoon.paul@gmail.com>
Date: Sat, 18 Jul 2020 00:48:26 +0900
Message-ID: <CAPK2DewCPYYb2q_nekE0+iExbbwR_Nzzs-bb93URetn6DxemPw@mail.gmail.com>
To: tom petch <daedulus@btconnect.com>
Cc: Jan Lindblad <janl@tail-f.com>, YANG Doctors <yang-doctors@ietf.org>, "i2nsf@ietf.org" <i2nsf@ietf.org>, skku-iotlab-members <skku-iotlab-members@googlegroups.com>, "Mr. Jaehoon Paul Jeong" <jaehoon.paul@gmail.com>
Content-Type: multipart/alternative; boundary="000000000000c5b38605aaa51817"
Archived-At: <https://mailarchive.ietf.org/arch/msg/i2nsf/ux4DCj95vI5dsLZfwviwvBhbnoo>
Subject: Re: [I2nsf] Yangdoctors last call review of draft-ietf-i2nsf-consumer-facing-interface-dm-07
X-BeenThere: i2nsf@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "*I2NSF: Interface to Network Security Functions mailing list*" <i2nsf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/i2nsf>, <mailto:i2nsf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/i2nsf/>
List-Post: <mailto:i2nsf@ietf.org>
List-Help: <mailto:i2nsf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/i2nsf>, <mailto:i2nsf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 17 Jul 2020 15:49:17 -0000

Hi Tom,
I will address your further comments in the revision.

Thanks for your review and comments.

Best Regards,
Paul

On Fri, Jul 17, 2020 at 8:07 PM tom petch <daedulus@btconnect.com> wrote:

> On 17/07/2020 10:51, tom petch wrote:
> > On 11/07/2020 08:44, Mr. Jaehoon Paul Jeong wrote:
> >> Hi Jan and Tom,
> >> I have revised our I2NSF Consumer-Facing Interface (CFI) Data Model
> Draft
> >> according to both your comments.
> >>
> >> Jan,
> >> I attach the revised draft and the revision letter to explain how I have
> >> reflected your comments one by one.
> >>
> >> Tom,
> >> the references to RFC inside our YANG module cannot be cited in my I-D
> >> XML
> >> file, so I cannot include them
> >> in Normative References.
> >
> > Yes you can and in fact you must:-)
> > You can put anything you want to in Normative References with a
> > corresponding [RFC0913] in the text part of the I-D so you add Section
> > 8.1 "This YANG module imports from [RFC6991], ... and makes reference to
> > [RFC0854], [RFC0913], ......"
> >
> > Note that all import must have a reference clause and the referenced
> > work must appear in Normative References; same technique applies.
>
> Some more minor tweaks
>
> s.5.1 /gorup/group/
>
> YANG module
>
> WG Chairs are not usually listed in the module - they used to be
>
> descripton is a bit terse - some quote the Abstract
>
> YARA, SNORT, SURICATA would benefit from references; they are not ones I
> see in TLS or SSH!
>
> typedef time I see in RFC6991bis
>
> does the ipv6 addresss ever need the interface?
>
> start/end ipv4/ipv6 could do with a must end > start
>
> geo-ip could do with a reference
>
> s.9.1  221.159 is not a documentation address - see RFC5737
>
> IESG often expect an ipv6 example alongside ipv4
>
> s.12 Registrant should be IESG
>
> prefix is not that of the module
>
>
> Tom Petch
>
>
> > Tom Petch
> >
> >>
> >> Also, the choice of the prefix  is i2nsf-cfi.
> >>
> >> I put "Note: This section is informative" for Sections 7 and 10, which
> >> include XML configuration examples.
> >>
> >> If you have further comments, please let me know by July 12, 2020, in
> >> EST.
> >> If possible, I want to post this revision on July 13, 2020 after
> >> reflecting
> >> your further comments on the revision.
> >>
> >> Thanks.
> >>
> >> Paul
> >>
> >>
> >> On Fri, Mar 20, 2020 at 2:25 AM Jan Lindblad <janl@tail-f.com> wrote:
> >>
> >>> Paul,
> >>>
> >>> Thank you for all your work with the module, and for the reminder for
> me
> >>> to verify all the changes.
> >>>
> >>> I am afraid I think the module is still not ready for last call, even
> if
> >>> it is better shape than ever thanks to your efforts. I went through the
> >>> module from top to bottom, so this is sorted in order of appearance.
> >>>
> >>> Line 107-204: The following identities are declared in the module, but
> >>> never referenced. They should either have a common base with
> >>> something, or
> >>> be referenced somewhere. If not, why are they defined here? They
> >>> currently
> >>> serve no purpose in this YANG module.
> >>>    identity ddos {
> >>>    identity enforce-type {
> >>>    identity admin {
> >>>    identity time {
> >>>
> >>> Line 377: Defining a custom date-and-time type seems odd. You should
> >>> probably use one that has already been defined
> >>>    typedef date-and-time {
> >>>
> >>> Line 513: The leaf represents the name of a user, but the format is
> >>> undefined. What should be the format for the string value? How would
> >>> a user
> >>> know what to configure here? Email addresses? If implementation
> >>> dependent,
> >>> say so.
> >>>      leaf name {
> >>>        type string;
> >>>        description
> >>>          "This represents the name of a user.";
> >>>
> >>> Line 518: If no IP address information is specified for the user-group,
> >>> what happens then? Is the user access accepted, rejected, or
> >>> something else?
> >>>      uses ip-address-info;
> >>>
> >>> Line 658: Key leaf declared mandatory. All keys are mandatory, so
> >>> mandatory is not needed on this leaf.
> >>>      leaf policy-name {
> >>>        type string;
> >>>        mandatory true;
> >>>
> >>> Line 664: Users mentioned in the owners-ref should have full CRUD
> >>> privileges to the policy. But what about everyone else? Should they
> have
> >>> R(ead) privileges? Can anyone create new policies? If not, who can? If
> >>> someone creates a policy, but does not mention his own name among
> owners
> >>> (e.g. misspells or does not get the format right), he will not be
> >>> able to
> >>> modify or remove the policy. If no owner is mentioned, then noone can.
> >>>      uses owners-ref;
> >>>
> >>> Line 673: Key leaf declared mandatory. All keys are mandatory, so
> >>> mandatory is not needed on this leaf.
> >>>          leaf rule-name {
> >>>            type string;
> >>>            mandatory true;
> >>>
> >>> Line 682: Users mentioned in the owners-ref should have full CRUD
> >>> privileges to the rule. But what about everyone else? Should they have
> >>> R(ead) privileges? Can anyone create new rules, or only those that have
> >>> full CRUD privileges for the policy? If someone creates a rule, but
> does
> >>> not mention his own name among owners (e.g. misspells or does not get
> >>> the
> >>> format right), he will not be able to modify or remove the rule.
> >>>      uses owners-ref;
> >>>
> >>> Line 697: Choice enforce-type has a description that I can't
> understand.
> >>> What does this mean?
> >>>            choice enforce-type {
> >>>              description
> >>>                "There are two different enforcement types;
> >>>                admin, and time.
> >>>                It cannot be allowed to configure
> >>>                admin=='time' or enforce-time=='admin'.";
> >>>
> >>> Line 703: In case of enforce-type admin (whatever that means), a string
> >>> value needs to be configured. What are the valid values for this leaf?
> >>>              case enforce-admin {
> >>>                leaf admin {
> >>>                  type string;
> >>>                  description
> >>>                    "This represents the enforcement type
> >>>                    based on admin's decision.";
> >>>
> >>> Line 711: In case of enforce-type time, three times can be configured.
> >>> What is the relation between enforce-time, and the other two
> >>> (begin-time,
> >>> end-time)?
> >>>              case time {
> >>>                container time-information {
> >>>                  description
> >>>                    "The begin-time and end-time information
> >>>                    when the security rule should be applied.";
> >>>                  leaf enforce-time {
> >>>                    type date-and-time;
> >>>                    description
> >>>                      "The enforcement type is time-enforced.";
> >>>                  }
> >>>                  leaf begin-time {
> >>>                    type date-and-time;
> >>>                    description
> >>>                      "This is start time for time zone";
> >>>                  }
> >>>                  leaf end-time {
> >>>                    type date-and-time;
> >>>                    description
> >>>                      "This is end time for time zone";
> >>>                  }
> >>>
> >>> Furthermore, the locally defined date-and-time type used includes both
> a
> >>> date and time, which seems to be at odds with the example
> >>> configurations in
> >>> the draft. Example 9.2:
> >>>    <rules>
> >>>      <rule>
> >>>        <rule-name>block_access_to_sns_during_office_hours</rule-name>
> >>>        <event>
> >>>          <time-information>
> >>>            <begin-time>2020-03-11T09:00:00.00Z</begin-time>
> >>>            <end-time>2020-03-11T18:00:00.00Z</end-time>
> >>>
> >>> In the example, the rule-name "block_access_to_sns_during_office_hours
> "
> >>> suggests that the begin-time and end-time should be times of day
> between
> >>> which the policy should be enforced. E.g. every day between 9.00 and
> >>> 18.00.
> >>> If that is a valid use case, using a time type with a date doesn't make
> >>> much sense. In the context of the policy that repeats "daily", how
> >>> should
> >>> the start date-and-time value "2020-03-11T09:00:00.00Z " be
> interpreted?
> >>> What if it was "monthly"?
> >>>
> >>> Line 736: In the frequency leaf, the enumeration value only-once is for
> >>> rules that don't repeat. But how long do they apply? A single packet? A
> >>> single time the rule is triggered? How does a user know if the rule is
> >>> still in effect, i.e. if the "once" has happened or not?
> >>>                enum only-once {
> >>>
> >>> Line 835: Maybe it's just my limited understanding of how threat-feeds
> >>> work, but I wonder i this construct with source and destinations for
> >>> threat
> >>> feeds is meaningful?
> >>>            container threat-feed-condition {
> >>>              description
> >>>                "The condition based on the threat-feed information.";
> >>>              leaf-list source {
> >>>                type leafref {
> >>>                  path
> >>> "/i2nsf-cfi-policy/threat-preventions/threat-feed-list/name";
> >>>                }
> >>>              description
> >>>                "Describes the threat-feed condition source.";
> >>>              }
> >>>              leaf dest-target {
> >>>                type leafref {
> >>>                  path
> >>> "/i2nsf-cfi-policy/threat-preventions/threat-feed-list/name";
> >>>                }
> >>>              description
> >>>                "Describes the threat-feed condition destination.";
> >>>
> >>> Line 920: Location groups can be configured, but there seems to be no
> >>> references to them. How are they supposed to be used?
> >>>        list location-group{
> >>>          key "name";
> >>>          uses location-group;
> >>>
> >>> Line 931: Regarding point 16.1 in your revision letter, you say "We
> >>> think
> >>> list type of threat-feed-list can be configured more than one feed of
> >>> the
> >>> same type". I'm afraid that is not the case with the current YANG
> >>> model. If
> >>> you do wish to allow more than one threat-feed-list for the same
> >>> threat-feed-type, you need to add an additional key to your
> >>> threat-feed-list.
> >>>        list threat-feed-list {
> >>>          key "name";
> >>>          description
> >>>            "There can be a single or multiple number of
> >>>            threat-feeds.";
> >>>          uses threat-feed-info;
> >>>
> >>> ...
> >>>    grouping threat-feed-info {
> >>>      description
> >>>        "This is the grouping for the threat-feed-list";
> >>>      leaf name {
> >>>        type identityref {
> >>>          base threat-feed-type;
> >>>
> >>>
> >>> Generally, the indentation in the module is much improved. Some lines
> >>> are
> >>> still a bit off, however, so I would recommend using a tool that
> indents
> >>> consistently.
> >>>
> >>> Generally, I also wonder whether there has been any discussion with
> >>> implementors around the admin security model proposed here. As noted
> >>> before, it's a bit different from everything else I have seen. Is it
> >>> well
> >>> thought through? Do implementors feel this is doable and user friendly?
> >>> Currently there are no examples involving owner. Perhaps an example
> that
> >>> sheds some light over how different users create, modify and see the
> >>> various rules would shed some light over this.
> >>>
> >>> Best Regards,
> >>> /jan
> >>>
> >>>
> >>>
> >>>
> >>>
> >>> On 18 Mar 2020, at 18:41, Mr. Jaehoon Paul Jeong
> >>> <jaehoon.paul@gmail.com>
> >>> wrote:
> >>>
> >>> Hi Jan,
> >>> Could you update the state of YANGDOCTORS Last Call Review on
> >>> I2NSF Consumer-Facing Interface YANG Data Model  if the updates are
> fine
> >>> to you?
> >>>
> >>>
> >>>
> https://datatracker.ietf.org/doc/draft-ietf-i2nsf-consumer-facing-interface-dm/
> >>>
> >>>
> >>> I think your comments are all addressed in this version.
> >>>
> >>> Thanks.
> >>>
> >>> Best Regards,
> >>> Paul
> >>>
> >>> On Thu, Mar 12, 2020 at 1:15 AM Mr. Jaehoon Paul Jeong <
> >>> jaehoon.paul@gmail.com> wrote:
> >>>
> >>>> Hi Jan,
> >>>> We authors have addressed your comments with the revision:
> >>>>
> >>>>
> https://tools.ietf.org/html/draft-ietf-i2nsf-consumer-facing-interface-dm-08
> >>>>
> >>>>
> >>>> I attach a revision letter to explain how to respond to your comments.
> >>>>
> >>>> If you have further comments, please let me know.
> >>>>
> >>>> Thanks.
> >>>>
> >>>> Best Regards,
> >>>> Paul
> >>>>
> >>>> On Tue, Nov 12, 2019 at 1:53 AM Jan Lindblad via Datatracker <
> >>>> noreply@ietf.org> wrote:
> >>>>
> >>>>> Reviewer: Jan Lindblad
> >>>>> Review result: Almost Ready
> >>>>>
> >>>>> This is my YD review of
> >>>>> draft-ietf-i2nsf-consumer-facing-interface-dm-07. I
> >>>>> have previously reviewed the -05 revision (~end June). I find the new
> >>>>> revision
> >>>>> much improved, but still with much to discuss. I will call this
> >>>>> "almost
> >>>>> ready".
> >>>>>
> >>>>> Generally speaking, I think the YANG module lacks the precision and
> >>>>> descriptions needed to foster interoperability. The examples at the
> >>>>> end
> >>>>> are
> >>>>> very enlightening however, and compensate for much of that, but their
> >>>>> informal
> >>>>> nature can never replace proper YANG. The module usage needs to be
> >>>>> mostly clear
> >>>>> from the module itself.
> >>>>>
> >>>>> The management access control model proposed here is, even with its
> >>>>> latest
> >>>>> adaptation towards NACM, is still quite different from NACM author's
> >>>>> original
> >>>>> ideas. I will therefore bring this use case up in the NETMOD WG for
> >>>>> discussion.
> >>>>>
> >>>>> 1. Network access control principles
> >>>>>
> >>>>> Network access control is about which users are able to use the
> >>>>> network
> >>>>> being
> >>>>> managed, for example connect to facebook. The purpose of the NSF
> >>>>> module
> >>>>> is to
> >>>>> control this access. This version of the YANG module is now based on
> a
> >>>>> list of
> >>>>> policies.
> >>>>>
> >>>>> Each policy has a list of rules. Each rule has an event --
> >>>>> condition --
> >>>>> action
> >>>>> triplet. This resembles traditional firewall management, which is a
> >>>>> good
> >>>>> thing,
> >>>>> because that concept is stable and much tried. This allows
> >>>>> operators to
> >>>>> create
> >>>>> lists of rules in this style:
> >>>>>
> >>>>> if pkt.x == 1: drop                     // Rule 1
> >>>>> elif pkt.y > 2: alert                   // Rule 2
> >>>>> elif pkt.z == 10: pass          // Rule 3
> >>>>> else: drop                              // Rule 4
> >>>>>
> >>>>> This pattern relies heavily on the ability to control the order of
> the
> >>>>> rules.
> >>>>> The current model relies on the alphabetical sorting of names rules
> >>>>> for
> >>>>> the
> >>>>> ordering. The YANG trick I would recommend to give operators the
> >>>>> ability
> >>>>> to
> >>>>> insert and move rules as they wish is to add ordered-by user on the
> >>>>> list:
> >>>>>
> >>>>>      list rule {
> >>>>>        ordered-by user;  // <== Add this line
> >>>>>        leaf rule-name {
> >>>>>
> >>>>> Nothing is said about what the system should do in case policies
> >>>>> conflict. What
> >>>>> if one policy says pass, the other drop for the same packet? Please
> >>>>> clarify.
> >>>>> What should happen to packets that do not match any of the policies?
> >>>>>
> >>>>> This module also assumes that all users in the operator's
> organization
> >>>>> are
> >>>>> listed in one or more NACM groups (e.g. "employees"). That wasn't
> >>>>> really
> >>>>> the
> >>>>> NACM authors' original intent. Even if this could be made to work
> >>>>> maybe,
> >>>>> there
> >>>>> is no strong reason to repurpose the NACM group concept for user
> >>>>> network
> >>>>> access
> >>>>> purposes. It could easily be modeled differently. So in the cases
> >>>>> where
> >>>>> there
> >>>>> are leafrefs to NACM groups when dealing with network access rather
> >>>>> than
> >>>>> management access, don't use NACM groups.
> >>>>>
> >>>>>                  leaf src-target {
> >>>>>                    type leafref {
> >>>>>                      path
> >>>>> /nacm:nacm/nacm:groups/nacm:group/nacm:user-name;  //
> >>>>>                      <== Point to some other list of network users
> >>>>>
> >>>>> 2. Management access control principles
> >>>>>
> >>>>> Management access control is about which users are able to
> >>>>> configure/run
> >>>>> actions/the policies and rules. IMO, the most controversial aspect of
> >>>>> this
> >>>>> module has always been its new and creative management access control
> >>>>> model. In
> >>>>> this revision, the management principles have been remodeled
> >>>>> greatly to
> >>>>> fit in
> >>>>> with NACM. I find this redesign very promising, but the result is
> >>>>> still
> >>>>> not
> >>>>> quite ready for publication.
> >>>>>
> >>>>> The point where integration with NACM concepts is important is when
> it
> >>>>> comes to
> >>>>> allow some users to CRUD the NSF policies and rules themselves.
> >>>>> There is
> >>>>> now a
> >>>>> leaf-list "owners" on each policy and rule which point to a list of
> >>>>> NACM
> >>>>> groups. My understanding is that the idea is that the NSF module
> >>>>> should
> >>>>> be seen
> >>>>> as a service model that translate high level ownership information to
> >>>>> specific
> >>>>> NACM rules. It would be good to mention these ideas somewhere in
> >>>>> the NSF
> >>>>> document.
> >>>>>
> >>>>>    leaf-list owners {
> >>>>>      type leafref {
> >>>>>        path /nacm:nacm/nacm:groups/nacm:group/nacm:name;
> >>>>>
> >>>>> I expect the intent is that any user listed in a NACM group
> >>>>> mentioned in
> >>>>> the
> >>>>> owners list would get full CRUD privileges for the contents of the
> >>>>> rule
> >>>>> the
> >>>>> owners leaf sits on. That is never spelled out anywhere, however.
> >>>>>
> >>>>> It is a little less clear how leaf-list owners on policy objects
> >>>>> should
> >>>>> be
> >>>>> handled. Should owners listed on a policy object get full CRUD powers
> >>>>> over the
> >>>>> entire policy, including all the rules inside? Or would they need
> >>>>> to be
> >>>>> listed
> >>>>> on the rules as well? Not clear. Is the intent that users not
> >>>>> listed on
> >>>>> the
> >>>>> policy object are unable to create new rules, but to be able to
> update
> >>>>> rules
> >>>>> they are listed as owners of, if any?
> >>>>>
> >>>>> Who is allowed to create new policy objects? Should users that are
> not
> >>>>> owners
> >>>>> get read access to all the policies and rules?
> >>>>>
> >>>>> Finally, there is an "owner" leaf on each rule with an identityref
> >>>>> allowing
> >>>>> operators to configure a role name like dept-head or sec-admin. It is
> >>>>> marked
> >>>>> mandatory, but never included in any of the examples at the end of
> the
> >>>>> document. This makes me think this may be a remnant from bygone
> >>>>> times and
> >>>>> should be removed from the YANG. If not, an explanation of how to use
> >>>>> this
> >>>>> leaf, and how it interacts with "owners" needs to be added, and the
> >>>>> examples
> >>>>> updated.
> >>>>>
> >>>>> 3. leafrefs crosspointing between policy instances
> >>>>>
> >>>>> There are six leafrefs that point to various objects inside a policy,
> >>>>> e.g. a
> >>>>> rule condition pointing to a device group name. None of them restrict
> >>>>> what can
> >>>>> be pointed to so that only names within the current policy are
> >>>>> valid. It
> >>>>> is
> >>>>> therefore possible to configure the name of a device group defined
> >>>>> in a
> >>>>> different policy. I suspect this is not the intention. In order to
> >>>>> restrict the
> >>>>> leafrefs to the same policy, the following predicate can be added:
> >>>>>
> >>>>>                  leaf-list src-target {
> >>>>>                    type leafref {
> >>>>>                    path
> >>>>>
> >>>>>
> "/i2nsf-cfi-policy[policy-name=current()/../../../../../policy-name]/endpoint-group/device-group/name";
> >>>>>
> >>>>>                     // <== Add predicate
> >>>>>
> >>>>> 4. Mandatory to implement all events, conditions, actions
> >>>>>
> >>>>> Each rule is defined with a choice of different events (admin, time),
> >>>>> conditions (firewall, ddos, custom, threat-feed) and actions (pass,
> >>>>> drop,
> >>>>> alert, mirror, ...). Is the intent that all of these options should
> be
> >>>>> mandatory to implement? The current model requires this. Also,
> >>>>> would it
> >>>>> make
> >>>>> sense to allow additional mechanisms here? If so, it may be good to
> >>>>> explain to
> >>>>> readers how the set of choices and identities can be extended by
> >>>>> implementations.
> >>>>>
> >>>>> 5. Optional and mandatory elements
> >>>>>
> >>>>> In this revision of the module, 8 leafs have been marked mandatory. A
> >>>>> few of
> >>>>> them are list keys, which are conventionally not marked mandatory,
> >>>>> since
> >>>>> list
> >>>>> keys are always mandatory. A few others are skipped in the XML
> >>>>> examples
> >>>>> at the
> >>>>> end of the NSF document, which makes me believe they might not
> >>>>> really be
> >>>>> mandatory after all.
> >>>>>
> >>>>> Three leafs have a default, but most leafs are left optional
> >>>>> without any
> >>>>> default. In many cases I think I understand what it means to not set
> a
> >>>>> leaf,
> >>>>> but with the ones listed here, I don't think it clear at all.
> >>>>> Either add
> >>>>> a
> >>>>> default to make it clear, make them mandatory if they should be, or
> >>>>> explain in
> >>>>> the leaf description what happens if not set.
> >>>>>
> >>>>> 493: leaf-list name
> >>>>> 513: leaf-list protocol
> >>>>> 531: leaf geo-ip-ipv4
> >>>>> 541: leaf continent
> >>>>> 562: leaf feed-server-ipv4
> >>>>> 585: leaf payload-description
> >>>>> 590: leaf-list content
> >>>>> 600: leaf-list owners
> >>>>> 870: leaf method
> >>>>>
> >>>>> 6. Indentation
> >>>>>
> >>>>> The YANG indentation is mostly wrong. Run the YANG text through
> >>>>> pyang or
> >>>>> some
> >>>>> other tool that can indent the content correctly before you put it
> >>>>> into a
> >>>>> document.
> >>>>>
> >>>>> 7. YANG element naming
> >>>>>
> >>>>> The YANG convention is to not have lists on the top level in the YANG
> >>>>> module,
> >>>>> but to surround lists with a container. The surrounding container
> >>>>> often
> >>>>> has a
> >>>>> name in the plural and the list in singluar, like this
> >>>>>
> >>>>> container interfaces {
> >>>>>      list interface {
> >>>>>
> >>>>> To better fit into the world of IETF YANG modules, I'd recommend
> >>>>> turning
> >>>>> the
> >>>>> top level list i2nsf-cfi-policy into this instead:
> >>>>>
> >>>>> container i2nsf-cfi-policies {
> >>>>>      list policy {
> >>>>>
> >>>>> Further down, I would change container rule to rules:
> >>>>>
> >>>>> container rules {
> >>>>>      list rule {
> >>>>>
> >>>>> Finally, it is customary to not repeat the names of parent object
> >>>>> in the
> >>>>> names
> >>>>> of elements. For example, in the following:
> >>>>>
> >>>>> list threat-feed-list
> >>>>>      leaf feed-name
> >>>>>      leaf feed-server-ipv4
> >>>>>      leaf feed-server-ipv6
> >>>>>      leaf feed-description
> >>>>>
> >>>>> all the leafs should normally not repeat "feed-". Just leaf name,
> leaf
> >>>>> server-ipv4, leaf server-ipv6, leaf description. There are many more
> >>>>> examples
> >>>>> of this throughout the module.
> >>>>>
> >>>>> The condition choice has many containers with a single leaf inside
> >>>>> (e.g.
> >>>>> ddos-source). Their purpose is rather unclear to me. Remove?
> >>>>>
> >>>>>                container ddos-source {
> >>>>>                  description
> >>>>>                  "This represents the source.";
> >>>>>                  leaf-list src-target {
> >>>>>
> >>>>> Also, I find the name "src-target" rather confusing. How about
> >>>>> "source"?
> >>>>>
> >>>>> 8. No date leaf
> >>>>>
> >>>>> The draft text near fig 2 talks about a date leaf. There is no date
> >>>>> object in
> >>>>> this revision of te YANG.
> >>>>>
> >>>>> "Date:  Date when this object was created or last modified"
> >>>>>
> >>>>> 9. leaf owner
> >>>>>
> >>>>> Near fig.3 leaf Owner is mentioned. Is this leaf still current?
> >>>>>
> >>>>> "Owner: This field contains the onwer of the rule.  For example,
> >>>>>               the person who created it, and eligible for modifying
> >>>>> it."
> >>>>>
> >>>>> 10. leaf packet-per-second
> >>>>>
> >>>>> This is now modeled as uint16. Is this future proof? Many packet
> flows
> >>>>> on the
> >>>>> internet exceed 64k pps.
> >>>>>
> >>>>> 11. container custon-source
> >>>>>
> >>>>> Misspelled. Should be custom-source
> >>>>>
> >>>>> 12. identity ddos
> >>>>>
> >>>>> Is ddos a malware file-type? This is not exactly in line with my
> >>>>> intuition.
> >>>>>
> >>>>> 13. identity protocol-type
> >>>>>
> >>>>> There are other modules that already define protocol-types. Would
> >>>>> it be
> >>>>> worth
> >>>>> reusing one of them?
> >>>>>
> >>>>> 14. identity palo-alto
> >>>>>
> >>>>> Is it a good IETF practice to list vendor names in modules? Can we
> >>>>> consider
> >>>>> this a protocol name? Is there perhaps an RFC/specification name
> >>>>> for it
> >>>>> that we
> >>>>> could reference instead?
> >>>>>
> >>>>> 15. grouping ipsec-based-method
> >>>>>
> >>>>> This grouping contains a list which allows listing none of, either
> >>>>> of or
> >>>>> both
> >>>>> of ipsecike and ikeless. Are all valid configurations?
> >>>>>
> >>>>> 16. leaf feed-name
> >>>>>
> >>>>> This leaf is the key in a list, which makes it possible to have at
> >>>>> most
> >>>>> one
> >>>>> feed of each type. If it would make sense to configure more than one
> >>>>> feed of
> >>>>> the same type, the YANG needs to be updated here.
> >>>>>
> >>>>> 17. leaf-list content
> >>>>>
> >>>>> This leaflist is of type string. What is the format of this string?
> >>>>> Does
> >>>>> the
> >>>>> name refer to something?
> >>>>>
> >>>>> 18. Event types
> >>>>>
> >>>>> container event has a choice between enforce-admin and time
> >>>>> alternatives. Each
> >>>>> of those choices have a leaf that allows the operator to configure an
> >>>>> identityref to an enforce-type value. What does that mean? What
> >>>>> would it
> >>>>> mean
> >>>>> if an operator configured admin == 'time' (or enforce-time ==
> >>>>> 'admin')?
> >>>>>
> >>>>> 19. leaf begin-time, end-time
> >>>>>
> >>>>> >From the examples, it can be seen that these are meant to be a
> >>>>> time of
> >>>>> day
> >>>>> values. Currently they are modeled as yang:date-and-time, which means
> >>>>> they are
> >>>>> a concrete time a specific day, e.g. 2019-11-11T16:07. This needs
> >>>>> to be
> >>>>> changed
> >>>>> in order to be what the modeler intended. Perhaps like this:
> >>>>>
> >>>>> typedef time-of-day {
> >>>>>      type string {
> >>>>>          pattern '(2[0-3]|[01]?[0-9]):[0-5][0-9]';
> >>>>>      }
> >>>>> }
> >>>>>
> >>>>> 20. leaf frequency
> >>>>>
> >>>>> This leaf is now modeled properly from a YANG perspective. But what
> >>>>> does
> >>>>> it
> >>>>> mean? If this leaf is set to 'once-only', what exactly will happen
> >>>>> only
> >>>>> once?
> >>>>> Please write a description that explains this.
> >>>>>
> >>>>> 21. Example in Fig.17
> >>>>>
> >>>>> The example contains XML that refers to "endpoint-group/user-group".
> >>>>> There is
> >>>>> no such element in the YANG.
> >>>>>
> >>>>> <endpoint-group
> >>>>> xmlns="urn:ietf:params:xml:ns:yang:ietf-i2nsf-cfi-policy">
> >>>>>    <user-group>
> >>>>>
> >>>>> Furthermore, there is nothing called range-ip-address,
> >>>>> start-ip-address,
> >>>>> end-ip-address. They are called range-ipv4-address,
> >>>>> start-ipv4-address,
> >>>>> end-ipv4-address.
> >>>>>
> >>>>>      <range-ip-address>
> >>>>>        <start-ip-address>221.159.112.1</start-ip-address>
> >>>>>        <end-ip-address>221.159.112.90</end-ip-address>
> >>>>>      </range-ip-address>
> >>>>>
> >>>>> Finally, there must not be any xmlns attribute on an closing XML
> >>>>> tag. So
> >>>>>
> >>>>> </endpoint-group
> >>>>> xmlns="urn:ietf:params:xml:ns:yang:ietf-i2nsf-cfi-policy">
> >>>>>
> >>>>> should be
> >>>>>
> >>>>> </endpoint-group>
> >>>>>
> >>>>> This happens in several of the examples.
> >>>>>
> >>>>> 22. Example in Fig.18
> >>>>>
> >>>>> There is no element called policy any more. It's now
> i2nsf-cfi-policy.
> >>>>>
> >>>>>     <policy
> xmlns="urn:ietf:params:xml:ns:yang:ietf-i2nsf-cfi-policy">
> >>>>>       <policy-name>security_policy_for_blocking_sns</policy-name>
> >>>>>
> >>>>> The rules are modeled in a container and list, both by the name
> >>>>> rule. So
> >>>>> there
> >>>>> needs to be two <rule> tags.
> >>>>>
> >>>>>       <rule>
> >>>>>
>  <rule-name>block_access_to_sns_during_office_hours</rule-name>
> >>>>>
> >>>>> The security-event element is marked mandatory in the YANG, but
> >>>>> missing
> >>>>> in the
> >>>>> example. The times given below may be what is intended, but do not
> >>>>> match
> >>>>> the
> >>>>> date format for the type used (which include a date, etc).
> >>>>>
> >>>>>         <event>
> >>>>>           <time-information>
> >>>>>             <begin-time>09:00</begin-time>
> >>>>>             <end-time>18:00</end-time>
> >>>>>           </time-information>
> >>>>>         </event>
> >>>>>
> >>>>> Since the example is not mentioning leaf frequency, it will have the
> >>>>> value
> >>>>> 'once-only'. Maybe explain what that means in the context of the
> >>>>> example?
> >>>>>
> >>>>> The condition/firewall-condition says the src-target is mandatory and
> >>>>> dest-target optional, exactly like below.
> >>>>> condition/custom-destination/dest-target is mandatory and
> >>>>> src-target is
> >>>>> optional, exactly like below. Is this pure luck, or is there a
> logical
> >>>>> explanation why exactly those should be mandatory, and the example
> use
> >>>>> precisely those?
> >>>>>
> >>>>>         <condition>
> >>>>>           <firewall-condition>
> >>>>>             <source-target>
> >>>>>               <src-target>employees</src-target>
> >>>>>             </source-target>
> >>>>>           </firewall-condition>
> >>>>>           <custom-condition>
> >>>>>             <destination-target>
> >>>>>               <dest-target>sns-websites</dest-target>
> >>>>>             </destination-target>
> >>>>>           </custom-condition>
> >>>>>
> >>>>> The current YANG model does not allow setting both a
> >>>>> firewall-condition
> >>>>> and
> >>>>> custom-condition. If that should be allowed, the model needs to
> >>>>> change.
> >>>>> Should
> >>>>> it be possible to have multiple firewall- or other conditions? That
> is
> >>>>> not
> >>>>> currently possible.
> >>>>>
> >>>>> This example leaves out the mandatory leaf owner. Is that a sign
> >>>>> that it
> >>>>> should
> >>>>> not be mandatory, or perhaps not exist at all?
> >>>>>
> >>>>> 23. Example in Fig.19
> >>>>>
> >>>>> This example lists a firewall-condition with no src-target, which is
> >>>>> mandatory.
> >>>>>
> >>>>>        <firewall-condition>
> >>>>>          <destination-target>
> >>>>>            <dest-target>employees</dest-target>
> >>>>>          </destination-target>
> >>>>>        </firewall-condition>
> >>>>>
> >>>>> Under condition, there is a container rate-limit with a leaf
> >>>>> packet-per-second.
> >>>>> Is this a trigger value for the condition, or is it an actual limit
> >>>>> that
> >>>>> the
> >>>>> system is expected to enforce? If it's a trigger, it may be good to
> >>>>> find
> >>>>> a
> >>>>> clearer name. If it's enforced, it's placement under condition is
> >>>>> deceiving.
> >>>>>
> >>>>> If a rule's action is set to 'rate-limit', to which rate will it be
> >>>>> limited?
> >>>>>
> >>>>> 24. Security Considerations
> >>>>>
> >>>>> Section 10 in the NSF document under review is the Security
> >>>>> Considerations. I
> >>>>> think it would make sense to mention something about the management
> >>>>> access
> >>>>> control mechanism here, and its relation to NACM.
> >>>>>
> >>>>> (End of list)
> >>>>>
> >>>>>
> >>>>> _______________________________________________
> >>>>> I2nsf mailing list
> >>>>> I2nsf@ietf.org
> >>>>> https://www.ietf.org/mailman/listinfo/i2nsf
> >>>>>
> >>>>
> >>>>
> >>>> --
> >>>> ===========================
> >>>> Mr. Jaehoon (Paul) Jeong, Ph.D.
> >>>> Associate Professor
> >>>> Department of Software
> >>>> Sungkyunkwan University
> >>>> Office: +82-31-299-4957
> >>>> Email: jaehoon.paul@gmail.com, pauljeong@skku.edu
> >>>> Personal Homepage: http://iotlab.skku.edu/people-jaehoon-jeong.php
> >>>> <http://cpslab.skku.edu/people-jaehoon-jeong.php>
> >>>>
> >>>
> >>>
> >>> --
> >>> ===========================
> >>> Mr. Jaehoon (Paul) Jeong, Ph.D.
> >>> Associate Professor
> >>> Department of Software
> >>> Sungkyunkwan University
> >>> Office: +82-31-299-4957
> >>> Email: jaehoon.paul@gmail.com, pauljeong@skku.edu
> >>> Personal Homepage: http://iotlab.skku.edu/people-jaehoon-jeong.php
> >>> <http://cpslab.skku.edu/people-jaehoon-jeong.php>
> >>>
> >>>
> >>>
> >>
>


-- 
===========================
Mr. Jaehoon (Paul) Jeong, Ph.D.
Associate Professor
Department of Computer Science and Engineering
Sungkyunkwan University
Office: +82-31-299-4957
Email: jaehoon.paul@gmail.com, pauljeong@skku.edu
Personal Homepage: http://iotlab.skku.edu/people-jaehoon-jeong.php
<http://cpslab.skku.edu/people-jaehoon-jeong.php>