Re: [Netconf] draft-ietf-netconf-rfc6536bis: one week review of a specific change

[Andy Bierman <[e-mail]>]

On Wed, Nov 22, 2017 at 5:41 AM, Benoit Claise <[e-mail]> wrote:

> Hi Rob,
>
> At that points in time, if it clarifies the spec. and the authors agree
> with this, let's do the right thing.
>
>
I will add the new text if that is what the WG wants.
There have not been any comments on this issue.


Andy


> Regards, Benoit.
>
> Hi Benoit,
>
> There is also one further, unrelated change that I am proposing is made
> the draft before it is published, to help better clarify the expected
> behavior.  It isn't the end of the world if this doesn't go in, but I think
> that it prevents sometime taking a different, but IMO reasonable,
> interpretation of how "when" statements are considered, and then having a
> future argument about what behavior it specified in the standard.
>
> If we clarify it now, then it closes that door :-)
>
> I've proposed text to Andy and Martin on Monday, but I've not heard back
> yet.
>
> Netconf email with proposed text attached.  The text doesn't necessarily
> have to match this, but personally I think that it is useful if the draft
> says something on this.
> Thanks,
> Rob
>
>
> On 22/11/2017 13:25, Benoit Claise wrote:
>
> On 11/10/2017 7:23 PM, Andy Bierman wrote:
>
> Hi,
>
> Here are some proposed edits to make the data rule consistent with the
> examples.
> Note that this issue is not related to the edit in the original 1-week
> change.
>
> That's right, but we found a source of misinterpretation in the draft and
> you have rightly corrected it in the github v9.
>
> Regards, Benoit
>
>
>
> sec. 3.3.5:
>
> OLD:
>
>
>       data node rule:  controls access for a specific data node, identified
>       by its path location within the conceptual XML document for the
>       data node.
>
>
> NEW:
>
>       data node rule:  controls access for a specific data node and its
> descendants,
>       identified by its path location within the conceptual XML document
> for the
>       data node.
>
>
> sec 3.4.5, step 6, bullet 2:
>
>
> OLD:
>
>         *  The rule does not have a "rule-type" defined or the "rule-
>            type" is "data-node" and the "path" matches the requested
>            data node, action node, or notification node.
>
>       NEW:
>
>         *  The rule does not have a "rule-type" defined or the "rule-
>            type" is "data-node" and the "path" matches the requested
>            data node, action node, or notification node. A path is
>            considered to match if the current data node is the data node
>            specified by the path, or is a descendant data node of this data node.
>
> appendix B.4: (2 bugs in explanation)
>
> OLD:
>
>       deny-nacm:  This rule denies the "guest" group any access to the
>       <nacm> subtree.  Note that the default namespace is only
>       applicable because this subtree is defined in the same namespace
>       as the <data-rule> element.
>
>  NEW:
>
>       deny-nacm:  This rule denies the "guest" group any access to the
>       <nacm> subtree.
>
> Andy
>
>
> On Fri, Nov 10, 2017 at 9:24 AM, Robert Wilton <[e-mail]> wrote:
>
>>
>>
>> On 10/11/2017 16:33, Andy Bierman wrote:
>>
>>
>>
>> On Fri, Nov 10, 2017 at 8:16 AM, Robert Wilton <[e-mail]> wrote:
>>
>>>
>>>
>>> On 10/11/2017 15:49, Andy Bierman wrote:
>>>
>>>
>>>
>>> On Fri, Nov 10, 2017 at 5:07 AM, Per Hedeland <[e-mail]> wrote:
>>>
>>>> On 2017-11-10 11:42, Robert Wilton wrote:
>>>> >
>>>> >
>>>> > On 10/11/2017 10:02, Mahesh Jethanandani wrote:
>>>> >>
>>>> >>
>>>> >>
>>>> >>
>>>> >> Mahesh Jethanandani
>>>> >> [e-mail] <mailto:[e-mail]>
>>>> >> On Nov 10, 2017, at 10:07 AM, Andy Bierman <[e-mail]
>>>> <mailto:[e-mail]>> wrote:
>>>> >>
>>>> >>> Hi,
>>>> >>>
>>>> >>> The term "data node" is used in the document to refer to the
>>>> top-level node
>>>> >>> of the specified object, not the entire subtree (if any).
>>>> >>>
>>>> >>> The data-rule /foo does not match /foo/child1 in the text below.
>>>> >>> The child nodes are omitted because of step 11.
>>>> >>> The admin has to explicitly permit individual child nodes (or
>>>> modules).
>>>> >>> This seems correct if the read-default is "deny".
>>>> >>>
>>>> >>> Should any text be added or changed to make this more clear?
>>>> >>
>>>> >> I would agree with Robert that it was not entirely clear that rule
>>>> applied on the parent data-node does not apply to the child nodes. So yes,
>>>> it would help to clarify it.
>>>> > We should be doing more than clarifying it.  We need to fix it so
>>>> that it works in a sensible way.  I.e. to make the normative text
>>>> consistent with the behaviour currently described in the examples in
>>>> > the appendix B.4.
>>>> >
>>>> > If we follow Andy's interpretation that a data rule doesn't match
>>>> child nodes then those examples are completely wrong.  E.g. the 4th rule is
>>>> described as "This rule gives the 'admin' group read-write
>>>> > access to all acme <interface> entries."  But If the path only
>>>> strictly matches "/acme:interfaces/acme:interface" then the admin
>>>> group rule achieves nothing useful at all.  The admin is not even
>>>> > allowed to create an interface because they would not even have
>>>> permission to write to the list key 'name' node required to create a list
>>>> entry!  Instead, a separate rule would be required for every
>>>> > single possible schema node under "/acme:interfaces/acme:interface"!
>>>> I think that this makes "permit" data-node rules completely unusable.
>>>>
>>>> I strongly agree with this, and I would say that it isn't only the case
>>>> for "permit" rules - e.g. denying some access to a subtree of the data
>>>> model that would otherwise be permitted due to defaults is at least as
>>>> common, and the rules would be just as unusable for that.
>>>>
>>>> Besides the examples, I think that the very use of the term "match",
>>>> though unfortunately not defined, strongly suggests that it is something
>>>> other than use of e.g. the term "identify" would imply. Additionally,
>>>> this text in the description of the 'path' leaf is consistent with the
>>>> match being a prefix match:
>>>>
>>>>        The special value '/' refers to all possible
>>>>        datastore contents.";
>>>>
>>>> FWIW, our NACM implementation, available to (and used by) customers
>>>> since 2012, follows the prefix match logic, and I have yet to hear of
>>>> any user expecting it to do otherwise.
>>>>
>>>> > To fix this properly, we need to make the data-node path rule a
>>>> prefix match.  In particular, we need text that specifies:
>>>> >
>>>> > (i) that a data-node path match succeeds if it matches the path
>>>> prefix from the root of the tree.  I.e. so the data-rule "/foo" matches
>>>> "/foo" and all of foo's descendant children nodes.
>>>>
>>>> Strongly agree.
>>>>
>>>>
>>>
>>> I do not see how the text can be interpreted this way.
>>>
>>>
>>> Because otherwise the path match part of the NACM solution is really
>>> broken, and the path based examples in the appendix are entirely misleading
>>> and wrong.  The only way those examples make sense is the paths match
>>> descendant children nodes as well.
>>>
>>>
>>
>> IMO the text does not support this interpretation.
>>
>> The examples in B.4, and the definition of "/" matching all nodes
>> supports this interpretation.
>>
>> Hence, my opinion is that it is the text in 3.4.5 that is incorrectly
>> specified; and that the examples, definition of "/" and standard practice
>> are right.
>>
>> Otherwise, how did IETF manage to publish an RFC where the path based
>> examples are so completely wrong?   Whoever wrote and reviewed those
>> examples clearly had a different interpretation of how these path based
>> ACLs worked.
>>
>>
>> There is nothing said about inheriting state from the parent data node.
>> I think no matter how the permissions are derived, one can
>> find examples that work better or worse because of it.
>>
>> No.  If the rules apply to descendant children, all normal examples work
>> well (including the ones in the appendix).
>>
>>
>> IMO the number of rules required to implement a use-case is not
>> very relevant or objective criteria.
>>
>> Yes it is, particularly when the difference is between needing a 1 line
>> rule, and a 100+ line rule.
>>
>>
>> Using the previous example of /home and /home/user1,
>> if the user1 is given read access to /home, then (according to you)
>> it also has read access to every user subtree under /home.
>>
>> Instead of 1 rule per user, 2 rules are needed
>>
>> No, just 1 rule per user:
>>    read-default=deny
>>    group=user1, path=/home/user1, action=permit
>>
>> This is because of my two proposed changes:
>>
>> (i) that a data-node path match succeeds if it matches the path prefix
>> from the root of the tree.  I.e. so the data-rule "/foo" matches "/foo" and
>> all of foo's descendant children nodes.
>> <- This means that you only need 1 entry instead of 100 entries.
>>
>> (ii) if a data-node rule has action "permit" then it implicitly allows
>> read access for all ancestor parent nodes up to the root.  (I.e. to
>> mitigate the original change proposed on this thread.)
>> <- This means that you don't need a separate read rule for "/home".  Read
>> access to that node it is implicitly given via "group=user1,
>> path=/home/user1, action=permit", hence meaning that the rule works the
>> same way as it does on an rfc6536 compliant implementation.
>>
>>
>>    read-default=deny
>>    group=*, path=/home, action=permit
>>    group=user1, path=/home/user1, action=permit
>>
>> The above rules would allow access for every user to every other user.
>> The 2nd rule has no effect, which is counter-intuitive.
>> Every user dir would need 2 rules
>>
>>    read-default=deny
>>    group=*, path=/home, action=permit
>>    group=user1, path=/home/user1, action=permit
>>    group=*, path=/home/user1, action=deny
>>
>> There is nothing that says this is how it works.
>>> If it did, once could never have privileged sub-fiolders
>>>
>>>      /var/log -> permit
>>>      /var/log/apache2  -> deny
>>>
>>>
>>> Yes, you can, you just list the longest path first in the list of rules:
>>>
>>>  (1)  /var/log/apache2  -> deny
>>>   (2) /var/log -> permit
>>>
>>> Any requests that attempt to access anything under /var/log/apache2
>>> would match rule (1) and be denied.
>>> Any requests that attempt to access anything under /var/log, but not
>>> under /var/log/apache2, would fail to match rule (1), but would match rule
>>> (2) instead and be permitted.
>>>
>>>
>>>
>> I do not see any text in the draft or RFC 7950 that
>> suggests that /var/log and /var/log/apache2 represent the same data node.
>>
>> They are different data nodes, but I don't see how that is relevant.
>>
>> Thanks,
>> Rob
>>
>>
>>
>>
>> Andy
>>
>>
>>
>>>
>>>
>>>
>>>> > (ii) if a data-node rule has action "permit" then it implicitly
>>>> allows read access for all ancestor parent nodes up to the root.  (I.e. to
>>>> mitigate the original change proposed on this thread.)
>>>>
>>>> This seems reasonable to me, although I haven't at this point evaluated
>>>> the suggestion in detail. In any case I think the main point both
>>>> regarding this and the prefix match is that this update to 6536 can't
>>>> make radical changes to the semantics compared to a "reasonable
>>>> interpretation" (hard to define, I know) of the under-specified
>>>> original.
>>>>
>>>
>>> The text does not say this at all so I do not approve of this change
>>>
>>>
>>> In the RFC version of the NACM this wasn't required because operation
>>> 'none' didn't require read access.  Now read access is required even for
>>> operation 'none', then this change makes sense to make the NACM changes
>>> backwards compatible, whilst still closing the security hole.
>>>
>>> Thanks,
>>> Rob
>>>
>>>
>>>
>>>
>>>>
>>>> --Per
>>>>
>>>>
>>>
>>> Andy
>>>
>>>
>>>> > Thanks,
>>>> > Rob
>>>> >
>>>> >
>>>> >>
>>>> >> Thanks
>>>> >>
>>>> >>>
>>>> >>>
>>>> >>> Andy
>>>> >>>
>>>> >>>
>>>> >>> On Thu, Nov 9, 2017 at 2:44 AM, Robert Wilton <[e-mail]
>>>> <mailto:[e-mail]>> wrote:
>>>> >>>
>>>> >>>     Hi Andy,
>>>> >>>
>>>> >>>     It isn't clear to me whether matching a path in NACM either:
>>>> >>>       (i) Only applies to the specific node, and not any children,
>>>> or
>>>> >>>       (ii) Applies to the specific node and all descendant children
>>>> nodes as well.
>>>> >>>
>>>> >>>     As an example, using the tree below.  if I have a rule that
>>>> matches path "A/B" then does that apply to only the specific node "A/B", or
>>>> does it also apply to all descendant children of "A/B" as
>>>> >>>     well?
>>>> >>>
>>>> >>>     In rfc6536bis-08, section "3.4.5.  Data Node Access
>>>> Validation", step 6 states:
>>>> >>>
>>>> >>>             *  The rule does not have a "rule-type" defined or the
>>>> "rule-
>>>> >>>                type" is "data-node" and the *"path" matches the
>>>> requested data node*, action node, or notification node.
>>>> >>>
>>>> >>>
>>>> >>>     My reading of this is that it implies that the interpretation
>>>> of the path rule is (i), but this is not how I would normally expect an ACL
>>>> rule to apply in a tree like object (e.g a directory
>>>> >>>     file system).
>>>> >>>
>>>> >>>     However, the examples in Appendix B.4. imply that the path rule
>>>> is to be interpreted like (ii), or otherwise the example rules seem to be
>>>> mostly pointless.
>>>> >>>
>>>> >>>     E.g. taking this example from appendix B.4:
>>>> >>>
>>>> >>>            <rule>
>>>> >>>              <name>permit-dummy-interface</name>
>>>> >>>              <path xmlns:acme="http://example.com/ns/itf" <
>>>> http://example.com/ns/itf>>
>>>> >>>                /acme:interfaces/acme:interface[acme:name='dummy']
>>>> >>>              </path>
>>>> >>>              <access-operations>read update</access-operations>
>>>> >>>              <action>permit</action>
>>>> >>>              <comment>
>>>> >>>                Allow the limited and guest groups read
>>>> >>>                and update access to the dummy interface.
>>>> >>>              </comment>
>>>> >>>            </rule>
>>>> >>>
>>>> >>>
>>>> >>>     If the rule is (i)  then the access rule allows the client to
>>>> read the specific node "/acme:interfaces/acme:interface[acme:name='dummy']"
>>>> but not any child leafs/containers of that interface,
>>>> >>>     this doesn't seem useful.
>>>> >>>
>>>> >>>     Further comments inline below ...
>>>> >>>
>>>> >>>     On 08/11/2017 20:05, Andy Bierman wrote:
>>>> >>>>     Hi,
>>>> >>>>
>>>> >>>>     This change has no impact on the server if /nacm/read-default
>>>> is "permit".
>>>> >>>>     In that case, the extra read rules for /A and /A/B are not
>>>> needed.
>>>> >>>     I agree.
>>>> >>>
>>>> >>>>     An operator worried about read access should set read-default
>>>> to "deny".
>>>> >>>     I agree.  This is the scenario that I'm considering.
>>>> >>>
>>>> >>>>     In that case, explicit rules to read /A and /A/B would be
>>>> needed
>>>> >>>>     in the new NACM.
>>>> >>>     Yes, if the interpretation of the rule is (i) above.
>>>> >>>     Otherwise if the interpretation is (ii) then you only need
>>>> "read /A" since that implies "read A/B" as well (as long as the rules are
>>>> listed in the correct order).
>>>> >>>
>>>> >>>
>>>> >>>>       The deny rules would not be needed.
>>>> >>>     Only if the interpretation of the rule is (i) above.  In which
>>>> case the  "read/write 'A/B/J' rule would not be sufficient.  It would be
>>>> necessary to define an Xpath expressions that contains
>>>> >>>     all children nodes as well.  Perhaps 'A/B/J//*'?
>>>> >>>
>>>> >>>     If the interpretation of the rule is (ii) then you would also
>>>> need all the explicit deny statements as well, otherwise they would be
>>>> allowed by the "read /A" rule above.
>>>> >>>
>>>> >>>     Thanks,
>>>> >>>     Rob
>>>> >>>
>>>> >>>
>>>> >>>>
>>>> >>>>
>>>> >>>>     Andy
>>>> >>>>
>>>> >>>>
>>>> >>>>     On Wed, Nov 8, 2017 at 8:33 AM, Robert Wilton <
>>>> [e-mail] <mailto:[e-mail]>> wrote:
>>>> >>>>
>>>> >>>>         Hi,
>>>> >>>>
>>>> >>>>         I'm not sure about this change.
>>>> >>>>
>>>> >>>>         I'm not that familiar with NACM, but if you want to give a
>>>> particular set of users read/write access to a subtree, but not allow them
>>>> to have any other access to the configuration in the
>>>> >>>>         running datastore then with the existing RFC, that could
>>>> be expressed with a single rule (example in 6536bis, appendix B.4)
>>>> >>>>
>>>> >>>>         With this new change, I think that you may need to
>>>> configure many more rules to achieve the same thing.  I think that you
>>>> would need to give read access to the top node in the desired
>>>> >>>>         path, and then separate explicit "deny" rules for every
>>>> sibling child node walking from the top of the tree down to the data node
>>>> that read/write access is actually being given to.  The
>>>> >>>>         example below may explain my understanding better:
>>>> >>>>
>>>> >>>>         E.g. For a tree of data nodes, rooted at A, if we wanted
>>>> to give read/write access only to "J" subtree, and no access for the rest
>>>> of the tree then:
>>>> >>>>
>>>> >>>>                          A
>>>> >>>>                          |
>>>> >>>>                 --------------------
>>>> >>>>                 |      |     |     |
>>>> >>>>                 B      C     D     E
>>>> >>>>                 |
>>>> >>>>            -----------
>>>> >>>>            |   |  |  |
>>>> >>>>            F   G  H  J
>>>> >>>>                      |
>>>> >>>>                     ...
>>>> >>>>
>>>> >>>>
>>>> >>>>         In the old model, I think that the ACL rules would be 1
>>>> rules long (assuming default deny all):
>>>> >>>>            "read/write 'A/B/J'
>>>> >>>>
>>>> >>>>         In the new model, I think that the equivalent ACL rules
>>>> would need to be 8 rules long (assuming default deny all):
>>>> >>>>            "read/write 'A/B/J'
>>>> >>>>            "read A"
>>>> >>>>            "deny C"
>>>> >>>>            "deny D"
>>>> >>>>            "deny E"
>>>> >>>>            "deny F"
>>>> >>>>            "deny G"
>>>> >>>>            "deny H"
>>>> >>>>
>>>> >>>>         Note, I am assuming that a "path" rule matches for the
>>>> given path and all descendant nodes.  The draft doesn't seem to be
>>>> particularly clear on this point (it states that the rule applies
>>>> >>>>         when the path matches, but this would seem to be counter
>>>> intuitive), and perhaps it could be clarified.
>>>> >>>>
>>>> >>>>         If this change is allowed, then the example in appendix
>>>> B.4 looks like it would need to be fixed, since the "limited-acl" probably
>>>> wouldn't give any access at all, unless default read
>>>> >>>>         access had been given.
>>>> >>>>
>>>> >>>>         But, possibly I'm misunderstanding how this all works!  If
>>>> so, apologies for the noise :-)
>>>> >>>>
>>>> >>>>         Thanks,
>>>> >>>>         Rob
>>>> >>>>
>>>> >>>>
>>>> >>>>         On 02/11/2017 14:18, Benoit Claise wrote:
>>>> >>>>>         Dear all,
>>>> >>>>>
>>>> >>>>>         Here is a major change in draft-ietf-netconf-rfc6536bis,
>>>> suggested by the Security AD Eric Rescola part of the IESG review, which I
>>>> would like to validate with the WG. See
>>>> >>>>>         https://tools.ietf.org/rfcdif
>>>> f?url2=draft-ietf-netconf-rfc6536bis-08.txt <
>>>> https://tools.ietf.org/rfcdiff?url2=draft-ietf-netconf-rfc6
>>>> 536bis-08.txt>
>>>> >>>>>
>>>> >>>>>         <dfpcfioondggippe.png>
>>>> >>>>>         The NETCONF WG was cc'ed for the entire discussion.
>>>> >>>>>         What do you think? I will draw the conclusions by Friday
>>>> Nov 10th.
>>>> >>>>>
>>>> >>>>>         Note: If the WG is fine, the next step is to approve this
>>>> document.
>>>> >>>>>
>>>> >>>>>         Regards, Benoit
>>>> >>>>>
>>>> >>>>>
>>>> >>>>>         _______________________________________________
>>>> >>>>>         Netconf mailing list
>>>> >>>>>         [e-mail] <mailto:[e-mail]>
>>>> >>>>>         https://www.ietf.org/mailman/listinfo/netconf <
>>>> https://www.ietf.org/mailman/listinfo/netconf>
>>>> >>>>
>>>> >>>>
>>>> >>>
>>>> >>>
>>>> >>> _______________________________________________
>>>> >>> Netconf mailing list
>>>> >>> [e-mail] <mailto:[e-mail]>
>>>> >>> https://www.ietf.org/mailman/listinfo/netconf
>>>> >>
>>>> >> Mahesh Jethanandani
>>>> >> [e-mail] <mailto:[e-mail]>
>>>> >
>>>> >
>>>> >
>>>> > _______________________________________________
>>>> > Netconf mailing list
>>>> > [e-mail]
>>>> > https://www.ietf.org/mailman/listinfo/netconf
>>>> >
>>>>
>>>>
>>>
>>>
>>
>>
>
>
> _______________________________________________
> Netconf mailing [e-mail]://www.ietf.org/mailman/listinfo/netconf
>
>
>
>
>