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

"Eric Voit (evoit)" <evoit@cisco.com> Wed, 01 May 2019 17:09 UTC

Return-Path: <evoit@cisco.com>
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 BCD6E1200A4; Wed, 1 May 2019 10:09:20 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.502
X-Spam-Level:
X-Spam-Status: No, score=-14.502 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_MED=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_HI=-5, SPF_PASS=-0.001, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=cisco.com
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 4u7t5G4o5sy0; Wed, 1 May 2019 10:09:16 -0700 (PDT)
Received: from alln-iport-5.cisco.com (alln-iport-5.cisco.com [173.37.142.92]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id A17811200A2; Wed, 1 May 2019 10:09:15 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=46366; q=dns/txt; s=iport; t=1556730555; x=1557940155; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=obS29gQaf38yq5DJqdKizNy0P2H3V6dXD4OPMSlIM98=; b=k1qhQzBzz8ALy/vAKyNpnLk/YFBNC4xLxCY6vl8ZmkVC0WlH2QbQv5tg V315McRz+9VTrdDYK1yuJ8Wq1qZnzrWF9B4IYt8zBgbDkFu0mTLtAwJJZ +bZKygobhn9kTXaHI0K+aQfFQvHhOozoaDq4/8j43xj8mWt49EhHB79SA o=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0BDAACg0clc/5JdJa1cAQIHDgsBAQE?= =?us-ascii?q?BAQEBAQEBAQEHAQEBAQEBgWWBZyppgQQoCoNGQJUvfpdmgWcOAQEjhEoCF4Y?= =?us-ascii?q?bIzgTAQMBAQQBAQIBAm0cDIVKAQEBAQIBDAIMAQgRMQcGBQIFCwIBBgIVAwI?= =?us-ascii?q?CCRYHAgICMBUQAgQODRGCPkyBew8Pkg6bZYEvhEZBhSgGgQsniiGBKxeBQD+?= =?us-ascii?q?BEYIUSTU+gmEBAQIBAYEyAQQOAQSDHYJYBIp2AxImC4IIhGqBbpISZQkCggm?= =?us-ascii?q?CU4NEjCAjgg2GN4xxkleOFgIRFYEwNiGBVnAVO4I4ATSCGhcUbQECBodWhQQ?= =?us-ascii?q?7QTEBkSEBJQOBCIEhAQE?=
X-IronPort-AV: E=Sophos;i="5.60,418,1549929600"; d="scan'208";a="267325017"
Received: from rcdn-core-10.cisco.com ([173.37.93.146]) by alln-iport-5.cisco.com with ESMTP/TLS/DHE-RSA-SEED-SHA; 01 May 2019 17:09:13 +0000
Received: from XCH-RTP-011.cisco.com (xch-rtp-011.cisco.com [64.101.220.151]) by rcdn-core-10.cisco.com (8.15.2/8.15.2) with ESMTPS id x41H9D7T015899 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=FAIL); Wed, 1 May 2019 17:09:13 GMT
Received: from xch-rtp-013.cisco.com (64.101.220.153) by XCH-RTP-011.cisco.com (64.101.220.151) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Wed, 1 May 2019 13:09:12 -0400
Received: from xch-rtp-013.cisco.com ([64.101.220.153]) by XCH-RTP-013.cisco.com ([64.101.220.153]) with mapi id 15.00.1473.003; Wed, 1 May 2019 13:09:12 -0400
From: "Eric Voit (evoit)" <evoit@cisco.com>
To: Benjamin Kaduk <kaduk@mit.edu>
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>
Thread-Topic: Benjamin Kaduk's Discuss on draft-ietf-netconf-subscribed-notifications-24: (with DISCUSS and COMMENT)
Thread-Index: AQHU/5lDbPOEHPnGa0+OxIdeqCy7a6ZVXYMAgAFN5gD//8vZMA==
Date: Wed, 1 May 2019 17:09:11 +0000
Message-ID: <a1f34f171f134805af8c80b141867a27@XCH-RTP-013.cisco.com>
References: <155665867115.7536.12392383474714269681.idtracker@ietfa.amsl.com> <fcc4646114ab4048a2c464e77ecf3ca2@XCH-RTP-013.cisco.com> <20190501153725.GG59871@kduck.mit.edu>
In-Reply-To: <20190501153725.GG59871@kduck.mit.edu>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.118.56.233]
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
MIME-Version: 1.0
X-Outbound-SMTP-Client: 64.101.220.151, xch-rtp-011.cisco.com
X-Outbound-Node: rcdn-core-10.cisco.com
Archived-At: <https://mailarchive.ietf.org/arch/msg/netconf/h7-aaqiRXcTFch5X1ZSCMVAg-78>
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:09:21 -0000

> 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.

Eric

> -Ben