[ipcdn] MIB doctor review of: draft-ietf-ipcdn-pktc-mtamib-01.txt (Part 1)

["C. M. Heard" <[e-mail]>]

On Wed, 23 Jul 2003, C. M. Heard wrote, in a message to the IPCDN
chairs and the responsible ADs:
> In case Bert Wijnen has not yet told you, I have volunteered to
> review draft-ietf-ipcdn-pktc-mtamib-01.txt
>
> I shall endeavour to complete this task on or before 11 August 2003.

As requested by Jean-Francois, the review comments are being posted
to the [e-mail] mailing list with cc: to co-authors and to Bert.
These follow the outline suggested by the MIB review checklist from
Appendix A of <draft-ietf-ops-mib-review-guidelines-01.txt>.

Unfortunately, I have not been able to complete checklist item #11,
which is the review of the technical content and so is the most
important thing.  This is mainly due to the large number of small
things that I've found wrong ... not huge glaring technical problems
but relatively small things like lack of clarity and poor English
grammar in the DESCRIPTION and REFERENCE clauses.  I have decided
that it would be better to send the authors and WG what I have
completed to date, rather than delay for another couple of weeks,
because a lot of the comments are following the same pattern.  My
suggestion is to go over these comments and where I have pointed
out a problem with lack of clarity, style, grammar, etc. don't
just correct that specific place where I pointed it out but also
look for other places that have similar problems and correct them,
too.  If you can produce a -02 draft that doesn't suffer this
"death of 1000 cuts" it will be much easier to concentrate on
purely technical issues and the 2nd pass review will go much
more quickly.

If the WG, authors, and AD feel that this is not reasonable, then
feel free to request another reviewer.

Anyway, here are the comments.

1.) I-D Boilerplate -- please remove the citation [RFC2026]
in the first paragraph of the "Status of this Memo" section.
See http://www.ietf.org/ID-nits.html Section 1.1, 8th bullet.

2.) Abstract -- OK

3.) MIB Boilerplate -- OK

4.) IPR Notices -- (a) the minimum required notices are present, but
are not verbatim copies of 10.4(A) and 10.4(B) of RFC 2026, and the
editing process has introduced at least one (and possibly three)
minor errors or stylistic anomalies that need to be corrected:

s/described in document/described in this document/

s/implementers or users/implementors or users/

s/that may cover technology that/which may cover technology that/

(b) no claims are noted (10.4(D) of RFC 2026 is absent), and this
appears to be OK since the following search of the IPR statements on
the IETF web site did not turn up anything:

                          IETF Search Engine
   _________________________________________________________________

 Query: cable modem_______________________________________

 Press button to submit your query or reset the form: Search Reset

 Query Options:
 Maximum number of hits: [1000]
     Scope: [IPR Statements________]
     Match: [All____]
     Format: [Long_]
     Sort by: [Score________]
   _________________________________________________________________

                              Search results
   _________________________________________________________________

 No matches were found for '(cable or cabled or cabling or cables)
 and (modem or modems)'

If any WG participants are aware of pertinent intellectual property
claims that would not be revealed by this search, they are reminded
of their obligation to disclose that knowledge.

5.) References -- in addition to the non-ASCII characters in one of
the references (detailed in #10(a) below), there are the following
substantive issues to address:

(a) Bert Wijnen already pointed out that:
> You IMPORT from IF-MIB (RFC2863) and SNMP-FRAMEWORK-MIB (RFC3411)
> and SNMPv2-MIB (RFC3418).  So those must be listed as normative
> references.

(b) The normative reference [RFC3289] should be removed.  It is not
cited anywhere in the text, nor does the PKTC-IETF-MTA-MIB import
anything from DIFFSERV-DSCP-TC or DIFFSERV-MIB.

(c) The informative reference [RFC2026] should be removed.  It is
cited only in the I-D boilerplate, which will be stripped off prior
to publication as an RFC and is not allowed to have a citation (see
#2 above).

(d) The following PacketCable references need to be updated to
point to the latest versions of the documents:

Current reference           Latest version

PKT-SP-MIB-MTA-I06-030415   PKT-SP-MIB-MTA-I07-030728
PKT-SP-PROV-I06-030415      PKT-SP-PROV-I07-030728
PKT-SP-SEC-I08-030415       PKT-SP-SEC-I09-030728

When you make these updates, please be sure to get the spelling
of the document number right.  In the current draft I see that
all occurrences of [PKT-SP-PROV-IO6-030415] have the letter "O"
in "IO6" instead of the numeral "0".  This is also true for the
citation of [PKT-SP-MIB-MTA-IO6-030415] at the beginning of
Section 3.

You may also want to consider including the URLs where these
documents can be found (in addition to including the title
and document number):

http://www.packetcable.com/downloads/specs/PKT-SP-MIB-MTA-I07-030728.pdf
http://www.packetcable.com/downloads/specs/PKT-SP-PROV-I07-030728.pdf
http://www.packetcable.com/downloads/specs/PKT-SP-SEC-I09-030728.pdf

(e) I could find no citations for the following informative
references:  [ETSI TS 101 909-4], [ETSI TS 101 909-9],
[EN 300 001], and [EN 300 659-1].  They should either be
cited or removed.

(f) Although [RFC3291] is indeed the correct reference for the
InetAddress/InetAddressType definitions, there is a possibility
that it will be replaced by draft-ietf-ops-rfc3291bis-01.txt or
a successor by the time this document is published.  To allow
for this possibility I recommend adding an RFC Editor note such
as the following:

  ************************************************************
  * NOTES TO RFC Editor (to be removed prior to publication) *
  *                                                          *
  * 1.) If <draft-ietf-ops-rfc3291bis-01.txt> (or its        *
  * successor) is to be published as an RFC concurrently     *
  * with this document, please update normative reference    *
  * [RFC3291] to point to that RFC, instead of RFC 3291.     *
  *                                                          *
  ************************************************************

(you may want to consider collecting all RFC Editor
notes together and putting them at the end, as was
done in <draft-ietf-atommib-rfc2493bis-01.txt> and
<draft-ietf-atommib-rfc2558bis-01.txt> ... but be
sure not to eliminate the required embedded notes
in the MIB module if you do that)

6.) Security Considerations Section -- the MIB security template has
been followed, but the presentation is not as clear as I would like
and there appear to be some some missing and misplaced entries in
the lists of writeable objects.  To address the latter the following
changes are suggested:

