Re: [netmod] Benjamin Kaduk's No Objection on draft-ietf-netmod-nmda-diff-09: (with COMMENT)

2021-08-06 Thread Alexander L Clemm
Hello Benjamin,

thank you for your review. Two quick additional responses inline, 

Kind regards

--- Alex

On 7/15/2021 11:27 AM, Benjamin Kaduk wrote:
> Hi Andy,
>
> On Mon, Jul 12, 2021 at 03:42:30PM -0700, Andy Bierman wrote:
>> On Thu, Jul 8, 2021 at 2:45 PM Benjamin Kaduk via Datatracker <
>> nore...@ietf.org> wrote:
>>
>>> Benjamin Kaduk has entered the following ballot position for
>>> draft-ietf-netmod-nmda-diff-09: No Objection
>>>
>>> When responding, please keep the subject line intact and reply to all
>>> email addresses included in the To and CC lines. (Feel free to cut this
>>> introductory paragraph, however.)
>>>
>>>
>>> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
>>> for more information about DISCUSS and COMMENT positions.
>>>
>>>
>>> The document, along with other ballot positions, can be found here:
>>> https://datatracker.ietf.org/doc/draft-ietf-netmod-nmda-diff/
>>>
>>>
>>>
>>> --
>>> COMMENT:
>>> --
>>>
>>> Thanks to Alexey Melnikov for the secdir review.
>>>
>>> I'm not experienced enough with YANG to know whether or how problematic
>>> it is that the "anydata subtree-filter" node contents are described by
>>> reference to the NETCONF specification, which has a particular (XML)
>>> representation of YANG data and does not give a clear presentation of
>>> the abstract YANG structure/semantics to be used.  Is it possible to use
>>> the filter-spec choice option when, for example, RESTCONF is used with
>>> JSON encoding?
>>>
>>>
>> There is not a standard specification for subtree filtering using JSON
>> encoding
>> instead of XML. RESTCONF has its own somewhat compatible filtering
>> mechanism.
>> It would require a new NETCONF work item to officially expand subtree
>> filtering
>> to support JSON.  (Unofficially, there is RFC for JSON encoding of YANG
>> data and an
>> implementation could map the RFC 6241 text to JSON).
> Okay, this sounds a little familiar now that you mention it.
>
> Should we mention in a comment of some form that this filtering mechanism
> is currently only usable with the XML encoding?

 Since we are not defining a new filtering mechanism here, just
making use of what has been defined, presumably the references in the
YANG spec should be sufficient. While the limitations you state are
correct, those lie in the nature of the current filtering
state-of-the-art and not specific to the specification here. it is well
possible that JSON filtering might be standardized in the future, in
which case such filter would equally apply / be accommodated.  So, I am
not sure that commenting on the current state in filter standardization
would be helpful and prefer to keep the text as is.  

>>
>>> Section 4
>>>
>>>o  report-origin: When set, this parameter indicates that origin
>>>   metadata should be included as part of RPC output.  When this
>>>   parameter is omitted, origin metadata in comparisons that involve
>>>is by default omitted.
>>>
>>> Why is it important to complicate the semantics of this parameter with a
>>> dependence on the datastore?  It seems like it would be simpler to get
>>> this effect by having clients specify report-origin when the target is
>>> not .  Note that changing the semantics would require text
>>> changes in subsequent parts of the document for consistency.  (If
>>> retaining the current semantics, please clarify whether "comparisons
>>> that involve " applies when operational is source, target,
>>> or either.)
>>>
>>>
>> IMO NMDA is way too complicated, so I cannot argue with that complaint.
>> The origin attribute only applies to the  datastore.
>> I think it is not allowed to appear elsewhere.
> Ah, I see what you are saying now (on second read): the "origin" being
> reported is the origin attribute of the data in the datastore being
> compared, but only the  datastore will every have that
> attribute available.  The text here would be a lot more clear if it
> reminded the reader that the origin attribute has such a limited scope,
> perhaps:
>
> o report-origin: data in the  datastore has an associated
>   "origin" attribute that indicates the origin of the reported value.  When
>   set, this parameter indicates that the origin metadata should be included
>   as part of the RPC output.  It has no effect when the target datastore is
>   anything other than .  When this parameter is omitted, the
>   origin metadata is omitted from the RPC output.
 I think this is clearly explained in the bullet item:  "Note that
