Subject: Re: [yang-doctors] [I2nsf] Yangdoctors last call review of draft-ietf-i2nsf-consumer-facing-interface-dm-05

From: "Mr. Jaehoon Paul Jeong" <[email-masked]>
Date: Thu, 25 Jul 2019 10:25:47 -0400

Hi Jan,
Here is the revision letter for the revised draft, reflecting your comments
along with the revised draft:
https://tools.ietf.org/html/draft-ietf-i2nsf-consumer-facing-interface-dm-06

If you have further comments and questions, please let me know.

Thanks.

Best Regards,
Paul

On Wed, Jun 26, 2019 at 12:00 PM Jan Lindblad via Datatracker <
[email-masked]> wrote:

> Reviewer: Jan Lindblad
> Review result: On the Right Track
>
> This is my YANG Doctor review of
> draft-ietf-i2nsf-consumer-facing-interface-dm-05.txt, and the YANG module
> ietf-i2nsf-cfi-policy in particular. The document is on the right track,
> but
> several important discussions remain, so is not quite ready for last call.
>
> There are plenty of smaller things to fix in this module, see below for a
> list.
> There are a couple of potentially major issues in this module, however,
> that I
> would like to start with. The first relates to the management access
> control
> for this module.
>
> 1) Management access control
>
> This RFC defines a new management access control mechanism, entirely
> unrelated
> to the traditional NACM model (RFC 8341/RFC 6536). It also specifies things
> like which auth mechanisms should be used by clients in specific domains
> when
> connecting to the management server. This appears highly disruptive to the
> existing base of NETCONF/RESTCONF servers.
>
> To make it worse, the new security model described in this document is
> rather
> heavily unspecified. It may be that this security model is well thought
> through, but there is no language or other evidence that shows this in the
> text. The YANG model representation of it is broken in many places.
>
> The RFC text refers to a device called "Security Controller". By reading
> the
> parent RFC 8329, which contains an overview of the architecture, I believe
> the
> Security Controller is the management (NETCONF/RESTCONF/...) server with
> the
> ietf-i2nsf-cfi-policy YANG model, and the user is supposed to connect
> directly
> with it. This means, I would think, that the management server would need
> to be
> the component to implement the new management access control model.
>
> Key in this new mechanism is a leaf owner. There isn't much information
> about
> the owner leaf, but the RFC text relating to "owner" above Fig. 3 says:
>
>    Owner: This field contains the owner of the rule.  For example,
>              the person who created it, and eligible for modifying it.
>
> And further, in the YANG module itself, we find:
>
>   leaf owner {
>         type string;
>           description
>            "This field defines the owner of this
>            policy. Only the owner is authorized to
>            modify the contents of the policy.";
>
> This is all that is mentioned about the owner leaf. That the type of the
> leaf
> is string does not give much clues as to how this is meant to work.
>
> Then there are Policy Domains. A Policy Domain has an (optional) auth
> method
> (which is not correctly modeled in YANG) and a list of Policy Tenants. Each
> Policy Tenant represents one department or other part of the organization.
>
> Policy Users are individual users that are allowed to access the management
> server to CRUD policies and rules. Either within a tenant (default) or
> domain.
> There is no linkage to which tenant or domain, however. Each Policy User
> has
> exactly one reference to a an access profile in the Policy Role object. The
> only information this linkage provides is whether the access is read-only
> or
> read-write.
>
> In summary, I believe significant work is required to get this to a
> workable
> state. In order to invent a new management access control mechanism, a
> wider
> discussion in the NETMOD working group would be needed.
>
> Below I note some details that I came up with while trying to figure this
> out.
> Fixing them will not fix this issue as a whole.
>
> - container policy-mgnt-auth-method should probably be a list
> - container policy-role should probably be a list, and the list
> access-profile
> removed - list policy-tenant should probably not have any leaf domain - the
> leaf owner description talks about owner of the policy, while the leaf
> sits on
> an individual rule. Either the description or the leaf placement must be
> wrong.
> - I believe there are more issues to look at as part of the "translation
> from
> UML to YANG" that this module has been subject to.
>
> 2) container policy
>
> The top level container policy can only exist in a single instance as
> currently
> modeled. Even if not stated explicitly in the document, I get the feeling
> that
> the author's intent is that it should be possible to have more than one
> policy
> in the system. If this is the case, container policy needs to change to
> list
> policy (and probably add a surrounding container policies). Doing so would
> also
> require updating all the leafrefs used throughout the module, to only
> select
> nodes in the current policy.
>
> Another concern is that the name "policy" is very generic. There are many
> modules out there which already define a top level object called policy.
> Searching for container policy and list policy on yangcatalog.org gives
> several
> hundred pages with hits. Most of them will not be on the top level, but
> this
> shows that "policy" is a popular concept. How about calling the top level
> container cfi-policy or i2nsf-cfi-policy?
>
> 3) Are rules ordered?
>
> In the RFC section 4, it says regarding list rule:
>
>              This field contains a list of rules.  If the rule does not
>              have a user-defined precedence, then any conflict must be
>              manually resolved.
>
> I'm not sure what a "user-defined precedence" is, or at least I can't find
> anything like that in the YANG. So how are the rules meant to be applied
> on the
> devices? What does "manually resolved" mean, who does that when and how? If
> different operators cannot see each other's rules, does that mean that
> "manually resolved" just got harder?
>
> Each policy contains a set of rules in a list. This list is keyed by rule
> name
> and modeled as ordered-by system, i.e. rules will be sorted
> alphabetically. Is
> it up to the server to decide in which sequence these rules are applied. I
> suppose they could be overlapping, leading to different results based on
> the
> order rules are applied, and there could be performance implications
> depending
> on how this is done. If several policies are allowed, the issue only grows.
>
> 4) Restricted to IPv4
>
> The module specifically uses IPv4 in many places. Would it not make sense
> to
> use ip-address instead, to allow the use of IPv6 also, or is there some
> particular reason to exclude IPv6? If so, it would be good to mention this
> reason.
>
> In a couple of places, an address range is specified, and the start
> address is
> an ipv4-address while the end address is an ip-address. Surely that must
> be a
> mistake.
>
> 5) Many optional leafs
>
> The RFC text repeats many times: The XYZ object SHALL have the following
> information... This could be understood as the succeeding information
> items are
> mandatory. There are not a single mandatory element (apart from keys) in
> the
> entire module, however. And only three leafs with a default statement.
>
> It would be appropriate to go through the module and add mandatory and
> defaults
> where they make sense. For the leafs that should remain optional, it would
> be
> good if something could be added to the description regarding what the
> server
> behavior is when the leaf has no value, when that is not entirely obvious.
> Actually, it is often good to write even when it is obvious, because
> otherwise
> the behavior is technically undefined, and implementations could get away
> with
> bad behavior even when all know it's wrong.
>
> A couple of examples: what happens if the owner leaf is left unset?
> primary-action? enforce-type? recur? policy-tenant/domain?
>
> 6) Many strings
>
> In the module, the type string is used for names, which is great, but also
> for
> a many cases where some certain format of the content is expected, but not
> defined. There is no reason to believe that will lead to interoperable
> solutions. It will also leave users frustrated with little information to
> guess
> what type of data in which format to fill in. This needs fixing.
>
> 294: leaf-list protocol
> 322: leaf-list content
> 388: leaf begin-time
> 393: leaf end-time
> 553: leaf primary-action
> 561: leaf secondary-action
> 581: leaf owner
> 697: leaf auth-method
> 823: leaf threat-feed-description
> 839: leaf-list signatures
>
> 7) leaf date
>
> The grouping meta contains a name and a date leaf. The name is clearly
> meant to
> be a configuration item filled in by the client. I'm not sure about the
> date
> leaf, however. The description says
>
>     description "This is the date when the entity is
>     created or modified.";
>
> The date leaf is currently modeled as config true, i.e. is expected to be
> filled in by the client. It is not customary to have this sort of meta
> information in YANG modules, but it would make some sense if this was
> config
> false, i.e. computed by the server itself.
>
> If that is the intent, the specification should only make the date leaf
> config
> false and clarify how these time stamps should be calculated. Do they
> pertain
> to this particular list entry, or would they be updated if any child entry
> to
> this list entry was modified (similar to etags).
>
> If the intent is that clients should fill this in if they want, for their
> own
> optional housekeeping, I think that needs to be stated clearly. If the
> intent
> is to make it mandatory for clients to keep these timestamps up to date, I
> think the model is broken. What implications would it have if they were not
> used?
>
> 8) container policy-mgnt-auth-method
>
> The container has a description that says
>
>         "This represents the list of authentication methods.";
>
> A container is obviously not a list, so exactly what the author has in
> mind is
> somewhat unclear. At the top of the container, there is a leaf
>
>       leaf auth-method {
>         type string;
>         description
>           "This represents the authentication method name.";
>
> The format and usage of this string is highly unclear, but more
> importantly, it
> seems to speak of a single authentication method, not a list.
>
> Following this leaf is a number of lists; list password-based, list
> token-based, list certificate-based, list ipsec-method, list
> single-sign-on.
> Are these lists under container policy-mgnt-auth-method mutually
> exclusive, or
> can several of them be configured at the same time?
>
> This would need some clearing up. As this is an area where we can
> anticipate
> new methods being introduced over time, it would be good if the model
> advised
> how to extend with further auth methods.
>
> 9) More on container policy-mgnt-auth-method
>
> RFC text sec 5.1 says
>
>              Authentication method to be used for this
>              domain.  It should be a reference to a "Policy-Management-
>              Authentication-Method" object
>
> But in fact there is only one instance of container
> policy-mgnt-auth-method.
> Maybe this is meant to be a list?
>
> In the RFC Fig. 8 YANG tree representation authentication-method points
> like
> this
>
>             +--rw authentication-method?   ->
>             /policy/multi-tenancy/policy-mgnt-auth-method/name
>
> In the YANG module itself, the path is different:
>
>           path
>
> "/policy/multi-tenancy/policy-mgnt-auth-method/ipsec-method/method";
>
> Also, the abbreviation of management as "mgnt" is not very common. It is
> usually "mgmt". In order to reduce misspellings of policy-mgnt-auth-method
> I
> suggest renaming it to policy-mgmt-auth-method. There is actually already
> one
> such misspelling in this YANG module itself.
>
> 10) One cert server per cert type?
>
> List certificate-based allows the configuration of certificate servers. Is
> there usually a different server based on certificate type? Max one server
> can
> be configured per certificate type, and max three total as there are three
> certificate types. If this is what you want, the YANG says it nicely.
>
> 11) Enumerations vs. identities
>
> 203: certificate-type is defined as an enumeration. This means the module
> will
> have to be revised if any new certificate types need to be supported one
> day.
> An identity may be more appropriate here, as I expect new certificate
> types (or
> formats) may come out.
>
> 366: enforce-type is defined as an enumeration. This means the module will
> have
> to be revised if any new enforcement types need to be supported one day. An
> identity or choice may be more appropriate here. Then an additional module
> could add new identities or augment the choice.
>
> 55: permission-type is defined as an identity. Unless you foresee the need
> for
> other values than read-only and read-write, you could use enumeration here
> instead. It could simplify the module a little, but identity is fine too.
>
> 168: continent is defined as an identity. Unless you foresee the need for
> new
> continents, you could use enumeration here instead. It could simplify the
> module a little, but identity is fine too.
>
> 145: identity ransomeware is misspelled. Should be identity ransomware
>
> 12) Reference RFC 6087
>
> The RFC text refers to RFC 6087, which is obsoleted by RFC 8407. Update the
> reference?
>
> 13) YANG trees
>
> The RFC text has many figure descriptions like this
>
>     Figure X show the XML instance
>
> What is shown is actually a YANG tree, so the text should be updated to say
>
>     Figure X show the YANG tree
>
> 14) Indentation
>
> The indentation of the YANG module is broken on many, many lines. Make
> sure to
> use a YANG indentation tool before publishing the result.
>
> 15) Revision statement
>
> The revision statement at the top of the file needs to be rewritten before
> publication.
>
> 16) container recursive
>
> The container recursive on line 399 seems to be about how rules recur, not
> recurse (which is something completely different). Maybe change to the
> whole
> container recursive to
>
>     leaf frequency {
>       type enumeration {
>         enum only-once;
>         enum daily;
>         enum weekly;
>         enum monthly;
>       }
>       default once-only;
>     }
>
> 17) String passwords
>
> Passwords are modeled as strings in a couple of places. This is not
> acceptable
> from a security point of view. Use one of the cryptographic types for
> passwords, such as ianach:crypt-hash RFC 7317.
>
> 666: leaf password
> 710: leaf password
>
> The leaf password on line 710 has the description
>
>             "This should be defined using the
>             regular expression.";
>
> What does that mean? Authentication, password, regular expression? I don't
> get
> it.
>
> 18) Grouping descriptions
>
> A number of groupings have descriptions like
>
>     This grouping is to remove repetition of
>         'name' and 'ip-address' fields.";
>
> This is not exactly true since these groupings are used only once (hence no
> repetition). Describe instead what the grouping is used for or what it
> contains.
>
> 231: grouping meta
> 280: grouping user-group
> 301: grouping location-group
> 316: grouping payload-string
>
> 19) container time-information
>
> Is container time-information only relevant for enforce-type ==
> time-enforced?
> If so, a when expression on the container may be useful. Or, even better,
> make
> the enforce-type a choice and have the time related content inside the
> choice
> case instead.
>
> 20) container rate-limit
>
> Is uint8 a good type to use for the rate limiting function? Would it never
> (now
> or in the future) make sense to limit to more than 255 packets per second?
>
> 486: container rate-limit
>
> 21) container condition
>
> This container seems to hold the configuration of many different triggering
> conditions. Would several of these apply at the same time in a given rule?
>
> There are many instances of container source-target and container
> destination-target nested underneath. They seem to serve no purpose. Maybe
> consider simplifying the model by removing them, unless they have some
> clever
> function for the future.
>
> 22) Transactionality
>
> Section 10.1 starts with the words
>
>    In order to create a rule of a security policy, it is essential to
>    first register data (those which are used to form such rule) to the
>    database.
>
> This could lead some implementors to believe it is acceptable to require
> that
> the endpoints are committed separately from the rest of the configuration.
> The
> actual requirement is that the referenced endpoints exist at the time the
> transaction is committed. I.e. they could very well be part of the same
> transaction.
>
> 23) Incomplete XML examples
>
> The XML snippets in examples in Fig 24-27 use XML namespaces incorrectly.
> It's
> easy to fix. Just make sure the first line with
> <ietf-i2nsf-cfi-policy:policy>
> looks like this instead:
>
> <policy xmlns="urn:ietf:params:xml:ns:yang:ietf-i2nsf-cfi-policy">
>
> The last example, in section 10.4 is talking about <encrypt>. There is no
> such
> in the YANG module (today).
>
>        <encrypt>
>          <ipsec-method>ipsec-ike</ipsec-method>
>        </encrypt>
>
> The XML loads fine if you just remove the encrypt tag, so maybe you mean:
>
>          <ipsec-method>ipsec-ike</ipsec-method>
>
> (End of list)
>
>
> _______________________________________________
> I2nsf mailing list
> [email-masked]
> 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: [email-masked], [email-masked]
Personal Homepage: http://iotlab.skku.edu/people-jaehoon-jeong.php
<http://cpslab.skku.edu/people-jaehoon-jeong.php>

