Re: [netconf] Éric Vyncke's Discuss on draft-ietf-netconf-https-notif-13: (with DISCUSS and COMMENT)

["Eric Vyncke (evyncke)" <[e-mail]>]

Mahesh and Kent,

Thanks for your patience (it seems that Kent’s reply was lost in my end-of-the-year shutdown period [1]) and for the revised I-D.

The combination of Kent’s very detailed reply and the revised I-D is sufficient for me to clear my DISCUSS.

BTW, it would have been easier to read draft-ietf-netconf-netconf-client-server first ;-)

Regards

-éric

[1] never hesitate to nudge an AD ! an AD is just another human being suffering from mailbox overload ;-)

From: Mahesh Jethanandani <[e-mail]>
Date: Thursday, 18 January 2024 at 21:31
To: Eric Vyncke <[e-mail]>
Cc: The IESG <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, "maqiufang (A)" <[e-mail]>, Kent Watsen <[e-mail]>
Subject: Re: Éric Vyncke's Discuss on draft-ietf-netconf-https-notif-13: (with DISCUSS and COMMENT)

Hi Eric,

We posted a -14 version of the draft that addresses your DISCUSS and COMMENT. Please review.

Thanks.


On Dec 15, 2022, at 8:49 AM, Kent Watsen <[e-mail]<mailto:[e-mail]>> wrote:

Hi Ėric,


Please see below for responses to your valuable comments.


Kent // co-author




On Dec 12, 2022, at 11:38 AM, Éric Vyncke via Datatracker <[e-mail]<mailto:[e-mail]>> wrote:

Éric Vyncke has entered the following ballot position for
draft-ietf-netconf-https-notif-13: Discuss

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/about/groups/iesg/statements/handling-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-https-notif/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------


# Éric Vyncke, INT AD, comments for draft-ietf-netconf-https-notif-13
CC @evyncke

Thank you for the work put into this document.

Please find below one blocking DISCUSS points (easy to address), some
non-blocking COMMENT points (but replies would be appreciated even if only for
my own education).

Special thanks to Qiufang Ma for the shepherd's detailed write-up including the
WG consensus *and* the justification of the intended status.

I hope that this review helps to improve the document,

Regards,

-éric

## DISCUSS

As noted in https://www.ietf.org/blog/handling-iesg-ballot-positions/, a
DISCUSS ballot is a request to have a discussion on the following topics:

### Section 6.1

`The module contains the TCP, TLS and HTTPS parameters` seems to prevent the
use of HTTP/3, which relies on QUIC, i.e., UDP. Is it on purpose ? If so, a
justification is probably required.

The parameters mentioned are inherited from draft-ietf-netconf-http-client-server, which was just published to the IESG for consideration.

Looking at the "http-client-server" draft, Section 1.1, entitled "Relation to other RFCs" [1], you will notice that the there is a hierarchy of protocol-specific drafts, each pertaining only to its layer in a protocol stack.  More specifically, each draft defines two YANG "groupings", one for the protocol-client and the other for the protocol-server.

[1] https://www.ietf.org/archive/id/draft-ietf-netconf-http-client-server-12.html#name-relation-to-other-rfcs

For instance, to define an HTTP-client, the stack is:

    tcp-client-grouping // from draft-ietf-netconf-tcp-client-server
    http-client-grouping // from draft-ietf-netconf-http-client-server

To define an HTTPS-client, the stack is:

    tcp-client-grouping // from draft-ietf-netconf-tcp-client-server
    tls-client-grouping // from draft-ietf-netconf-tls-client-server
    http-client-grouping // from draft-ietf-netconf-http-client-server

Now, to support a QUIC-client, we'd want a stack something like:

    udp-client-grouping // from TBD
    quic-client-grouping // from TBD
    http-client-grouping // from draft-ietf-netconf-http-client-server

The "TBD" above means that there are currently no drafts in progress for "udp-client-server" or "quic-client-server".   Such drafts may be developed at any time, and "case" statements may be "augmented" into the instances of the groupings defined the "http-client-server" draft.

All this to say, support for HTTP/3 can be added when desired.



Related to the above sentence, the tree has:
```
         +--rw https-receiver
            +--rw (transport)
            |  +--:(tls) {tls-supported}?
            |     +--rw tls
```
Please, bear with my ignorance of YANG, but does the '?' indicate that the tls
sub-tree may not be supported ? Weird for an IETF draft whose title include
HTTPS ;-) (or is it to support QUIC ?)

Yes, the "{foo}?" syntax in YANG tree-diagrams (RFC 8340) means that the node is present only when the specified feature (foo) is enabled.

