Re: [Netconf] Reminder: WG Last Call for draft-ietf-netconf-access-control-04.txt

[Andy Bierman <[e-mail]>]

On 08/01/2011 09:27 AM, Wes Hardaker wrote:
>>>>>> On Fri, 29 Jul 2011 12:31:19 -0400, "Bert (IETF) Wijnen"<[e-mail]>  said:
>
> BW>  Pls review BEFORE or BY August 1 if you have not already done so.
> BW>  A "I read it and see no issues or have no concerns" is also very welcome!
>
> I read this document on the way home.  There is no really easy way to
> say this, so I'll be blunt: this document is not ready to go forward.
> I'm very happy that this document exists and is trying to go forward,
> and I very much want a standardized access control model, so I'm making
> this statement even knowing it'll hamper my desire to get something
> standardized.
>
> Though I have technical comments too, the real show-stoppers were the
> document layout and readability.  I started marking up the paper with
> editorial notes and wording suggestions, but eventually started marking
> only the major issues.  So the items listed below are a summary of the
> larger issues I have and I skipped documenting my minor-edits.
>

I think you should review the next version.
Many of the issues you raise are going to be addressed,
and were already raised by Juergen.  Will will look at
your new issues as well.

The normative text will be moved from the objectives section,
but a separate draft for objectives is already out of scope.

> Unfortunately, my available time in the next month especially is pretty
> much non-existent (which is why I read it on the plane: I knew I
> couldn't get to at all otherwise).  I'd love to help out with some real
> edits, but my time [and job] won't permit it.  I feel guilty dropping
> this on the doorstep, but I don't have another alternative.  I suppose
> that gives you the right to say "not helpful" and delete this message :-)
>
>
> == Document structure issues ==
>
> * Document structure: requirements vs NACM definition
>
>    The document actually tries to do 2 things:
>
>       + define a set of requirements that every netconf ACM must fulfill
>       + define an instance of a netconf ACM that meets some of the requirements
>
>    The problem is that the document isn't divided into parts that make
>    this double-role readable.  Section 2 is listed as "access control
>    design requirements" but isn't really worded with an introduction that
>    indicates this and is also sprinkled with text that seems to apply to
>    the specific implementation of the requirements too (eg, 2.4.1 isn't
>    really a requirement but is one implementation).
>
>    IMHO, if we need to define a set of requirements they should really be
>    in a separate draft.  The choices I see to go forward:
>
>    1) split the requirements into a second draft
>    2) remove the requirements and don't try to state what NACM is being
>       held up against.
>    3) do a really good job separating them from the NACM specific implementation
>
>
>
> * Section 2 even starts off with an "Editor's note", which made me
>    immediately wonder if I really was reading the "last call" document,
>    but I checked in the second airport to confirm I really was reading
>    the most recent version.  Multiple other "Editor's notes" exist in the
>    document as well and certainly shouldn't be there.

This was resolved at the WG meeting and will be finalized in the next rev.

>
> * Figure 1 makes no sense as currently drawn.  It likely needs some
>    missing arrows or something (and it's also not referenced anywhere in
>    the text).

It will be redrawn, along with some changes to fig. 2

>
> * Notification content is referred to in a number of places restricting
>    access on "event types" but doesn't discuss other included data that
>    will go along inside the notification.


this inside data is out of scope.
NACM only filters notifications on event type, not the contents.
Same for protocol operations that do not touch a datastore.

One lesson from VACM: you can make access control so hard to use,
that it doesn't get used.

