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>