Re: [netconf] Roman Danyliw's Discuss on draft-ietf-netconf-http-client-server-17: (with DISCUSS and COMMENT)

[Kent Watsen <[e-mail]>]

Hi Roman,

Thank you for another thoughtful review.
Please find my responses below.

Kent


> On Feb 14, 2024, at 2:53 PM, Roman Danyliw via Datatracker <[e-mail]> wrote:
> 
> Roman Danyliw has entered the following ballot position for
> draft-ietf-netconf-http-client-server-17: 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-http-client-server/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> (Let me know if I am misunderstanding how this YANG module would be deployed. 
> I was envisioning this being a stand-alone configuration.)

Two answers:

1) Since both YANG modules define only groupings, the YANG modules would never
be used stand-alone, as there would always have to be a “higher level” module to
`import` the module and `use` its groupings.

2) Unlike the tcp/ssh/tls-client-server drafts, the http/netconf/restconf-client-server
drafts define so called “stack” groupings, in addition to the normal layer-specific 
groupings.  A “stack” grouping is one that brings together a number of layer-specific
groupings for common use-cases.  For instance, this document defines the 
"http-server-stack-grouping” that “stacks” the "tcp-server-grouping”, "tls-server-grouping”,
and the "http-server-grouping”.  Hence, a single grouping bringing together three layers.

Answer #2 doesn’t takeaway from answer #1, but it does lend itself to being perceived
as being somewhat more “stand-alone” than in the tcp/ssh/tls-client-server documents.

Did I answer your question, or maybe you were asking something else?


> Constraining credential use:
> ** Section 2.3.  Per “grouping http-client-identity-grouping” how does one
> constrain the use of the username/password to a particular webserver?

This is a “grouping” statement.  Grouping statements have to be “used” by a
“higher-level" YANG module in order to have protocol-accessible configuration.

One example is that there are three higher-level modules, each for a different
client, connecting to a different webserver.  In this case, each client’s config would
be independent of the others', and hence the username/password would be
“constrained” that way,

In another example, as will be seen in the "restconf-client-server" document, there
can be “list” of endpoints, each for a different webserver (e.g., RESTCONF-servers),
each again having a distinct instance of the "http-client-identity-grouping”.

Both examples are really the same, in showing how distinct instances can be
configured for their specific use.  This is normal arrangement in the real world.

E.g., device-1 is configured to use credential-1 when checking for patches from
vendor-1, while device-2 is configured to use credential-2 when checking for 
patches from vendor-2.  These are completely separate devices, and thus there
is no overlap in their configurations.


> ** Section 2.3.  Per “tls-client-parameters” and support for a client
> certificate.
> 
> -- Editorial.  I was expected statement in the text to explain to me that this
> is a client certificate for mutually authenticated TLS (mTLS) -- If a client
> certificate is configured, does that imply the implementation will always use
> this certificate if mTLS is requested by the server?  How does one constrain
> the places/servers a certificate would be used?

To the first question, yes, the intention is that this would be the certificate
the client would send if the server requested mTLS.   Since this comment
regards the “tls-client-grouping”, defined in the “tls-client-server” document,
I would think that such a statement would be in that document more so than
In this one (to be a DRY as possible).  Looking to that document, I see here
(https://datatracker.ietf.org/doc/html/draft-ietf-netconf-tls-client-server-38#section-3.1.2.1)
the following text:

	The "client-identity" node, which is optionally configured (as client
	authentication MAY occur at a higher protocol layer), configures
	identity credentials, each enabled by a "feature" statement defined
	in Section 3.1.1.

	[SIde note:  RESTCONF allows TLS- and/or HTTP- level client-auth]

This blurb doesn’t use text similar to yours, perhaps because it seemed
too obvious that it is what a “client identity” would be for.  Just the
same, I’m happy to improve this text to clear your DISCUSS.  Is there a
specific sentence that you would like to see?

To the second question, the “constraint” happens in the same way as
described above with regards to the “http-client-identity-grouping”.  Is
this an acceptable answer?


> -- If configured with multiple client certificates, how does which is the
> appropriate certificate to use?

The "tls-client-grouping” (linked above)  supports at most one certificate
to be configured at a time.  There is no ability for there to be “multiple
client certificates”.   Is this DISCUSS cleared?



> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> ** Section 3.3.  Is it possible to configure a
> client-auth-supported/basic-auth-support without tls-supported?  If so, can
> there be caution added about such arrangements

Yes, it is possible.  For instance, when an external device fronts the
webserver to offload TLS-termination (e.g., using crypto accelerators).
In this case, from the webserver’s perspective, “tls-supported” is NOT
enabled, while yet still allowing basic-auth to be configured.

I added the following text to the Security Considerations section for
the “ietf-http-server” module (Section 4.2):

	The feature"tls-supported" is not required, in order to support
	cases where an external device is used to terminate TLS
	connections, but such arrangements leave the client-credentials
	to be unprotected by the transport.


> ** Section 4.1 and 4.2.  Certain nodes are called out a vulnerable with
> write-access.  Can the rationale and impact of misuse please be explained.

Added to Section 4.1:

	A misconfigured "client-identity" node may result in loss of
	connectivity or, perhaps, unexpectedly elevated authorization.

	and

	A misconfigured “proxy-connect" node may result in loss of
	connectivity or, perhaps, unexpectedly elevated authorization.
	

Added to Section 4.2:

	A misconfigured "server-name" may mislead clients into not knowing
	how to interoperate with the server (e.g., "foo v1.0" vs "foo 2.0”).

	and

	Misconfigured "client-authentication" may lead the server to
	authenticate invalid client credentials.

Good?


> ** Section 4.1 and 4.2 use a very concerning editorial construct that could be
> easily misinterpreted.  Paragraph one reads “Please be aware that this YANG
> module uses groupings from other YANG modules that define nodes that may be
> considered sensitive or vulnerable in network environments.  Please review the
> Security Considerations for dependent YANG modules for information as to which
> nodes may be considered sensitive or vulnerable in network environments.”  This
> text followed by another paragraph which then says “None of the readable data
> nodes defined in this YANG module are considered sensitive or vulnerable in
> network environments.”
> 
> The first paragraph is unambiguous (i.e., check the references and their
> Security Considerations apply).  The second paragraph’s accuracy depends on an
> interpretation that “defined in this YANG module” excludes imported modules.  I
> believe that clearer language is needed here.

Good catch, how about this?

OLD: defined in this YANG module, 
NEW: defined in this YANG module, i.e., excluding imported modules,



Thanks again!
Kent