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

Bert and Mike,

Let's hold off on the IETF Last Call and I'll look over and work the issues.

- Clay

Wijnen, Bert (Bert) wrote:

>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
>>
>>    
>>
>
>_______________________________________________
>Adslmib mailing list
>[email protected]
>https://www1.ietf.org/mailman/listinfo/adslmib
>  
>

-- Paradyne Mail --