---

From: Jan Lindblad <[email-masked]>
Date: Fri, 26 Jul 2019 16:51:40 +0200

Paul, I2NSF WG,

Thank you for going through all my comments. I can see you have worked hard to fix up the module. I still see ample opportunities to improve it, but let's come back to those details later. What I miss most of all is a discussion within the NETMOD WG around this new proposed management access control model. 

The proposed model, as far as I understand it, would have a high impact on all existing NETCONF/RESTCONF server implementations and break some architectural principles, so I think we should expect a lot of skepticism there if we propose the current module.

Having read RFC 8192, I can see the vision you are painting. I think it's definitely worthwhile looking at ways to realize that idea. I believe the vision there can be realized in a much less disruptive fashion if we reorganized ietf-i2nsf-cfi-policy.yang to rely on the existing RFC 8341 Network Configuration Access Control Model.

I volunteer to provide a sketch of how the current module could be restructured to fit in with NACM, if you are interested. If you ask me to do this, I will bring the result back to the I2NSF WG for further discussion and possibly further iterations.

In the end, any management access control functionality changes would have to be discussed in the NETMOD WG.

Also, if you already know of any particular party that is interested to implement this, it would be great to have implementor involvement in the design process from this point onward.

Let me know if you want me to propose a way to redesign ietf-i2nsf-cfi-policy.yang to fit better with NACM.

