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 protected]> 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. > > 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 protected]> > 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 > > > 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 protected]> 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 protected] >> 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 protected], [email protected] > 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> > > > -- =========================== Mr. Jaehoon (Paul) Jeong, Ph.D. Associate Professor Department of Software Sungkyunkwan University Office: +82-31-299-4957 Email: [email protected], [email protected] Personal Homepage: http://iotlab.skku.edu/people-jaehoon-jeong.php <http://cpslab.skku.edu/people-jaehoon-jeong.php>
_______________________________________________ I2nsf mailing list [email protected] https://www.ietf.org/mailman/listinfo/i2nsf
