Re: [secdir] SECDIR review of draft-ietf-netmod-system-mgmt-11

[Martin Bjorklund <[e-mail]>]

Hi,

Thank you for your review!

Donald Eastlake <[e-mail]> wrote:
> Hi,
> 
> Sorry this review is late.
> 
> I have reviewed this document as part of the security directorate's
> ongoing effort to review all IETF documents being processed by the
> IESG. Document editors and WG chairs should treat these comments just
> like any other last call comments.
> 
> I believe this draft is ready with issues.
> 
> This draft specifies a YANG data model for configuration and
> identification of NETCONF server device information. You might think
> there would not be much in the way of Security Considerations for a
> "data model" but the model includes User Authentication,  sensitive
> writable data objects, and the like.
> 
> For user password authentication, there are provisions for storing a
> plain text of the password or a salted hash. Hash functions available
> are MD5, SHA-256, and SHA-512.
> 
> Security Considerations:
> 
> The Security Considerations section seems pretty thorough in covering
> NETCONF security features such as SSH transport and access controls.
> However, I believe the Security Considerations should recommend not
> storing passwords as plaintext but rather as a salted hash.

Actually, the clear text password is never stored:

       The '$0$' prefix signals that the value is clear text.  When
       such a value is received by the server, a hash value is
       calculated, and the string '$<id>$<salt>$' or
       $<id>$<parameter>$<salt>$ is prepended to the result.  This
       value is stored in the configuration data store.

The client can send a clear text password (over SSH or TLS) but the
server will never store it in clear text.

> While the
> Security Considerations section refers to RFC 6151 for MD5 Security
> Considerations and having that reference is good, I believe this
> document should also recommend that MD5 not be used as the password
> salted hash function.

This was discussed in the WG, and we felt that whatever we would say
would be better stated in RFC 6151.  Maybe we can be more explicit
about the recommendation:

OLD:

This YANG model defines a type "crypt-hash" that can be used to store
MD5 hashes.  ^RFC6151^ discusses security considerations for MD5.


NEW:

This YANG model defines a type "crypt-hash" that can be used to store
MD5 hashes.  ^RFC6151^ discusses security considerations for MD5.  The
usage of MD5 is NOT RECOMMENDED.


> For the list of sensitive readable data and sensitive remote procedure
> call operations, the draft is careful to say "It is thus important to
> control access to these operations." However, while it is pretty
> obvious, these words or equivalent seem to be missing in reference to
> the sensitive writable data.

The Security Consideration section is modeled after a template found
at:

http://trac.tools.ietf.org/area/ops/trac/wiki/yang-security-guidelines

I don't mind an update, but I believe we should update the template as
well in this case.

The current text says that "write operations [...] without protection
can have a negative effect [...]".

The other paragraphs use the phrase:

"It is thus important to control [...] access [to these data nodes]".

I think the current text says the same thing, but I guess it can be
made more explicit:

OLD:

There are a number of data nodes defined in this YANG module which are
writable/creatable/deletable (i.e. config true, which is the
default). These data nodes may be considered sensitive or vulnerable
in some network environments. Write operations (e.g. edit-config) to
these data nodes without proper protection can have a negative effect
on network operations. These are the subtrees and data nodes and their
sensitivity/vulnerability:


NEW:

There are a number of data nodes defined in this YANG module which are
writable/creatable/deletable (i.e. config true, which is the
default). These data nodes may be considered sensitive or vulnerable
in some network environments. Write operations to these data nodes can
have a negative effect on network operations.  It is thus important to
control write access (e.g., via edit-config) to these data nodes.
These are the subtrees and data nodes and their
sensitivity/vulnerability:


> Trivial:
> 
> Section 2.3, first line: "need" -> "needs"
> Section 2.3, 2nd paragraph, second line: "need" -> "needs"
> I believe RPC should be expanded to "remote procedure call" at its one
> use in the text of the draft, unless I've expanded the acronym wrong,
> which would be proof that whatever it stands for it should be spelled
> out.

All fixed.


/martin