FROM:
 ! - All writable objects in the pktcMtaDevServer and
 ! pktcMtaDevRealmTable groups share the potential, if SET maliciously,
 ! to prevent an MTA from provisioning properly, hence there are
 ! considered very sensitive for service delivery, in particular:
       pktcMtaDevProvisioningTimer,
       pktcMtaDevServerAddressType,
       pktcMtaDevServerDns1,
       pktcMtaDevServerDns2 - these two objects, if SET maliciously,
   will prevent the MTA from being authenticated and consequently from
 ! getting the telephony services.
       pktcMtaDevSnmpEntity,
       pktcMtaDevProvConfigHash,
       pktcMtaDevProvConfigKey,
       pktcMtaDevProvSolicitedKeyTimeout,
       pktcMtaDevRealmName,
       pktcMtaDevRealmOrgName,
 !     pktcMtaDevRealmStatus - this object, if SET maliciously, would
 ! cause the whole row of the table to be deleted which will deny the
 ! prevent the MTA from getting the telephony services.
TO:
 ! - All writable objects in the pktcMtaDevServer group and some in
 ! the pktcMtaDevRealmTable share the potential, if SET maliciously,
 ! to prevent an MTA from provisioning properly.  Hence they are
 ! considered very sensitive for service delivery.  In particular:
       pktcMtaDevProvisioningTimer,
       pktcMtaDevServerAddressType,
       pktcMtaDevServerDns1,
       pktcMtaDevServerDns2 - these two objects, if SET maliciously,
   will prevent the MTA from being authenticated and consequently from
 ! getting telephony services.
 +     pktcMtaDevTimeServer,
 +     pktcMtaDevConfigFile,
       pktcMtaDevSnmpEntity,
       pktcMtaDevProvConfigHash,
       pktcMtaDevProvConfigKey,
       pktcMtaDevProvSolicitedKeyTimeout,
       pktcMtaDevRealmName,
       pktcMtaDevRealmOrgName,
 +     pktcMtaDevRealmUnsolicitedKeyMaxTimeout,
 +     pktcMtaDevRealmUnsolicitedKeyNomTimeout,
 +     pktcMtaDevRealmUnsolicitedKeyMaxRetries,
 !     pktcMtaDevRealmStatus - this object, if SET maliciously, could
 ! cause the whole row of the table to be deleted which may prevent
 ! MTA from getting telephony services.

FROM:
   - All writable objects in the pktcMtaDevCmsTable table share the
   potentional, if SET maliciously, to disrupt the telephony service by
   altering which Call Management Server the MTA must send signaling
   registration to, in particular:
       pktcMtaDevCmsFqdn,
       pktcMtaDevCmsKerbRealmName,
       pktcMtaDevCmsMaxClockSkew,
       pktcMtaDevCmsSolicitedKeyTimeout,
       pktcMtaDevCmsUnsolicitedKeyMaxTimeout,
       pktcMtaDevCmsUnsolicitedKeyNomTimeout,
 !     pktcMtaDevRealmUnsolicitedKeyMaxRetries - this object, if set to
 ! a zero value '0', may prevent an MTA from retry its attempt to
   establish a security association with the CMS,
       pktcMtaDevCmsStatus.
TO:
   - All writable objects in the pktcMtaDevCmsTable table share the
   potentional, if SET maliciously, to disrupt the telephony service by
   altering which Call Management Server the MTA must send signaling
   registration to, in particular:
       pktcMtaDevCmsFqdn,
       pktcMtaDevCmsKerbRealmName,
       pktcMtaDevCmsMaxClockSkew,
       pktcMtaDevCmsSolicitedKeyTimeout,
       pktcMtaDevCmsUnsolicitedKeyMaxTimeout,
       pktcMtaDevCmsUnsolicitedKeyNomTimeout,
 !     pktcMtaDevCmsUnsolicitedKeyMaxRetries - this object, if set to
 ! a zero value '0', may prevent an MTA from retrying its attempt to
   establish a security association with the CMS,
       pktcMtaDevCmsStatus.

