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

From: Donald Eastlake <[email-masked]>
Date: Sun, 2 Feb 2014 12:22:32 -0500

Hi Martin,

On Fri, Jan 31, 2014 at 3:16 PM, Martin Bjorklund <[email-masked]> wrote:
> Hi,
>
> Thank you for your review!
>
> Donald Eastlake <[email-masked]> 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.

Ahh, OK.

>> 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.

That would make me happy.

>> 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:

OK.

>> 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.

Thanks,
Donald
=============================
 Donald E. Eastlake 3rd   +1-508-333-2270 (cell)
 155 Beaver Street, Milford, MA 01757 USA
 [email-masked]

> /martin