comments on draft-ietf-manet-report-mib-04

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