Best Regards,
/jan

> On 25 Jul 2019, at 16:25, Mr. Jaehoon Paul Jeong <[email-masked]> wrote:
> 
> Hi Jan,
> Here is the revision letter for the revised draft, reflecting your comments along with the revised draft:
> https://tools.ietf.org/html/draft-ietf-i2nsf-consumer-facing-interface-dm-06 <https://tools.ietf.org/html/draft-ietf-i2nsf-consumer-facing-interface-dm-06> 
>  
> On Wed, Jun 26, 2019 at 12:00 PM Jan Lindblad via Datatracker <[email-masked] <mailto:[email-masked]>> wrote:
> Reviewer: Jan Lindblad
> ietf-i2nsf-cfi-policy in particular. The document is on the right track, but
> There are plenty of smaller things to fix in this module, see below for a list.
> There are a couple of potentially major issues in this module, however, that I
> would like to start with. The first relates to the management access control
> for this module.
> 
> This RFC defines a new management access control mechanism, entirely unrelated
> like which auth mechanisms should be used by clients in specific domains when
> To make it worse, the new security model described in this document is rather
> The RFC text refers to a device called "Security Controller". By reading the
> parent RFC 8329, which contains an overview of the architecture, I believe the
> Security Controller is the management (NETCONF/RESTCONF/...) server with the
> ietf-i2nsf-cfi-policy YANG model, and the user is supposed to connect directly
> with it. This means, I would think, that the management server would need to be
> Key in this new mechanism is a leaf owner. There isn't much information about
> This is all that is mentioned about the owner leaf. That the type of the leaf
> Then there are Policy Domains. A Policy Domain has an (optional) auth method
> server to CRUD policies and rules. Either within a tenant (default) or domain.
> There is no linkage to which tenant or domain, however. Each Policy User has
> only information this linkage provides is whether the access is read-only or
> read-write.
> 
> In summary, I believe significant work is required to get this to a workable
> state. In order to invent a new management access control mechanism, a wider
> Below I note some details that I came up with while trying to figure this out.
> - container policy-role should probably be a list, and the list access-profile
> leaf owner description talks about owner of the policy, while the leaf sits on
> an individual rule. Either the description or the leaf placement must be wrong.
> - I believe there are more issues to look at as part of the "translation from
> The top level container policy can only exist in a single instance as currently
> modeled. Even if not stated explicitly in the document, I get the feeling that
> the author's intent is that it should be possible to have more than one policy
> in the system. If this is the case, container policy needs to change to list
> policy (and probably add a surrounding container policies). Doing so would also
> require updating all the leafrefs used throughout the module, to only select
> Searching for container policy and list policy on yangcatalog.org <http://yangcatalog.org/> gives several
> hundred pages with hits. Most of them will not be on the top level, but this
> anything like that in the YANG. So how are the rules meant to be applied on the
> Each policy contains a set of rules in a list. This list is keyed by rule name
> and modeled as ordered-by system, i.e. rules will be sorted alphabetically. Is
> suppose they could be overlapping, leading to different results based on the
> order rules are applied, and there could be performance implications depending
> The module specifically uses IPv4 in many places. Would it not make sense to
> In a couple of places, an address range is specified, and the start address is
> an ipv4-address while the end address is an ip-address. Surely that must be a
> mistake.
> 
> 5) Many optional leafs
> 
> information... This could be understood as the succeeding information items are
> mandatory. There are not a single mandatory element (apart from keys) in the
> It would be appropriate to go through the module and add mandatory and defaults
> where they make sense. For the leafs that should remain optional, it would be
> good if something could be added to the description regarding what the server
> Actually, it is often good to write even when it is obvious, because otherwise
> the behavior is technically undefined, and implementations could get away with
> In the module, the type string is used for names, which is great, but also for
> solutions. It will also leave users frustrated with little information to guess
> The grouping meta contains a name and a date leaf. The name is clearly meant to
> be a configuration item filled in by the client. I'm not sure about the date
> information in YANG modules, but it would make some sense if this was config
> If that is the intent, the specification should only make the date leaf config
> false and clarify how these time stamps should be calculated. Do they pertain
> to this particular list entry, or would they be updated if any child entry to
> If the intent is that clients should fill this in if they want, for their own
> optional housekeeping, I think that needs to be stated clearly. If the intent
> A container is obviously not a list, so exactly what the author has in mind is
> The format and usage of this string is highly unclear, but more importantly, it
> token-based, list certificate-based, list ipsec-method, list single-sign-on.
> Are these lists under container policy-mgnt-auth-method mutually exclusive, or
> This would need some clearing up. As this is an area where we can anticipate
> new methods being introduced over time, it would be good if the model advised
> But in fact there is only one instance of container policy-mgnt-auth-method.
> In the RFC Fig. 8 YANG tree representation authentication-method points like
> this
> 
> usually "mgmt". In order to reduce misspellings of policy-mgnt-auth-method I
> suggest renaming it to policy-mgmt-auth-method. There is actually already one
> there usually a different server based on certificate type? Max one server can
> 203: certificate-type is defined as an enumeration. This means the module will
> have to be revised if any new certificate types need to be supported one day.
> An identity may be more appropriate here, as I expect new certificate types (or
> formats) may come out.
> 
> 366: enforce-type is defined as an enumeration. This means the module will have
> 55: permission-type is defined as an identity. Unless you foresee the need for
> 168: continent is defined as an identity. Unless you foresee the need for new
> The indentation of the YANG module is broken on many, many lines. Make sure to
> recurse (which is something completely different). Maybe change to the whole
> container recursive to
> 
>     leaf frequency {
>       type enumeration {
>         enum only-once;
>         enum daily;
>         enum weekly;
>         enum monthly;
>       }
>       default once-only;
>     }
> 
> 17) String passwords
> 
> Passwords are modeled as strings in a couple of places. This is not acceptable
> What does that mean? Authentication, password, regular expression? I don't get
> it.
> 
> repetition). Describe instead what the grouping is used for or what it contains.
> 
> 231: grouping meta
> 280: grouping user-group
> Is container time-information only relevant for enforce-type == time-enforced?
> If so, a when expression on the container may be useful. Or, even better, make
> the enforce-type a choice and have the time related content inside the choice
> case instead.
> 
> 20) container rate-limit
> 
> Is uint8 a good type to use for the rate limiting function? Would it never (now
> consider simplifying the model by removing them, unless they have some clever
> function for the future.
> 
> 22) Transactionality
> 
> This could lead some implementors to believe it is acceptable to require that
> the endpoints are committed separately from the rest of the configuration. The
> The XML snippets in examples in Fig 24-27 use XML namespaces incorrectly. It's
> easy to fix. Just make sure the first line with <ietf-i2nsf-cfi-policy:policy>
> looks like this instead:
> 
> The last example, in section 10.4 is talking about <encrypt>. There is no such
> [email-masked] <mailto:[email-masked]>
> https://www.ietf.org/mailman/listinfo/i2nsf <https://www.ietf.org/mailman/listinfo/i2nsf>
> 
> 
> -- 
> Email: [email-masked] <mailto:[email-masked]>, [email-masked] <mailto:[email-masked]>
> 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-20190725.pdf>

