Re: [Disman] [MIB-DOCTORS] comments on draft-ietf-manet-report-mib-04

[Benoit Claise <[e-mail]>]

Thanks Randy for your time.

draft-ietf-manet-report-mib authors,
What really concerns me is Randy's comment: "It's pretty obvious that 
this module has not seen a MIB compiler recently."
This is inappropriate to ask for a MIB doctor review for a MIB module 
that doesn't even compile.

Regards, Benoit
> Hi -
>
> I was asked to take a look at draft-ietf-manet-report-mib-04.
> This is *not* a full MIB doctor review; in my opinion some
> fundamental issues need to be resolved before a detailed
> review (either from a MIB doctor or disman perspective)
> would be worthwhile.
>
> These are mostly just top-level issues that absolutely need to
> be resolved.  I fear that even though the questions aren't
> terribly difficult, the fixes may result in significant document
> churn.  There are also some odds and ends that jumped out that
> should be fixed before bothering with a serious review. I didn't
> go into a lot of detail because the sheer number of problems
> would make any such attempt seem like a DoS attack.  The categories
> here are:
>    (1) data type difficulties
>    (2) time stamping
>    (3) access control
>    (4) odds and ends
>    (5) questions of power
>
> Data Type Difficulties
>    
>    Location: Abstract and elswhere
>
>    Example of Current text:
>      In particular, it describes objects for configuring autonomous report
>      generation on any device that supports MIBs containing objects that
>      resolve to type Integer32 (i.e., Integer32, Counter, Gauge, or
>      TimeTicks). to be used for performance monitoring.
>
>    Problems:
> 	- typographical nit: misplaced "." after ")"
>          - technical: "Counter" is not an SMIv2 type.  I assume
>                       "Counter32" is intended.  If inclusion of
>                       Counter64 is also intended, there are other problems.
>          - technical: "Gauge" is not an SMIv2 type.  I assume
>                       "Gauge32" is intended.
> 	- technical:  the range of type Integer32 (defined in RFC 2578)
>                        does not cover the entire ranges of type Counter32,
>                        Gauge32, or TimeTicks, as defined in RFC 3416.
>                        The SMI type *Unsigned32* does cover the same ranges.
> 	- technical: the use of "resolve" is questionable, since all
>                       of the data types mentioned have distinct ASN.1
>                       tags.  (RFC 3416 section 3)
>
>    It gets worse.  The definitions for reportSampledCurrentMeasurementValue
>    and reportSampledHistoricalReportsValue limit their ranges to
>    1 to 2,147,483,647.  This means that samples of Counter32, Gauge32,
>    and TimeTicks with values of 0 or anything in the range from
>    2,147,483,647 to 4,294,967,295 cannot be represented.
>
>    The reportSampledCurrentMeasurementStatus and the
>    reportSampledHistoricalMeasurementStatus OBJECT-TYPEs appear
>    to be an attempt to address this problem, and would *almost*
>    work if the MeasurementValue were of type Unsigned32, and
>    if its range were not constrained.  However, if the module
>    functioned as though sampled data, regardless of type, were
>    "cast" to Unsigned32, then the valuePositive(2) and valueNegative(3)
>    aren't really needed, since the signedness of the sampled values
>    can be recovered from reportSampledStudyOid.
>
>
> Time Stamping
>    There are a bunch of concerns related to time stamping.
>
>    - several OBJECT-TYPE definitions have impossible syntaxes.
>      "sysUptime" is not a syntax.  It should not be IMPORTed.
>      For reportSampledCurrentMeasurementTime and for
>      reportSampledHistoricalMeasurementTime you intend to
>      use a syntax of "TimeStamp"
>
>    - HOWEVER, there is a health warning attached to the
>      TimeStamp textual convention in RFC 2579, and it applies
>      here.  Firstly, it applies because the set of values
>      permitted for reportSampledStudySamplingInterval means
>      it is possible to construct a single report in which
>      the sample times are ambiguous.  More significantly,
>      for historical reports, especially for "archival" purposes,
>      the sample times are almost certainly ambiguous.  Consequently,
>      I think there needs to be a re-think of whether this is
>      the correct data type for the intended use.
>
>    - Many counter objects have "discontinuity indicators" other
>      than sysUpTime. This module does not provide a straightforward
>      way to cope with these.  See RFC 2981 (search for "discontinuity")
>      to see one approach.
>
>    - reportSampledStudySamplingInterval may be modified during a
>      report.  This means that conventional algorithms for detecting
>      sysUpTime discontinuities within a report may not work.
>
> Access Control
>    There are a couple of ways in which access control considerations
>    might affect the design of this module.  The presence of
>    reportSampledStudyOwner suggests that someone was thinking about
>    this, but some changes are needed to make things work.
>
>      (1) reportSampledControlEntry should have an INDEX of
>          { reportSampledStudyOwner, reportSampledStudyIndex }
>          This is necessary if you want to be able to limit access
>          to specific studies to specific users in a reasonable way,
>          and to prevent collisions between users during the creation
>          of studies.
>
>      (2) Likewise, reportSampledCurrentReportsStatusEntry should
>          also have an INDEX of
>          { reportSampledStudyOwner, reportSampledStudyIndex },
>          reportSampledCurrentReportsEntry should have
>          { reportSampledStudyIndex, reportSampledCurrentMeasurementIndex }
>          reportSampledHistoricalReportsEntry should have
>          { reportSampledStudyOwner, reportSampledStudyIndex,
>            reportSampledHistoricalReportIndex,
>            reportSampledHistoricalMeasurementIndex }
>
>      (3) A consequence is that (for example) reportSampledStudyIndex
>          only needs to be unique for a given owner, and not among
>          all studies.
>
>    Note that reportSampledDataCollectionFailure suffers from the
>    "who's buried in Grant's Tomb" problem, and can be simplified.
>
>    A second way in which access control matters here is that the
>    sampling process should not circumvent access control.  I strongly
>    urge putting in explicit language about this.  I'd suggest the following
>    as a starting point for discussion:
>
>       Security for local access effectively requires recording appropriate
>       security credentials of the creator of a reportSampledControlEntry
>       and using those to access the local objects.  These security
>       credentials are parameters necessary as inputs to isAccessAllowed
>       from the Architecture for Describing SNMP Management Frameworks.
>       The system MUST (conceptually) use isAccessAllowed at the time of
>       each access (not just creation of the reportSampledControlEntry)
>       to ensure that it does not permit unauthorized access.
>
>       When using VACM, the security administrator SHOULD ensure that
>       access permitted to the collected data and notifications (that is,
>       access to reportSampledCurrentReportsStatusTable.*.<user>,
>       reportSampledCurrentReportsTable.*.<user>, and
>       reportSampledHistoricalReportsTable.*.<user>) is
>       limited to the <user>'s appropriate VACM group.
>
> Odds and ends:
>    It's pretty obvious that this module has not seen a MIB compiler
>    recently.  In addition to the syntactic problem, there are some
>    glaring stylistic issues that, if fixed now, will make an eventual
>    MIB doctor review go a lot smoother.
>
>     (1) Use of data types.  Every single use of Integer32 in this
>         module is arguably incorrect.  Unsigned32 would be a better
>         choice, since these are all sub-typed to exclude negative
>         values.
>
>    (2) Use of data types.  reportSampledStudySamplingInterval
>        seems like it should make use of RFC 2579 TimeInterval, perhaps
>        constrained to be at least 10.
>
>    (3) Use of UNITS.  Though it's a little unusual (but not unheard-of)
>        to use UNITS with non-counter types, a value of "count" in,
>        for example, reportSampledStudyMaximumNumberOfHistoricalReports,
>        is really not helpful.  In that particular case, "reports"
>        would make more sense.
>
>    (4) Attention to error cases: there are a bunch of error situations
>        that should be given a little attention.  What happens when
>        there's no longer enough memory to keep accumulating studies?
>        What happens when the object instance referred to by
>        reportSampledStudyOid is the wrong data type?
>
>    (5) The use of non-volatile storage doesn't seem to have been
>        completely thought through.  It doesn't make sense to me, for
>        example, to have the control entries in backing storage, but
>        not the "archival" historical data.  Likewise, if a control
>        entry is restored from non-volatile upon reboot, does that
>        cause a new report to be started or does it pick up where
>        it left off on a existing report?
>
>    (6) Notification throttling.
>
> Questions of Power
>    I think the decision to limit the module to local access is reasonable
>    given the intended applicability.  I do think, however, there should
>    be a clear explanation of the rationale behind limiting its functionality
>    to the current SNMP context, unlike RFC 1981.
>
>    I think it would be helpful to at least suggest how it is envisioned
>    that future "statistical" functions alluded to in the text would work
>    with this MIB module -   are they expected to be extensions of this
>    module or completely new ones?
>
>    I honestly have some difficulty seeing what good this module would
>    do; perhaps the WG has discussed concrete use cases, but don't be
>    surpised if a future reviewer raises questions on this point.
>
> Randy
>
> _______________________________________________
> MIB-DOCTORS mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/mib-doctors
> .
>