origin metadata only applies to ; it is therefore also
omitted in comparisons that do not involve ; regardless of
whether or not the parameter is set. Adding the first sentence would
seem a bit redundant.  
>> Section 9
>>> In addition to noting that the "compare" RPC is sensitive and should be
>>> restricted to authorized parties, I suggest to rei

Re: [netmod] Benjamin Kaduk's No Objection on draft-ietf-netmod-nmda-diff-09: (with COMMENT)

2021-07-15 Thread Benjamin Kaduk
Hi Andy,

On Mon, Jul 12, 2021 at 03:42:30PM -0700, Andy Bierman wrote:
> On Thu, Jul 8, 2021 at 2:45 PM Benjamin Kaduk via Datatracker <
> nore...@ietf.org> wrote:
> 
> > Benjamin Kaduk has entered the following ballot position for
> > draft-ietf-netmod-nmda-diff-09: No Objection
> >
> > When responding, please keep the subject line intact and reply to all
> > email addresses included in the To and CC lines. (Feel free to cut this
> > introductory paragraph, however.)
> >
> >
> > Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> > for more information about DISCUSS and COMMENT positions.
> >
> >
> > The document, along with other ballot positions, can be found here:
> > https://datatracker.ietf.org/doc/draft-ietf-netmod-nmda-diff/
> >
> >
> >
> > --
> > COMMENT:
> > --
> >
> > Thanks to Alexey Melnikov for the secdir review.
> >
> > I'm not experienced enough with YANG to know whether or how problematic
> > it is that the "anydata subtree-filter" node contents are described by
> > reference to the NETCONF specification, which has a particular (XML)
> > representation of YANG data and does not give a clear presentation of
> > the abstract YANG structure/semantics to be used.  Is it possible to use
> > the filter-spec choice option when, for example, RESTCONF is used with
> > JSON encoding?
> >
> >
> 
> There is not a standard specification for subtree filtering using JSON
> encoding
> instead of XML. RESTCONF has its own somewhat compatible filtering
> mechanism.
> It would require a new NETCONF work item to officially expand subtree
> filtering
> to support JSON.  (Unofficially, there is RFC for JSON encoding of YANG
> data and an
> implementation could map the RFC 6241 text to JSON).

Okay, this sounds a little familiar now that you mention it.

Should we mention in a comment of some form that this filtering mechanism
is currently only usable with the XML encoding?

> 
> 
> > Section 4
> >
> >o  report-origin: When set, this parameter indicates that origin
> >   metadata should be included as part of RPC output.  When this
> >   parameter is omitted, origin metadata in comparisons that involve
> >is by default omitted.
> >
> > Why is it important to complicate the semantics of this parameter with a
> > dependence on the datastore?  It seems like it would be simpler to get
> > this effect by having clients specify report-origin when the target is
> > not .  Note that changing the semantics would require text
> > changes in subsequent parts of the document for consistency.  (If
> > retaining the current semantics, please clarify whether "comparisons
> > that involve " applies when operational is source, target,
> > or either.)
> >
> >
> IMO NMDA is way too complicated, so I cannot argue with that complaint.
> The origin attribute only applies to the  datastore.
> I think it is not allowed to appear elsewhere.

Ah, I see what you are saying now (on second read): the "origin" being
reported is the origin attribute of the data in the datastore being
compared, but only the  datastore will every have that
attribute available.  The text here would be a lot more clear if it
reminded the reader that the origin attribute has such a limited scope,
perhaps:

o report-origin: data in the  datastore has an associated
  "origin" attribute that indicates the origin of the reported value.  When
  set, this parameter indicates that the origin metadata should be included
  as part of the RPC output.  It has no effect when the target datastore is
  anything other than .  When this parameter is omitted, the
  origin metadata is omitted from the RPC output.

