Re: [I2nsf] Yangdoctors last call review of draft-ietf-i2nsf-consumer-facing-interface-dm-07
"Mr. Jaehoon Paul Jeong" <jaehoon.paul@gmail.com> Wed, 11 March 2020 16:15 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 104413A0C97; Wed, 11 Mar 2020 09:15:12 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.077
X-Spam-Level:
X-Spam-Status: No, score=-2.077 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, HTML_MESSAGE=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_FREEMAIL_DOC_PDF=0.01, T_HK_NAME_FM_MR_MRS=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 c6hVqV4_DTyQ; Wed, 11 Mar 2020 09:15:02 -0700 (PDT)
Received: from mail-lf1-x130.google.com (mail-lf1-x130.google.com [IPv6:2a00:1450:4864:20::130]) (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 4AD1D3A0C64; Wed, 11 Mar 2020 09:14:57 -0700 (PDT)
Received: by mail-lf1-x130.google.com with SMTP id q9so2210769lfc.10; Wed, 11 Mar 2020 09:14:57 -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=Sm5QwU+zNdgHP1BLcDp9yuXkjBrAUR7jzA/7pEaex38=; b=XxkV39VKxH7GThDTqI8SaLimTK+RS9qHy2sp51hskJb0MsbsR0e0gSokD/59JhoY7n 2vZEvm+lVUCYC1hKMIplZk9Lrk9AsIlsvd27Cybqj26NMyTmGm5c+zPExtCFipYv+yIn +mc7y7ABknao6OHAl8lKmLTiCXoiFdbpF8Gk/WyZFHGWVS2s4rDmbVi3QXWsHKvKe5fU ptxwPpxtGLOave3eb9Uq03I1mhM7qxwJPMtFO5X2FR1b1lnCQoAf9YrHdrWqPLKZ9m3z nOY9vNiPheje3G0qFa6J0G5AuuFw/1yzpQO3l7LbAtM8y3OqLZ7XftldT6106ctoqnSY MzNg==
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=Sm5QwU+zNdgHP1BLcDp9yuXkjBrAUR7jzA/7pEaex38=; b=K4HO4Q6ppd/kCIBzU9YO8jdfgictXa9vG8kl94xcxu7KP99t5rKZPD9exuJLnpglhT WatK+u69HZmEomIUIUbGZVpu7gdOVUxv1A30wf0HpSGQy+7NwkRNN3Q4g9a7nRrelUVW AJFcBpYL8AN0FK8lbQhXMQIW2e2+Ue8k3CTBee1HnttF9uNIpQJTyWXexH17GkuwO2o4 TOH/yCNfTOlMK9SSPZs9fRxNtrGY3zUB92tc5nSyXvceWtUIOanCZA4vWlkXwsRe41V3 yzqVrpcDIq4Qr1pcbeBIchVuSFYVWWYQ6txcSLDuDp14qbrsDGwAh5Z5oIu8LfURwSuJ RsMg==
X-Gm-Message-State: ANhLgQ2KmUQQvEo7W61F2H5L8eUbSNM5vXifhMhYc9FAsy7n8Zx+SaW8 Cq203gtUYfXVgtQ1YXRYGh2cAVKgGci9/ysACJo=
X-Google-Smtp-Source: ADFU+vuxSeOhv6qs9K8RtMfBxFtaQfZABuPG8JD6gMB2TBh9/c67kxkGgmNdOjVXroGEmJaPPKUevZd0XLZ53cB36Xk=
X-Received: by 2002:ac2:5303:: with SMTP id c3mr2678593lfh.132.1583943293631; Wed, 11 Mar 2020 09:14:53 -0700 (PDT)
MIME-Version: 1.0
References: <157349122063.7571.1978842562243958252@ietfa.amsl.com>
In-Reply-To: <157349122063.7571.1978842562243958252@ietfa.amsl.com>
From: "Mr. Jaehoon Paul Jeong" <jaehoon.paul@gmail.com>
Date: Thu, 12 Mar 2020 01:15:28 +0900
Message-ID: <CAPK2Dexgk81Saufei3z67E4XZg=LLra1HdTUWU-kU33Pj_o+eg@mail.gmail.com>
To: Jan Lindblad <janl@tail-f.com>
Cc: YANG Doctors <yang-doctors@ietf.org>, "i2nsf@ietf.org" <i2nsf@ietf.org>, last-call@ietf.org, draft-ietf-i2nsf-consumer-facing-interface-dm.all@ietf.org, skku-iotlab-members <skku-iotlab-members@googlegroups.com>, "Mr. Jaehoon Paul Jeong" <jaehoon.paul@gmail.com>
Content-Type: multipart/mixed; boundary="0000000000005d4d4d05a09689d4"
Archived-At: <https://mailarchive.ietf.org/arch/msg/i2nsf/RywWXZbZHVf-IHQCFSFBEfbcroU>
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: Wed, 11 Mar 2020 16:15:20 -0000
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>
- [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