---

From: "Mr. Jaehoon Paul Jeong" <[email-masked]>
Date: Tue, 30 Jul 2019 20:52:57 +0900

Hi Jan,
Thanks for your volunteering to improve our Consumer-Facing Interface YANG
Module.
Could you propose a way to redesign ietf-i2nsf-cfi-policy.yang to befit
NACM?

I am not aware of any particular party interested to implement our data
model.

Thanks.

Best Regards,
Paul

On Fri, Jul 26, 2019 at 11:51 PM Jan Lindblad <[email-masked]> wrote:

> Paul, I2NSF WG,
>
> Thank you for going through all my comments. I can see you have worked
> hard to fix up the module. I still see ample opportunities to improve it,
> but let's come back to those details later. What I miss most of all is a
> discussion within the NETMOD WG around this new proposed management access
> control model.
>
> The proposed model, as far as I understand it, would have a high impact on
> all existing NETCONF/RESTCONF server implementations and break some
> architectural principles, so I think we should expect a lot of skepticism
> there if we propose the current module.
>
> Having read RFC 8192, I can see the vision you are painting. I think it's
> definitely worthwhile looking at ways to realize that idea. I believe the
> vision there can be realized in a much less disruptive fashion if we
> reorganized ietf-i2nsf-cfi-policy.yang to rely on the existing RFC 8341
> Network Configuration Access Control Model.
>
> I volunteer to provide a sketch of how the current module could be
> restructured to fit in with NACM, if you are interested. If you ask me to
> do this, I will bring the result back to the I2NSF WG for further
> discussion and possibly further iterations.
>
> Also, if you already know of any particular party that is interested to
> implement this, it would be great to have implementor involvement in the
> design process from this point onward.
>
> Let me know if you want me to propose a way to redesign
> ietf-i2nsf-cfi-policy.yang to fit better with NACM.
>
> Best Regards,
> /jan
>
>
>