The "https-notif" module (defined in this draft) inherits a snippet of YANG from the "ietf-http-client" module (defined in draft-ietf-netconf-http-client-server).   The "http-client-server" draft correctly defines features enabling the various supported transports (e.g., TCP and TLS currently, QUIC when defined) so that the specific transport(s) supported by an application may be specified.


FWIW, this draft's YANG explicitly disables the ability for HTTP (i.e., w/o TLS), but the tree diagram still renders the only remaining "case" statement with its "feature" statement.  This should be seen as a positive, as it enables other secure transports to be exclusively supported in the future (e.g., a QUIC-only future).  Perhaps this draft's title could be changed, e.g., s/HTTPS/Secure/?



Finally, and for sure it is my ignorance about YANG, but I do not see an
obvious link between the tree view and the YANG module. Should there be a
written explanation in the text ? (this is of course a COMMENT-level point).

Good catch, this draft is missing a reference to RFC 8340.




### Section 7

The security section is mainly about the YANG RESTCONF/NETCONF template, but
does not address the security of sending notifications over HTTP. E.g., as
written below, what about TLS mutual authentication ? How is the HTTP server
certificate is trusted ?

HTTP (without TLS) is not supported by this draft.

The Security Considerations section, as written, accurately addresses the "northbound API" presented by NETCONF and RESTCONF servers.  What is not addressed is the "eastbound API" this draft introduces, which is just "HTTPS" and does not have a mutual-auth requirement.

Would the fix be to add the following to the Security Considerations section?
- the publisher (the HTTPS-client) MUST authenticate the server.
- the receiver (the HTTPS-server) MUST authenticate the client.




----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------


## COMMENTS

### Abstract
```
  This document requires that the publisher is a "server" (e.g., a
  NETCONF or RESTCONF server), but does not assume that the receiver is
  a server.
```
This last paragraph adds more confusion on the roles... Suggest to remove it.

The paragraph was added due to comments about how things were confusing before.

To be clear:

- the publisher (the HTTPS-client) MUST also be a YANG-based server, i.e., presenting a YANG-based protocol such as NETCONF and RESTCONF.

- the receiver (the HTTPS-server), is an HTTPS-server without further stipulation





### Roles

Suggest to prefix server always by HTTPS (i.e., "HTTPS server") and use
"publisher" for the NETCONF/RESTCONF "server".

Agreed.  Mark Nottingham provided similar comments, and v14 version of the document (currently in GitHub here<https://github.com/netconf-wg/https-notif/pull/27>) carries the prefix HTTP or NETCONF/RESTCONF before the word “server” to clarify what kind of server is being assumed.



### Which TLS version ?

I will let the transport/security ADs to ballot a DISCUSS if required, but
wouldn't a specification of which HTTP and TLS versions are required ?

The "http-client-server" draft does not have any "feature" statements enabling configuration to vary the configuration HTTP-layer by HTTP-version.  And hence is unconstrained.

The "tls-client-server" draft does have "feature" statements enabling configuration to vary by TLS-version, specifically 1.0, 1.1, 1.2, and 1.3.  Note that 1.0 and 1.1 have "status obsolete", and TLS 1.2 has "status deprecated".

This draft inherits the above stance without imposing any new limitations.


Should the certificate validation procedure be specified ?

No, such is for protocol drafts to define.  This draft, and those it depends on, regard only on how to *configure* the protocol stacks.



### Section 2

Probably linked to my ignorance of the system (a global archicture or a more
detailed introduction would be appreciated), but how is the
"relay-notifications" known by the publisher ? In the examples, there is not
such item.

There is a typo in Section 2: s/relay-notifications/relay-notification/.

Search again without the trailing 's'?


Minor comment: is it a "path" or a "common prefix" ? Also, doesn't having a
common prefix in the URI prevent having two "HTTP servers" for the same
subscription. E.g., one for the capabilities and one for the notifications
(e.g., for geographica load distribution).

It is a common prefix.  This was fixed per another comment.




### Section 3.2

`a publisher can issue an HTTPS GET request` should the 'can' be 'upgraded' to
a "SHOULD" or "MAY" ?

Good grief, as the beginning of that sentence is written, it's more like a MUST ;)

But equally good, IMO: s/can issue/issues/

Thoughts?


### Section 3.4

Please replace 'nnn' by the exact Content-Length.

Or remove "Content-Length" line from the examples?




### Section 4.1

s/HTTPS POST request/HTTP POST request/ ?

Good catch.




### Section 4.2

```
  ... In case of
  corrupted or malformed event, the response should be an appropriate
  HTTP error response.
```

Suggest to replace "should" by "SHOULD" or even "MUST".

Agreed.



## NITS

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues.

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments



Thanks again!
Kent


Mahesh Jethanandani
[e-mail]<mailto:[e-mail]>