> 
> Section 9
> >
> > In addition to noting that the "compare" RPC is sensitive and should be
> > restricted to authorized parties, I suggest to reiterate that the
> > "compare" operation should not provide a mechanism to work around access
> > control on other nodes -- that is, a result should only be returned if
> > the requestor would be allowed to access both the "source" and "target"
> > trees independently of the RPC.  In particular, even a "no-matches"
> > output should not be returned, as that might provide a way to determine
> > the structure of the datastore even without accessing it.
> >
> 
> Fortunately the access control model (NACM) is per server and not per
> datastore
> so the same access rules apply to all datastores. I agree the no-access
> subtrees
> should be silently skipped, the same way that GET operations are treated in
> NACM.

Ah, okay.  (I think Roman may have mentioned a similar concern about
no-access subtrees, too.)

Thanks,

Ben

> 
> 
> > We might also incorporate by reference the security considerations for
> > subtree filtering (RFC 6241) and xpath filtering (RFC 6991).
> >
> > NITS
> >
> > Section 1
> >
> >an unusually long time to do so.  This can be the

Re: [netmod] Benjamin Kaduk's No Objection on draft-ietf-netmod-nmda-diff-09: (with COMMENT)

2021-07-12 Thread Andy Bierman
On Thu, Jul 8, 2021 at 2:45 PM Benjamin Kaduk via Datatracker <
nore...@ietf.org> wrote:

> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-netmod-nmda-diff-09: No Objection
>
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
>
>
> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about DISCUSS and COMMENT positions.
>
>
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-netmod-nmda-diff/
>
>
>
> --
> COMMENT:
> --
>
> Thanks to Alexey Melnikov for the secdir review.
>
> I'm not experienced enough with YANG to know whether or how problematic
> it is that the "anydata subtree-filter" node contents are described by
> reference to the NETCONF specification, which has a particular (XML)
> representation of YANG data and does not give a clear presentation of
> the abstract YANG structure/semantics to be used.  Is it possible to use
> the filter-spec choice option when, for example, RESTCONF is used with
> JSON encoding?
>
>

There is not a standard specification for subtree filtering using JSON
encoding
instead of XML. RESTCONF has its own somewhat compatible filtering
mechanism.
It would require a new NETCONF work item to officially expand subtree
filtering
to support JSON.  (Unofficially, there is RFC for JSON encoding of YANG
data and an
implementation could map the RFC 6241 text to JSON).



> Section 4
>
>o  report-origin: When set, this parameter indicates that origin
>   metadata should be included as part of RPC output.  When this
>   parameter is omitted, origin metadata in comparisons that involve
>is by default omitted.
>
> Why is it important to complicate the semantics of this parameter with a
> dependence on the datastore?  It seems like it would be simpler to get
> this effect by having clients specify report-origin when the target is
> not .  Note that changing the semantics would require text
> changes in subsequent parts of the document for consistency.  (If
> retaining the current semantics, please clarify whether "comparisons
> that involve " applies when operational is source, target,
> or either.)
>
>
IMO NMDA is way too complicated, so I cannot argue with that complaint.
The origin attribute only applies to the  datastore.
I think it is not allowed to appear elsewhere.


Section 9
>
> In addition to noting that the "compare" RPC is sensitive and should be
> restricted to authorized parties, I suggest to reiterate that the
> "compare" operation should not provide a mechanism to work around access
> control on other nodes -- that is, a result should only be returned if
> the requestor would be allowed to access both the "source" and "target"
> trees independently of the RPC.  In particular, even a "no-matches"
> output should not be returned, as that might provide a way to determine
> the structure of the datastore even without accessing it.
>

Fortunately the access control model (NACM) is per server and not per
datastore
so the same access rules apply to all datastores. I agree the no-access
subtrees
should be silently skipped, the same way that GET operations are treated in
NACM.



