Re: [netconf] Benjamin Kaduk's Discuss on draft-ietf-netconf-subscribed-notifications-24: (with DISCUSS and COMMENT)

Benjamin Kaduk <kaduk@mit.edu> Wed, 01 May 2019 17:12 UTC

Return-Path: <kaduk@mit.edu>
X-Original-To: netconf@ietfa.amsl.com
Delivered-To: netconf@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id D813D1200A2; Wed, 1 May 2019 10:12:48 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.201
X-Spam-Level:
X-Spam-Status: No, score=-4.201 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id A7tHqtpf45E7; Wed, 1 May 2019 10:12:45 -0700 (PDT)
Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id A1B5C12008F; Wed, 1 May 2019 10:12:44 -0700 (PDT)
Received: from kduck.mit.edu (24-107-191-124.dhcp.stls.mo.charter.com [24.107.191.124]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id x41HCcOt025983 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 1 May 2019 13:12:40 -0400
Date: Wed, 1 May 2019 12:12:38 -0500
From: Benjamin Kaduk <kaduk@mit.edu>
To: "Eric Voit (evoit)" <evoit@cisco.com>
Cc: The IESG <iesg@ietf.org>, "draft-ietf-netconf-subscribed-notifications@ietf.org" <draft-ietf-netconf-subscribed-notifications@ietf.org>, Kent Watsen <kent+ietf@watsen.net>, "netconf-chairs@ietf.org" <netconf-chairs@ietf.org>, "netconf@ietf.org" <netconf@ietf.org>
Message-ID: <20190501171238.GK59871@kduck.mit.edu>
References: <155665867115.7536.12392383474714269681.idtracker@ietfa.amsl.com> <fcc4646114ab4048a2c464e77ecf3ca2@XCH-RTP-013.cisco.com> <20190501153725.GG59871@kduck.mit.edu> <a1f34f171f134805af8c80b141867a27@XCH-RTP-013.cisco.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <a1f34f171f134805af8c80b141867a27@XCH-RTP-013.cisco.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Archived-At: <https://mailarchive.ietf.org/arch/msg/netconf/pyoiv5jqt_o84be538zvOlDoDBY>
Subject: Re: [netconf] Benjamin Kaduk's Discuss on draft-ietf-netconf-subscribed-notifications-24: (with DISCUSS and COMMENT)
X-BeenThere: netconf@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: NETCONF WG list <netconf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netconf>, <mailto:netconf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netconf/>
List-Post: <mailto:netconf@ietf.org>
List-Help: <mailto:netconf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netconf>, <mailto:netconf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 01 May 2019 17:12:49 -0000

On Wed, May 01, 2019 at 05:09:11PM +0000, Eric Voit (evoit) wrote:
> > From: Benjamin Kaduk,  May 1, 2019 11:37 AM
> > 
> > On Wed, May 01, 2019 at 05:48:45AM +0000, Eric Voit (evoit) wrote:
> > > Hi Benjamin,
> > >
> > > > From: Benjamin Kaduk, April 30, 2019 5:11 PM
> > > >
> > > > Benjamin Kaduk has entered the following ballot position for
> > > > draft-ietf-netconf-subscribed-notifications-24: Discuss
> > > >
> > > > --------------------------------------------------------------------
> > > > --
> > > > DISCUSS:
> > > > --------------------------------------------------------------------
> > > > --
> > > >
> > > > Thanks for this document; I just have a few minor "housekeeping"
> > > > points that should get resolved before publication.  (Please also
> > > > note the substantive comments in the COMMENT section as well,
> > > > particularly those relating to the transport requirements and
> > > > security considerations.)
> > > >
> > > > I'm not sure that we state clearly enough what is required to have a
> > > > specification for a transport for notifications.  Specifically (see
> > > > COMMENT), in the module we seem to say that the specification must
> > > > indicate what the default encoding is to be used if not otherwise
> > > > specified, but that's not enumerated as a requirement on such a specification
> > anywhere I see.
> > >
> > > I provide more info in-line below with your comment.  But here is a summary...
> > >
> > > For dynamic subscriptions, the default encoding is normally specified by
> > whatever is used within the establish-subscription RPC.  So there is not a need
> > for a default here as the subscriber has already encoded in the RPC what it
> > wants.
> > >
> > > For configured subscriptions, the default (and only) encoding is often driven by
> > the transport selection.  And this is defined by existing transport RFCs which
> > themselves define supported encodings (e.g., "NETCONF + XML" , "RESTCONF +
> > JSON").
> > >
> > > But encodings can vary (e.g., the use of CBOR within draft-birkholz-yang-push-
> > coap-problemstatement).  Supporting this need is the purpose of the identity
> > "configurable-encoding" in the YANG model.   And this identity does say that
> > "Further details for any specific configurable encoding would be explored in a
> > transport document based on this specification."  I guess this information could
> > be repeated as a separate requirement in Section 5.3 if you feel this is
> > insufficiently defined.
> > 
> > I'm mostly concerned about the case of configured subscriptions and future
> > extensibility.  If someone wants to make a new "QUIC + CBOR" transport, do we
> > have a clear list of what details that specification needs to provide?
> > (From my reading of this document, there should be an explicit "this is the
> > default encoding" statement, even if there is only one encoding specified in the
> > transport specification.)
> 
> I have added the following statement to Section 5.3...
> "A specific transport specification MUST identity any encoding supported.  Where a configured subscription's transport allows different encodings, the specification MUST identify the default       encoding."
> 
>  
> > > > I also think that we could
> > > > probably require (as opposed to "highly recommend" in the current
> > > > security
> > > > considerations) that the transport provide message confidentiality
> > > > and integrity protection.
> > >
> > > I for sure like the idea of encryption. And I absolutely like the idea of
> > signatures too.  However several years ago on based on work with MSDC / cloud
> > providers desiring telemetry, there were requests for deployments where the
> > volume of telemetry generated could overwhelm the CPU of the host router.
> > And as the telemetry was being delivered within a fully protected MSDC
> > domain/environment, these MSDC providers said they wanted *all* the router
> > data more than they needed message confidentiality and integrity protection.
> > I.e., the wanted to have more data rather than being constrained the host CPU
> > doing functions which reduced the volume of data being delivered.
> > >
> > > So personally, I want low volume telemetry with full security protections
> > applied.  But by leaving it at "highly recommend" we don't unintentionally inhibit
> > applicability to MSDC implementations in a protected domain.  They explicitly
> > said they didn't want it.
> > 
> > We do sometimes have cases where we leave an opening for people to assert
> > that the physical security of their network provides "equivalent protection" to
> > cryptographic confidentiality/integrity protection, if we want to try to
> > wordsmith something.  But, given the above, I won't push very hard on this
> > front.
> 
> Ok, will leave as is.
> 
> > > > I'm also unsure that I properly understand the use of the "reset"
> > > > RPC -- if it has no effect when transit connectivity already exists,
> > > > then what entity is going to call "reset" in the case of publisher timeout when
> > trying to reach a receiver?
> > > > Surely not the publisher itself, since that would just be
> > > > publisher-internal functionality with no need for an external-facing RPC.
> > >
> > > The reset RPC of section 2.5.5 is YANG model exposed operational knob based
> > on the YANG 1.1 language construct "action":
> > > https://tools.ietf.org/html/rfc7950#section-7.15
> > >
> > > An operational interface can call this action.  I.e., an entity with the proper
> > administrative privileges can access "reset".    This means a REST interface could
> > be exposed upon a controller for that publisher.  And when somebody find that
> > the subscription isn't responsive (for a variety of reasons) that REST interface
> > can be called, and the publisher can start trying to make connections (such as
> > RFC 8071 call home connections) to a receiver.
> > 
> > Okay, thanks for helping me out here; consider this one resolved.
> > 
> > >
> > > > I'm also a little unclear on the mechanics of the list of
> > > > subscriptions described in Section 3.3.  Does it provide a way for a
> > > > low-privilege client (that can only access one or a handful of the
> > > > subscriptions) to enumerate all subscriptions that exist, including
> > > > subscriptions used by high-privilege clients?  If so, we may want to
> > > > think about some security considerations text to that effect; if
> > > > not, we may want to say that the access-control will limit which leafs are
> > visible to some clients.
> > >
> > > Your functional requirements completely valid.  I believe they are covered by a
> > set of other YANG RFC which dictate who/what can access various elements of
> > YANG data.  Good example documents to look at here are ones like:
> > >
> > > RFC-8341 - Network Configuration Access Control Model
> > >
> > > An understanding of these documents are assumed, and listed in places like the
> > security considerations in Section 5.4.
> > 
> > I admit to having not fully absorbed the NACM, and given the number of
> > documents I have left to ballot on this week, I'll take your word that it's covered
> > well enough there.
> > 
> > > > Finally, we have a few instances of "leaf filter-failure-hint"
> > > > that's of type "string", providing
> > > >              "Information describing where and/or why a provided filter
> > > >               was unsupportable for a subscription."; I don't
> > > > understand why it's a string as opposed to some form of
> > > > machine-readable data.  Is it supposed to be human-readable?  Does that
> > bring in any internationalization considerations?
> > >
> > > There are many reasons why a filter might fail.  The authors didn't believe that
> > there was enough operational experience to sufficiently document the universe
> > of failure types across the set of interested parties.  As a result, the option
> > seemed to be to provide no guidance on the failure reason, or to let the vendors
> > provide some guidance (albeit just a 'string').  So at this point with the
> > specification, it would be up to the vendor to determine whether it is human
> > readable, or whether internationalization considerations should be covered.
> > Hopefully with enough deployments this might be revisited at a future date.
> > 
> > We should probably think about saying something about how its syntax and
> > semantics are implementation-specific, then -- there doesn't seem to be real
> > guidance for how to use it, at present.
> 
> Added text in leaf to clarify this approach...
> 
>       leaf filter-failure-hint { 
>         type string;
>           description
>             "Information describing where and/or why a provided filter
>              was unsupportable for a subscription. The syntax and
>              semantics of this hint are implementation-specific.";
>       }
> 
> > > > --------------------------------------------------------------------
> > > > --
> > > > COMMENT:
> > > > --------------------------------------------------------------------
> > > > --
> > > >
> > > > Section 1.3
> > > >
> > > >    Also note that transport specific transport drafts based on this
> > > >    specification MUST detail the life cycle of dynamic subscriptions, as
> > > >    well as the lifecycle of configured subscriptions (if supported).
> > > >
> > > > I'm not sure I understand what additional specification is needed
> > > > for the lifecycle of configured subscriptions -- doesn't the
> > > > previous text tightly tie their lifecycle to the presence of configuration in the
> > configuration datastore?
> > >
> > > For the most part it does.  However there is a relationship for exactly when to
> > establish transport connectivity based on the state of each receiver of a
> > configured subscription.
> > >
> > > As an example of what should be specified, see Section 5.2 of
> > > https://datatracker.ietf.org/doc/draft-ietf-netconf-netconf-event-noti
> > > fications/10/ Here you can see how NETCONF call home is invoked at the
> > > proper points in the configured subscription state machine.
> > 
> > Ah, thanks for the pointer.
> > 
> > > > nit: please be consistent about "life cycle" vs. "lifecycle" throughout.
> > >
> > > Fixed nit to "lifecycle".
> > >
> > > > Section 2.1
> > > >
> > > >    An event stream is a named entity on a publisher which exposes a
> > > >    continuously updating set of YANG encoded event records.  [...]
> > > >
> > > > nit: I don't think "YANG encoded" is well-defined (here and below),
> > > > in that YANG structures generate data models but can be encoded into
> > > > various formats (like XML and JSON).
> > >
> > > This is true.  I made the two occurrences of "YANG encoded" into "YANG
> > defined".
> > >
> > > > Section 2.3
> > > >
> > > >    If the publisher supports the "dscp" feature, then a subscription
> > > >    with a "dscp" leaf MUST result in a corresponding [RFC2474] DSCP
> > > >    marking being placed within the IP header of any resulting
> > > >    notification messages and subscription state change notifications.
> > > >    Where TCP is used, a publisher which supports the "dscp" feature
> > > >    SHOULD ensure that a subscription's notification messages are
> > > >    returned within a single TCP transport session where all traffic
> > > >    shares the subscription's "dscp" leaf value.  Where this cannot be
> > > >    guaranteed, any "establish subscription" RPC request SHOULD be
> > > >    rejected with a "dscp-unavailable" error
> > > >
> > > > I can't decide whether we need to be more explicit in the second
> > > > and/or third sentences that this remains contingent on the
> > > > subscription in question having a "dscp" leaf.
> > >
> > > The sentences were already long, it feels to me like it would be more
> > confusing to put in more words here.
> > >
> > > > nit: end sentence with a full stop
> > >
> > > Period added.
> > >
> > > > I share the TSV-ART reviewer's confusion about how the weighting (as
> > > > well as
> > > > DSCP) functionality is intended to work.
> > >
> > > Short answer:  Earlier versions of this draft made explicit parallels of the
> > weighting method to HTTP2 RFC-7450 section 5.3.2.  The operation of weighting
> > is exactly the same HTTP2 dependency weighting.  There is additional definition
> > of how this is supposed to work in Section 4 of draft-ietf-netconf-restconf-notif
> > (which is also currently up for review).
> > >
> > > Long answer: TSV-ART is absolutely correct that a publisher might generate a
> > lot of traffic.  In many ways, high traffic volumes would be a successful outcome
> > for this work.   As a result, mitigating different dimensions of this volume risk has
> > been a design goal since this effort’s inception.  And this is the reason robust
> > QoS mechanisms were included.  This includes both DSCP and weighting.
> > >
> > > Here is a quick summary of some QoS mechanisms available in this draft...
> > >
> > > 1. The publisher is allowed to decline a dynamic subscription.  One of these
> > reasons is that the incremental traffic generated by a particular incremental
> > subscription will be too high.  There are errors back to the subscriber indicating
> > this condition exist.
> > > 2. A publisher can suspend any subscription for capacity reasons, and a
> > receiver must be able to gracefully accept this suspension.
> > > 3. Much like with HTTP2 streams, higher priority subscriptions intended for a
> > particular receiver can be dequeued first from a publisher.
> > > 4. Much like with HTTP2 streams, proportional subscription dequeuing
> > intended for a particular receiver can be performed a publisher.
> > > 5. DSCP markings can be placed on subscribed data.
> > > 6. Mechanisms for detecting and reacting to different types of subscribed data
> > loss have been embedded.
> > > 7. Methods have been included to ensure a configured receiver “ok’s”
> > > information push before subscribed data is sent. (This should reduce one DDoS
> > vector.) 8. Keep alive mechanisms are established for different transport types,
> > so that subscribed data isn’t being sent when the is no receiver listening.
> > >
> > > Mechanism (4) is in question here.   As defined by draft-ietf-netconf-restconf-
> > notif, this is supposed to work identically to HTTP2 RFC-7450 section 5.3.2.
> > However there were WG members who did not want to include a functional
> > requirement normative reference to that HTTP2 transport in this document
> > however.  Therefore the reference to HTP2 was removed.
> > >
> > > In the end, I don't think we can afford to get rid of support for (4) from the
> > specification.   This weighting capability is quite powerful, and can easily be
> > added to GRPC based subscription alternatives.
> > 
> > I don't think I was trying to suggest removing the mechanism entirely.
> > What you've written here includes a mental model of having local queues of
> > messages that get dequeued according to a specific algorithm, and the weights
> > influencing the rate of dequeuing across queues; that mental model (combined
> > with the knowledge that larger weight values get more traffic
> > faster) gives me the picture I wanted for "how it's supposed to work".
> > Maybe we could distill that down a bit and put it in the draft, as the current text
> > had some level of "magic occurs here", to me.
> 
> I am hoping to leave things as they are for now.  Basically the magic that occurs can be gleaned from Section 4 of draft-ietf-netconf-restconf-notif.   And nobody in the NETCONF world is likely to implement this type of Dequeuing, so they shouldn't have the extra text in the base specification trip them up..   
> 
> > > > Section 2.4.2.1
> > > >
> > > >    Replay provides the ability to establish a subscription which is also
> > > >    capable of passing recently generated event records.  In other
> > > > words,
> > > >
> > > > nit/editorial: this language could probably be more clear about
> > > > "recently generated" meaning "in the past", that is, records for
> > > > events that have already happened when the subscription is established.
> > >
> > > Tweaked to "event records generated in the recent past"
> > >
> > > > Section 2.4.3
> > > >
> > > >    any number of times.  Dynamic subscriptions can only be modified via
> > > >    this RPC using a transport session connecting to the subscriber.
> > > > If
> > > >
> > > > nit(?): is this more readable as "connecting to" or "connecting from"
> > > > the subscriber?  (The same wording shows up throughout the document,
> > > > but I'll try to just comment once.)
> > >
> > > I expect that it should be clear enough either way.  I believe leaving the current
> > text should not impact implementations.
> > >
> > > > Section 2.4.5
> > > >
> > > > Is there any distinction between "killing a subscription" and
> > > > "administratively deleting a subscription"?  It's unclear to me that
> > > > we need the extra connotations of the word "kill", here.
> > >
> > > We chose to use a different RPC name so that administrative access control
> > permissions differences between the kill-subscription and delete-subscription
> > would be explicit and obvious.  And once we had a new RPC name, it seemed
> > easier for the reader to continue using the "kill" word for that RPC.
> > 
> > In vacuum, "admin-delete-subscription" seems like a fine name, to me.  But this
> > is a non-blocking comment so I'll stop pushing now.
> > 
> > > > Section 2.4.6
> > > >
> > > >    As a result of this mixture, how subscription errors are encoded
> > > >
> > > > nit: "mixture" doesn't seem like the right word to me; "variety" or
> > > > "varied population" are the first alternatives that come to mind,
> > > > though they are not perfect either.
> > >
> > > Changed to 'variety'.
> > >
> > > > Is there some sort of "access denied" error that could result from
> > > > kill- subscription RPCs?  (The table/artwork only lists
> > > > "no-such-subscription".)
> > >
> > > Access denied when an RPC fails access control is part of the base transport
> > error types for the RPC.   I.e., transports like NETCONF and RESTCONF already
> > have non-subscription-specific errors like this one already covered.
> > 
> > Ah, of course, and we require processing of transport-level errors before these
> > ones, so it all works out fine.
> > 
> > > > Section 2.5
> > > >
> > > > It's interesting to see a slight dichotomy between dynamic and
> > > > configured subscriptions, in that (IIUC) for dynamic subscriptions,
> > > > a modification event un- suspends a subscription immediately, but
> > > > for configured subscriptions the publisher seems to have some
> > > > latitude to retain the subscription in the suspended state for some
> > > > time before un-suspending it and sending the "subscription-modified" state
> > change message.
> > > >
> > > >                  In this case, a separate dynamic subscription can be
> > > >    established to retransmit the missing event records.
> > > >
> > > > Do you want to put in a pointer to replay-start-time here?
> > >
> > > I thought getting to that level of detail in the intro was confusing for this intro
> > section.
> > 
> > Okay.  (Maybe a parenthetical "replay" as a search term would help without
> > being confusing, but entirely your call.)
> 
> I will leave as-is.
> 
> > > > Section 2.5.1
> > > >
> > > >          Event records are only sent to active receivers.  Receivers of
> > > >    a configured subscription remain active if both transport
> > > >    connectivity can be verified to the receiver, and event records are
> > > >    not being dropped due to a publisher buffer overflow.  [...]
> > > >
> > > > nit: "buffer overflow" is a technical term in security circles
> > > > indicating a potentially exploitable security flaw; would "publisher
> > > > buffer capacity being reached" be an acceptable substitute (here and
> > below)?
> > >
> > > Change made
> > >
> > > > Section 2.7.7
> > > >
> > > >    This notification indicates that all of the event records prior to
> > > >    the current time have been passed to a receiver.  It is sent before
> > > >    any notification message containing an event record with a timestamp
> > > >    later than (1) the "stop-time" or (2) the subscription's start time.
> > > >
> > > > nit(?): this text seems to imply that a notification message with a
> > > > timestamp later than the "stop-time" might actually be sent, which IIUC is
> > not the case.
> > >
> > > You are correct that it is not sent.  But the event record does exist on the
> > stream.
> > >
> > > I think it obvious, but if you want I could add the following sentence to the end
> > of the subsequent paragraph.  "If a stop-time has been exceed during replay, no
> > subsequent event records are sent following the sending of a "replay-
> > completed" notification."
> > 
> > I don't think there's a need to add that sentence, but thanks for offering.
> > 
> > >
> > > > Section 2.9
> > > >
> > > > nit: the name "supports-vrf" for the feature doesn't really parallel
> > > > the other feature names, that don't include a word like "support"
> > > > and rather just describe the actual feature.
> > >
> > > This is true.  Several years ago when someone proposed this name, there was
> > a reason.  But I can't remember what it is right now.  So hopefully we can leave
> > it so as not to break someone's thinking.
> > >
> > > > Section 3.1
> > > >
> > > > Is there any risk of collision in event stream names from
> > > > vendor-specific streams?  (We don't seem to create an IANA registry
> > > > for them, for example.)
> > >
> > > In theory yes.  But it has not proven to be an issue with RFC-5277 streams, so it
> > doesn't seem worth it to do something new now.   So in this document, we have
> > one IETF stream "NETCONF" defined in Section 2.1.  Hopefully it is enough to
> > hard-code this.   We can always revisit if IETF groups want to start defining
> > streams
> > >
> > > > Section 4
> > > >
> > > >    identity subscription-suspended-reason {
> > > >       description
> > > >        "Problem condition communicated to a receiver as part of a
> > > >        'subscription-terminated' notification.";
> > > >    }
> > > >
> > > >    identity subscription-terminated-reason {
> > > >       description
> > > >        "Problem condition communicated to a receiver as part of a
> > > >        'subscription-terminated' notification.";
> > > >    }
> > > >
> > > > Are these descriptions supposed to be the same?
> > >
> > > Good catch.   Fixed the 'subscription-suspended' description.
> > >
> > > >    identity configurable-encoding {
> > > >      description
> > > >        "If a transport identity derives from this identity, it means
> > > >         that it supports configurable encodings.  An example of a
> > > > [...]
> > > >
> > > > Is it intended that such future transports (or future encodings?)
> > > > will derive from both this and the normal base identity (i.e., "transport")?
> > > > I guess I'm asking why this identity does not derive from the
> > > > corresponding base, but that's just a style question and not really
> > > > a protocol matter, making this question a side note.
> > >
> > > Yes.  Future identities can derive from configurable-encoding
> > >
> > > >      leaf weighting {
> > > >        if-feature "qos";
> > > >        type uint8 {
> > > >           range "0 .. 255";
> > > >        }
> > > >        description
> > > >          "Relative weighting for a subscription. Allows an underlying
> > > >           transport layer perform informed load balance allocations
> > > >           between various subscriptions";
> > > >        reference
> > > >          "RFC-7540, section 5.3.2";
> > > >
> > > > Do we want to chase the reference for readers and say that larger
> > > > weights get more resources?
> > >
> > > I put in "Larger weights get more resources." as sentence two of the
> > description.
> > >
> > > >      leaf encoding {
> > > >        when 'not(../transport) or derived-from(../transport,
> > > >        "sn:configurable-encoding")';
> > > >        type encoding;
> > > >        description
> > > >          "The type of encoding for notification messages.   For a
> > > >          dynamic subscription, if not included as part of an establish-
> > > >          subscription RPC, the encoding will be populated with the
> > > >          encoding used by that RPC.  For a configured subscription, if
> > > >          not explicitly configured the encoding with be the default
> > > >          encoding for an underlying transport.";
> > > >
> > > > Where is the default encoding for an underlying transport specified?
> > > > Section 5.3 does not seem to say that a transport specification must
> > > > provide this information, nor does Section 1.3 (which mentions that
> > > > transport specifications must detail the lifecycle of dynamic
> > > > subscriptions), nor does Section 2.5.7 (that discusses the need for
> > > > a separate model augmenting
> > > > /subscriptions/subscription/receivers/receiver
> > > > to provide transport details).
> > >
> > > For many transports, the encoding is redundant information.  The
> > > reason is that transport RFCs themselves define supported encodings
> > > (e.g., "NETCONF + XML" , "RESTCONF + JSON").  And WG members who built
> > > those transport RFCs did not want to allow operators to misconfigure
> > > anything here.  (As an aside, this desire to significantly reduce the
> > > opportunity for misconfiguration is what drove the interesting 'when'
> > > statement above.)
> > >
> > > But encodings can vary.  This is the purpose of the identity "configurable-
> > encoding" in the YANG model.   And this identity does say that "Further details
> > for any specific configurable encoding would be explored in a transport
> > document based on this specification."  I guess this information could be
> > repeated in Section 5.3 if necessary.
> > 
> > I think that duplication would be my preference, to get things consolidated in
> > one place.
> 
> Per above in the discuss, text added to 5.3.
> 
> > > >        choice notification-message-origin {
> > > >          if-feature "configured";
> > > >          description
> > > >            "Identifies the egress interface on the publisher from which
> > > >             notification messages are to be sent.";
> > > >
> > > > nit: given the address-originated case, perhaps "the egress interface"
> > > > is not quite correct anymore.
> > >
> > > Every alternative I come up with seems more problematic.   I believe readers
> > should be able to understand based on the cases below.
> > 
> > (I don't have any better suggestions, either.)
> > 
> > > >                enum connecting {
> > > >                  value 3;
> > > >                  if-feature "configured";
> > > >                  description
> > > >                    "A subscription has been configured, but a
> > > >                    'subscription-started' subscription state change
> > > >                    notification needs to be successfully received before
> > > >                    notification messages are sent.
> > > >
> > > > nit: successful receipt happens on the receiver but sending happens
> > > > on the publisher, so there's a bit of a mismatch in the sentence
> > > > subject here.  Perhaps we could talk about "successfully sent" state-changes
> > to resolve things.
> > >
> > > This wording is as intended.  Basically it is highly desirable for
> > > configured receivers will need to acknowledgement of successful
> > > receipt of a "subscription-started" before sending event records.
> > > This helps prevent DDOS attacks.  The mechanism for gaining an
> > > acknowledgement varies by transport.  As an example of what we have
> > > been thinking about here, see
> > >
> > > https://datatracker.ietf.org/doc/draft-ietf-netconf-restconf-notif/04/
> > Section 3.1.2
> > > Hopefully this type of mechanism will be revived in future drafts.
> > 
> > I agree that getting explicit acknowledgment of delivery is highly desirable; my
> > qualm here is solely about the grammar of the sentence.
> > Does it preserve the desired meaning to replace "successfully received"
> > with "successfully acknowledged" (or "successfully received and
> > acknowledged")?
> 
> I would love to have had acknowledged.  However I have heard from implementers that NETCONF is not as robust in this way as HTTP.  And acknowledged might imply things to developers they might not be able to deliver.
> 
> > > >              config false;
> > > >              mandatory true;
> > > >              description
> > > >                "Specifies the state of a subscription from the
> > > >                perspective of a particular receiver.  With this info it
> > > >                is possible to determine whether a subscriber is
> > > >                currently generating notification messages intended for
> > > >                that receiver.";
> > > >
> > > > nit: is it the subscriber that is generating messages or the publisher?
> > >
> > > Good catch.  Changed to publisher.
> > >
> > > > Section 5.3
> > > >
> > > >    A secure transport is highly recommended and the publisher MUST
> > > >    ensure that the receiver has sufficient authorization to perform the
> > > >    function they are requesting against the specific subset of content
> > > >    involved.
> > > >
> > > > "secure transport" usually means that it provides message
> > > > confidentiality, integrity protection, and source authenticity (akin to the
> > mutual authentication).
> > > > This is qualitatively different from implementing authorization
> > > > checks, and it's surprising to see them squashed into the same sentence.
> > >
> > > Tweaked to "A secure transport is highly recommended.  Beyond this, the
> > publisher MUST".
> > >
> > > > Do we want to say anything about discovery for support of new
> > > > transports, and what workflow would be used to negotiate the use of a new
> > transport?
> > >
> > > Not at this point.
> > 
> > Okay.
> > 
> > > > Section 5.4
> > > >
> > > > I can think of a couple other considerations to mention here:
> > > >
> > > > - Using DNS names for receiver "name" lookup can cause situations where
> > > >   the name resolves differently on the publisher and subscriber, so the
> > > >   recipient would be different than expected.
> > > >
> > > > - Using the publisher's boot time for deduplication protection on
> > > >   replayed messages introduces a dependency on accurate time.  We don't
> > > >   have a great secure time story at present, so this is more of a
> > > >   "beware of risk" situation than something we can mitigate, but it does
> > > >   seem that an attacker that could (e.g.) spoof the NTP traffic to the
> > > >   publisher on boot could cause it to send duplicate notifications to
> > > >   recipients that requested historical data.
> > >
> > > I tweaked and inserted the two paragraphs from you...
> > >
> > > "Using DNS names for configured subscription receiver "name" lookup can
> > cause situations where the name resolves unexpectedly differently on the
> > publisher, so the recipient would be different than expected."
> > >
> > > "Using the publisher's boot time for deduplication protection on replayed
> > messages introduces a dependency on accurate time."
> > 
> > Can we also say something like "An attacker that can cause the publisher to use
> > an incorrect time can induce message replay by setting the time in the past, and
> > introduce a risk of message loss by setting the time in the future"?
> 
> Text you suggest above has been inserted instead of " Using the publisher's boot time for deduplication protection...".
> 
> > >
> > > > Some other comments on what's already there (which is pretty good;
> > > > thanks for
> > > > it!) follow.
> > > >
> > > >    Container: "/filters"
> > > >
> > > >    o  "stream-subtree-filter": updating a filter could increase the
> > > >       computational complexity of all referencing subscriptions.
> > > >
> > > >    o  "stream-xpath-filter": updating a filter could increase the
> > > >       computational complexity of all referencing subscriptions.
> > > >
> > > > Do we want to say anything about modifying either of these causing
> > > > the set of notifications delivered to recipients to shrink (or become
> > unmanageably large)?
> > > > I guess it may not be necessary, since the recipient gets a
> > > > notification about the modification that includes the details of the filter.
> > >
> > > Yes, that was the thinking.
> > >
> > > >    Container: "/subscriptions"
> > > >
> > > >    o  "excluded-event-records": leaf can provide information about
> > > >       filtered event records.  A network operator should have
> > > >       permissions to know about such filtering.
> > > >
> > > > To be clear, this is event records filtered either via explicit
> > > > filter or via access control restrictions.
> > >
> > > Yes.
> > 
> > I think that the semantic difference is worth noting.
> > 
> > > > Specifically, it can allow a receiver to learn that there exists
> > > > access controls that deny it access to some data, in the case when
> > > > they did not apply any filtering rules explicitly.
> > >
> > > Yes.
> > >
> > > >This potential for information leakage (of the  existence of
> > > > ACLs) should be mentioned explicitly.
> > >
> > > Added the sentence: "Improper configuration could provide a receiver with
> > information leakage consisting of the dropping of event records."
> > 
> > Can I counter-propose: "Improper configuration could allow a receiver to learn
> > that event records were dropped due to an ACL when the existence of that ACL
> > would otherwise be transparent."
> > 
> > >
> > > > Appendix A
> > > >
> > > > This example transport module does not specify the life cycle of
> > > > dynamic subscriptions per Section 1.3, and a couple other things
> > > > required from a transport module specification.  Perhaps we are okay
> > > > claiming that since this is just an example YANG model and not a
> > > > full transport example, the omission is okay, but it may be worth mentioning
> > the omission for clarity.
> > >
> > > I added as the second sentence
> > > " This example is not intended to be a complete transport model."
> > >
> > > Thanks again for the review, it was excellently done.
> > 
> > And thanks for the clear explanations of the parts that I didn't understand, it was
> > very helpful to me.
> 
> No problem.  This thread has been good for me too.  I will post v25 as soon as you give the ok.

Please go ahead and post v25.

Thanks,

Ben