Subject: [Netconf] Eric Rescorla's No Objection on draft-ietf-netconf-rfc6536bis-07: (with COMMENT)

From: Eric Rescorla <[email-masked]>
Date: Sat, 21 Oct 2017 14:00:34 -0700

Eric Rescorla has entered the following ballot position for
draft-ietf-netconf-rfc6536bis-07: No Objection

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/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-netconf-rfc6536bis/

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

Line 231
      object.  One of "none", "read", "create", "delete", "update", or
      "execute".
Your table below does not make use of "none" but rather makes use of N/A. Are
they the same?

Line 469
   The RESTCONF protocol [RFC8040] is used for network management
   purposes within this document.
Maybe just merge these sentences?

Line 559
      is rejected with an "access-denied" error.

   The following sequence of conceptual processing steps is executed for
How does this discussion correspond to the diagram above. For instance, what
does pre-read data node acc tl correspond to?

Line 648
      operation for these nodes is considered to be "none".  The edit
      begins at the target resource.
IMPORTANT: I believe this creates a modest vulnerability to treat
PATCH (and potentially PUT) this way because it allows the attacker to
confirm a guess for the contents of a resource it could not have read
(because the parents were unreadable). The attack here is to provide a
patch with the "before" being a guess for parts of the file and then
look for success or failure. How hard this is depends on file
structure (characters per line), etc.

Line 670
   | POST    | datastore, data | <edit-config>       | create          |
   | POST    | operation       | specified operation | N/A             |
   | PUT     | data            | <edit-config>       | create, replace |
I don't folllow why "Access operation" is N/A for, e.g. "GET". an you explain?

Line 673
   | PUT     | datastore       | <copy-config>       | replace         |
   | PATCH   | data, datastore | <edit-config>       | merge           |
   | DELETE  | data            | <edit-config>       | delete          |
"merge" does not appear in the list of access oeprations above.

Line 1057
        match any of the user's groups, proceed to the next rule-list
        entry.
Assuming I am reading this correctly, it is not possible to have
access control entries for individuals but only only for
groups. There's nothing wrong with that, but you should make it clear
earlier. If I'm wrong, can you correct me and explain what I'm
missing.

---

From: Andy Bierman <[email-masked]>
Date: Wed, 25 Oct 2017 10:44:45 -0700

> https://datatracker.ietf.org/doc/draft-ietf-netconf-rfc6536bis/

Thanks for your review.
These line numbers do not line up with the draft in my editor.
I will try to guess the section and paragraph anyway.

> Your table below does not make use of "none" but rather makes use of N/A.

This is a bug.
I will change N/A to "none" in Table 1.
Also note a bug in table 1. The access operation for POST operation should
be "execute", not "N/A". I will fix that as well. (see new table 1 below)

> Maybe just merge these sentences?
OLD:

3.1.2.  External Dependencies

   The NETCONF protocol [RFC6241] is used for network management
   purposes within this document.

NEW:

   The NETCONF protocol [RFC6241] and RESTCONF protocol [RFC8040]
   are used for network management purposes within this document.

Line 559
> does pre-read data node acc tl correspond to?

The "pre-read" applies to actions not "rpc" so there is no text in page 13,
para 1, 4 bullet lists
I do not have any proposed changes for this issue.

> structure (characters per line), etc.

The purpose of this text is to support user-specific access within a
more-restricted resource.
As an analogy, I can do "ls /home" on my linux PC and see all the users
that are setup,
but I cannot access any of the user dirs (except my own).

I think this was raised as a RESTCONF issue.
The server can return 403 or 404 if access is denied.
Here is some text from the RFC 8040, 4.6:

   If the user is not authorized to alter the target resource, an error
   response containing a "403 Forbidden" status-line SHOULD be returned.
   A server MAY return a "404 Not Found" status-line, as described in
   Section 6.5.4 in [RFC7231]
<https://tools.ietf.org/html/rfc7231#section-6.5.4>.

 I do not have any suggested edits for this issue.

> "merge" does not appear in the list of access oeprations above.
Corrected Table 1:

changed rows: HEAD, GET, POST, PUT, PATCH

   +---------+-----------------+---------------------+-----------------+
   | method  | resource class  | NETCONF operation   | Access

   | OPTIONS | all             | none                | none            |
   | HEAD    | all             | <get>               | read            |
   | GET     | all             | <get>               | read            |
   | POST    | datastore, data | <edit-config>       | create          |
   | POST    | operation       | specified operation | execute         |
   | PUT     | data            | <edit-config>       | create, update |
   | PUT     | datastore       | <copy-config>       |  update         |
   | PATCH   | data, datastore | <edit-config>       | update           |
   | DELETE  | data            | <edit-config>       | delete          |
   +---------+-----------------+---------------------+-----------------+

               Table 1: Mapping RESTCONF Methods to NETCONF