>
> == Technical Reservations ==
>
> * I'm not convinced, but not against, that a single ACM model should be
>    applied across candidate, running and startup.  I could see not
>    wanting people to copy the config from running to startup, eg.
>
> * I'd make the text more clear about that access control is only applied
>    to netconf sessions.  In a few places it says things like "after
>    boot", etc, but the wording is fuzzy.  Simply stating that it is only
>    applied to operations received over the netconf protocol would make it
>    more clear.
>
> * the security considerations needs to discuss that the protocol can be
>    used to probe for information via some of the error/no-error cases
>    that are discussed in a few places.
>
> * The whole "read and execute" by default still makes me cringe inside.
>    Especially with the execute bit being on by default.  The ACM should
>    stop a user from executing a command that twiddles configuration,
>    because the write-access implemented within an rpc operation would be
>    trumped on data-modification.  But I could certainly see people
>    defining dangerous execute operations that don't twiddle config.  EG,
>    does the equivalent of "<interface-down>" actually modify config data
>    (insert long standing debate about control vs config data here).  Or
>    what about "<clear-counters>" (which hasn't been written yet, but
>    thinking about the future is critical in cases like this).
>
>    Fortunately, not every system user will have netconf access without
>    being assigned into a group.  This is straight forward for groups that
>    were configured manually, but is much less so for group memberships
>    configured via RADIUS or another "central server" outside the box.
>
> * 2.4.4 (copy-config) needs a discussion on data deletion too.
>
> * 2.4.4 (copy-config) discusses that a operator MUST have full read
>    access but doesn't say what happens when an operator doesn't.
>
> * 2.9 (data shadowing) fails to bring a clear point to the text and
>    seems to be discussing actually two different topics.
>
> * 3.1.3 (Message Processing Model) Is figure 2 a netconf standard
>    processing model?  Is implementing this flow as is a requirement?  The
>    document doesn't state this (or is it an "example" model).
>
> * 3.1.3 (and other places) mentions that the close-session message is
>    exempt from processing.  I think it'd be easier to define a list of
>    message types that *don't* get access control processing with only a
>    single entry in it for the time being.  But doing it this way would
>    let other documents add to the list, if need be, and the named list
>    could be used consistently in the rest of the document.
>
>    Sub question: is the<hello>  message technically an RPC message or not?
>    (I didn't read other documentation to find out).
>
> * 3.1.2 (users) needs to discuss naming conflicts.  I believe the
>    document intentionally shys away from this, but that doesn't solve the
>    problem.  At a minimum the security consideration section needs to
>    mention that users coming in over the different channels may have the
>    same user name since the users are not zoned by incoming protocol.
>
> * 3.2.5.3 write-default switch: does this apply to create, update and
>    delete uniformly.  If so, it needs to be stated.
>
> * 3.3.2 session establishment: why is the prohibition against sensitive
>    data in the capability elements a SHOULD NOT and not a MUST NOT?
>
> * 3.3.5 and following: the rules don't discuss what happens when data
>    exists in the config that doesn't exist (but maybe should).  IE, what
>    happens when the access-operations node doesn't exist?  The default
>    for a access-operations is a '*' but what if a user deletes the config
>    node?
>
>    (the answer is probably mostly obvious but needs to be stated; though
>    some would argue that if the result is supposed to be a 'deny' and
>    there is missing data, then it should always deny by default because
>    the rule is broken and you shouldn't skip it, where as a 'permit' rule
>    should deny by default with missing data)
>
> * The "secure" and "very-secure" names are cute, but they're generally
>    meaningless to the reader.  I'd suggest renaming them to keywords that
>    include the functionality underneath.  Maybe:
>
>       secure        ->  nacm:default-deny-modification
>       very-security ->  nacm:default-deny-all
>
> * yang model:
>
> ** user-name-type max: implementations must support any string length
>     that a transport model returns for a user name?  Even if it's really
>     long (consider a base64 name of an x.509 certificate; yes I realize
>     this example is silly).
>
> ** node-instance-identifier
>
>     So xpath 1.0 is required for nacm, right?  What happened to the whole
>     "we can't make everything require xpath because it's too much
>     overhead" from the early netconf days.  Or is it expected that
>     devices that don't want to do xpath won't want access control mechanisms?
>
> * installing yang models into devices may change the security
>    ramifications on the system.  EG, if the internal implementation is
>    reading yang modules to apply access control settings on (IE, they're
>    not hard-coded but the definitions are read from a .yang file itself),
>    then a user might have a way to install a new version of the module
>    without the yang:very-secure flags, etc, thus suddenly allowing
>    access.  I'd mention this in the security considerations at a minimum.
>
> * somewhere (security considerations) should mention that when editing
>    rules on the running config container, it is critical to insert rules
>    in an order that doesn't give access to users by mistakenly inserting
>    the permissive rules before the denial rules.  There is a paragraph in
>    the security considerations that hints at this, but I'd make it more
>    clear by stating "operators should insert denial rules first" or
>    something and specifically mention that the running container is
>    critical to get right.
>
> * the security considerations (and maybe elsewhere) should discuss data
>    references, where dealing with external keys, etc, are explicitly
>    spelled out.
>
> * might want to list rollbacks, etc, somewhere as write-operations that
>    need to be checked too.
>
> * Nowhere does it discuss what it means to lock (or partial-lock) a
>    container when you don't have access to the whole container.
>
> * If the exmaples in A.3 don't have an end "deny-all" rule or
>    "permit-all" rule, then text needs to be inserted stating what will
>    happen to help the user understand the dependency on the global flags,
>    etc.
>
> * A.3, example rule-list #2: access-operations is 'exec' but the comment
>    talks about writing.
>
> * A.3, since the "monitor" group is used in many places to provide more
>    than just monitoring access, I'd suggest renaming the group to
>    "limited" or something.
>