Subject: [Gen-art] Gen-ART review of draft-ietf-netconf-with-defaults-12.txt

From: "Richard L. Barnes" <[email-masked]>
Date: Mon, 25 Oct 2010 22:08:24 -0400

I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft. For background on Gen-ART, please see the FAQ 
at http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq

Please resolve these comments along with any other comments you may receive.

Document: draft-ietf-netconf-with-defaults-12.txt
Reviewer: Richard Barnes <[email-masked]>
Review Date: 2010-10-25

Summary: This document is almost ready to go.  Some minor ambiguities 
need to be resolved, and some meat needs to be added to the security 
considerations.

Major issues:

S4:
I'm confused by the existence of Section 4 in light of the fact that 
Sections 2.1.2, 2.2.2., and 2.3.2 say that a server MUST support a 
<with-defaults> value for each of the basic modes.  If there are cases 
where a server doesn't support a given mode, what does it mean for it to 
"support the <with-defaults> parameter" with a given value?

S8:
I'll defer to the SECDIR reviewer for more details, it seems like there 
are some more security considerations to be discussed.  For example, if 
the underlying protocol is used in a mode without authentication and 
integrity protection a man in the middle could abuse both the 
<with-defaults> element and the :with-defaults capability to suppress 
information from the server to the client.  In principle, the same MitM 
could change the values in the data messages directly, but it is simpler 
just to prevent them from being sent in the first place.

Minor issues:

S2.1.3 P1: "...MUST return an <rpc-error response..."
Is the correct error-tag here really "unknown-attribute"?  It seems like 
the server could support the attribute without accepting it in these cases.

S3.4:
It seems like the 'report-all-tagged' mode should have its own 
sub-section under Section 2, like the basic modes.

S1.1:
Suggest changing ":with-defaults" to ":with-defaults" or ":with-defaults 
capability".  As it is, I was expecting to see this with colons on both 
sides.

S4.1:
This subsection heading could be deleted, hanging the text directly 
under Section 4.

Questions:

S2.1.3 P2: "... the data node is immediately replaced with the default 
value"
If that's true, how do you ever change a value away from the default?

---

From: Andy Bierman <[email-masked]>
Date: Tue, 26 Oct 2010 09:34:27 -0700

S4:

All new NETCONF capabilities need to be documented using a template
in RFC4741.  This is sec. 4. The text is confusing I guess.  Here
is a fix for 2.1.2, 2.2.2, and 2.3.2:

2.1.2:

OLD:

   The server MUST support the <with-defaults> parameter with a value
   equal to 'report-all', as specified in Section 3.1.

NEW:

   If the 'report-all' basic mode is supported, the server MUST support 
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   the <with-defaults> parameter with a value equal to 'report-all', as
   specified in Section 3.1.

2.2.2:

   The server MUST support the <with-defaults> parameter with a value
   equal to 'trim', as specified in Section 3.1.

NEW:

   If the 'trim' basic mode is supported, the server MUST support
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   the <with-defaults> parameter with a value equal to 'trim',
   as specified in Section 3.1.

2.3.2:

OLD:

   The server MUST support the <with-defaults> parameter with a value
   equal to 'explicit', as specified in Section 3.3.

NEW:

   If the 'explicit' basic mode is supported, the server MUST support
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   the <with-defaults> parameter with a value equal to 'explicit',
   as specified in Section 3.3.

S8:  The NETCONF protocol (4741) lists the protocol requirements.
Altering the <with-defaults> leaf is not some special security hole.
The reason the default leaf is left out is that it is a well-known
value, defined in the data model schema.

All modes for NETCONF have authentication and integrity protection.

S2.1.3:  This is the error-tag that the NETCONF protocol spec says to use.
It means unknown or unexpected.  In this 'report-all' basic mode, there are never
any tagged nodes since all nodes are returned by default.  Therefore,
a server working in this mode is not required to have any concept
at all of 'returning the node to its default value'.  The WG decided
it was better to have all servers operating in this mode work the same way.
A server that understands defaults will be using one of the other modes anyway.

S3.4: The 'report-all-tagged' mode is not a basic mode.  There was no interest
within the WG to make it a basic mode.  It is only used when the client sets
the <with-defaults> parameter to this value.  Sec. 2 is documenting all the
basic modes.

S1.1: The ':foo' syntax is defined in RFC 4741 and is the expected syntax
for referring to a NETCONF capability.  It is arbitrary, but all NETCONF
capabilities use it.

S4.1: This documentation format is defined in RFC 4741.  All capabilities
are supposed to follow it.  See Appendix D on page 113:
http://www.ietf.org/id/draft-ietf-netconf-4741bis-06.txt

S2.1.3 P2: This text refers to the corner-case of deleting a node
with a default.  The 'merge' or 'replace' operations are used to
change to a non-default value.

Andy

-----Original Message-----
From: Richard L. Barnes [mailto:[email-masked]] 
Sent: Monday, October 25, 2010 7:08 PM
To: Russ Housley
Cc: Gen-ART; [email-masked]
Subject: Gen-ART review of draft-ietf-netconf-with-defaults-12.txt

Major issues:

S4:

S8:

Minor issues:

S3.4:

S1.1:

S4.1:

Questions: