A YANG Data Model for System Management
draft-ietf-netmod-system-mgmt-16

It might be nice at some stage to write a short RFC on data model tree
diagrams to avoid repeating the terminology in each YANG document, and
to provide a general home for discussion of the format of those 
diagrams. Such a document could also include some worked examples with
explanations.

1) I agree with Brian's DISCUSS point (2) about some flexibility for new and different types of encryption algorithms.  I'm particularly concerned that the algorithm-id appears to be tucked into the string.  How can one extend that?   The approach taken in authentication/user/ssh-key/algorithm with IANA allocated algorithms looks much better.

2) Not that I'm a security expert, but are these really the best encryption method for securing the configuration to routers via RADIUS?  This is more minor, since it can obviously be extended as needed.

3) Is it possible to do any nesting of features?  What does it mean to not support ntp but support ntp-udp-port?

Comments below thanks to Wes George's quick review ; I agree with them.

4) 3.5 – since SSH is mandatory, “over any transport” doesn’t make sense. Should be something like MUST support SSH, MAY support others, not “password over any transport” as currently. Plaintext transport should be explicitly prohibited, or if not prohibited, at least strongly recommended against, and it should be in the body of the document, not buried in the security considerations.

5) 5 – MD5 is deprecated and a new YANG module should not support it and should instead support TCP-AO. I don’t see the point in having it here, and then burying a “not recommended” in the end of the security considerations. 

General:
Odd that TACACS is not discussed at all as an alternative to RADIUS or DIAMETER.

You rightly raise the concern about the level of access to the system this model proposes (ability to reload/shutdown, etc). I think this doc is making the implicit assumption that the AAA method being used will also include some level of role definition that controls which commands are usable and which are prohibited by user/client, including the potential for different roles for different systematic uses of this interface — different IDs with different authentication. This is obliquely discussed in the security considerations, but it would be very useful to make this assumption more explicit in the body of the document so that people are thinking in that way when implementing support for the different AAA models. It may even be worth explicitly defining a set of commands which must be separately authenticated before they are executed so that there’s a method for “are you sure?” before triggering reloads and the like.

As to which Hashes and algorithms, I agree that SHA256 seems sort of puny but aren’t there much better documents discussing which algorithms and key lengths are appropriate that this document could refer to? 
For something like an RFC, it makes sense to define the most secure algorithm as mandatory to implement, while allowing enough extensibility to support others, whether they are newer/better or lesser due to hardware/scale limitations.  This probably means SHA-512 with salt, but I defer to folks with a better background in security.

An updated last call went out on 29 April, which calls out the two downrefs.  Thanks!

Thank you for adding a reference to Section 4 of RFC3579 for the threats to radius.  This will close out my discuss as the other references describe the protocol without going deep enough into the threats for both privacy and security (the otehr references are from 2000, so that makes sense).

The reference for RADIUS and other authentication methods are good and help to clear one of my discuss points, but should also include RFCs that have the threats included.  The current references in the draft discuss the protocol, not really the threats and don't have security consideration sections.  The best reference I could find is a discussion of RADIUS with IPsec in RFC3579.  Then there is an experimental RFC that discusses the threats in detail, advocating for TLS is RFC6614, but that would require a downref.  So adding in a reference to the security considerations in RFC3579 would help.




The DISCUSS that was previously listed did not result in a change to the boilerplate or RFC6241 and I think there is an issue with the wording still.  Since the only available transport options include session encryption, the concern is addressed, but the requirement for transport encryption should be more explicit in the language.  Text included below.

2. Although SSH is Mandatory to Implement, it may not be mandatory to use and the risks should be called out or point to a discussion on the risks explicitly in this section.

In the draft under review and the referenced RFC6241, there is no explicit requirement for session encryption to provide protection against passive or active attacks (Man in the Middle).  Although the referenced RFC calls for integrity and confidentiality, it only says this "could" be provided through SSH or TLS.  It does rely on transport to provide theses features, but falls short of covering the current threat landscape where we have to worry about passive and active monitoring.  In other words, you could provide confidentiality and integrity in other ways (not likely, but possible).  Stronger language to require session encryption explicitly would help either in this draft or section 2.2 of RFC6241.  Even better would be to explicitly call out the threat of passive and active monitoring as the only one listed is replay protection right now.  Then say protections are provided through a requirement for session encryption.  Proposals of where this should be and what wording is preferred are welcome (this draft, the template, or the base RFC6241).

My discuss ended up being basically "what about securre NTP and
more fully featured SSH admin and DNSSEC and other trust
anchors?" to which the responses were that those could be done
as extensions, but had not yet been done because there was a 
desire to start with a simple model. And they might also  be a lot
of work to get done. 

On that basis, I've cleared my discuss and hope someone does
get back around to thnking about those issues sometime.

--- OLD Comments below here

- 2.3: why not also Diameter? I can buy doing that later if
you say that is less important for enterprise use-cases I
guess.

- 3.1: where is inet:domain-name defined? Is it ok that all
hostnames (managed this way) must be valid DNS names?

- 3.5.3: I agree with Kathleen's discuss point#1 that more
details of which RADIUS schemes would be good to add.

- The crypt-hash typedef seems very specific to Linux.  Is
that linux specific of glibc? And a wikipedia article might
not make the RFC editor happy.

- I agree with Kathleen's discuss point #2  that it might be
reasonable to go beyond the boilerplate security
considerations for this data model since it is transporting
cleartext passwords and public keys used to authenticate and
device settings (e.g. DNS) that could easily subvert almost
any security function on the device.