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>
- [I2nsf] Yangdoctors last call review of draft-iet… Jan Lindblad via Datatracker
- Re: [I2nsf] Yangdoctors last call review of draft… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] Yangdoctors last call review of draft… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] Yangdoctors last call review of draft… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] Yangdoctors last call review of draft… Jan Lindblad
- Re: [I2nsf] Yangdoctors last call review of draft… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] [Last-Call] Yangdoctors last call rev… tom petch
- Re: [I2nsf] [Last-Call] Yangdoctors last call rev… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] [Last-Call] Yangdoctors last call rev… tom petch
- Re: [I2nsf] [Last-Call] Yangdoctors last call rev… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] Yangdoctors last call review of draft… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] Yangdoctors last call review of draft… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] Yangdoctors last call review of draft… tom petch
- Re: [I2nsf] Yangdoctors last call review of draft… tom petch
- Re: [I2nsf] Yangdoctors last call review of draft… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] Yangdoctors last call review of draft… Jan Lindblad
- Re: [I2nsf] Yangdoctors last call review of draft… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] Yangdoctors last call review of draft… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] Yangdoctors last call review of draft… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] Yangdoctors last call review of draft… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] Yangdoctors last call review of draft… tom petch
- Re: [I2nsf] Yangdoctors last call review of draft… tom petch
- Re: [I2nsf] Yangdoctors last call review of draft… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] Yangdoctors last call review of draft… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] Yangdoctors last call review of draft… tom petch
- Re: [I2nsf] Yangdoctors last call review of draft… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] Yangdoctors last call review of draft… Mr. Jaehoon Paul Jeong
- Re: [I2nsf] Yangdoctors last call review of draft… tom petch