[netconf] AD review of draft-ietf-netconf-https-notif-12

["Rob Wilton (rwilton)" <[e-mail]>]

Hi Kent, Mahesh,

Thanks for a well written document and thank you to Quifang for the great and detailed shepherd writeup. I think that the document is in great shape and I only have minor comments.


Minor level comments:

(1) p 4, sec 3.1.  Applicability

   For publishers using Subscription to YANG Notifications [RFC8639],
   dynamic discovery of a receiver's supported encoding is necessary
   only when the "/subscriptions/subscription/encoding" leaf is not
   configured, per the "encoding" leaf's description statement in the
   "ietf-subscribed-notification" module.

I presume that this is only for configured subscriptions (or otherwise this document would need to update RFC 8639, specificially the 3rd bullet in section 2.4.2.)  Would it be helpful to clarify here at all?


(2) p 5, sec 3.2.  Request

   To learn the capabilities of a receiver, a publisher can issue an
   HTTPS GET request to the "capabilities" resource (see Section 2) on
   the receiver with "Accept" header set using the "application/xml"
   and/or "application/json" media-types, with the latter as mandatory
   to implement, and the default in case the type is not specified.

I'm not suggesting that you change the text, but when I first read this I thought that the Accept header was being used to indicate what encoding the server supported, or preferred, rather that just being used to select the encoding of the capabilities.


(3) p 7, sec 4.2.  Response

   The response should be "204 (No Content)".

Is this response still used if the client thought that the notification was corrupted or malformed?


(4) p 16, sec 7.  Security Considerations

   *  The "path" node in "ietf-subscribed-notif-receivers" module can be
      modified by a malicious user to point to an invalid URI.

Are there other fields that would also be sensitive if they were written, e.g., overwriting the certificate, or cert-to-name mapping?  Or changing the remote address or port?


(5) p 25, sec Appendix A.  Configuration Examples

   The authors would like to thank for following for lively discussions
   on list and in the halls (ordered by first name): Eric Voit, Henning
   Rogge, Martin Bjorklund, Reshad Rahman, and Rob Wilton.

Please also consider acknowledging Quifang.



Nit level comments:

(6) p 4, sec 2.  Overview of Publisher to Receiver Interaction

   The POST messages MAY be "pipelined" (not illustrated in the diagram
   above), whereby multiple notifications are sent without waiting for
   the HTTP response for a previous POST.

Suggest 'previous POST' -> 'previous POST message'.

Grammar nits from a tool to consider:

Grammar Warnings:
Section: 1.1, draft text:
While the YANG modules have been defined as an augmentation of [RFC8639], the notification method defined in this document MAY be used outside of [RFC8639] by using some of the definitions from this module along with the grouping defined in [I-D.ietf-netconf-http-client-server]. 
Warning:  This phrase is redundant. Consider using outside.
Suggested change:  "outside"

Section: 1.1, draft text:
While the YANG modules have been defined as an augmentation of [RFC8639], the notification method defined in this document MAY be used outside of [RFC8639] by using some of the definitions from this module along with the grouping defined in [I-D.ietf-netconf-http-client-server]. 
Warning:  If the text is a generality, 'of the' is not necessary.
Suggested change:  "some"

Section: 3.4, draft text:
In this example, the "Accept" states that the publisher wants to receive the capabilities response in XML but, if not supported, then in JSON.
Warning:  Apostrophe might be missing.
Suggested change:  "capabilities'"

Section: 7, draft text:
Some of the readable data nodes in the YANG modules may be considered sensitive or vulnerable in some network environments. 
Warning:  If the text is a generality, 'of the' is not necessary.
Suggested change:  "Some"

Section: 7, draft text:
Some of the RPC operations in the YANG modules may be considered sensitive or vulnerable in some network environments. 
Warning:  If the text is a generality, 'of the' is not necessary.
Suggested change:  "Some"

Section: A.2, draft text:
In the case that it is desired to use HTTPS-based notifications outside of Subscribed Notifications, an application-specific module would need to define the configuration for sending the notification.
Warning:  This phrase is redundant. Consider using outside.
Suggested change:  "outside"

Thanks,
Rob