Subject: [netmod] AD review: draft-ietf-netmod-snmp-cfg-05

From: Benoit Claise <[email-masked]>
Date: Tue, 03 Jun 2014 18:15:08 +0200

Dear all,

This is my AD review.
Good work on a complete YANG model. It depends heavily on the knowledge 
of the different SNMP MIB modules: SNMP-TARGET-MIB, 
SNMP-NOTIFICATION-MIB, SNMP-PROXY-MIB, SNMP-COMMUNITY-MIB, etc. This is 
good as there is no specification redefinition. And I'm glad to find the 
examples in the appendix for the operators wanted "standard" SNMP 
configurations.

My feedback:
-

    The configuration data model in particular_targets_SNMP deployments
    where SNMP runs in read-only mode and NETCONF is used to configure
    the SNMP agent.  Nevertheless, the data model has been_designed_to
    allow implementations that support write access both via SNMP and
    NETCONF in order to interwork with SNMP-managed management
    applications manipulating SNMP agent configuration using SNMP.

    The YANG data model focuses on configuration.

I don't understand what you mean by "targets" or "designed to"
So you made some tradeoffs in the design? Is this the way we should understand this?
If which one?

- At the beginning of VACM and SNMP, we faced one issue. Someone with a read-only community string could query the read-write community string.

Router#sh run | i snmp

snmp-server engineID local 000000090200009092827820

snmp-server group v1group v1 read includeeverything

snmp-server view includeeverything internet included

snmp-server community _claise _RW

snmp-server user _public _v1group v1

...

"snmpwalk -v 1 <Router> _public _internet.6.3.16 | grep _claise_"... you 
would be surprised

Basically, the trick is that we need a default view on VACM.
I see http://tools.ietf.org/html/rfc3415#section-7.4
Do we need something specific in this draft to stress that issue?

Editorial:

-

One small alignment issue below: see the port line

     +--rw snmp
          +--rw engine
             +--rw enabled?               boolean
             +--rw listen* [name]
             |  +--rw name    snmp:identifier
             |  +--rw (transport)
             |     +--:(udp)
             |        +--rw udp
             |           +--rw ip      inet:ip-address
            |           +--rw port?   inet:port-number
             +--rw version
             |  +--rw v1?    empty
             |  +--rw v2c?   empty
             |  +--rw v3?    empty
             +--rw engine-id?             snmp:engine-id
             +--rw enable-authen-traps?   boolean

-

    The "/snmp/engine/version" container can be used to enable/disable
    the different message processing models.

"message processing models": that's a specific SNMP term, well not well known to SNMP users.
I would add a reference to RFC 3412

-
Similarly to
    This submodule defines the feature "tsm".  A server implements this
    feature if it supports the Transport Security Model (tsm) [RFC5591  <http://tools.ietf.org/html/rfc5591>].

    ...

      feature tsm {
        description
          "A server implements this feature if it supports the
          Transport Security Model for SNMP.";
        reference
          "RFC5591  <http://tools.ietf.org/html/rfc5591>: Transport Security Model for the
                    Simple Network Management Protocol (SNMP)";
      }

It would be great to have references for:

    This submodule defines the feature "notification-filter".  A server
    implements this feature if it supports SNMP notification filtering.

    ...

      feature notification-filter {
        description
          "A server implements this feature if it supports SNMP
          notification filtering.";
      }

And for

    This submodule defines the feature "proxy".  A server implements this
    feature if it can act as an SNMP Proxy.

    ...

      feature proxy {
        description
          "A server implements this feature if it can act as an
          SNMP Proxy";
      }

-

      augment /snmp:snmp/snmp:target {
        when "snmp:v1 or snmp:v2c";
        leaf mms {
          type union {
            type enumeration {
              enum "unknown" { value 0; }
            }
            type int32 {
              range "484..max";
            }
          }
          default "484";
          reference
            "SNMP-COMMUNITY-MIB.snmpTargetAddrMMS";
        }
      }

mms: I thought it was a mistake: nms versus mms.
I had to lookup snmpTargetAddrMMS to understand "maximum packet size"
Proposal: either add a description or have a better name

Regards, benoit

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Sat, 7 Jun 2014 09:02:14 +0200

> So you made some tradeoffs in the design? Is this the way we should 

I think "in particular targets" should be read as in "will likely be
primarily used in". Or to say it in other words: If a deployment uses
SNMP to configure SNMP agents, then this data model may be of limited
value.

Concerning "designed to": The data model has been designed to cover
everything that the SNMP configuration models describe. It allows an
implementation that supports both configuration via NETCONF and
configuration via SNMP. The details are discussed in section 3.2.

> - At the beginning of VACM and SNMP, we faced one issue. Someone with a 
> read-only community string could query the read-write community string.
> 
> 
> Router#sh run | i snmp
> 

I do not think that this document is the place to discuss how default
VACM rules should look like. This should go into a separate document
because this is not specific to the YANG data model but rather
specific to VACM.

>             +--rw enable-authen-traps?   boolean
 
fixed 
 
>    <http://tools.ietf.org/html/rfc5591>].

OK. Although I think this should be a reference to RFC 3411 since RFC
3411 defines the components (called models in SNMP speak) and RFC 3412
is just one specific instance.

>    implements this feature if it supports SNMP notification filtering.

Added reference to RFC 3413.
 
>          notification filtering.";

Added reference	to RFC 3413.

>    feature if it can act as an SNMP Proxy.

Added reference to RFC 3413.

>          "A server implements this feature if it can act as an

Added reference to RFC 3413.

> Proposal: either add a description or have a better name

I prefer to keep the name since the SNMPv3 specifications use
this name. I added a description clause (and I surprised that
a leaf without a description clause slipped through).

Do you want us to post a new revision?

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: +49 421 200 3587         Campus Ring 1, 28759 Bremen, Germany
Fax:   +49 421 200 3103         <http://www.jacobs-university.de/>

---

From: Benoit Claise <[email-masked]>
Date: Tue, 17 Jun 2014 15:45:39 +0200

Jüergen,

Sorry for the delay in getting back to you.
> SNMP to configure SNMP agents, then this data model may be of limited
I didn't read this way the first time. Maybe it's only me...
> configuration via SNMP. The details are discussed in section 3.2.
I didn't read this way the first time. Your new wording makes it clearer 
to me.
Again, maybe it's just me...
> because this is not specific to the YANG data model but rather
Yes, this is specific to VACM, but having an extra document just for 
this might be an overkill.
Adding one sentence in the Security Considerations would be make IMO.
> is just one specific instance.
Sure.
> a leaf without a description clause slipped through).
A description makes sense.
> Do you want us to post a new revision?
Yes please.

Regards, Benoit