FROM:
   - The following objects, if SET maliciously will not have an
   immediate effect on service but they may impact the service
   performace and may cause avalanche attacks on provisioning servers
   including Kerberos KDC servers on massive device reboots:
       pktcMtaDevResetKrbTickets - if set to true, will cause an MTA to
   request a new Kerberos ticket at reboot.
       pktcMtaDevRealmPkinitGracePeriod - if set to short periods, will
   cause MTA to renew its tickets more frequently,
       pktcMtaDevRealmTgsGracePeriod (same as above).
 -     pktcMtaDevRealmUnsolicitedKeyMaxTimeout,
 -     pktcMtaDevRealmUnsolicitedKeyNomTimeout.
TO:
   - The following objects, if SET maliciously will not have an
   immediate effect on service but they may impact the service
   performace and may cause avalanche attacks on provisioning servers
   including Kerberos KDC servers on massive device reboots:
       pktcMtaDevResetKrbTickets - if set to true, will cause an MTA to
   request a new Kerberos ticket at reboot.
       pktcMtaDevRealmPkinitGracePeriod - if set to short periods, will
   cause MTA to renew its tickets more frequently,
       pktcMtaDevRealmTgsGracePeriod (same as above),

(changed lines indicated by '!' in left margin, added lines by '+',
deleted lines by '-').

Note that suggested addition/regrouping of objects in the lists
above assumes that

   pktcMtaDevRealmUnsolicitedKeyMaxTimeout
   pktcMtaDevRealmUnsolicitedKeyNomTimeout
   pktcMtaDevRealmUnsolicitedKeyMaxRetries

are for communication with a Key Distribution Center (KDC) and so
can affect the provisioning process whilst

   pktcMtaDevCmsUnsolicitedKeyNomTimeout
   pktcMtaDevCmsUnsolicitedKeyMaxTimeout
   pktcMtaDevCmsUnsolicitedKeyMaxRetries

are used for communication with a Call Management server (CMS) and
so can affect telephony services but not provisioning (that, at
least, is what I concluded from Section 3.4 and from the object
DESCRIPTION clauses).

Even with the above fixes the Security Considerations section would
not be a model of clarity.  The problem is that the presentation
style doesn't make it perfectly clear exactly what vulnerabilities
are associated with each object.  Here is an example of a possibly
better style for the first section above:

   - All writable objects in the pktcMtaDevServer group and some in
   the pktcMtaDevRealmTable share the potential, if SET maliciously,
   to prevent an MTA from provisioning properly.  Hence they are
   considered very sensitive for service delivery.  The objects in
   question are:

       pktcMtaDevProvisioningTimer
       pktcMtaDevServerAddressType
       pktcMtaDevServerDns1
       pktcMtaDevServerDns2
       pktcMtaDevTimeServer
       pktcMtaDevConfigFile
       pktcMtaDevSnmpEntity
       pktcMtaDevProvConfigHash
       pktcMtaDevProvConfigKey
       pktcMtaDevProvSolicitedKeyTimeout
       pktcMtaDevRealmName
       pktcMtaDevRealmOrgName
       pktcMtaDevRealmUnsolicitedKeyMaxTimeout
       pktcMtaDevRealmUnsolicitedKeyNomTimeout
       pktcMtaDevRealmUnsolicitedKeyMaxRetries
       pktcMtaDevRealmStatus

   Certain of the above objects have additional specific vulnerabilites.
   pktcMtaDevServerDns1 and pktcMtaDevServerDns2, if SET maliciously,
   could prevent the MTA from being authenticated and consequently from
   getting telephony services.  pktcMtaDevRealmStatus, if SET
   maliciously, could cause the whole row of the table to be deleted
   which may prevent MTA from getting telephony services.

The authors may wish to consider reworking the style of the entire
section to something along these lines.  It would also be good to
make consistent statements about retry objects, RowStatus objects,
and so on in the different lists.

Finally, this section includes only the standard "deployment of
SNMP versions prior to SNMPv3 is NOT RECOMMENDED."  It has been
my understanding that versions of SNMP prior to SNMPv3 are not
permitted for DOCSIS devices.  If that's true, then the last
paragraph should say that instead of the standard stuff.

7.) IANA Considerations Section -- OK (none present and none
required).

8.) Copyright notices -- the Full Copyright statement has a second
copy of the standard Intellectual Property notices in front of it.
That shouldn't be there and needs to be removed.

9.) MIB compilation -- Bert already pointed out the following
warning message from SMICng:

 W: f(ipcdnPktcMta.mi2), (1347,4) NOTIFICATION-GROUP
    "pktcMtaNotificationGroup" is not used in a
    MODULE-COMPLIANCE in current module