> earlier. If I'm wrong, can you correct me and explain what I'm
Sec 3.3.2 para 3:

OLD:

   A group member is identified by a username string.

NEW:

   A group contains zero or more group members.
   A group member is identified by a username string.

Andy

---

From: Mahesh Jethanandani <[email-masked]>
Date: Wed, 25 Oct 2017 11:23:51 -0700

Eric,

I will take a stab at trying to respond, and I will let the authors correct me if I am wrong.

> On Oct 21, 2017, at 2:00 PM, Eric Rescorla <[email-masked]> wrote:
> 

> does pre-read data node acc tl correspond to?

The way I read the diagram is that the left/middle side of the diagram corresponds to first set of bullet points below the diagram, and relate to active sessions sending <rpc> message and actions to the server, while the second set of bullet points correspond to the right side of the diagram and cover the access control as they relate to notifications. As an example, if the user is not authorized to execute the specified operation shown on the left/middle side of the diagram, they would get a "access-denied" error.

Andy, the pre-read data acc. ctl, I thought implied the reading/caching of the NACM rules that had been configured before the first message/notification is issued - no?

> earlier. If I'm wrong, can you correct me and explain what I'm

Mahesh Jethanandani
[email-masked]

---

From: Andy Bierman <[email-masked]>
Date: Wed, 25 Oct 2017 11:29:33 -0700

On Wed, Oct 25, 2017 at 11:23 AM, Mahesh Jethanandani <
[email-masked]> wrote:

> The way I read the diagram is that the left/middle side of the diagram
> corresponds to first set of bullet points below the diagram, and relate to
> active sessions sending <rpc> message and actions to the server, while the
> second set of bullet points correspond to the right side of the diagram and
> cover the access control as they relate to notifications. As an example, if
> the user is not authorized to execute the specified operation shown on the
> left/middle side of the diagram, they would get a "access-denied" error.
>
> Andy, the pre-read data acc. ctl, I thought implied the reading/caching of
> the NACM rules that had been configured before the first
> message/notification is issued - no?
>
>

No.

If there was an action called /interfaces/interface/reset-interface
then the group must be authorized to read /interfaces and read
/interfaces/interface
and execute /interfaces/interface/reset-interface.

The pre-read refers read access to data node ancestors of an action or
notification.
Do you have text change suggestions?

Andy

---

From: Mahesh Jethanandani <[email-masked]>
Date: Wed, 25 Oct 2017 12:14:19 -0700

> On Oct 25, 2017, at 11:29 AM, Andy Bierman <[email-masked]> wrote:
> 
> 
> 
> On Wed, Oct 25, 2017 at 11:23 AM, Mahesh Jethanandani <[email-masked] <mailto:[email-masked]>> wrote:
> Eric,
> 
> > On Oct 21, 2017, at 2:00 PM, Eric Rescorla <[email-masked] <mailto:[email-masked]>> wrote:
> >
> > Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html <https://www.ietf.org/iesg/statement/discuss-criteria.html>
> > https://datatracker.ietf.org/doc/draft-ietf-netconf-rfc6536bis/ <https://datatracker.ietf.org/doc/draft-ietf-netconf-rfc6536bis/>
> >
> >
> >
> then the group must be authorized to read /interfaces and read /interfaces/interface
> The pre-read refers read access to data node ancestors of an action or notification.

How about text to the same effect?

pre-read data acc. ctl refers to group read access as it relates to data node ancestors of an action or notification. As an example, if an action is defined as /interfaces/interface/reset-interface, the group must be authorized to read /interfaces and /interfaces/interface, and execute on /interfaces/interface/reset-interface.

> [email-masked] <mailto:[email-masked]>
> 
> 
> 
> 

Mahesh Jethanandani
[email-masked]

---

From: Eric Rescorla <[email-masked]>
Date: Wed, 25 Oct 2017 15:23:31 -0700

On Wed, Oct 25, 2017 at 10:44 AM, Andy Bierman <[email-masked]> wrote:

> The "pre-read" applies to actions not "rpc" so there is no text in page
> 13, para 1, 4 bullet lists
>    Section 6.5.4 in [RFC7231] <https://tools.ietf.org/html/rfc7231#section-6.5.4>.
>
>

