Re: [yang-doctors] [I2nsf] Yangdoctors last call review of draft-ietf-i2nsf-consumer-facing-interface-dm-07
"Mr. Jaehoon Paul Jeong" <jaehoon.paul@gmail.com> Fri, 28 August 2020 13:43 UTC
Return-Path: <jaehoon.paul@gmail.com>
X-Original-To: yang-doctors@ietfa.amsl.com
Delivered-To: yang-doctors@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id D9D8B3A05AC; Fri, 28 Aug 2020 06:43:27 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.096
X-Spam-Level:
X-Spam-Status: No, score=-2.096 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, SPF_PASS=-0.001, 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 6-Cyge4ghZ1D; Fri, 28 Aug 2020 06:43:23 -0700 (PDT)
Received: from mail-lf1-x12a.google.com (mail-lf1-x12a.google.com [IPv6:2a00:1450:4864:20::12a]) (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 64DD93A058F; Fri, 28 Aug 2020 06:43:22 -0700 (PDT)
Received: by mail-lf1-x12a.google.com with SMTP id v12so729001lfo.13; Fri, 28 Aug 2020 06:43:22 -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=bjTYOki75nHAqZMhffhkiCJU8lkoYf/p2dvGd+ljP5M=; b=ZV0i1hw0fubGd+bTnZGk8iZ8lGQnb/xmn7R7Tl3yn2BmddaCt/oUZntCaQVFINJsgy PswkKmGokt9d9EltKc6rceRdyxLJdJdix53waTSbO9PcVBwSk/aB8S0SK6zSdrKMKJfF 5mstMHcWsQw9T/AJ6sLSkB66ZbQXhSjAwk4KRloDl6ZYTQh8R+TiAh5Jhc9uHqwBxMpD 9+tDdu8iVcYtre4OLbMnJv54oVdKRaNOXbnPoVVjRLG6aUFBJUHjGu6JmAlPyDz6jzfT u1OI+/EkLDWu87thrmluGmMrFa/IvCyXOhLsPKvbXX/eQ1eejHnX+AOQ0cdzErvg/GXW ehoQ==
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=bjTYOki75nHAqZMhffhkiCJU8lkoYf/p2dvGd+ljP5M=; b=t+WVb+zOZwXIzU+/BctedCtm3yQqVCckSu5x2A30yfXaHMeXgZaqdNizsGX9W6o/Jf CHfr2URrMpxh8nmKEHsWzocUEHad0pFeBiWNEZfcy6sOFe2G3NNouIWClZ0eopiIPVR1 g70lPe+ns3vlO/GKCmoEa69D2WZBBDqfcvkigBUbSx/4VApozrC961oJCl0Hyr1ST5v1 bMWsRCERMUdmqR1IB/7Pv94og2Mozch7VFqzKG+Lyp0W0Zk/1eqA0JN9/17UX4c2gcCQ c/9cdcuLffy/IvWYD8pvC8pd3LCLw0aEQw2Ln9HZVPHxEC08qiU8zSzalCf/l0kRVu+0 oRMQ==
X-Gm-Message-State: AOAM532jzrZdZ4bTGwSfgZGkbSn79xh6jcPh8ORMwwhAQWSLUVEgpFB6 R/KyP/KE5y1VXhs+o67j/RgawLiCwdGXXNaWp7g=
X-Google-Smtp-Source: ABdhPJw3GY3Lo9IffOrboz/6XJneb1Dx9pl7Z51/S/9RDUKWyO2n2uvisCjDPPq4VihbaD4ciG20EAbmgNrREgvA0Vc=
X-Received: by 2002:a19:be0e:: with SMTP id o14mr855088lff.203.1598622200150; Fri, 28 Aug 2020 06:43:20 -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>
In-Reply-To: <5F117489.6060709@btconnect.com>
From: "Mr. Jaehoon Paul Jeong" <jaehoon.paul@gmail.com>
Date: Fri, 28 Aug 2020 22:42:40 +0900
Message-ID: <CAPK2Dezsp-J462GAEYuKO7gKe9wkxiPmyAXX0pjhXFCnJTwqig@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>
Content-Type: multipart/alternative; boundary="0000000000005f151f05adf03c36"
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/tvAJV8qXpjNhU2LpQkEOQcbXuWw>
Subject: Re: [yang-doctors] [I2nsf] Yangdoctors last call review of draft-ietf-i2nsf-consumer-facing-interface-dm-07
X-BeenThere: yang-doctors@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Email list of the yang-doctors directorate <yang-doctors.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/yang-doctors/>
List-Post: <mailto:yang-doctors@ietf.org>
List-Help: <mailto:yang-doctors-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 28 Aug 2020 13:43:28 -0000
Hi Tom, I have included RFCs used in the YANG module as follows: ---------------------------------------------------------------------------- 8.1. YANG Module of Consumer-Facing Interface This section describes a YANG module of Consumer-Facing Interface. This YANG module imports from [RFC6991] and uses the typedef of time in [I-D.ietf-netmod-rfc6991-bis]. It makes references to [RFC0854] [RFC0913][RFC0959][RFC1081][RFC1631][RFC2616][RFC2818][RFC4250] [RFC5321]. ---------------------------------------------------------------------------- Thanks. Best Regards, Paul On Fri, Jul 17, 2020 at 6:51 PM tom petch <daedulus@btconnect.com> 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. > > 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>
- [yang-doctors] Yangdoctors last call review of dr… Jan Lindblad via Datatracker
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Mr. Jaehoon Paul Jeong
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Mr. Jaehoon Paul Jeong
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Mr. Jaehoon Paul Jeong
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Jan Lindblad
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Mr. Jaehoon Paul Jeong
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Mr. Jaehoon Paul Jeong
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Mr. Jaehoon Paul Jeong
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … tom petch
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … tom petch
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Mr. Jaehoon Paul Jeong
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Jan Lindblad
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Mr. Jaehoon Paul Jeong
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Mr. Jaehoon Paul Jeong
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Mr. Jaehoon Paul Jeong
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Mr. Jaehoon Paul Jeong
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … tom petch
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … tom petch
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Mr. Jaehoon Paul Jeong
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Mr. Jaehoon Paul Jeong
- Re: [yang-doctors] [I2nsf] Yangdoctors last call … Mr. Jaehoon Paul Jeong