[BEHAVE] Issue tracking for NAT MIB

[Tom Taylor <[e-mail]>]

The NAT MIB document, draft-ietf-behave-nat-mib-11, is going through the
process of review as an AD-sponsored document. David Harrington had a
long list of issues. I am posting his E-mail here before breaking these
issues down and putting them into an issue tracker so that I have a
message reference for each issue.

I will let you know the details for reaching the tracker when it is
ready. I'm creating it directly in the Behave Wiki rather than using TRAC.

Tom Taylor

> -----Original Message----- From: David Harrington
> [mailto:[e-mail]
<mailto:[e-mail]>]
> Sent: Tuesday, June 17, 2014 2:22 AM To:
> [e-mail]
<mailto:[e-mail]>
> Cc: [e-mail] <mailto:[e-mail]>;
[e-mail] <mailto:[e-mail]>
> Subject: MIB Doctor review of draft-ietf-behave-nat-mib-11
>
> Hi,
>
> I have done a MIB Doctor review of draft-ietf-behave-nat-mib-11. I
> have reviewed this document as part of the MIB Doctor directorate's
> ongoing effort to review all IETF documents being processed by the
> IESG.  These comments were written primarily for the benefit of the
> operational area directors. Document editors and WG chairs should
> treat these comments just like any other last call comments.
>
> My impression is that this document is not ready for publication.
>
> Technical issues: The nature of the realm usage in NAT-MIB could have
> serious consequences to existing deployments of other MIB modules.
> The use of calculated thresholds, and calculated values to compare
> to thresholds, could be problematic, especially to
> resource-constrained agent implementations.
>
> Editorial issues: A significant cause for concern is the naming,
> which doesn't follow RFC
4181
> guidelines. This will make the MIB harder for operators to use in
> the
field.
> Another significant concern is the lack of REFERENCE clauses. This
> will
make
> the MIB harder for operators to use in the field. The description
> clauses are sometimes rather sparse. This will make the
MIB
> harder for operators to use in the field. Some of the descriptions in
> the module are sparse, while the surrounding text contains a better
> description. But MIB modules are frequently distributed without the
> surrounding RFC text. This will make the MIB
harder
> for operators to use in the field. The IMPORTS clauses don't identify
> the RFC that contains the MIB module being imported from. This will
> make the MIB harder for operators to use in the field. The
> redefinition of realm in the middle of a bis could be a problem,
> especially for managers. This will make the MIB harder for operators
> to
use
> in the field.
>
>
> Detailed review:
>
> 1) We have discussed whether the new objects really should be a new
> MIB module, published under a new MIB module name. Reading through
> this document, I feel that deprecating the whole previous NAT-MIB
> within the same document is a mistake. It makes the document harder
> to deal with. This really should be two separate documents and two
> separate MIBs - with different module names.
>
> The current approach might be perfectly viable for an agent
implementation,
> where the NAT-MIB is likely to be an implementation of EITHER the
> first
half
> of this document, or the second half of this document. But it seems
> problematic for an NMS implementation that has to support BOTH
> implementations based on RFC4008, and implementations based on this
> document. I especially think about the fact that NMSs import MIB
> modules, including the Description clauses for the managed objects,
> and can display the description clauses so operators can see what the
> definition of the object is. With this document, NMS applications
> MUST parse all the deprecated objects, even if, at some future time,
> there are no devices that implement the RFC4008 NAT-MIB. If the
> deprecation was handled in a separate RFC from the new MIB module,
> and an NMS no longer had to support the "old" NAT-MIB, then NMSs in
> the future would only need to import the new document.
>
> I found it especially irritating to review, because I am used to
> finding Textual Conventions at the beginning of the MIB module, which
> is usually
at
> the beginning of the document containing the MIB module, but here, I
> had to look for the T-Cs in the middle of a long MIB module, because
> I had to
work
> past the first 54 pages of deprecations to find the T-Cs of the new
module.
> Every user of this MIB document will be similarly inconvenienced,
> over and over.
>
> 2) This new I-D is redefining a term "realm" that is defined in the
original
> NAT-MIB. (see section 3.3). RFC4008 defined realm objects to be of
> type INTEGER, but the new NAT-MIB defines realm objects as
> SnmpAdminStrings. An NMS (and operators) needs to be able to support
> devices that implement the "old" NAT-MIB and devices that implement
> the "new" NAT-MIB. With this new NAT-MIB, an NMS user must be aware
> when they look at the description of a realm-related object, whether
> that object was defined in the RFC4008 NAT-MIB or in the <new RFC>
> NAT-MIB, in order to understand which meaning and datatype of the
> term "realm" applies. But since RFC4008 is updated by being
> deprecated in the same document, in the same-named MIB module, the
> reader of the new NAT-MIB must determine whether the object was
> defined in the first half of this document, i.e.,
the
> first half of the NAT-MIB, i.e., the "old" deprecated NAT-MIB, or in
> the second half of this document,  to understand the meaning of the
> word realm. I think this is problematic.
>
> 3) Section 3.3 explains that implementations of MIB modules that
implicitly
> support realms (e.g.,OSPFv2-MIB), and utilize SNMPv3 contexts (or
> presumably communities in community-based SNMP) in an SNMP-standard
> compliant manner, will no longer be able to do things in those
> standard-compliant manners, because the NAT-MIB design spans realms.
>
> This I-D should list the names and references for the "many MIBs"
> that "implicitly support realms in one form or another" that may be
> broken by this NAT-MIB's usage of realm. I suggest an "Operational
> Considerations" section for this information. I think the IETF
> community needs to better understand this claim before approving this
> document.
>
> 4) I will assume all the RFC4008 objects that are being deprecated
> have
been
> included here, and each has had its status changed to deprecated. I
> have
not
> gone through the document looking at each and every deprecation.
>
> 5) ProtocolNumber is a rather generic name for this textual
> convention. It doesn't start with "nat". I fear there is a potential
> for conflict. This does not follow the conventions recommended in
> RFC4181 Appendix C: - Names of TCs that are specific to the MIB
> module and names of SEQUENCE types that are used in conceptual table
> definitions should start with a prefix Xxx that is the same as xxx
> but with the initial letter changed to uppercase.  OPT-IF-MIB uses
> the prefix OptIf on the names of TCs and SEQUENCE types.
>
> 6) ProtocolNumber is dependent on an IANA registry. There should be a
> REFERENCE clause with a link to the specific IANA registry, so
> implementers/users can see the list of valid values.
>
> 7) This document does not provide REFERENCE clauses; it really
> should.
>
> 8) natPoolID - the description does not describe what a pool is, or
provide
> a REFERENCE clause.
>
> 9) natBehaviorType has only a reference in the description. In
> general it
is
> better to actually have a description in the description clause and
provide
> the reference in the REFERENCE clause. This is not merely an academic
> nit. Because GUI-based NMSs frequently display the Description clause
> in a window so the user can understand the meaning of the object, it
> is important that the description actually explain the meaning.
> Providing a reference in the description implies that the user would
> then need to go find the RFC and find the section of the RFC, and
> read the whole section to understand what this object means. Not very
> helpful. On top of that, an NMS might know how to track down an RFC
> and provide a link to the document in the REFERENCE clause, but it
> wouldn't necessarily know to parse the Description clause to find the
> reference.
>
> 10) NatPoolingType is only used to define the type of one object. The
> description clause in the object is indirect - " "Type of address
> pooling behavior that was used to create this mapping." To understand
> this object, you need to look at the textual convention. But the
> textual convention is also indirect - "Pooling type as described
in
> [RFC4787] sections 4.1." Even looking at RFC4787 section 4.1 isn't
> immediately helpful, because the discussion of arbitrary versus
> paired is a subset of section 4.1 (that is not called out as a
> subsection); it is found in the one-paragraph explanatory text for
> REQ-2. A short description in the OBJECT description clause could be
> made from a couple sentences from section 4.1, supplemented with a
> REFRERENCE clause, which would be much easier for users than the
> current indirect-indirect-someplace-in-this-text  approach.
>
> 11) SubscriberIdentifierType concerns me. We are supposed to be
> standardizing things here. But this appears to be a standardized
> "bucket" which implementers can stuff with their own proprietary
> identifiers, especially the "other" value. How is a vendor-neutral
> NMS supposed to know how to handle the "other" value? I feel
> uncomfortable with this whole identifierType, which implies that an
> implementation needs to go parse the packet to pull out additional
> information to identify the subscriber, without standardizing how
> this information should be extracted from the message in a
> standardized vendor-neutral manner.
>
> I feel uncomfortable that for the "other" type, each implementation
defines
> the semantics of AND and OR combinations. How should a
> vendor-neutral NMS handle the implementation-specific variations;
> where is the AND/OR semantic decision documented by each implementer,
> to make this vendor- interoperable?
>
> For the interfaces choice, how is the set of interface indexes
constructed?
> I'm confused by this. The classifying information is in the packet
(identify
> the subscriber from an incoming packet); so does the packet contain a
> set
of
> interface indexes that the receiving implementation is supposed to
extract?
> There is no reference to the type of packet involved, and I haven't
> had to unpack a set of interface indices from a packet before. What
> type of
packet
> contains a set of one or more ifIndexes? The RFC2863 textual
> convention doesn't seem to describe this.
>
> 12) SubsInterfaceIdRowIndex - unique within what scope? Within the
> agent?
>
> 13) I am concerned about abbreviations is naming. RFC4181 Appendix C
> suggests - When abbreviations are used, then they should be used
> consistently. Inconsistent usage such as
>
> xxxYyyDestAddr xxxZzzDstAddr
>
> should be avoided.
>
> But sometimes, object names in this module use Identifier and other
> times ID: natSubscriberVpnIdentifier vs. natSubscriberIPEncapsIdType
> Sometimes it's subscriber, other times it's "subs".
> natSubscriberVpnIdentifier vs. natSubsInterfaceIndex Sometimes it's
> threshold, sometimes it's "thresh": natMappingsNotifyThreshold  vs.
> natSubscriberMapNotifyThresh For an operator, especially one under
> stress, using a command-line SNMP tools to query a value, it is
> really irritating to have some objects abbreviated one way and other
> objects abbreviated other ways.
>
> 14) This is made worse by inconsistent name construction, such as a
> set of nat limits being named nat + "Limit" + <specific>, as in
> natLimitMappings but the set of notify thresholds are constructed as
> nat + <specific> + "NotifyThreshold", as in
> natMappingsNotifyThreshold. So operators need to recall which way you
> chose to construct the names, on a case-by-case
basis,
> because you are not being consistent. Why not do nat + <specific> +
"Limit"
> to match the way you construct NotifyThreshold names, or use
> nat+"NotifyThreshold"+<specific> to match how you construct the
> Limit names?
>
> 15) Once again, I will point out consistency in naming, following
> Appendix
C
> conventions: "   - The descriptor associated with a conceptual table
> should be of the form xxxZzzTable; the descriptor associated with the
> corresponding conceptual row should be of the form xxxZzzEntry; the
> name of the associated SEQUENCE type should be of the form
> XxxZzzEntry; and the descriptors associated with the subordinate
> columnar objects should be of the form xxxZzzSomeotherName.  An
> example from the OPT-IF-MIB is the OTMn table.  The descriptor of the
> table object is optIfOTMnTable, the descriptor of the row object is
> optIfOTMnEntry, the name of the associated SEQUENCE type is
> OptIfOTMnEntry, and the descriptors of the columnar objects are
> optIfOTMnOrder, optIfOTMnReduced, optIfOTMnBitRates,
> optIfOTMnInterfaceType, optIfOTMnTcmMax, and optIfOTMnOpticalReach."
>
> But we find in this NAT-MIB module inconsistency: natCountersTable
> OBJECT-TYPE ... natCountersEntry OBJECT-TYPE ... NatCountersEntry
> ::= SEQUENCE { natTranslations             Counter64,
> natOutOfPortErrors          Counter64, natResourceErrors
> Counter64, natQuotaDrops               Counter64, natMappingCreations
> Counter64, natMappingRemovals          Counter64,
> natAddressMappingCreations  Counter64, natAddressMappingRemovals
> Counter64 }
>
> And natLimitsTable OBJECT-TYPE ... natLimitsEntry OBJECT-TYPE ...
> NatLimitsEntry ::= SEQUENCE { natLimitMappings
> Unsigned32, natMappingsNotifyThreshold  Unsigned32,
> natLimitAddressMappings     Unsigned32, natAddrMapNotifyThreshold
> Unsigned32, natLimitFragments           Unsigned32,
> natLimitSubscribers         Unsigned32 } (notice Limits vs Limit, as
> well as objects with no natLimit prefix, as
well
> as "AddressMappings" vs "AddrMap".)
>
> 16) If there is a concern that descriptor length can become
> problematic, "Threshold" is never used without the "Notify" prefix.
> You could probably shorten the NotifyThreshold names to just use
> Threshold.
>
> 17) Consistency. Watermark vs Threshold.
>
> 18) natLimitMappings - this is in a table of limits for a NAT
> instance. What does "Global" mean in this context? See others in this
> table as well.
>
> 19) natMappingsNotifyThreshold - the description is "See
> natNotifMappings." Which says "This notification is generated when
> the number of active mappings exceeds the value of
> natMappingsNotifyThreshold." Why not write the (modified) description
> into both places rather than
using
> indirection?
>
> 20) natNotifMappings Typically we try to not derive values when
> defining objects. For example, RFC4188 states as a principle "
> Exclude objects that are simply derivable from others in this or
> other MIB modules." But natNotifMappings is triggered when " the
> number of active mappings exceeds the value of
> natMappingsNotifyThreshold" But we don't actually track active
> mappings; we track creations and removals, and we calculate active by
> subtracting removals from creations (active=creations-removals). If
> you want to use active against the threshold, why not simply track
active
> and skip tracking creations and removals?
>
> Is there a reason to track creations and removals? If the maximum
> number of creations performed  is important (maybe for memory
> management or something?), then why not track creations and active
> and let somebody else calculate the removals
> (removals=creations-active) If the number of removals is important (I
> have no idea why), then you can track active and removals, and let
> somebody else calculate (creations=active+removals) This way a
> notification doesn't have to wait to calculate active.
>
> 21) natNotifMappings - the relationship between the notification and
> thresholds is clear. The relationship to "which mappings" is a bit
> less clear. I see a natMapIntAddrTable, and a natMappingTable; which
> one is related to natNotifMappings? (and is the savings from dropping
> the y from notify significant?)
>
> I assume the fact we are sending a notification means it is important
> for the operator to know right away when "active exceeds threshold"
> happens. I have to guess that this might be related to
> memory/resource management. But I have to guess because nothing is
> said about why these particular values are tracked. Nothing is said
> about table row re-use when a mapping is removed. Is it really
> important to know the creation and removal counts, rather
than
> just the active count? Is it important to not reuse an index value to
> make sure that the NMS doesn't associate previously acquired data
> with a new mapping. (This is important, for example, if ifIndex
> changes during a reboot,
because
> an NMS could take data associated with an old ifIndex and confuse it
> as being associated with a new ifIndex, and totally misinterpret
> trends, and
so
> on. Other MIB modules need to protect against reuse of an index
> value, at least for some period of time, or until rollover.)
>
> 22) It's a good thing the description of natPoolTable is "Table of
> pools" because I might have confused it as being something else.
> 8-ball anyone? Description please?
>
> 23) natpoolRealm - "Realm to which this pool's addresses belong." MIB
> modules are often distributed without the surrounding text from the
> RFC.
>
> So it is important to be sure the description is meaningful without
> the whole RFC being present. At a minimum, especially since you are
> defining realm in this document,
you
> should provide a REFERENCE to the text in this RFC that contains the
> definition for realm.
>
> 24) natNotifPoolWatermarkLow and WatermarkHigh
>
> we usually try to not use derived values in MIB modules. We try to
> keep
the
> agent simple, and push complexity toward the NMS, which normally has
> far more resources available. When you set a watermark/threshold as a
> percentage, then the system needs to use CPU cycles to calculate the
> percentage - CPU cycles that might be
better
> used forwarding packets.
>
> The calculations you expect are rather complex - you require summing
> (allocations - deallocations) over all address ranges, then dividing
> by
the
> total number of ip addresses in the poll and by the size of the port
> range (calculated by portmax-portmin+1).
>
> I'm not sure that that formula is unambiguous; is that
> (sum(allocations-deallocation)/ipaddresses)/portrange or am I
> calculating something with (ip addresses and size of portrange)
> before dividing? Can
you
> use parentheses or something?
>
> How often should this complex calculation be done to monitor the
> percentage versus the threshold? Should this be re-calculated
> ***every time*** one of the following changes: allocation or
> deallocation across any address
range,
> total number of ip addresses, or size of the port range? (and of
> course,
the
> watermark setting).
>
> Wow! That seems like a lot of CPU cycles.
>
> 25) natpoolIndex - the description doesn't mention the scope of
uniqueness.
> Is this unique across all address pools in all instances? Unique
> within a nat instance, but not necessarily across other nat
> instances?
>
> 26) natPool watermarks There is mention of low usage mode and high
> usage mode. Is there a REFERENCE you can make, maybe to NAT
> requirements or something, that explains this a bit more? Is there a
> reason why BEHAVE is not trying to standardize low/high usage
> behaviors? (isn't standardizing behaviors what behave is all about?)
>
> Are the "may" terms here RFC2119-compliant? Usually RFC2119 "may"
> terms are capitalized.
>
> 27) It is very helpful to add comments to your IMPORTS identifying
> the RFC that contains the MIB you are importing from.
>
> 28) natPoolRangeType - can this support any of the address types
> allowed
by
> this textual convention? Or should the text constrain the acceptable
values?
> I'm not sure which ways to limit this are acceptable. Let me know if
> all
the
> address types cannot be supported, and I'll check with other mib
> doctors
to
> determine the best way to constrain it.
>
> 29) natMappingTable - "Table of mappings indexed by external
> 3-tuple." I'm not sure this is accurate, given INDEX {
> natInstanceIndex, natMappingProto, natMappingExtRealm,
> natMappingExtAddressType, natMappingExtAddress, natMappingExtPort }
>
> 30) natMappingExtAddress - can you point me to the REFERENCE for
> "the undefined address"? I didn't find it in RFC4001. Ditto for
> natMappingIntAddress
>
> 31) natMappingMapBehavior and natMappingFilterBehavior - for reasons
> mentioned above, please include a description, not just a reference
> in the description text. Put the reference in a REFERENCE clause.
>
> 32) natMappingSubsIndex - you don't really need the Index suffix.
> This would be more meaningful as natMappingSubscriber
>
> 33) natSubscribersTable, natSubscribersEntry, and natSubscriberIndex
> (et
al)
> are not consistent. I recommend losing the plural.
>
> 34) do the existence of fields natSubscriberVlanIdentifier,
> natSubscriberVpnIdentifier, natSubscriberIPEncapsIdType, and
> natSubscriberIPEncapsIdAddr depend on the value of
> natSubscriberIdentifierType? If so, then it might be better to define
> this as a sparse augments table. Let me know, and I'll check with the
> MIB
Doctors
> list about the best way to do this.
>
> 35) natSubscriberIdentifierType - " Unused identifier values MUST be
> zero
or
> equivalent". I think this is under-specified. I think the
> zero-equivalents should be specified here for each type. When the
> type is none(0), and the identifier is "none", does this mean the
> identifier field is not implemented, or has a zero value or empty
> string
or
> what? Which of these is the zero equivalent? The TC
> SubscriberIdentifierType says the interfaces(1) type means the
> identifier is a set of **one or more** interface indexes. What is
> the zero-equivalent of a set of one or more interface indexes?
> VPNIdorZero says "           The semantics of the value zero-length
> OCTET STRING are object-specific and must therefore be defined as
> part of the description of any object that uses this syntax." So
> presumably, the zero equivalent is a zero-length OCTET STRING. But
> the description here does not define the semantics of the zero-length
> string;
it
> only implies the semantics. And for other(5), we have no idea what a
> zero-equivalent is.
>
> 36) natSubscriberIntPrefixType is a "Subscriber's internal prefix
> type." While the corresponding address is a "Prefix assigned to a
> subscriber's CPE." I tend to think of a CPE as being the border
> between a subscriber's
network
> and an ISP's network. If the subscriber entry represents a host
> served by a managed enterprise NAT, is this still the prefix type for
> the CPE? Please clarify if the type is the internal prefix type for
> the
subscriber's
> CPE, or simply the subscriber's internal prefix type.
>
> 37) natSubscriberLimitMappings - again, we should avoid calculated
objects.
> This depends on "active" mappings, which is a calculated value.
>
> 38) natSubscriberMapNotifyThresh - consistency in naming please;
> this seems to be a real problem throughout the subscriber table.
> (natSubscribers vs natSubscriber, thresh, mappngs vs Map)
>
> 39) natSubscriberIPEncapsIdType -  I find the "when not unknown(0)"
> to be an odd description. The InetAddressType description is " A
> value that represents a type of Internet address." I think that
> should suffice here
as
> well, even though unknown(0) has special semantics described in the
> T-C.
>
> 40) natSubsInterfaceIdentifierTable - the description contains a
> sentence fragment; I don't know what it means - " 'OR' semantics if
> multiple interface indexes are present." I find the other sentences a
> bit short; I recommend rewriting the description to make it clearer.
>
> 41) natSubsInterfaceIdSubsIndex - I have some concerns about the use
> of Index as parts of object descriptors and part of object
> descriptions, for objects that are not INDEXes, when INDEX is a SMI
> reserved word (RFC2578, section  3.7). I don't find the "Index"
> suffix necessary. I think SubscriberIndex would be fine as
> Subscriber, and SubsInterfaceIdRowIndex would be fine as
> SubsInterfaceIdRow, and natSubsInterfaceIdRowIndex would be fine as
> natSubsInterfaceIdRow, etc.
>
> 42) natSubsInterfaceIdRowIndex - the description is "Row index."
> There is no description of why this is being defined, or what it is
> expected to be used for. The T-C has a better explanation of its
> uniqueness and its assignment to rows, but no description of why it
> is needed. It might have been better explained in the
> natSubsInterfaceIdentifierEntry description, where it is used in the
> INDEX. But I don't understand the description there - "Each entry
> provides a single interface index." Can you provide an example of how
> this table works?
>
> 43) I am a bit concerned about the compliance options. If we are
> standardizing behaviors, why do we have five compliance levels? Lots
> of compliance levels = lack of standardization.
>
> --- Security considerations --- 44) What objects reveal host
> identities? Do  you mean host addresses? I think the community has
> made a statement about identity versus address in efforts like HIP
> and SNMPv3. 45) If a disgruntled former employee can break into
> private hosts, can attackers who never worked for the employer do so
> as well? I don't have
any
> employees, and no former employees; does that mean this is an attack
> vector I can totally ignore?
>
>
> Hope this helps,
>
> David Harrington [e-mail]
> <mailto:[e-mail]> [phone]