> We might also incorporate by reference the security considerations for
> subtree filtering (RFC 6241) and xpath filtering (RFC 6991).
>
> NITS
>
> Section 1
>
>an unusually long time to do so.  This can be the case due to certain
>conditions not being met, certain parts of the configuration not
>propagating because considered inactive, resource dependencies not
>being resolved, or even implementation errors in corner conditions.
>
> "because considered inactive" seems like an incomplete clause; maybe
> "because they are considered inactive"?
>
> Section 4
>
>o  differences: This parameter contains the list of differences.
>   Those differences are encoded per YANG-Patch data model defined in
>
> s/YANG-Patch/the YANG-Patch/
> I'd also consider s/per/according to/, since this is not exactly a
> logic-driven deduction but rather more of a new requirement.
>
> Section 6
>
>for the management of interfaces defined in [RFC8343].  The excerpt
>of the data model whose instantiation is the basis of the comparison
>is as follows:
>
> I feel like this phrasing is a little misleading, as not only is the
> following snippet only a subset of the nodes contained within "container
> interfaces" but the descriptions have been greatly abbreviated as well.
> Perhaps we could say something about "for the purposes of understanding
> the subsequent example, the following subset of the [RFC8343] data model
> is provided".
>
>Accept: application/yang-d
>
> (I believe this 

[netmod] Benjamin Kaduk's No Objection on draft-ietf-netmod-nmda-diff-09: (with COMMENT)

2021-07-08 Thread Benjamin Kaduk via Datatracker
Benjamin Kaduk has entered the following ballot position for
draft-ietf-netmod-nmda-diff-09: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-netmod-nmda-diff/



--
COMMENT:
--

Thanks to Alexey Melnikov for the secdir review.

I'm not experienced enough with YANG to know whether or how problematic
it is that the "anydata subtree-filter" node contents are described by
reference to the NETCONF specification, which has a particular (XML)
representation of YANG data and does not give a clear presentation of
the abstract YANG structure/semantics to be used.  Is it possible to use
the filter-spec choice option when, for example, RESTCONF is used with
JSON encoding?

Section 4

   o  report-origin: When set, this parameter indicates that origin
  metadata should be included as part of RPC output.  When this
  parameter is omitted, origin metadata in comparisons that involve
   is by default omitted.

Why is it important to complicate the semantics of this parameter with a
dependence on the datastore?  It seems like it would be simpler to get
this effect by having clients specify report-origin when the target is
not .  Note that changing the semantics would require text
changes in subsequent parts of the document for consistency.  (If
retaining the current semantics, please clarify whether "comparisons
that involve " applies when operational is source, target,
or either.)

Section 9

In addition to noting that the "compare" RPC is sensitive and should be
restricted to authorized parties, I suggest to reiterate that the
"compare" operation should not provide a mechanism to work around access
control on other nodes -- that is, a result should only be returned if
the requestor would be allowed to access both the "source" and "target"
trees independently of the RPC.  In particular, even a "no-matches"
output should not be returned, as that might provide a way to determine
the structure of the datastore even without accessing it.

We might also incorporate by reference the security considerations for
subtree filtering (RFC 6241) and xpath filtering (RFC 6991).

NITS

Section 1

   an unusually long time to do so.  This can be the case due to certain
   conditions not being met, certain parts of the configuration not
   propagating because considered inactive, resource dependencies not
   being resolved, or even implementation errors in corner conditions.

"because considered inactive" seems like an incomplete clause; maybe
"because they are considered inactive"?

Section 4

   o  differences: This parameter contains the list of differences.
  Those differences are encoded per YANG-Patch data model defined in

s/YANG-Patch/the YANG-Patch/
I'd also consider s/per/according to/, since this is not exactly a
logic-driven deduction but rather more of a new requirement.

Section 6

   for the management of interfaces defined in [RFC8343].  The excerpt
   of the data model whose instantiation is the basis of the comparison
   is as follows:

I feel like this phrasing is a little misleading, as not only is the
following snippet only a subset of the nodes contained within "container
interfaces" but the descriptions have been greatly abbreviated as well.
Perhaps we could say something about "for the purposes of understanding
the subsequent example, the following subset of the [RFC8343] data model
is provided".

   Accept: application/yang-d

(I believe this truncated header field was already noted by another
reviewer.)



___
netmod mailing list
netmod@ietf.org
https://www.ietf.org/mailman/listinfo/netmod