Re: [netconf] Benjamin Kaduk's No Objection on draft-ietf-netconf-notification-capabilities-18: (with COMMENT)

[Benjamin Kaduk <[e-mail]>]

Hi Benoît,

Also inline

On Sun, Oct 10, 2021 at 10:01:11PM +0200, Benoit Claise wrote:
> Hi Benjamin,
> 
> Thanks for your review.
> See inline for addressing your most important comments (in coordination 
> with our AD)
> 
> On 10/7/2021 8:12 AM, Benjamin Kaduk via Datatracker wrote:
> > Benjamin Kaduk has entered the following ballot position for
> > draft-ietf-netconf-notification-capabilities-18: 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/blog/handling-iesg-ballot-positions/
> > for more information about how to handle DISCUSS and COMMENT positions.
> >
> >
> > The document, along with other ballot positions, can be found here:
> > https://datatracker.ietf.org/doc/draft-ietf-netconf-notification-capabilities/
> >
> >
> >
> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> >
> > Thanks to Barry Leiba for the secdir review.
> >
> > Section 1
> >
> >     Implementation-time information is needed by Network Management
> >     [...]
> >     new network node type can be introduced into the network.
> >
> >     Implementation-time information is needed by system integrators.
> >     [...]
> >     node type sometimes depends on these management possibilities.
> >
> > Commenting here since I read this draft after
> > draft-ietf-netmod-yang-instance-file-format, but there seems to be
> > pretty strong overlap between the discussion of how it's expensive to
> > require NMS implementors to have correclty configured nodes available,
> > the need for operators to have information about device capabilities
> > for purchasing decisions, etc.  It would be good to check that the
> > specific language used to describe these scenarios is in agreement
> > across documents, to the extent possible.  (The phrasing in this
> > document is probably a better starting point for the convergence, in my
> > opinion.)
> >
> > Section 4.2
> >
> >          1) search for a datastore-capabilities list entry for
> >          the specific datastore. When stating a specific capability, the
> >          relative path for any specific capability must be the same
> >          under the system-capabilities container and under the
> >          per-node-capabilities list: the same grouping for defining
> >          the capabilities MUST be used.
> >
> > It's a little surprising to find the "MUST use same grouping"
> > requirement buried in the procedure for finding a capability value for a
> > specific data node in a specific datastore.
> We propose to  delete “the same grouping for definingthe capabilities 
> MUST be used.”, and instead pull this out to a separate paragraph in the 
> module description:
> 
>     The same grouping MUST be used to define hierarchical capabilities
>     supported both at the system level and datastore/data node level.

Excellent; thanks!

> > Section 5.2
> >
> >             leaf minimum-update-period {
> >               [...]
> >               description
> >                 "Indicates the minimal update period that is
> >                  supported for a 'periodic' subscription.
> >
> >                  A subscription request to the selected data nodes with
> >                  a smaller period than what this leaf specifies is
> >                  likely to result in a 'period-unsupported' error.";
> >               [...]
> >             }
> >             leaf-list supported-update-period {
> >               [...]
> >               description
> >                 "Supported update period values for a 'periodic'
> >                  subscription.
> >
> >                  A subscription request to the selected data nodes with a
> >                  period not included in the leaf-list will result in a
> >                  'period-unsupported' error.";
> >
> > Is it intended that these disagree on whether it is "likely to result"
> > or "will result" in a period-unsupported error?
> >
> >           leaf-list supported-excluded-change-type {
> >             if-feature "yp:on-change";
> >             type union {
> >               type enumeration {
> >                 enum none {
> >                   value -2;
> >                   description
> >                     "None of the change types can be excluded.";
> >                 }
> >                 enum all {
> >                   value -1;
> >                   description
> >                     "Any combination of change types can be excluded.";
> >                 }
> >               }
> >               type yp:change-type;
> >
> > It seems like this allows for some nonsensical edge cases, like a list
> > that includes both "none" and "any", or all the actual values from RFC
> > 8641 as well as "none", etc.  Do we need to say anything about how to
> > handle such invalid scenarios?
> >
> > Section 6
> >
> > I agree with Roman that it would be nice to be able to say something
> > about the potential scope of security considerations for future
> > augmentations, if nothing else to aid authors of such future
> > augmentations in documenting their own security considerations.
> We now have in the security considerations section.
> 
>        All protocol-accessible data nodes in augmented modules are 
> read-only
>          and cannot be modified

Thanks, that's worth stating clearly.
I was also imagining something like "This document outlines a framework for
conveying system capability information that is inherently flexible and
extensible.  While the full set of use cases is not known now, they may range
as wide as conveying the minimum update period for periodic subscription
updates and what protocols might be used for such notifications.  Knowledge
of this type of value might, for example, allow an attacker to gain insight
into how long unauthorized configuration changes might be active prior to
detection, and what communications channels might be disrupted to extend
the period of non-detection.  Documents adding additional capabilities via
augmenting this module are encouraged to document the security
considerations of the new YANG nodes, according to the guidance in BCP
216."

(Note that my imagination is probably not very broad about how wide the
scope for future augmentation is, and I would be happy to see the specifics
of the above text changed as appropriate.)
Anyway, that's just my thought; do with it what you may.

Thanks again,

Ben

> > NITS
> >
> > Section 4.2, 5.2
> >
> > I think only one "<CODE ENDS>" tag is needed.
> Indeed, already addressed in the latest version.
> 
> Thanks and regards, Benoit
> >
> >
> >
> > .
>