---

From: Jan Lindblad <[email-masked]>
Date: Wed, 31 Jul 2019 10:21:52 +0200

Paul, I2NSF team,

> Thanks for your volunteering to improve our Consumer-Facing Interface YANG Module.
> Could you propose a way to redesign ietf-i2nsf-cfi-policy.yang to befit NACM?

Certainly. Find my proposed sketch for the module structure attached.

I think it is important for the adoption of this module that it is reasonably easy to implement it on top of existing NETCONF/RESTCONF/YANG servers. They all implement the NACM management access control mechanism today, so the ietf-i2nsf-cfi-policy module should build on that. It's therefore important to leverage the existing NACM mechanisms and concepts for groups, users, permissions. 

It would be technically possible to set up all the management access control rules needed to implement the I2NSF ideas by only creating rules in NACM. The NACM rules are massively more complex than the simple owner leaf proposed in your YANG module, however. From a usability perspective I think it makes good sense to keep the abstraction in ietf-i2nsf-cfi-policy and let the module implementor make sure this high level authorization view is translated into NACM specifics.

In order to make this feasible, I changed the owner string leaf into a leafref pointer to NACM groups, and removed the module's separate identities for permissions. Let's adopt the NACM counterparts instead. The structure of the rules was very flat, i.e. the domains, tenants, policies and rules were mostly side by side, not reflecting their logical hierarchy in the YANG. This would make the number of NACM rules to control access to each individual item very high. By arranging them in a tree structure, I believe the number of NACM rules can be kept to a minimum. NACM rules may have a high impact on server performance, so it's important to not have excessive amounts of them.

