Re: [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:40 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 E443F3A052C; Fri, 28 Aug 2020 06:40:39 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.086
X-Spam-Level:
X-Spam-Status: No, score=-2.086 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, T_FREEMAIL_DOC_PDF=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 WpQf1JCOxX59; Fri, 28 Aug 2020 06:40:33 -0700 (PDT)
Received: from mail-lf1-x135.google.com (mail-lf1-x135.google.com [IPv6:2a00:1450:4864:20::135]) (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 F1F743A0528; Fri, 28 Aug 2020 06:40:31 -0700 (PDT)
Received: by mail-lf1-x135.google.com with SMTP id c8so731136lfh.9; Fri, 28 Aug 2020 06:40:31 -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=//g+zJloFgIdeodYq4A6ebNMgcpuCkCRrd2mSw+4D3w=; b=kJXYbwpd7b5PNwU6o+3m3kR2bCH842comTFxSb5JO3uYlI1a31ZZ5p4DJZijUyrY9D t4mmDvFASZUqkCNWSBGSBijVwTSLWPuv8IUJWEMbx8wiNJtde/0Ql1iKggEQIMFFOKMo GhZUX1tS1DBBUBsqR4CGzF29vg+pk3xULqmrUQK2QuyOGImX9i0Ud2FQj3XRaHSY9+4z dXfUF3T4CnKo5brs7+jdYrHyHulpPhLo8dDNcI+Dbdddik7yYziESAFIQ/roDzuclE6Z cipFclAPrApfBMNFJOwYeI2kgEMDT+m23kcGlFeGqsdYh4mvZohXORb16JmGotp3oB0f gDTQ==
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=//g+zJloFgIdeodYq4A6ebNMgcpuCkCRrd2mSw+4D3w=; b=JzbouGd4Fx+gIEttXtPeX0esy4doRKSRJbzGpqhNODM90V+i/5CWSJ+SdKic9ZZ5KH n5pf9EU2u8p3sQ13uUOyKOAoIYyu3dAwskrmiA9ulrHmLU6ppNFIppK/H3iTr3reGcAT D7lqKt8egSuWhWD9+p8Pwql7rHR++pA3z9GAEuteieXmIh/oMFEp6spg5JKnnGfx+uiL r3VKB9jjLfCXoeozYAumzaupPNIzVjmChExTsR4jGNZNXPhfwLxhCyDs+LQ6u7cNqooA 9CH340u8ye3jZXSRLG88+R94nZe5WZ67tXWNxP1SXy2IqRjWbzcrQWlcqZKUNUBCsWCM H1ng==
X-Gm-Message-State: AOAM5319SFTrIkBFH3sa6nFhLEip1q/3Zwe3C/6QpxBgA0V0x1stCD0V MqlrFngf3KuSyHDw3TOLWVvChrEVC2vzCaJHG7gfbQY1ychLAQ==
X-Google-Smtp-Source: ABdhPJysXzwysjFUWkHSKRzzvbNSKh7VLf61zXJi9DGfbLbjCTTDUaJEY2LJT5elmMXnoDemnriok57CuH9daw+DJYs=
X-Received: by 2002:a19:40c8:: with SMTP id n191mr870143lfa.29.1598622029119; Fri, 28 Aug 2020 06:40:29 -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> <CAPK2DewR2CXNBFaiN_ZoAT0RzLmswC2JD9xx51Xh7W_zRU8D7Q@mail.gmail.com> <9DEEFD92-5DC6-4E30-A706-20EBAFDAFC2D@tail-f.com>
In-Reply-To: <9DEEFD92-5DC6-4E30-A706-20EBAFDAFC2D@tail-f.com>
From: "Mr. Jaehoon Paul Jeong" <jaehoon.paul@gmail.com>
Date: Fri, 28 Aug 2020 22:39:48 +0900
Message-ID: <CAPK2Dex3Q-TYm2_GJ5nYoSH5HYUiHNDWbq8HfAcw+7z4ovVnKA@mail.gmail.com>
To: Jan Lindblad <janl@tail-f.com>
Cc: YANG Doctors <yang-doctors@ietf.org>, tom petch <daedulus@btconnect.com>, "i2nsf@ietf.org" <i2nsf@ietf.org>, skku-iotlab-members <skku-iotlab-members@googlegroups.com>
Content-Type: multipart/mixed; boundary="0000000000002d583405adf0322f"
Archived-At: <https://mailarchive.ietf.org/arch/msg/i2nsf/Z8lNPXTfWJmHO3bEXQ-5Wdzq6QM>
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, 28 Aug 2020 13:40:40 -0000
Hi Jan, I have addressed all your comments and submitted the revised draft: https://tools.ietf.org/html/draft-ietf-i2nsf-consumer-facing-interface-dm-10 I attach the revision letter to explain how I have addressed your comments. Thanks. Best Regards, Paul On Thu, Jul 23, 2020 at 2:00 AM Jan Lindblad <janl@tail-f.com> wrote: > Paul, > > Good work with the module, and sorry for the slow response. I have been > OOO. I read through this again today, and I have some comments, if you are > interested. I guess this is not part of any formal review any more. > > > o Figure 1: Not sure I understand what the arrow from "Consumer-Facing > Interface Information Model" ---> "Consumer-Facing Interface Data Model" > means, but it probably does no harm. > > o Figure 5: The tree diagram leafref paths are strange (e.g. > -> /../../user-group/name) > > o Figure 7, figure 12: The UML diagram cardinality is given as "1..n" in > several places. In the actual YANG, the cardinality is "0..n" > > o Section 5: The endpoint groups are mapped to a single IP or IP range. > Is that sufficient for your use cases? Also, much of this information re IP > addresses for users, devices and geo locations in the world are probably > available in other systems with most network operators. Is it advisable to > duplicate that information here? Sounds difficult to keep all this > information in sync. > > o Section 6: Threat signatures and content patterns can be configured > here. Is the expectation that the I2NSF client (operator?) configures these > patterns, and the I2NSF server communicates these patterns to the threat > feed servers, as a sort of controller? How this part of the model would be > used is not clear to me. > > o Section 9: The examples should use prefixed identity names. For example > <protocol>http</protocol> should be > <protocol>i2nsf-cfi:http</protocol> > and > <day>monday</day> should be <day>i2nsf-cfi:monday</day> > > o Section 9.2 and 9.3: Even though the examples text talks about the value > of "destination", no such tag is actually present in the XML. > > > Then in the YANG module itself: > > o In ip-ranges like container range-ipv4-address, what happens if either > of start- or end- address is omitted? Maybe explain in the description, or > make both leafs mandatory? > > o In container period there are several when-expressions that mean > something else than you think: > container period{ > when > > "/i2nsf-cfi-policy/rules/rule/event/frequency!='only-once'"; > By using an abosolute path like this, the XPath expression looks for > matching instances across all rules. This means this expression is true as > soon as there is at least one rule with a frequency != 'only-once'. There > are several other when expressions here with the same problem. What I think > you mean is this: > container period{ > when "../../frequency!='only-once'"; > This expression looks only at the frequency leaf in the same rule instance > as the period container is in. The other when expressions can be fixed in a > similar way. > > o leaf frequency: What happens if this leaf is set to weekly and no day > is specified? Or monthly, etc? One way of modeling this to avoid the > problem is to make the leaf-list day, leaf-list date, leaf-list month etc a > choice, so that the frequency is implicit by configuring a day, date or > month. If none of them are set, that would mean only-once. Just a thought. > > > Best Regards, > /jan > > > On 13 Jul 2020, at 14:20, Mr. Jaehoon Paul Jeong <jaehoon.paul@gmail.com> > wrote: > > Hi Jan, > I have uploaded the revised draft into the IETF repository: > > https://tools.ietf.org/html/draft-ietf-i2nsf-consumer-facing-interface-dm-09 > > > I attach the revision letter, too. > > Could you review the draft and complete your YANG review if you are > satisfied with the revision? > > https://datatracker.ietf.org/doc/draft-ietf-i2nsf-consumer-facing-interface-dm/ > > > Thanks. > > Best Regards, > Paul > > > On Sat, Jul 11, 2020 at 4:44 PM Mr. Jaehoon Paul Jeong < > jaehoon.paul@gmail.com> 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. >> >> 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> >> > > > -- > =========================== > 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> > > <Revision-Letter-for-I2NSF-Consumer-Facing-Interface-YANG-Data-Model-20200711-v1.pdf> > > > -- =========================== 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