Re: [Netconf] LC on subscribed-notifications-10

[Martin Bjorklund <[e-mail]>]

Hi,

"Eric Voit (evoit)" <[e-mail]> wrote:
> Hi Martin,
> 
> > From: Martin Bjorklund, March 14, 2018 4:39 AM
> > 
> > Hi,
> > 
> > "Eric Voit (evoit)" <[e-mail]> wrote:
> > > > From: Robert Wilton -X, March 12, 2018 12:41 PM
> > > >
> > > > Hi,
> > > >
> > > > Some minor comments on this draft.  I've not read/reviewed all of it
> > > > yet, so some more comments may follow.  But generally I am
> > > > supportive of publishing this draft.
> > >
> > > Thanks Robert.
> > >
> > > I have incorporated tweaks as-per below.  These updates can be seen
> > > within the working draft -v11 at:
> > > https://github.com/netconf-wg/rfc5277bis/blob/master/draft-ietf-netcon
> > > f-subscribed-notifications-11.txt
> > >
> > > >    I'm happy to leave it to the authors discretion on whether and
> > > > how  they want to address these.
> > > >
> > > > 1. I noted that neither the title nor abstract mention YANG or
> > > > network configuration.  Nor do they mention that they define a YANG
> > > > data model.
> > >
> > > This is true, and I think ok.  Just like RFC-5277, it is possible to
> > > subscribe to non-YANG streams.
> > 
> > Perhaps "A YANG Data Model for Event Stream Subscriptions"?
> > 
> > Or something along these lines in the abstract.
> > 
> > Even though the streams may not be YANG-defined, the subscription
> > mechanism is YANG based.
> 
> Ok.   I made the first sentence of the abstract:
> "This document defines capabilities, operations, and a YANG Data Model
> for the customized establishment of subscriptions upon a publisher's
> event streams."

Ok.

> > [...]
> > 
> > > > - State change notification => "Subscription state change
> > > > - notifications"?
> > >
> > > Every time "state change notification" is used, the word subscription
> > > is easily matched up.  So it seems to me to be more readable to use a
> > > shorter term versus a longer one.
> > 
> > I think that in most cases you are right.  But I suggest you clarify
> > this the first
> > time the term is used, in 1.1:
> > 
> > OLD:
> > 
> >    o  state change notifications (e.g., publisher driven suspension,
> >       parameter modification)
> > 
> > NEW:
> > 
> >    o  subscription state change notifications (e.g., publisher driven
> >       suspension, parameter modification)
> 
> Done
> 
> > [...]
> > 
> > > > I wasn't sure how this tied in with YANG push where a client may not
> > > > have access to read some nodes.  In that case, nodes that can't be
> > > > read are left out (e.g. like a NETCONF GET request).  I know that
> > > > YANG push covers this is more detail, but didn't know whether this
> > > > paragraph correctly stands on its own?
> > >
> > > Current paragraph is correct.  The idea is you shouldn't allow someone
> > > to subscribe to a stream containing any content they are not allowed
> > > to see.  Access control is to the stream rather than the content.
> > 
> > But it seems that YANG push filters out the nodes that you don't have
> > access
> > to.
> 
> Yes.  For subscriptions to datastores, YANG push filters out data
> nodes for which the receiver has no access (just like a Get).

Ok.

> But for
> subscription to event streams, it is assumed that any event records
> placed on a stream permitted for that receiver is authorized content
> (just like RFC-5277).

Hmm.  This is not how it is defined in RFC 5277:

   After generation of the <notification> element, access control is
   applied by the server.  If a session does not have permission to
   receive the <notification>, then it is discarded for that session,
   and processing of the internal event is completed for that session.

Also, NACM is designed to drop notifications that the client doesn't
have access to.

> Effects like this are why the two drafts, as
> well as the YANG model targets and filters for datastores and to
> streams have been separated.
>
> > Your statement:
> > 
> >   Access control is to the stream rather than the content.
> > 
> > seems to imply that in order to subscribe to changes to the datastore,
> > you
> > need full access to all nodes covered by the filter.
> 
> As a stream and a datastore are different, hopefully my comment above
> clears this up.
> 
> Eric
> > /martin


/martin