I created a hierarchy with domains on top, each domain containing zero or more tenants, each with zero or more policies that in turn consist of zero or more rules. At each level it is possible to list owners in the form of NACM groups. The module implementor would then have to translate these owner references to actual NACM rules.

Here is an example sketch configuration and the resulting NACM rules (in CLI style syntax for readability):

i2nsf-cfi domains domain example.com
 owners [ example.com--eng-it ]
 tenants tenant dev
  policies policy team-black
   owners [ example.com--dev ]
   rules rule 2
   !
   rules rule allow-malware-sites
    owners [ example.com--dev ]

This is supposed to mean that members of the example.com--eng-it group have full ownership of everything in the example.com domain. Within this domain, there is a tenant called dev, with a policy called team-black. That policy is owned by example.com--dev. This means this policy may be updated by members in example.com--dev and example.com--eng-it. Within the policy there are two rules ("2" and "allow-malware-sites"). The "allow-malware-sites" rule has the example.com--dev group listed as owner; this is superfluous. In this example, the rules are otherwise empty.

In order for existing NC/RC/YANG servers to enforce the above, the ietf-i2nsf-cfi-policy module implementation would need to translate the intent above to NACM rules like the ones below. In this example, the implementation created a rule to allow members of the dev and eng-it groups within the example.com org to see the example.com domain and everything within it. Next there is a rule to allow members of the example.com dev group to update the policy named team-black within the dev tenant. Finally, there is a rule to allow the eng-it group members to update anything within the example.com domain. The default nacm policy per statement in the YANG is to deny anyone else to see anything within the i2nsf domain.