The smilint e-mail robot says the same thing:

 This command (smilint 0.4.2-pre1, as of Wed Jul 23 17:37:01 2003)
 has been processed to get the following results:
 smilint -m -s -l 6 -i namelength-32 PKTC-IETF-MTA-MIB

 PKTC-IETF-MTA-MIB:1340: [5] {group-unref} warning: current group
 `pktcMtaNotificationGroup' is not referenced in this module

Although this is not a violation of the SMIv2 rules, we recommend in
the MIB review guidelines that a GROUP clause be present for each
optional group so that it is obvious that it was not an inadvertent
omission; see <draft-ietf-ops-mib-review-guidelines-01.txt>,
Sec. 4.8, last bullet on p. 23.  In this case it appears that this
group was indeed inadvertently omitted from the MANDATORY-GROUPS
clause;  see #11(w) below for details.

10.) Other issues, e.g., stuff in http://www.ietf.org/ID-nits.html
that is not covered above:

(a) Bert Wijnen noted 2 lines containing non-US-ASCII characters:
> > Bad chars at 2053
> > Bad chars at 2056
> > -: 2 lines containing non-US-ASCII characters

The fix that I recommend is to change the text starting at line 2053
from:
   [EN 300 659-1] EN 300 659-1: ôPublic Switched Telephone Network 
                  (PSTN); Subscriber line protocol over the local loop 
                  for display (and related) services; Part 1: On hook 
                  data transmissionö. 
to:
   [EN 300 659-1] EN 300 659-1: "Public Switched Telephone Network 
                  (PSTN); Subscriber line protocol over the local loop 
                  for display (and related) services; Part 1: On hook 
                  data transmission". 
assuming, of course, that this reference is retained.

(b) There are a few lines that go over 72 characters, however, that
goes away if the trailing blanks are removed.  It would be nice to
fix that in the next version of the draft, but it's not essential,
since the trailing blanks are stripped by the RFC Editor do that as
a side-effect of "nroff'ing" the document.

(c) Please make the following change globally: s/RFC-3495/RFC 3495/
Note that the MIB module and the reference [RFC3495] are affected.

(d) Section 2.2:  s/over the DOCSIS/over a DOCSIS/

(e) Section 2.3:  s/the cable or hybrid system/a cable or hybrid system/
(multiple occurrences);  s/the CM part/the CM part,/

(f) Section 2.8:  Consider appending the following sentence:

 Commonly used DHCP options are defined in [RFC2132].

(g) Section 2.10:  s/algorithm/device/

(h) Section 2.11:  s/system of the back office/a system of back office/

(i) Section 3.1, 5th paragraph:  s/First two groups/The first two groups/

(j) Section 3.1, 6th paragraph:  s/Third group/The third group/

(k) Section 3.2, 1st paragraph:  suggest to revise as follows:

 This group contains management information describing the parameters
 of the MTA device itself.  It also contains some objects to control
 the MTA state.  Some highlights are as follows:

(l) Section 3.3, 1st paragraph:  suggest to revise as follows:

 This group contains management information describing the back
 office servers and the parameters assigned to the communication
 timeouts.  It also contains some objects controlling the initial
 MTA interaction with the Provisioning Server.

(m) Section 3.4, 1st paragraph:  suggest to revise as follows:

 This group contains management information describing the
 security-related characteristics of the MTA.  It also contains
 two tables describing logical dependencies and parameters necessary
 to establish security associations between the MTA and other
 components of the back office.  Some higliights are:

(n) Section 3.5:  add blank lines between consecutive paragraphs,
and also before and after each bullet item in the bullet list.
Please rewrite the 2nd paragraph so that it is clear what it means.
In the 1st sentence of the 2nd paragraph after the bullet list
s/defined by the number of parameters/defined by a number of parameters/
Please use the definite and indefinite articles when customary
(e.g., s/Realm Table is indexed/The Realm Table is indexed/ and
s/contains conceptual column/contains a conceptual column).  This
whole section is hard to read and needs major editorial rework.

11.) Technical content -- here are the comments on the content of
Section 4, "Definitions," which contains the PKTC-IETF-MTA-MIB.

(a) In the MODULE-IDENTITY DESCRIPTION clause:
s/set if requirements/requirements/

(b) The ASN.1 comment preceding DocsX509ASN1DEREncodedCertificate
promises to do something that is not allowed:

   --
   -- The following textual convention will be imported from BPI+ RFC
   -- when introduced there.
   --

It is ILLEGAL to move a TC from one module to another or to
remove a definition from a published MIB module.  If the TC
is to be imported from the BPI-plus MIB, then do so before
this module is published;  otherwise, plan on leaving it in
in perpetuity (in which case there is no point in IMPORTING
it).  In the latter case, you may want to change the name
to PktcMtaX509ASN1DEREncodedCertificate.  In any case, that
comment has to go.

(c) The REFERENCE clauses do not have a consistent style throughout.
Sometimes a document is referred to by title alone;  sometimes a
document is referred to by number alone;  and in some cases both
styles are used in the same REFERENCE clause, e.g.,

 REFERENCE
     "PacketCable Provisioning Specification. RFC 3495."

I find this a little confusing:  it's not clear at a glance
whether RFC 3495 is the number of the PacketCable Provisioning
Specification or is a separate document.  For this reason I ask
that all REFERENCE clauses be updated to use a consistent
style when referring to documents.  My suggestion would be to
see something line this, which has both document number and
(abbreviated) title, and which has distinct document references
separated by a semicolon:

 REFERENCE
     "PKT-SP-PROV-I07-030728, PacketCable MTA Device Provisioning
      Specification;  RFC 3495, DHCP Option for CableLabs Client
      Configuration."

(d) In the DESCRIPTION clause of pktcMtaDevSwCurrentVers:
s/'sysDecr'/'sysDescr'/

(e) Should the DESCRIPTION clause of pktcMtaDevFQDN say what
is returned if the device is queried before a domain name
has been assigned to it?

(f) In the DESCRIPTION clause of pktcMtaDevEndPntCount:
s/The physical end points/The number of physical end points/

(g) In the definition of pktcMtaDevTypeIdentifier:  please
add a REFERENCE clause pointing to RFC 2132, where option #60
is defined.  NOTE:  there are several other objects that need
to have a REFERENCE to RFC 2132.  Among them are
pktcMtaDevServerDns1 (option #6), pktcMtaDevServerDns2
(option #6), and pktcMtaDevTimeServer (option #4).

(h) In the DESCRIPTION clause of pktcMtaDevErrorOidsTable:
it might be a good idea to specify when in the provisioning
process this table is cleared (presumably you do want the
table cleared if provisioning is restarted).

(i) The DESCRIPTION clause of pktcMtaDevErrorOid says:

   "If the error was due to a reason which cannot be associated
    with 'recognizable' OID of the parameter, then this MIB
    object must contain an empty value."

It is not clear what the words "an empty value" mean.  If you
mean "{ 0 0 }" please say so.

(also, s/Parameter, which caused/Parameter that caused/)

(j) The DESCRIPTION clause of pktcMtaDevErrorValue says:

   "If the error was due to unrecognizable OID of the
    parameter in the Configuration File, then this MIB
    Object contains the value of the OID as interpreted
    MTA. Otherwise (the error happens due to a wrong value
    of the parameter) - the MIB Object contains the value
    from the Configuration File converted to SnmpAdminString."

Putting on my implementor's hat I have a number of problems
with this DESCRIPTION clause.  First, it's not clear what is
meant by the first sentence.  I'm guessing that it means that
if I see an OID in a configuration file that I don't recognize
then I must populate this object (rather than pktcMtaDevErrorOid)
with that OID.  I shouldn't have to guess.  Second, there is no
guidance on how I am suppose to turn OIDs or other values into
an SnmpAdminString.  If this is left unspecified then it's quite
likely that different implementations will do different things.
So, I must ask that you clarify this DESCRIPTION clause.

(k) This comment deals with some problems related to the usage of
the InetAddressType/InetAddress TCs that affect objects of those
types in the pktcMtaDevServer group.

First: the DESCRIPTION clause of pktcMtaDevServerAddressType says:

 This object defines the address type for the following servers:
 pktcMtaDevServerDhcp1, pktcMtaDevServerDhcp2, pktcMtaDevServerDns1,
 pktcMtaDevServerDns2, pktcMtaDevTimeServer.

However, the DESCRIPTION clauses of pktcMtaDevServerDhcp1,
pktcMtaDevServerDhcp2, pktcMtaDevServerDns1, pktcMtaDevServerDns2,
and pktcMtaDevTimeServer do not have this information, as required
by the DESCRIPTION clause of the InetAddress TC:

| An InetAddress value is always interpreted within the context
| of an InetAddressType value. Every usage of the InetAddress
| textual convention is required to specify the InetAddressType
| object which provides the context.

I strongly recommend, for reasons of ease of maintenance, that the
InetAddress/InetAddressType association be specified in the
DESCRIPTION clauses of the InetAddress objects _only_.  The reason
is that it is possible to add new InetAddress objects -- possibly
in a different MIB module -- that use a pktcMtaDevServerAddressType
as the type discriminator, and if the associations are listed in the
pktcMtaDevServerAddressType DESCRIPTION clause the it would need to
be updated whenever this happens.  It's clearly undesirable (and not
always feasible) to do that.

Second: although InetAddressType object pktcMtaDevServerAddressType
is formally allowed to assume values other than ipv4 -- which may
indeed be desirable for future extensibility -- the DESCRIPTION
and REFERENCE clauses for the corresponding InetAddress objects
pktcMtaDevServerDhcp1, pktcMtaDevServerDhcp2, pktcMtaDevServerDns1,
pktcMtaDevServerDns2, and pktcMtaDevTimeServer do not currently
provide any meaning for any address types other than ipv4(1).  The
main reference PacketCable MTA Device Provisioning Specification,
PKT-SP-PROV-I07-030728, only discusses provisioning with DHCPv4 and
not DHCPv6, and only discusses the use of IPv4 for communication
with the DHCP server.  So it's not clear what a client would do
with anything other than IPv4 addresses for pktcMtaDevServerDhcp1
and pktcMtaDevServerDhcp2.  Also, the DHCP options that correspond
to the pktcMtaDevServerDns1, pktcMtaDevServerDns2, and
pktcMtaDevTimeServer objects (cf. #11(g) above) are values of
DHCPv4 options, which can only contain IPv4 addresses.

If future extensibility to IPv6 is desirable and possible, then
the suggested fixes for the problems are to revise the DESCRIPTION
clauses for pktcMtaDevServerDhcp1, pktcMtaDevServerDhcp2,
pktcMtaDevServerDns1, pktcMtaDevServerDns2, and
pktcMtaDevTimeServer along the following lines:

            "The type of this address is determined by the value of
             the pktcMtaDevServerAddressType object.  When the latter
             has the value ipv4(1), this object is [ ... ].

             The behavior of this object when the value of
             pktcMtaDevServerAddressType is other than ipv4(1)
             is not presently specified, but may be specified
             in future versions of this MIB module."

where [ ... ] contains the existing description text (modified as
needed for grammatical correctness and to remedy the deficiencies
noted in #11(g) above and #11(l) through #11(n) below) and to
remove the second sentence of the pktcMtaDevServerAddressType
DESCRIPTION clause.

If, on the other hand, there is no realistic possibility of
re-using this MIB module with a future IPv6-capable version of the
PacketCable MTA specs, then it would probably be better to eliminate
pktcMtaDevServerAddressType and change the SYNTAX value of
pktcMtaDevServerDhcp1, pktcMtaDevServerDhcp2, pktcMtaDevServerDns1,
pktcMtaDevServerDns2, pktcMtaDevTimeServer to InetAddressIPv4.
Note that this is permitted (although not encouraged) by RFC 3291
(and its successor draft-ietf-ops-rfc3291bis-01.txt).

In subsequent comments I'll assume that the first solution is
adopted;  if it isn't please modify suggested text as appropriate.

(l) The DESCRIPTION clauses of pktcMtaDevServerDhcp1 and
pktcMtaDevServerDhcp2 should explicitly state that these parameters
are normally provided by the Cable Modem (CM) entity associated
with the MTA, and that the CM gets them as the values of suboptions
1 and 2 (respectively) of the DHCP CableLabs Client Configuration
Option (option #122) defined in RFC 3495.  Note that RFC 3495 (and
the provisioning spec) explicitly say that these suboptions apply
only to the CM.

For example, the DESCRIPTION clause for pktcMtaDevServerDhcp1
might become:

            "The type of this address is determined by the value of
             the pktcMtaDevServerAddressType object.  When the latter
             has the value ipv4(1), this object is the IP address of
             the primary DHCPv4 server which will cater to the MTA
             during its provisioning.  It is provided by the CM to
             the MTA from suboption 1 of the DHCP Option for
             CableLabs Client Configuration (option #122) defined
             in RFC 3495.  It contains the value 255.255.255.255 if
             there was no preference given with respect to the DHCP
             servers for MTA provisioning.

             The behavior of this object when the value of
             pktcMtaDevServerAddressType is other than ipv4(1)
             is not presently specified, but may be specified
             in future versions of this MIB module."

Similarly, the DESCRIPTION clause for pktcMtaDevServerDhcp2
might become:

            "The type of this address is determined by the value of
             the pktcMtaDevServerAddressType object.  When the latter
             has the value ipv4(1), this object is the IP address of
             the primary DHCPv4 server which will cater to the MTA
             during its provisioning.  It is provided by the CM to
             the MTA from suboption 2 of the DHCP Option for
             CableLabs Client Configuration (option #122) defined
             in RFC 3495.  It contains the value 0.0.0.0 if there
             is no specific secondary DHCP server to be considered
             during MTA provisioning.

             The behavior of this object when the value of
             pktcMtaDevServerAddressType is other than ipv4(1)
             is not presently specified, but may be specified
             in future versions of this MIB module."

Question:  do these objects apply only to E-MTA or do they
apply to S-MTA also?  If they don't apply to S-MTA (i.e.,
have the value 255.255.255.255/0.0.0.0) then say so;  if
they apply to both, I am curious to know how the CM communicates
the values to the MTA in that case.

(m) The DESCRIPTION clauses for pktcMtaDevServerDns1 and
pktcMtaDevServerDns2 claim that these objects get their
values from DHCP option 6 "as defined in RFC-3495".  That
is incorrect:  DHCP option 6 is defined in RFC 2132.  The
REFERENCE clause also contains this error.  These clauses
also read awkwardly.  See #11(l) above for a model DESCRIPTION
clause and #11(c) for a model REFERENCE clause.

(n) The DESCRIPTION clause for pktcMtaDevTimeServer does not mention
that the value MAY appear as option #4 in a DHCP Offer/DHCP Ack.  In
fairness, neither does the provisioning spec PKT-SP-PROV-I07-030728,
but it references the DOCSIS spec, which clearly spells this out.
Here is an excerpt I picked up from DOCSIS 1.0:

 The protocol by which the time of day MUST be retrieved is defined
 in [RFC-868].  Refer to Figure 7-10.  The request and response MUST
 be transferred using UDP. The time retrieved from the server (UTC)
 MUST be combined with the time offset received from the DHCP
 response to create the current local time.

 The DHCP server may offer a CM multiple Time of Day server IP
 addresses to attempt.  The CM MUST attempt all Time of Day servers
 included in the DHCP offer until local time is established.

I presume that something like this applies to an S-MTA.  I also
presume that populating this object is optional for an E-MTA since
the latter could get the time-of-day from the CM.

If these presumptions are correct, please revise the DESCRIPTION
clause to say so, and add a reference clause pointing to RFC 2132.

It these presumptions are not correct, you will probably still want
to note that this is the address of an RFC 868 timer server and
maybe add a REFERENCE clause pointing to RFC 868.

In any case, please fix this spelling error:  s/SMTA/S-MTA/
It caused me some confusion in trying to decipher what the
DESCRIPTION clause means.  And please also make this change too:
s/populated/populated with a value other than 0.0.0.0/

(o) In the DESCRIPTION clause of pktcMtaDevConfigFile I see:

 The object returns NULL if the server address is unknown.

It's not clear what NULL means in this context:  the SYNTAX
value is SnmpAdminString, and the TC definition in RFC 3411
does not define a NULL value.  If you mean a zero-length
string then you must say so explicitly.

In addition, the English needs to be re-worked a little to get
definite and indefinite articles in the right places.

(p) In the DESCRIPTION clause of pktcMtaDevSnmpEntity:
s/the DHCP Option CCC/DHCP Option 122/ to be consistent with
other DESCRIPTION clauses (and fix the English).

(q) Objects pktcMtaDevProvConfigHash and pktcMtaDevProvConfigKey
have SIZE constraints in the definitions that are appropriate to
the currently supported algorithms -- MD5 and SHA-1 for
authentication, null or DES for privacy -- but may be overly
constraining if new algorithms are added in a future version of
the PacketCable security spec.  The designers need to be aware
that removing or changing SIZE constraints after a MIB module has
been published constitutes an illegal revision (see RFC 2578
Section 10).  If there is a possibility of adding new algorithms,
then the SIZE constraints should be removed before initial
publication as an RFC.

... skipping down to the read-create tables ...

(r) The conceptual rows pktcMtaDevRealmEntry and pktcMtaDevCmsEntry
support dynamic creation of rows, but do not have a StorageType
column, nor do their DESCRIPTION clauses state what happens to
dynamically created rows after a reboot.  You must have one or the
other;  see <draft-ietf-ops-mib-review-guidelines-01.txt> Section
4.6.3.  If the fate of a row depends on some conditions such as the
value of some other object, then putting a statement to that effect
in the conceptual row DESCRIPTION clause is the best way to satisfy
this requirement.

(s) The DESCRIPTION clause of pktcMtaDevRealmStatus needs many
improvements.  In particular, in view of #11(z) below, the following
is not correct:

             There is no restriction on setting columns in this table
             when the value of pktcMtaDevRealmStatus is active(1).

Furthermore, the following stuff just adds unneeded words that have
no value at all to the implementor:

             The  RowStatus TC [RFC2579] requires that this DESCRIPTION
             clause states under which circumstances other objects in
             this row can be modified:

Please eliminate the above paragraph here and wherever else it appears.
Also eliminate the following, which (in context) appears to be a
duplicate of the first (erroneous) statement:

             The value of this object has no effect on whether other
             objects in this conceptual row can be modified."

Here is the suggested replacement for all of the above text:

             The columnar objects pktcMtaDevRealmName and
             pktcMtaDevRealmOrgName may not be modified when
             the value of this object is active(1).  The value
             of this object has no effect on whether other
             columns in this conceptual row can be modified."

pktcMtaDevCmsStatus has similar problems (pktcMtaDevCmsFqdn and
pktcMtaDevCmsKerbRealmName can't be modified while it is active(1))
and its DESCRIPTION clause needs similar modifications.

... skipping down to the notifications section ...

(t) Regarding the following:

   --
   -- notification group is for future extension.
   --

   pktcMtaNotificationPrefix OBJECT IDENTIFIER ::= { pktcMtaMib 2 }
   pktcMtaNotification OBJECT IDENTIFIER ::= {
   pktcMtaNotificationPrefix 0 }
   pktcMtaConformance  OBJECT IDENTIFIER ::= { pktcMtaMib 3 }
   pktcMtaCompliances  OBJECT IDENTIFIER ::= { pktcMtaConformance 1 }
   pktcMtaGroups       OBJECT IDENTIFIER ::= { pktcMtaConformance 2 }

Please remove the ASN.1 comment, as it is now incorrect.

For the remainder, consider instead using the following definitions,
placed right at the front of the MIB module, just below the
TEXTUAL-CONVENTION section (currently page 9):

   pktcMtaNotification OBJECT IDENTIFIER ::= { pktcMtaMib 0 }
   pktcMtaMibObjects   OBJECT IDENTIFIER ::= { pktcMtaMib 1 }
   pktcMtaConformance  OBJECT IDENTIFIER ::= { pktcMtaMib 2 }

and consider relocating the following

   pktcMtaCompliances  OBJECT IDENTIFIER ::= { pktcMtaConformance 1 }
   pktcMtaGroups       OBJECT IDENTIFIER ::= { pktcMtaConformance 2 }

down to the conformance section (p. 32).  These changes are not
mandatory, but they cost nothing and will save one sub-identifier
in each OID that identifies a notification.

(u) In the definitions of the pktcMtaDevProvisioningEnrollment
and pktcMtaDevProvisioningStatus notifications, the following
REFERENCE clause is bogus:

       REFERENCE
           " Inform as defined in RFC 1902"

An "inform" or "trap" is a protocol construct that transports a
notification, and has nothing directly to do with the SMI.  You
won't find them defined in RFC 1902;  the definitions of the
SNMPv2-Trap-PDU and the InformRequest-PDU are in RFC 3416 (formerly
RFC 1905).  You seem to be trying to define a notification that can
be transported in an inform, but that's not possible with the SMI.
If you want to require use of informs instead of traps, the place to
do so is in a PacketCable protocol spec (where I believe such a
restriction indeed exists), not in a MIB module.  In any case, RFC
1902 is obsolete;  it has been replaced by 2578.

... skipping down to the conformance section ...

(v) The DESCRIPTION clause for the compliance statement
pktcMtaBasicCompliance says:

           " The compliance statement for devices that implement
             PacketCable/IPCablecom MTA. It is left to manufacturers to
             determine whether to support both PacketCable and
             IPCablecom objects in the same MTA."

This is somewhat incomprehensible in the context of a MIB module that
contains neither PacketCable nor IPCablecom objects but rather IETF
objects.  May I suggest instead borrowing the text from the PKTC-MTA-MIB
in PKT-SP-MIB-MTA-I07-030728:

           " The compliance statement for devices that implement
             MTA feature."

If you decide to retain pktcMtaDevServerAddressType for future
extensibility to IPv6 (cf. #11(k) above), you might want to add to
this an additional sentence like this:

             This compliance statement applies to implementations that
             support DOCSIS 1.0/1.1/2.0, which are not IPv6-capable.

modified as appropriate for MTAs.  See this e-mail message:

http://ops.ietf.org/lists/mreview/mreview.2003/msg00503.html

for a discussion of the issues (and other recommended modifications).

(w) pktcMtaNotificationGroup either needs to be added to the list
of MANDATORY-GROUPS of pktcMtaBasicCompliance, if it is indeed
mandatory to implement, or else there should be a GROUP clause
explaining that it is optional, as explained in comment #9 above.
In this case it appears that the correct course would be to add
the group to the MANDATORY-GROUPS clause;  that is what was done
in the PKTC-MTA-MIB in PKT-SP-MIB-MTA-I07-030728.

(x) If you decide to retain the object pktcMtaDevServerAddressType
then it would be proper to add the following OBJECT clause:

           OBJECT  pktcMtaDevServerAddressType
               SYNTAX      { ipv4(1) }
               DESCRIPTION
                   " Support for address types other than ipv4(1)
                     is not required."

(y) The following OBJECT clause looks suspect to me:

           OBJECT  pktcMtaDevSnmpEntity
               MIN-ACCESS  read-only
               DESCRIPTION
                   " The behavior of the SNMP Agent when the object is
                     set is currently undefined. Object should be
                     implemented as read-only."

Unless there is some clear way that the behavior could in the future
be well-defined if this object is changed, then the correct thing to
do is to eliminate this OBJECT clause and change the MAX-ACCESS value
in the object definition to read-only.

(z) The following OBJECT clauses are all bogus and need to be
removed:

         OBJECT  pktcMtaDevRealmName
             MIN-ACCESS  read-only
             DESCRIPTION
                 " The behavior of the SNMP Agent when the object is
                   set is currently undefined. Object should be
                   implemented as read-only after the conceptual
                   row is created."

         OBJECT  pktcMtaDevRealmOrgName
             MIN-ACCESS  read-only
             DESCRIPTION
                 " The behavior of the SNMP Agent when the object is
                   set is currently undefined. Object should be
                   implemented as read-only after the conceptual
                   row is created."

         OBJECT  pktcMtaDevCmsFqdn
             MIN-ACCESS  read-only
             DESCRIPTION
                 " The behavior of the SNMP Agent when the object is
                   set is currently undefined. Object should be
                   implemented as read-only after the conceptual
                   row is created."

         OBJECT  pktcMtaDevCmsKerbRealmName
             MIN-ACCESS  read-only
             DESCRIPTION
                 " The behavior of the SNMP Agent when the object is
                   set is currently undefined. Object should be
                   implemented as read-only after the conceptual
                   row is created."

Each of the objects in question has a MAX-ACCESS of read-create.
Using a MIN-ACCESS clause in this way means that an agent may
(but need not) allow full read-create access.  That is clearly
not what you want, if the DESCRIPTION clause means what it says.

The correct way to do what you want is to say, in the DESCRIPTION
clause of the status column, that the object in question may not
be modified when the status is active(1).  See #11(s) above,
where specific changes are recommended.

This concludes the review of draft-ietf-ipcdn-pktc-mtamib-01.txt,
Part 1.  Much of the MIB module still need be reviewed, but that is
deferred until the -02 draft and/or another reviewer is available.

Regards,

Mike Heard


_______________________________________________
IPCDN mailing list
[e-mail]
https://www1.ietf.org/mailman/listinfo/ipcdn