RE: MIB Doctor Review of draft-ietf-adslmib-gshdslbis-07.txt

Thanks very much Mike for your thourough review.

I think we can do one of two things:

- I go ahead with IETF Last Call and the editors/authors address
  your comment as part of any other IETF last Call comments
- The editors/authors address your comments forst before I issue
  IETF Last Call.

I slightly prefer the latter. Editors/authors?

Bert

> -----Original Message-----
> From: C. M. Heard [mailto:[email protected]]
> Sent: Sunday, January 09, 2005 05:04
> To: ADSL MIB (E-mail)
> Cc: Wijnen, Bert (Bert)
> Subject: MIB Doctor Review of draft-ietf-adslmib-gshdslbis-07.txt
> 
> 
> Howdy,
> 
> Bert Wijnen asked me to do take a quick look at
> draft-ietf-adslmib-gshdslbis-07.txt before it is sent out for IETF
> last call.  I think that there are some issues with the Security
> Considerations section in the draft that warrant a clean-up before
> the document goes to the IESG, and I have also found a few minor
> editorial ssues.  Regarding the MIB module itself, I looked only at
> the changes that have been made since RFC 3276;  based on that, it
> appears to be in good shape and ready to go.
> 
> Details are in the MIB Doctor check list below.  (It does not quite
> match Appendix A of draft-ietf-ops-mib-review-guidelines-03.txt
> because it is based on a new version of the checklist that the MIB
> Doctors have been discussing on the mreview list.  You are the first
> guinea pigs :)
> 
> 1.) I-D Boilerplate -- OK
> 
> 2.) Abstract -- OK
> 
> 3.) MIB Boilerplate -- OK
> 
> 4.) Security Considerations Section -- this section does not comply
> with the Security Guidelines for IETF MIB Modules at
> http://www.ops.ietf.org/mib-security.html.  Editorially, the
> material appears in an unexpected order, which makes it hard it hard
> to read.  Also, there are several redundant/obsolete paragraphs that
> appear to be the result of splicing the text from the current
> guidelines onto the front of the old security text.  There are also
> some substantive deficiencies.  Because of this I believe that a
> rewrite is needed before submission to the IESG.  Specific
> suggestions follow.
> 
> (a) The opening paragraph, viz.,
> 
>    There are a number of management objects defined in this MIB module
>    with a MAX-ACCESS clause of read-write and/or read-create.  Such
>    objects may be considered sensitive or vulnerable in some network
>    environments.  The support for SET operations in a non-secure
>    environment without proper protection can have a negative effect on
>    network operations.
> 
> is fine.  However, the MIB security guidelines say:
> 
> -- if you have any read-write and/or read-create objects, please
> -- describe their specific sensitivity or vulnerability.
> 
> and I have interpreted that to mean that the sensitivity or
> vulnerability of all read-write and read-create objects has to be
> explicitly described.  That description is supposed to follow the
> first paragraph.  I don't see any such descriptions.  The
> read-create objects in this MIB module are:
> 
> hdsl2ShdslSpanConfNumRepeaters
> hdsl2ShdslSpanConfProfile
> hdsl2ShdslSpanConfAlarmProfile
> hdsl2ShdslEndpointAlarmConfProfile
> hdsl2ShdslMaintLoopbackConfig
> hdsl2ShdslMaintPowerBackOff
> hdsl2ShdslMaintSoftRestart
> hdsl2ShdslMaintLoopbackTimeout
> hdsl2ShdslSpanConfWireInterface
> hdsl2ShdslSpanConfMinLineRate
> hdsl2ShdslSpanConfMaxLineRate
> hdsl2ShdslSpanConfPSD
> hdsl2ShdslSpanConfTransmissionMode
> hdsl2ShdslSpanConfRemoteEnabled
> hdsl2ShdslSpanConfPowerFeeding
> hdsl2ShdslSpanConfCurrCondTargetMarginDown
> hdsl2ShdslSpanConfWorstCaseTargetMarginDown
> hdsl2ShdslSpanConfCurrCondTargetMarginUp
> hdsl2ShdslSpanConfWorstCaseTargetMarginUp
> hdsl2ShdslSpanConfUsedTargetMargins
> hdsl2ShdslSpanConfReferenceClock
> hdsl2ShdslSpanConfLineProbeEnable
> hdsl2ShdslSpanConfProfileRowStatus
> hdsl2ShdslEndpointThreshLoopAttenuation
> hdsl2ShdslEndpointThreshSNRMargin
> hdsl2ShdslEndpointThreshES
> hdsl2ShdslEndpointThreshSES
> hdsl2ShdslEndpointThreshCRCanomalies
> hdsl2ShdslEndpointThreshLOSWS
> hdsl2ShdslEndpointThreshUAS
> hdsl2ShdslEndpointAlarmConfProfileRowStatus
> 
> Some of these objects undoubtedly could, if misconfigured, cause
> traffic disruptions. Others (such as hdsl2ShdslEndpointThreshSNRMargin
> and hdsl2ShdslEndpointThreshLoopAttenuation) could possibly be
> misconfigured in such a way as to allow notification floods.  Some
> might cause only minor (or maybe even no) ill effects.  Whatever
> is the case, this section needs to spell that out explicitly for
> each writeable object.  If some considerations apply to several
> objects, it is of course OK to say to list the those objects rather
> than repeating the text for each;  the important thing is to make
> the information available for every writeable object.
> 
> The following text (which is presently in the draft) is NOT an
> acceptable substitute for the specific descriptions required by the
> guidelines.  Furthermore, the first sentence is simply wrong, since
> it contradicts the recommendation elsewhere in the text to use
> SNMPv3 security to do just the thing that it it says is out of
> scope.
> 
>    Blocking unauthorized access to the HDSL2-SHDSL MIB via the element
>    management system is outside the scope of this document.  It should
>    be noted that access to the MIB permits the unauthorized entity to
>    modify the profiles such that both subscriber service and network
>    operations can be interfered with.  Subscriber service can 
> be altered
>    by modifying any of a number of service characteristics 
> such as rate
>    partitioning and maximum transmission rates.  Network 
> operations can
>    be impacted by modification of notification thresholds such as SES
>    thresholds.
> 
> (b) The text on readable objects and the remainder of the
> boilerplate should be condensed and reorganized into standard form.
> I suggest:
> 
>    Some of the readable objects in this MIB module ( i.e., 
> objects with
>    a MAX-ACCESS other than not-accessible) may be considered sensitive
>    or vulnerable in some network environments.  In particular, certain
>    objects will reveal information about which vendor's 
> equipment is in
>    use on the network, which in many enviromments may be considered
>    sensitive for competitive reasons.  It is thus important to control
>    even GET and/or NOTIFY access to these objects and possibly even to
>    encrypt their values when sending them over the network via SNMP.
> 
>    These identifying objects in the inventory group are:
> 
>    - hdsl2ShdslInvVendorID
>    - hdsl2ShdslInvVendorModelNumber
>    - hdsl2ShdslInvVendorSerialNumber
>    - hdsl2ShdslInvVendorEOCSoftwareVersion
>    - hdsl2ShdslInvStandardVersion
>    - hdsl2ShdslInvVendorListNumber
>    - hdsl2ShdslInvVendorIssueNumber
>    - hdsl2ShdslInvVendorSoftwareVersion
>    - hdsl2ShdslInvEquipmentCode
>    - hdsl2ShdslInvVendorOther
>    - hdsl2ShdslInvTransmissionModeCapability
> 
>    SNMP versions prior to SNMPv3 did not include adequate security.
>    Even if the network itself is secure (for example by using IPSec),
>    even then, there is no control as to who on the secure network is
>    allowed to access and GET/SET (read/change/create/delete) 
> the objects
>    in this MIB module.
> 
>    It is RECOMMENDED that implementers consider the security 
> features as
>    provided by the SNMPv3 framework (see [RFC3410], section 8),
>    including full support for the SNMPv3 cryptographic mechanisms (for
>    authentication and privacy).
> 
>    Further, deployment of SNMP versions prior to SNMPv3 is NOT
>    RECOMMENDED.  Instead, it is RECOMMENDED to deploy SNMPv3 and to
>    enable cryptographic security.  It is then a customer/operator
>    responsibility to ensure that the SNMP entity giving access to an
>    instance of this MIB module is properly configured to give 
> access to
>    the objects only to those principals (users) that have legitimate
>    rights to indeed GET or SET (change/create/delete) them.
> 
>    It should be noted that interface indices in this MIB module are
>    maintained persistently.  View-based Access Control Model (VACM)
>    data relating to these should be stored persistently.
> 
> (c) Regarding this:
> 
>    HDSL2-SHDSL layer connectivity from the xtuR will permit the
>    subscriber to manipulate both the HDSL2-SHDSL link directly and the
>    HDSL2-SHDSL embedded operations channel (EOC) for their own loop.
>    For example, unchecked or unfiltered fluctuations initiated by the
>    subscriber could generate sufficient notifications to potentially
>    overwhelm either the management interface to the network or the
>    element manager.
> 
> It is not sufficient just to state that notification flooding can
> occur.  It is necessary to provide a mechanism to prevent it.  Cf.
> the following discussion on the mreview list from December 2002:
> 
> | On Sat, 28 Dec 2002 Bert Wijnen wrote:
> | > >    2) DoS attacks (as described in some of the ADSL MIBs'
> | > >       security considerations sections) based on the
> | > >       conditions under which notifications are generated.
> | > > 
> | > Mmm... I wonder... in the end it depends on
> | > - having proper access control to those objects that control/limit
> | >   the sending of notifications (for example access to the 
> tables in
> | >   RFC3413).
> | > - ensuring that no notification flooding will take place.
> | >   That I guess depends on proper mib design and the MIB 
> doctors should
> | >   be looking for such issues. I don't think it is OK to 
> just say that
> | >   DoS attacks are possible. Better to build in controls 
> to prevent it.
> | 
> | There is now text in 4.7 to the effect that notifications which can
> | be generated by rapidly changing external conditions SHOULD have a
> | rate-limiting mechanism in order to avoid overwhelming the network
> | with floods of notifications.  RFC 2108/RFC 2737 are cited 
> as examples.
> 
> The "text in 4.7" referred to above is this:
> 
>    In many cases notifications will be triggered by external 
> events, and
>    sometimes it will be possible for those external events to 
> occur at a
>    sufficiently rapid rate that sending a notification for each
>    occurrence would overwhelm the network.  In such cases a mechanism
>    MUST be provided for limiting the rate at which the 
> notification can
>    be generated.  A common technique is to require that the 
> notification
>    generator use throttling -- that is, to require that it generate no
>    more than one notification for each event source in any given time
>    interval of duration T.  The throttling period T MAY be 
> configurable,
>    in which case it is specified in a MIB object, or it MAY 
> be fixed, in
>    which case it is specified in the notification definition. 
>  Examples
>    of the fixed time interval technique can be found in the SNMP-
>    REPEATER-MIB [RFC2108] and in the ENTITY-MIB [RFC2737bis].
> 
> If I correctly understood what I read in the MIB module (and it is
> certainly possible that I did not), then it would appear that the
> notifications that could be flooded in case of fluctuations
> initiated by the subscriber are hdsl2ShdslLoopAttenCrossing and
> hdsl2ShdslSNRMarginCrossing, which are controlled by the writeable
> objects hdsl2ShdslEndpointThreshLoopAttenuation and
> hdsl2ShdslEndpointThreshSNRMargin.  At the very least the user
> should be warned that incorrect configuration of these objects could
> lead to that exposure (cf. (a) above).  You may also want to
> consider adding a throttling mechanism to those notifications.  As
> far as I could tell, the other notifications are already
> rate-controlled.
> 
> (d) Regarding this:
> 
>    It is conceivable that a management application that was 
> designed to
>    support G.SHDSL as defined in RFC 3276 [RFC3276] could be 
> broken by a
>    G.shdsl.bis agent which reports objects for additional 
> wire pairs (as
>    noted in Section 7).
> 
>    For example, if a management application blindly loaded object
>    instances into an array until the object changes (during repeated
>    GET-NEXT requests).  It is anticipated that the 
> modifications to the
>    management application code would be straightforward.
> 
> This is not really a security issue, it is an implementation
> consideration, and it is already dealt with (very well, I might add)
> in the Implementation Analysis section.  Please remove it from here.
> 
> (e) Please get rid of all redundant/obsolete stuff from previous
> versions of the security boilerplate.  If you include the material I
> requested in (a) and (b) above that should be enough.
> 
> 5.) IANA Considerations Section -- OK
> 
> 6.) References -- there are a few minor issues here.
> 
> (a) I don't think that RFC 3276 should be normative, since it is not
> necessary to consult with it in order to implement the new revision
> of the MIB module.  Please move it to the Informative References
> section.
> 
> (b) The current version of Security Guidelines for IETF MIB Modules
> at http://www.ops.ietf.org/mib-security.html no longer requires the
> USM and VACM documents as references.  If as requested in (4) above
> the Security Considerations section is rewritten to conform to the
> guidelines, then [RFC3414] and [RFC3415] will no longer be needed
> and so should be eliminated (the RFC Editor will remove them if
> there is no citation in the text.)
> 
> (c) there are some typos in informative reference [RFC3410]:
> OLD:
>    [RFC3410]  Case, J., Mindy, R., Partain, D. and B. Stewart,
>               "Introduction and Applicability Statements for Internet
>               Standard Management Framework", RFC 3410, December 2002.
> NEW:
>    [RFC3410]  Case, J., Mundy, R., Partain, D. and B. Stewart,
>               "Introduction and Applicability Statements for Internet-
>               Standard Management Framework", RFC 3410, December 2002.
> 
> (d) For completeness, I verified that the documents containing MIB
> modules from which definitions are imported (viz., RFCs 2578, 2579,
> 2863, 3593, 3411, and 2580) are included among the Normative
> References and that they are cited in the text.
> 
> 7.) Copyright Notices -- OK
> 
> 8.) IPR Notice -- I see the following paragraph that is not actually
> required by RFC 3668:
> 
>    The IETF has been notified of intellectual property rights claimed
>    in regard to some or all of the specification contained in this
>    document.  For more information consult the online list of claimed
>    rights.
> 
> I don't think that this is a problem, however, since this text will
> be reviewed and if necessary revised during the RFC publication
> process.
> 
> 9.) Other issues -- editorial nits, typos, and anything mentioned in
> http://www.ietf.org/ID-Checklist.html that are not covered elsewhere:
> 
> (a) typo/punctuation in second paragraph of Section 3.1.2:
> OLD:
>    The following attributes are part of the mandatory 
> ifGeneral group in
>    RFC 2863 [RFC2863], and are not duplicated in the HDSL2/SHDSL Line
>    MIB.
> NEW:
>    The following attributes are part of the mandatory
>    ifGeneralInformationGroup in RFC 2863 [RFC2863] and are not
>    duplicated in the HDSL2/SHDSL Line MIB.
> 
> (b) in Figure 1, the following was omitted:
> 
>       ifAlias                  See interfaces MIB [RFC2863].
> 
> (c) in the DESCRIPTION clause of hdsl2ShdslSpanConfUsedTargetMargins:
> OLD:
>         "Contains indicates whether a target SNR margin is enabled or
>          disabled.  This is a bit-map of possible settings.  The
>          various bit positions are:
> NEW:
>         "Indicates whether a target SNR margin is enabled or
>          disabled.  This is a bit-map of possible settings.  The
>          various bit positions are:
> 
> (d) punctuation in next-to-last paragraph of Section 7:
> OLD:
>    A management application intended to manage G.shdsl.bis agents,
>    should be modified to accept this sequence.
> NEW:
>    A management application intended to manage G.shdsl.bis agents
>    should be modified to accept this sequence.
> 
> 10.) Technical content -- review of actual technical content for
> compliance with <draft-ietf-ops-mib-review-guidelines-03.txt>:
> 
> (a) MIB compilation:
> 
> Running smilint identified no errors or warnings.
> 
> Running smidiff to evaluate the changes with respect to rfc3276
> produced numerous messages.  The ones that warrant comment are
> discussed below.
> 
> HDSL2-SHDSL-LINE-MIB.mi2:418 [3] {range-changed} range of type used
> in `hdsl2ShdslStatusMaxAttainableLineRate' changed from
> `(0..4112000)' to `(0..4294967295)'
> HDSL2-SHDSL-LINE-MIB.mi2:432 [3] {range-changed} range of type used
> in `hdsl2ShdslStatusActualLineRate' changed from
> `(0..4112000)' to `(0..4294967295)'
> HDSL2-SHDSL-LINE-MIB.mi2:1646 [3] {range-changed} range of type used
> in `hdsl2ShdslSpanConfMinLineRate' changed from
> `(0..4112000)' to `(0..4294967295)'
> HDSL2-SHDSL-LINE-MIB.mi2:1664 [3] {range-changed} range of type used
> in `hdsl2ShdslSpanConfMaxLineRate' changed from
> `(0..4112000)' to `(0..4294967295)'
> 
> These are flagged as level 3 by smidiff because liberalization of a
> range is not among the changes permitted by RFC 2578.  However, I
> think that what you are doing here is correcting a bug in the
> original MIB module by relaxing an arbitrarily restrictive subrange,
> and I agree that it is a good thing to do.  Furthermore, I see that
> SYNTAX refinements have been added to the compliance statement
> requiring only the subrange (0..4112000) for these objects:
> 
> HDSL2-SHDSL-LINE-MIB.mi2:2362 [5] {refinement-added} warning:
> object refinement for `hdsl2ShdslStatusMaxAttainableLineRate'
> added to `hdsl2ShdslLineMibCompliance'
> HDSL2-SHDSL-LINE-MIB.mi2:2370 [5] {refinement-added} warning:
> object refinement for `hdsl2ShdslStatusActualLineRate' added
> to `hdsl2ShdslLineMibCompliance'
> HDSL2-SHDSL-LINE-MIB.mi2:2378 [5] {refinement-added} warning:
> object refinement for `hdsl2ShdslSpanConfMinLineRate' added
> to `hdsl2ShdslLineMibCompliance'
> HDSL2-SHDSL-LINE-MIB.mi2:2386 [5] {refinement-added} warning:
> object refinement for `hdsl2ShdslSpanConfMaxLineRate' added
> to `hdsl2ShdslLineMibCompliance'
> 
> I see also that this issue is explicitly discussed in Section 7 of
> the text, which I would not have asked for but which is a very good
> idea.  So I think you are good to go on this score.
> 
> I see also that some enumerations have been added in a couple of
> places.  One is in the definition of the Hdsl2ShdslWirePair TC:
> 
> HDSL2-SHDSL-LINE-MIB.mi2:221 [5] {named-number-added} warning: named
> number `wirePair3' added to type `Hdsl2ShdslWirePair'
> HDSL2-SHDSL-LINE-MIB.mi2:221 [5] {named-number-added} warning: named
> number `wirePair4' added to type `Hdsl2ShdslWirePair'
> 
> This TC is used in the definition of hdsl2ShdslEndpointWirePair:
> 
> HDSL2-SHDSL-LINE-MIB.mi2:740 [5] {named-number-added} warning: named
> number `wirePair3' added to type used in `hdsl2ShdslEndpointWirePair'
> HDSL2-SHDSL-LINE-MIB.mi2:740 [5] {named-number-added} warning: named
> number `wirePair4' added to type used in `hdsl2ShdslEndpointWirePair'
> 
> which is an INDEX object used only in tables that do not support
> dynamic row creation.  Thus, the agent decides unilaterally for
> which values a row is instantiated, hence adding these values has no
> effect on the semantics of the compliance statement.  So this is OK.
> 
> Another place is in the definition of hdsl2ShdslSpanConfWireInterface:
> 
> HDSL2-SHDSL-LINE-MIB.mi2:1628 [5] {named-number-added} warning: named
> number `sixWire' added to type used in 
> `hdsl2ShdslSpanConfWireInterface'
> HDSL2-SHDSL-LINE-MIB.mi2:1628 [5] {named-number-added} warning: named
> number `eightWire' added to type used in 
> `hdsl2ShdslSpanConfWireInterface'
> 
> and I see that it now has a refinement so that only the original
> values twoWire(1) and fourWire(2) are required to be supported:
> 
> HDSL2-SHDSL-LINE-MIB.mi2:2350 [5] {refinement-added} warning:
> object refinement for `hdsl2ShdslSpanConfWireInterface' added
> to `hdsl2ShdslLineMibCompliance'
> 
> So you are good to go here, too.
> 
> I see that there are some new objects:
> 
> HDSL2-SHDSL-LINE-MIB.mi2:453 [5] {node-added} warning: column
> `hdsl2ShdslStatusMaxAttainablePayloadRate' has been added
> HDSL2-SHDSL-LINE-MIB.mi2:467 [5] {node-added} warning: column
> `hdsl2ShdslStatusActualPayloadRate' has been added
> HDSL2-SHDSL-LINE-MIB.mi2:1141 [5] {node-added} warning: column
> `hdsl2ShdslEndpointCurrTipRingReversal' has been added
> HDSL2-SHDSL-LINE-MIB.mi2:1155 [5] {node-added} warning: column
> `hdsl2ShdslEndpointCurrActivationState' has been added
> 
> and that they are packaged into two new object groups:
> 
> HDSL2-SHDSL-LINE-MIB.mi2:2645 [5] {node-added} warning: group
> `hdsl2ShdslWirePairGroup' has been added
> HDSL2-SHDSL-LINE-MIB.mi2:2658 [5] {node-added} warning: group
> `hdsl2ShdslPayloadRateGroup' has been added
> 
> which in turn have been added to the compliance statement as
> optional groups:
> 
> DSL2-SHDSL-LINE-MIB.mi2:2338 [2] {option-added} optional group
> `hdsl2ShdslWirePairGroup' added to `hdsl2ShdslLineMibCompliance'
> HDSL2-SHDSL-LINE-MIB.mi2:2344 [2] {option-added} optional group
> `hdsl2ShdslPayloadRateGroup' added to `hdsl2ShdslLineMibCompliance'
> 
> Again, these are flagged at level 2 since RFC 2580 does not
> specifically permit adding optional groups to a compliance
> statement.  However this really is OK since it does not change
> the semantics.  So you are good to go here, too.
> 
> (b) I have also verified that all the changes reported by smidiff
> are accounted for in the current REVISION clause change log.
> 
> This concludes the review of draft-ietf-adslmib-gshdslbis-07.txt.
> 
> Mike Heard
>