nacm rule-list example.com
 group [ example.com--dev example.com--eng-it ]
 rule read-all
  path              /i2nsf-cfi/domains/domain[name='example.com']
  access-operations read
  action            permit
 !
!
nacm rule-list example.com--dev
 group [ example.com--dev ]
 rule 1
  path   /i2nsf-cfi/domains/domain[name='example.com']/tenants/tenant[name='dev']/policies/policy[name='team-black']
  action permit
 !
!
nacm rule-list example.com--eng-it
 group [ example.com--eng-it ]
 rule 1
  path   /i2nsf-cfi/domains/domain[name='example.com']
  action permit
 !
!

NACM also contains a mapping from user names to groups. Is this in line with your expectations? Do we need additional infrastructure to control this mapping?

nacm groups group example.com--dev
 user-name [ jan vasilij ]
!
nacm groups group example.com--eng-it
 user-name [ chris victor ]
!
nacm groups group example.com--finance
 user-name [ clara sakura ]
!

What do you think about this approach to the management access control? I'm not sure I got the relations between domains, tenants, policies and rules as you want them. Are all these levels needed? Do you believe this is this is a workable approach to your vision?

Please let me know if you would like me to take any further steps with this sketch. I should mention that I also have plenty of other comments on your updated module, but I want to get the access control approach resolved before looking at anything else.

> I am not aware of any particular party interested to implement our data model.

Then it is all the more important that the solution can be implemented on top of the existing servers out there without modifying them.

Best Regards,
/jan