So I understand the notion of having the parent not be required, but the
point here is that having
asymmetry between the read and write conditions leads to unexpected
consequences, including
security failures.

>    A group member is identified by a username string.

Hmm... I was more thinking
"Access control is applied at the level of groups. A group consists of zero
or more members"

---

From: Eric Rescorla <[email-masked]>
Date: Wed, 25 Oct 2017 15:23:57 -0700

OK.

On Wed, Oct 25, 2017 at 12:14 PM, Mahesh Jethanandani <
[email-masked]> wrote:

>> Andy, the pre-read data acc. ctl, I thought implied the reading/caching
>> of the NACM rules that had been configured before the first
> pre-read data acc. ctl refers to group read access as it relates to data
> node ancestors of an action or notification. As an example, if an action is
> defined as /interfaces/interface/reset-interface, the group must be
> authorized to read /interfaces and /interfaces/interface, and execute on
> /interfaces/interface/reset-interface.
>
>
>
> Andy
>
>
> >
>> > Line 648

---

From: Adam Roach <[email-masked]>
Date: Wed, 25 Oct 2017 17:50:35 -0500

On 10/25/17 5:23 PM, Eric Rescorla wrote:
>
>
> On Wed, Oct 25, 2017 at 10:44 AM, Andy Bierman <[email-masked] 
> <mailto:[email-masked]>> wrote:
>
>
>
>
>     On Sat, Oct 21, 2017 at 2:00 PM, Eric Rescorla <[email-masked]
>     <mailto:[email-masked]>> wrote:
>
>
>         Line 648
>     The purpose of this text is to support user-specific access within
>     a more-restricted resource.
>     As an analogy, I can do "ls /home" on my linux PC and see all the
>     users that are setup,
> So I understand the notion of having the parent not be required, but 
> the point here is that having

Andy -- if your analogy holds, then the ability to perform PUT, PATCH, 
or DELETE on a resource implies that the user must also be authorized to 
*read* ancestor nodes of that resource. Is that what is intended here? 
If so, I think text to that effect may address the concern that Eric has 
raised.

/a

---

From: Andy Bierman <[email-masked]>
Date: Wed, 25 Oct 2017 16:08:58 -0700

On Wed, Oct 25, 2017 at 3:50 PM, Adam Roach <[email-masked]> wrote:

> Andy -- if your analogy holds, then the ability to perform PUT, PATCH, or
> DELETE on a resource implies that the user must also be authorized to
> *read* ancestor nodes of that resource. Is that what is intended here? If
> so, I think text to that effect may address the concern that Eric has
> raised.
>
>
OK -- I will work on some new text that explain the pre-read steps in sec.
3.1.3.
Definitely a bug that the pre-read boxes are not properly explained.

/a

Andy

---

From: Andy Bierman <[email-masked]>
Date: Wed, 25 Oct 2017 17:06:32 -0700

> /interfaces/interface/reset-interface.
OK -- I added new text before the diagram explaining the new boxes.

Andy

---

From: Andy Bierman <[email-masked]>
Date: Wed, 25 Oct 2017 17:11:55 -0700

Hi,

draft-07, Sec. 3.2.5:

OLD:

   If the effective access operation is "none" (i.e., default-
   operation="none") for a particular data node, then no access control
   is applied to that data node.

NEW:

   If the effective access operation is "none" (i.e., default-
   operation="none") for a particular data node, then read permissions
   are required for that data node.

Note that this is a normative change that requires server code changes.

Is this change OK?

Andy

---

From: Eric Rescorla <[email-masked]>
Date: Wed, 25 Oct 2017 21:08:23 -0700

This seems clear to me. I'm not taking a position on whether it's advisable.

-Ekr

---

From: Andy Bierman <[email-masked]>
Date: Thu, 26 Oct 2017 09:04:46 -0700

> This seems clear to me. I'm not taking a position on whether it's
OK -- I would rather not make this change unless the IESG says we need to
do it.

Andy

---

From: Eric Rescorla <[email-masked]>
Date: Thu, 26 Oct 2017 09:17:39 -0700

My position is that the document needs to carefully describe the security
properties of the system. Either you should remove this property (which
seems a bit unfortunate) or explain the implications carefully. Which one
you do seems up to the WG.

-Ekr

---

From: Andy Bierman <[email-masked]>
Date: Thu, 26 Oct 2017 10:07:25 -0700

> you do seems up to the WG.

We can leave the change in.
It is more secure.

-Ekr

Andy