Subject: Re: mib-doctor review of draft-ietf-ipsp-spd-mib

From: Michael Baer <[email-masked]>
Date: Tue, 11 Jan 2005 14:10:00 -0800

Hi Harrie,

Responses are in-line below,

>>>>> "HH" == Harrie Hazewinkel <[email-masked]> writes:

    HH> Hi all,

    HH> On Berts request I have done a review of the
    HH> draft-ietf-ipsp-spd-mib-01.txt.  Hereby my comments. (I am
    HH> subscribe to this list so I do not need an explicit CC).

    HH> - General, it may be subjective, but I do not like the use
    HH> of 'we' and 'us' in an RFC. Therefore, I suggest, for
    HH> example change "we should do" into "one should do".

Changed, I rephrased sentences to not use 'we' except for the text
in the example which I left.

    HH> - There are also parts of this MIB which use similar ideas
    HH>   as the
    HH> packet filtering in the DIFF-SERV MIB. Maybe it is good to
    HH> make a check wheter that can be used. I believe that at
    HH> least some terms can be used from that MIB. For instance,
    HH> incoming -> ingress.  Actually, also the draft itself speaks
    HH> sometimes of inbound and sometimes of incoming (in
    HH> particular the FORMAL MIB part).

incoming|inbound|outgoing|outbound replaced by ingress and egress
respectively in the text.

    HH> - Beyond that I believe it would be a good thing first to
    HH>   clean up the
    HH> usage of the language. Sometimes, many object descriptions
    HH> can be made way more precise.

    HH> - Some comments on the objects, need to be repeated for
    HH>   other
    HH> simila objects also. These are not explicitly added.

    HH> - What is the rational of defining a filter and a subfilter?
    HH> Is the subfilter not just another filter.

Yes it is basically just another filter.  The sub filter table is
used by the compound filter table to create a set of filters which
can be used as a single filter.  The tables that use filters such as
the ruleDefinitionTable can in this way point to a single filter or
to a set of filters via the compound filter table.

    HH> - I beleive it may help a reader/implementor a lot if the
    HH> logical structure is explained at front of the MIB module.
    HH> This descrription is currently brief by the size/complexity/
    HH> relations of the tables in the MIB module.

    HH> (more detailed comments below)

    HH> regards, Harrie

    >> Abstract
    >>
    >>
    >> This document defines a SMIv2 Management Information Base
    >> (MIB) module for configuring the security policy database of
    >> a device implementing the IPSec protocol.  The policy-based
    >> packet filtering and the corresponding execution of actions
    >> is of a more general nature than for IPsec configuration
    >> only, such as for configuration of a firewall.  This MIB
    >> module is designed with future extensibility in mind.  It is
    >> thus possible to externally add other packet filters and
    >> actions to the policy-based packet filtering system defined
    >> in this document.

    HH> I am not sure of the usefulnes of 'future extensibility'
    HH> here. I would also indicate that it is therefore possible to
    HH> extends with enterprises specific managed objects.

I rephrased the sentence and dropped 'future'.  Enterprise filters
and actions additions are explicitly mentioned. New ending:

   This MIB module is designed to be extensible.  It is thus
   possible to externally add other enterprise or standards based
   packet filters and actions to the policy-based packet filtering
   system defined in this document.

    >> 1.  Introduction [snip] It is possible to add other packet
    >> transforming actions to this MIB module if those actions
    >> needed to be performed conditionally on filtered traffic.

    HH> The last part of the sentence sounds odd.

Rephrased and also changed 'filtered' to 'network'. New Text:

   It is possible to extend this MIB module and add other packet
   transforming actions that are performed conditionally on network
   traffic.

    >> Companion documents [RFCXXXX], [RFCYYYY] document actions
    >> which are specific to IPsec and IKE.  No IPsec or IKE
    >> specific actions are defined within this document.

    HH> Add note that RFCXXXX and RFCYYYY must be replaced by the
    HH> RFC-editor.

Note added.

    >> 3.  Relationship to the DMTF Policy Model
    >>
    >>
    >> The Distributed Management Task Force (DMTF) has created an
    >> object oriented model of IPsec policy information known as
    >> the IPsec Policy Model White Paper [IPPMWP].  The contents of
    >> this document are also reflected in the "IPsec Configuration
    >> Policy Model" (IPCP) [RFC3585].

    HH> Is it not opposite?  The "IPsec Configuration Policy Model"
    HH> is also reflected in this document.

    >> This MIB module is a task specific derivation of the IPCP for
    >> use with SNMPv3.

    HH> But how does the MIB relate to the previous documents?

Ah, It's suppose to mean that the IPCP reflects the DMTF's Model
(i.e. the original design is the DMTF's model) and that the SPD-MIB
is an instantiation of the IPCP for use with SNMPv3.

I rephrased this paragraph to make it clearer (hopefully). New text:

   The Distributed Management Task Force (DMTF) has created an object
   oriented model of IPsec policy information known as the IPsec Policy
   Model White Paper [IPPMWP].  The "IPsec Configuration Policy Model"
   (IPCP) [RFC3585] is based in large part on the DMTF's IPsec policy
   model.  The IPCP document describes a model for configuring IPsec.
   This MIB module is a task specific derivation (i.e.  an SMIv2
   instantiation) of the IPCP's IPsec configuration model for use with
   SNMPv3.

    >> o Policies, Groups, Conditions, and some levels of Action are
    >> generically named.  That is we dropped prefixes like "SA", or

    HH> 'we dropped' -> 'this document drops' What is 'SA'?

Rephrased with first person plural dropped.  SA is an IPsec acronym
for Security Association.  I added the acronym's expansion to the
text.  changed to, '...IPsec specific prefixes like "SA" (Security
Association)...'

    >> "ipsec".  This is because we feel that packet classification
    >> and matching of conditions to actions is more general than
    >> IPsec and could possibly be reused by other packet
    >> transforming actions which need to conditionally act on
    >> packets matching filters.

    HH> If this are general packet filters, why not using diff-serv
    HH> packet filters.

It can be used somewhat generally, but it is specific to the IPSP
WG's IPsec Policy Model (and the DMTF's model that is based on).

    >> o Filters are implemented in a more generic and scalable
    >>    manner,
    >> rather than enforcing the condition/filtering pairing and
    >> their restrictions upon the user.  The MIB module offers a
    >> compound filter object to provide for greater flexibility
    >> when creating complex filters.

    HH> Implementations hints are fine, but is it really that this
    HH> document implement things in a 'more generic and scalable
    HH> manner'. I guess not, since the RFC does not specify
    HH> implementation specific things and there contradicts.

The introductory text for this item was "The high-level areas where
this MIB module diverges from the IPCP model are:". That is, we are
saying this MIB model is more generic and scalable than the IPCP
model.

rewritten to be more explicit:

   Filters are implemented in a more generic and scalable manner,
   rather than enforcing the condition/filtering pairing of the IPCP
   and its restrictions upon the user.  This MIB module offers a
   compound filter object to provide for greater flexibility than
   the IPCP when creating complex filters.

    >> 4.  MIB Module Overview
    >>
    >>
    >> The MIB module is modularized into several different parts:
    >> rules, filters, and actions.

    HH> Start new paragraph as you do for the other 'parts'.

done

    >> The rules section connects endpoints and groups of rules
    >> together.

    HH> The rules section does not do anything. Maybe the objects
    HH> defined in the section. It is not really clear what a 'rule'
    HH> is.

    HH> The rules section provides endpoint relationships and
    HH> grouping of the 'rules'.

rephrased to:

The rules section provides the data structure needed to represent
the relationship between endpoints and groupings of rules.

    >> This is partially made up of the ipspEndpointToGroupTable,
    >> ipspGroupContentsTable, and the ipspRuleDefinitionTable.

    HH> Why partially? In that case this MIB is incomplete.

It is a partial list because the writer (in this case, me) was
trying to describe the general idea of how the MIB has been designed
and not an exact listing of all the tables within the rules section.
Also, if I remember correctly, when this was written there was still
some flux in the MIB.  In any case, I changed it to a complete list
for this paragraph and the next.  I also fixed the table prefixes
(ipsp -> spd) which had changed in the MIB and were incorrect here.

    >> Each row of the ipspRuleDefinitionTable connects a filter(s)
    >> with an action(s).  It is structured to allow for reuse
    >> through the future creation of extension tables that provide
    >> additional filters and/or actions.  In fact, the companion
    >> documents to this one do just that to define IPsec and IKE
    >> specific actions to be used within this SPD configuration
    >> MIB.
    >>
    >>
    >> The filter section of the MIB module is composed of all the
    >> different

    HH> 'of all the' -> ' of the'

done.

    >> types of filters in the Policy Model.  It is partially made
    >> up of the

    HH> Again 'partially'...

changed.

    >> trueFilter, ipspCompoundFilterTable, ipspIpHeaderFilterTable,
    >> ipspIpOffsetFilterTable, ipspTimeFilterTable, and the
    >> ipspIpsoHeaderFilterTable.
    >>
    >>
    >> The action section of the MIB module contains different
    >> action types

    HH> 'contains different' -> 'contains the different'

hmm, no that isn't the intention of the text.  Obviously, the
original text is not clear.  I rewrote it to:

    The action section of this MIB module contains only the simple
    static actions required for the firewall processing that an
    IPsec SPD implementation requires (e.g. accept, drop, log, ...).
    The companion documents to this document define the complex
    actions necessary for IPsec and IKE negotiations.

    >> from the Policy Model.  This document contains only the basic
    >> actions needed for firewall processing (accept, drop, log,
    >> ...) that an SPD implementation will need.  The companion
    >> documents define the IPsec and IKE specific actions.
    >>
    >>
    >> 4.1 Usage Tutorial
    >>
    >>
    >> In order to make use of the tables contained in this
    >> document, a general understanding of firewall processing must
    >> be first understood.  The processing of the security policy
    >> database involves applying a set of firewall rules to an
    >> interface on a device.  The given set of rules to apply to
    >> any given interface is defined within the
    >> ipspEndpointToGroupTable table.  This table maps a given
    >> interface to a group of rules.  The interface itself with in
    >> this table is specified using its assigned address.  There is
    >> also one group of rules per direction (inbound and outbound).

    HH> This sounds more like introduction.

    >>
    >>
    >> 4.1.1 Notational conventions
    >>
    >>
    >> Notes about the following example operations:
    >>
    >> 1.  All of the example operations in the following section
    >>     make use
    >> of default values for all columns not listed.  The operations
    >> and close are the minimal SNMP Varbinds that must be sent.

    HH> 'operations and close' do sound odd here. Maybe it is they
    HH> are eplained later, but a read (me) is confused.

typo, changed to 'operations and column'

    >> 2.  The example operations are formatted such that a row
    >>     (i.e.  the
    >> table's Entry object) is operated on by using the indexes to
    >> that row and the column values for the that row.  It is left
    >> as an exercise for the reader to turn these into real SNMP
    >> operations using real OIDs and real varbinds in a SET PDU.
    >> Example:
    >>
    >>
    >>
    >> rowEntry(index1 = value1, index2 = value2) = (column1 =
    >> column_value1, column2 = column_value2)

    HH> I am not getting this example. Maybe it is better if you
    HH> once explain how the excercise for the reader must be done.

added a more specific example of the usage notation as follows,

   2.  The example operations are formatted such that a row (i.e.  the
       table's Entry object) is operated on by using the indexes to that
       row and the column values for the that row.

   3.  The following is a generic example of the notation used in the
       following section's examples of this MIB's usage:

       rowEntry(index1     = value1,
                index2     = value2)
             = (column1        = column_value1,
                column2        = column_value2)

   4.  The following is a specific example of the notation used in the
       following section's examples of this MIB's usage.  In order to
       set the address status column to deprecated for a row in the
       IP-MIB::ipAddressTable with the index values of ipAddressAddrType
       = ipv4 and ipAddressAddr = = 10.0.0.1.  The example notation
       would look like the following:

       ipAddressEntry(ipAddressAddrType = 1,           -- ipv4
                      ipAddressAddr     = 0x0a000001 ) -- 10.0.0.1
                   = (ipAddressStatus   = 2)           -- deprecated

    >> IMPORTS MODULE-IDENTITY, OBJECT-TYPE, NOTIFICATION-TYPE,
    >> Integer32, mib-2 FROM SNMPv2-SMI

    HH> Add rfc as comment. (also other imports)

done

    >> spdMIB MODULE-IDENTITY

    >> DESCRIPTION "This MIB module defines configuration objects
    >> for managing IPsec Security Policy.

    HH> Policy -> Policies

done

    >> Copyright (C) The Internet Society (2003). This version of

    HH> We are in the year 2004.

changed to 2005 :)

    >> this MIB module is part of RFC XXXX, see the RFC itself for
    >> full legal notices."
    >>
    >>
    >> -- Revision History
    >>
    >>
    >> REVISION "200401070000Z" -- 7 January 2004 DESCRIPTION
    >> "Initial version, published as RFC xxxx."
    >> -- RFC-editor assigns xxxx

    HH> Hmm, you use here also RFC XXXX. That was also a reference.
    HH> Change one of them.

generic temporary references changed to specific temporary
references

    >>
    >> spdConfigObjects OBJECT IDENTIFIER ::= { spdMIB 1 }
    >> spdNotificationObjects OBJECT IDENTIFIER ::= { spdMIB 2 }
    >> spdConformanceObjects OBJECT IDENTIFIER ::= { spdMIB 3 }
    >> spdActions OBJECT IDENTIFIER ::= { spdMIB 4 }

    HH> In your introduction text I saw there are three parts.  For
    HH> instance, rules and actions, I see actions here, but no
    HH> rules.

The introduction is an abstracted way of looking at the structure
for processing packets that is modeled from the IPsec Configuration
Policy Information Model.  The borders of the model are not the MIBs
themselves.  In the model, there are rules, filters, and actions.
The IPSEC-SPD-MIB includes rules, filters and some actions
(i.e. simple static actions) from that conceptual view.  The more
specific configuration objects for the IPSEC and IKE negotiation
Actions are then defined within the IPSEC-IPSECACTION-MIB and
IPSEC-IKEACTION-MIB respectively.

    >> -- Note: the following subassignments have been used in other
    >>    MIBs:
    >> -- IPSEC-IPSECACTION-MIB:
    >> -- ipsaMIB MODULE-IDENTITY ::= { spdActions 1 }
    >> -- IPSEC-IKEACTION-MIB:
    >> -- ipiaMIB MODULE-IDENTITY ::= { spdActions 2 }

    HH> Why do you not import them?

The values are not used within the IPSEC-SPD-MIB.  The comment is
just here for a reader's informational purposes

    >> SpdBooleanOperator ::= TEXTUAL-CONVENTION STATUS current
    >> DESCRIPTION "The SpdBooleanOperator operator is used to
    >> specify whether sub-components in a decision making process
    >> are ANDed or ORed together to decide if the resulting
    >> expression is true or false."  SYNTAX INTEGER { or(1), and(2)
    >> }

    HH> How about XOR and NOT/negate? Or not possible to use?

This type is directly created from the IPsec Policy Model as
defined.  That is, the IPCP model just has the AND and OR values.
With negation, however, there is also the ability to create NAND and
NOR logic as well as the ability use the same filter set for both
AND and NAND in different rules.

    >> SpdIPPacketLogging ::= TEXTUAL-CONVENTION STATUS current
    >> DESCRIPTION "SpdIPPacketLogging specifies whether or not an
    >> audit message should be logged when a packet is passed
    >> through an SA.

    HH> It does not really indicate whether logging i must be done,
    HH> but more precise how many bytes of the message must be
    HH> logged. I guess, it would be better to express that.

It does indicate whether logging should be done (e.g., a value of -1
indicates no logging, a value >= 0 indicates logging), but I changed
the initial DESCRIPTION sentence to make it more explicit,

    SpdIPPacketLogging specifies whether or not an audit message
    should be logged when a packet is passed through a Security
    Association (SA) and if some of that packet should be included
    in the log event.

    HH> Beyond that what is an SA??

An SA is a Security Association, a commonly used acronyms in the
IPsec doc's.  I expanded it (de-acronymed it?) in the DESCRIPTION
above.

    >> A value of '-1' indicates no logging.  A value of '0' or
    >> greater indicates that logging should be done and how many
    >> bytes of the beginning of the packet to place in the
    >> log. Values greater than the size of the packet being
    >> processed indicate that the entire packet should be sent.
    >>
    >>
    >> Examples: '-1' no logging '0' log but do not include any of
    >> the packet in the log '20' log and include the first 20 bytes
    >> of the packet in the log."

    HH> The examples roughly duplicate the text before and do not
    HH> added much.

    >> spdIncomingPolicyGroupName OBJECT-TYPE SYNTAX SnmpAdminString
    >> (SIZE(0..32)) MAX-ACCESS read-write STATUS current
    >> DESCRIPTION "This object indicates the policy group
    >> containing the global system policy that is to be applied on
    >> incoming packets (I.E., arriving at a interface) when a given
    >> endpoint does not contain a policy definition in the
    >> spdEndpointToGroupTable.  Its value can be used as an index
    >> into the spdGroupContentsTable to retrieve a list of
    >> policies.  A zero length string indicates no system wide
    >> policy exits and the default policy of 'accept' should be
    >> executed for incoming packets until one is imposed by either
    >> this object or by the endpoint processing a given packet."
    >> ::= { spdLocalConfigObjects 1 }
    >>

    >> spdOutgoingPolicyGroupName OBJECT-TYPE SYNTAX SnmpAdminString
    >> (SIZE(0..32)) MAX-ACCESS read-write STATUS current
    >> DESCRIPTION "This object indicates the policy group
    >> containing the global system policy that is to be applied on
    >> outgoing packets (I.E., leaving an interface) when a given
    >> endpoint does not contain a policy definition in the
    >> spdEndpointToGroupTable.  Its value can be used as an index
    >> into the spdGroupContentsTable to retrieve a list of
    >> policies.  A zero length string indicates no system wide
    >> policy exits and the default policy of 'accept' should be

    HH> I beleive that the description is close to the InComing, but
    HH> for outgoing packets, I do not believe you could use
    HH> 'accept'.

Agreed, I changed this to 'drop' for both outgoing and incoming
policyGroupNames (which better matches the IPsec documents as well).

    >> executed for outgoing packets until one is imposed by either
    >> this object or by the endpoint processing a given packet."
    >> ::= { spdLocalConfigObjects 2 }
    >>
    >>
    >>
    >>
    >> spdEndpointToGroupTable OBJECT-TYPE SYNTAX SEQUENCE OF
    >> SpdEndpointToGroupEntry MAX-ACCESS not-accessible STATUS
    >> current DESCRIPTION "This table is used to map policy
    >> (groupings) onto an endpoint where traffic is to pass by.
    >> Any policy group assigned to an endpoint is then used to
    >> control access to the traffic passing by it.
    >>
    >>
    >> If an endpoint has been configured with a policy group and no
    >> contained rule matches the incoming packet, the default
    >> action in this case shall be to drop the packet.
    >>
    >>
    >> If no policy group has been assigned to an endpoint, then the
    >> policy group specified by spdSystemPolicyGroupName should be
    >> used for the endpoint."  ::= { spdConfigObjects 2 }
    >>
    >>
    >> spdEndpointToGroupEntry OBJECT-TYPE SYNTAX
    >> SpdEndpointToGroupEntry MAX-ACCESS not-accessible STATUS
    >> current DESCRIPTION "A mapping assigning a policy group to an
    >> endpoint."  INDEX { spdEndGroupIdentType, spdEndGroupAddress
    >> }

    HH> Why is the spdEndGroupDirection not part of the index?  It
    HH> looks to me you make a distinction and thus need to be
    HH> reflected in making configurations also.

Yep, it should be part of the index, fixed.

    >> ::= { spdEndpointToGroupTable 1 }
    >>
    >> spdEndGroupDirection OBJECT-TYPE SYNTAX INTEGER {
    >> incoming(1), outgoing(2) } MAX-ACCESS not-accessible STATUS
    >> current DESCRIPTION "The direction of the packet crossing the
    >> interface.  As packets arrive or leave an interface, the
    >> appropriate policy is applied according to the direction it
    >> is traveling: into or out of the device."

    HH> The text does like to me partial repeating itself.

changed to:

   This object indicates which direction of packets crossing the
   interface should be associated with which spdEndGroupName object.
   Ingress packets, or packets into the device match when this value
   is inbound(1).  Egress packets or packets out of the device match
   when this value is outbound(2).

    >> ::= { spdEndpointToGroupEntry 1 }
    >>
    >>
    >> spdEndGroupIdentType OBJECT-TYPE SYNTAX InetAddressType
    >> MAX-ACCESS not-accessible STATUS current DESCRIPTION "The
    >> Internet Protocol version of the address associated with a
    >> given endpoint.  All addresses are represented as an array of
    >> octets in network byte order.  When combined with the
    >> spdEndGroupAddress these objects can be used to uniquely
    >> identify an endpoint that a set of policy groups should be
    >> applied to.  Devices supporting IPv4 MUST support the ipv4
    >> value, and devices supporting IPv6 MUST support the ipv6
    >> value.
    >>
    >>
    >> Values of unknown, ipv4z, ipv6z and dns are not legal values
    >> for this object."  ::= { spdEndpointToGroupEntry 2 }
    >>
    >>
    >> spdEndGroupAddress OBJECT-TYPE SYNTAX InetAddress (SIZE
    >> (4|16)) MAX-ACCESS not-accessible STATUS current DESCRIPTION
    >> "The address of a given endpoint, the format of which is
    >> specified by the spdEndGroupIdentType object."  ::= {
    >> spdEndpointToGroupEntry 3 }
    >>
    >>
    >>
    >> spdEndGroupName OBJECT-TYPE SYNTAX SnmpAdminString
    >> (SIZE(1..32)) MAX-ACCESS read-create STATUS current
    >> DESCRIPTION "The policy group name to apply to this endpoint.
    >> The value of the spdEndGroupName object should then be used
    >> as an index into the spdGroupContentsTable to come up with a
    >> list of rules that MUST be applied to this endpoint."

    HH> This text looks to me a duplicating. If the object has a
    HH> value I would indeed think it must be used. But how can you
    HH> make sure that these are used in the spdGroupContentsTable.

Well it is how this object is supposed to be used according to the
MIB design.  But, if the referenced row does not exist, the default
action when the endpoint has been configured and nothing matches a
packet is to drop the packet (this is in the DESCRIPTION of the
psdEndpointToGroupTable).

In other words, for completely un-configured machines the default is
spdIncomoing/OutgoingPolicyGroupName (which, from above, use to be
'accept' but is now 'drop').  For machines (mis)configured to have a
spdEndpointToGroupTable entry for an endpoint but no matching
spdGroupContentsTable entry exists, the default is to drop the
packet.

    HH> While thinkin of it, why not using the index of this table
    HH> in the spdGroupTable? Looks easier to me.

The advantages to the current set up are mostly flexibility and
scalability.  A group can be made up of multiple subgroups with the
current method.  Also, any of these groups can be applied to
different endpoints.

For example, for a rule of 'only accepting html traffic' that is
then applied to three different endpoints, only one group would have
to be created in the spdGroupContentsTable and then all the
endpoints could point to it from the spdEndpointToGroupTable.  A
complex set of rules forming a group or subgroup could also be
created and applied to multiple endpoints this way without
duplicating the rows making up the group.

    >> ::= { spdEndpointToGroupEntry 4 }

    >> This object may not be set to active until one or more active
    >> rows exist within the spdGroupContentsTable for the group
    >> referenced by the spdEndGroupName object."  ::= {
    >> spdEndpointToGroupEntry 7 }

    HH> What happens when you do set it active and those rows do not
    HH> exist.

It should respond with an inconsistentValue error.  But I made this
more explicit:
     
    This object is considered 'notReady' and may not be set to
    active until one or more active rows exist within the
    spdGroupContentsTable for the group referenced by the
    spdEndGroupName object."

    >> spdGroupContentsTable OBJECT-TYPE SYNTAX SEQUENCE OF
    >> SpdGroupContentsEntry MAX-ACCESS not-accessible STATUS
    >> current DESCRIPTION "This table contains a list of rules
    >> and/or subgroups contained within a given policy group.

    HH> Is this table used for two purposes?

Yes and no, this table is recursive and the
spdGroupContComponentName can refer to a rule or to another entry in
this table.  That is, groups can be made up of rules and subgroups
(a subgroup is otherwise identical to a group).  So the table is
used for the purpose of creating groups, but a group can be made up
of rules and/or other groups.

    >> The entries are sorted by the spdGroupContPriority object and
    >> MUST be executed in order according to this value, starting
    >> with the lowest value.  Once a group item has been processed,
    >> the processor MUST stop processing this packet if an action
    >> was executed as a result of the processing of a given group.
    >> Iterating into the next policy group item by finding the next
    >> largest spdGroupContPriority object shall only be done if no
    >> actions were run when processing the last item for a given
    >> packet."  ::= { spdConfigObjects 3 }
    >>
    >>
    >> spdGroupContentsEntry OBJECT-TYPE SYNTAX
    >> SpdGroupContentsEntry MAX-ACCESS not-accessible STATUS
    >> current DESCRIPTION "Defines a given sub-component within a
    >> policy group."

    HH> The table does contain a list of rules and/or subgroups, but
    HH> the entries do consist of sub-components.  On top of that a
    HH> sub-component is not defined either.

a sub-component is either a rule or a another group as referenced by
spdGroupContComponentName depending on the value of
sdpgroupContComponentType.  I changed the above DESCRIPTION to:

        "Defines a given sub-component within a policy group.  A
         sub-component is either a rule or another group as
         indicated by spdGroupContCompontentType and referenced by
         spdGroupContCompontentName."

    >> INDEX { spdGroupContName, spdGroupContPriority } ::= {
    >> spdGroupContentsTable 1 }

    >> spdGroupContPriority OBJECT-TYPE SYNTAX Integer32 (0..65535)
    >> MAX-ACCESS not-accessible STATUS current DESCRIPTION "The
    >> priority (sequence number) of the sub-component in this
    >> group."  ::= { spdGroupContentsEntry 2 }
    >>
    >>
    >> spdGroupContFilter OBJECT-TYPE SYNTAX VariablePointer

    HH> Since below the pointer refers to entries in tables.  Is a
    HH> RowPointer not prefered then?

It can also point to an object, for example the object
spdTrueFilterInstance is a valid value.

    >> MAX-ACCESS read-create STATUS current DESCRIPTION
    >> "spdGroupContFilter points to a filter which is evaluated to
    >> determine whether the sub-component within this group should
    >> be exercised.

    HH> What is considered a sub-component?
see above for sub-component definition. I change this to be more
explicit:

        "spdGroupContFilter points to a filter which is evaluated
         to determine whether the spdGroupContComponentName within
         this row should be exercised.

    >>
    >> If this column is set to a VariablePointer value which
    >> references a non-existent row in an otherwise supported
    >> table, the inconsistentName exception should be returned.  If
    >> the table or scalar pointed to by the VariablePointer is not
    >> supported at all, then an inconsistentValue exception should
    >> be returned."

    HH> What should the value be if the variable pointed to is
    HH> destroyed?

That shouldn't be allowed. The following additions have been made,

to DESCRIPTION clause of spdIpHeadFiltRowStatus,
spdIpOffFiltRowStatus, spdTimeFiltRowStatus,
spdIpsoHeadFiltRowStatus, 

   If active, this object must remain active if it is referenced by
   an active row in another table.  An attempt to set it to anything
   other than active while it is referenced by an active row in
   another table will result in an inconsistentValue error.

to spdSubFiltRowStatus's  DESCRIPTION

   If active, this object must remain active unless one of the
   following two conditions are met.  An attempt to set it to
   anything other than active while the following conditions are not
   met will result in an inconsistentValue error.  The two
   conditions are:

   I.  No active row in the SpdCompoundFilterTable exists which has
       a matching spdCompFiltName.
   II. Or at least one other active row in this table has a
       matching spdCompFiltName."

to  spdSubActRowStatus's DESCRIPTION

   This object can only be set to notInService or destroy if one of
   following two conditions are met.  If a set of 'destroy' is
   attempted and neither condition is met an inconsistentValue error
   is returned.  The two conditions are:

   I.  No active row in the spdCompoundActionTable exists which has
       a matching spdCompActName.

   II. Or at least one other active row in this table has a matching
       spdCompActName."

to the DESCRIPTION clause of ipsaSaPerActRowStatus,
ipiaIkeActRowStatus, ipiaIpsecActRowStatus,

   An attempt to set it to anything other than active while it is
   referenced by an active row in another table will result in an
   inconsistentValue error.

    >> DEFVAL { spdTrueFilterInstance }

    HH> Why do you not use a zeroDotZero?

I'm not sure what's meant here.  But we wanted this value to default
to true because we believe this will be used more often than any
single other filter at this location.

    >> ::= { spdGroupContentsEntry 3 }
    >>
    >>
    >> spdGroupContComponentType OBJECT-TYPE SYNTAX INTEGER {
    >> reserved(0), group(1), rule(2) } MAX-ACCESS read-create
    >> STATUS current DESCRIPTION "Indicates whether the
    >> spdGroupContComponentName object is the name of another group
    >> defined within the spdGroupContentsTable or is the name of a
    >> rule defined within the spdRuleDefinitionTable."

    HH> What do the values mean here? I believe group and rule are
    HH> ok, but reserved?

Hmm, we were thinking it was standard way of protecting from misuse
of 0 as value in SMIv2.  But I could only find one other MIB that
did this so the reserver(0) values have been removed.

    >> DEFVAL { rule } ::= { spdGroupContentsEntry 4 }
    >>
    >>
    >> spdGroupContComponentName OBJECT-TYPE SYNTAX SnmpAdminString
    >> (SIZE(1..32))

    HH> I notice a lot of times that this indicates a group or rule
    HH> name.  Why not making a single TC providing the semantics
    HH> for such a name.

Mainly because the TC SnmpAdminString already exists, is commonly
used, and provided what we wanted.

    >> MAX-ACCESS read-create STATUS current DESCRIPTION "The name
    >> of the policy rule or subgroup contained within this group,
    >> as indicated by the spdGroupContComponentType object."  ::= {
    >> spdGroupContentsEntry 5 }

    >> spdRuleDefDescription OBJECT-TYPE SYNTAX SnmpAdminString
    >> MAX-ACCESS read-create STATUS current DESCRIPTION "A user
    >> definable string.

    HH> 'definable' -> 'defined'

changed (as mentioned above, I also deleted unnecessary pronoun
'your' from following sentence).

    >> spdRuleDefFilter OBJECT-TYPE

    HH> DEF-VAL?? It looks to me a similar object as before.

It is similar, but the previous filter pointer was for the special
case of filtering before processing a group whereas this is the
filter associated to an action via this rule.  That case had a
default value that will often be used.  In this case, there was no
default value that made sense.

    >> ::= { spdRuleDefinitionEntry 3 }
    >>
    >>
    >> spdRuleDefFilterNegated OBJECT-TYPE SYNTAX TruthValue
    >> MAX-ACCESS read-create STATUS current DESCRIPTION
    >> "spdRuleDefFilterNegated specifies whether the filter
    >> referenced by the spdRuleDefFilter object should be negated
    >> or not."  DEFVAL { false }

    HH> What is the rational for the negation, is not better to
    HH> added the negated rule instead of this mechanism?

This mechanism allows for a negated rule.  A row in this table is a
'rule'.  There is also negation at the compound filter level.  Both
of these locations derive from the IPCP model.

    >> ::= { spdRuleDefinitionEntry 4 }
    >>
    >>
    >> spdRuleDefAction OBJECT-TYPE

    HH> DEFVAL?

No default value makes sense in this case.

    >> spdCompFiltDescription OBJECT-TYPE SYNTAX SnmpAdminString
    >> MAX-ACCESS read-create STATUS current DESCRIPTION "A user
    >> definable string.  You may use this field for your
    >> administrative tracking purposes."  DEFVAL { ''H }

    HH> Before this was defined as "".

changed to "".

    >>
    >> spdTrueFilter OBJECT-TYPE SYNTAX Integer32 MAX-ACCESS
    >> read-only STATUS current DESCRIPTION "This scalar indicates a
    >> (automatic) true result for a filter.  I.e. this is a filter
    >> that is always true, useful for adding as a default filter
    >> for a default action or a set of actions."  ::= {
    >> spdStaticFilters 1 }

    HH> What does this add in the boolean logic?

It allows a user to have an action automatically run.

    >>
    >>
    >> spdTrueFilterInstance OBJECT IDENTIFIER ::= { spdTrueFilter 0
    >> }
    >>
    >>
    >> --
    >> -- Policy IPHeader filter definition table
    >> --
    >>
    >>
    >> spdIpHeaderFilterTable OBJECT-TYPE SYNTAX SEQUENCE OF
    >> SpdIpHeaderFilterEntry MAX-ACCESS not-accessible STATUS
    >> current DESCRIPTION "This table contains a list of filter
    >> definitions to be used within the spdRuleDefinitionTable or
    >> the spdSubfilterTable table."  ::= { spdConfigObjects 8 }

    HH> I believe this is similar as defined in the DIFF-SERV-MIB.
    HH> Would it not be better to use those filters. For instance,
    HH> also the IPCDN WG could use the same filters for traffic
    HH> filtering.

They are very similar.  The main difference is that
spdIpHeaderFilterTable fits into the SPD design of referencing the
filters by a name (octet string) as opposed to the
diffServMultiFieldClfrTable's integer index.  That said, if the
community prefers the usage of the diffServMultiFieldClfrTable and
the loss of the text references, we could switch to using the
DIFF-SERV-MIB's table.

    >> spdTimeFiltTimeOfDayMaskStart OBJECT-TYPE SYNTAX DateAndTime
    >> MAX-ACCESS read-create STATUS current DESCRIPTION "Indicates
    >> the starting time of day for which this filter evaluates to
    >> true.  The date portions of the DateAndTime TC are ignored
    >> for purposes of evaluating this mask and only the time
    >> specific portions are used."  DEFVAL {
    >> '00000000000000002b0000'H } ::= { spdTimeFilterEntry 7 }

    HH> I beleive it would be better to make a new Time TC.  You add
    HH> more semantics to a value that you want.

DateAndTime is a commonly used TC.  We could make a specific TC, but
it would be an entire TC that was specific to only these two
(i.e. mask start and stop) inter-related values in one table in one
MIB.

    >> spdTimeFiltTimeOfDayMaskEnd OBJECT-TYPE SYNTAX DateAndTime
    >> MAX-ACCESS read-create STATUS current DESCRIPTION "Indicates
    >> the ending time of day for which this filter evaluates to
    >> true.  The date portions of the DateAndTime TC are ignored
    >> for purposes of evaluating this mask and only the time
    >> specific portions are used.  If this starting and ending time
    >> values indicated by the spdTimeFiltTimeOfDayMaskStart and
    >> spdTimeFiltTimeOfDayMaskEnd objects are equal, the filter is
    >> expected to be evaluated over the entire 24 hour period."
    >> DEFVAL { '00000000000000002b0000'H } ::= { spdTimeFilterEntry
    >> 8 }

    HH> I would earlier follow a schema as in RFC 2445 is used.
    HH> Mostly a full day starts at, 00:00:00 and ends at 23:59:00

One of the main uses for the filter is to be able to create a way to
filter out parts of a day.  These are not defining a 'day' but are
only supposed to be a way to filter a range of hours.  For Example,
only allow html traffic Monday through Friday from 9 am to 5 pm.
The start would be value would be set to 9 am the end value to 5 pm
(and the day of week BITS value would have the first 5 bits set).

I changed the spdTimeFiltTimeOfDayMaskEnd to
spdTimeFiltStopTimeOfDayMask and the spdTimeFiltTimeOfDayMaskStart
to spdTimeFiltStartTimeOfDayMask to make it clearer.

    >> spdIPInterfaceType OBJECT-TYPE SYNTAX InetAddressType
    >> MAX-ACCESS accessible-for-notify STATUS current DESCRIPTION
    >> "Contains the interface type for the interface that the

    HH> The value should represent an interface type, but you use an
    HH> InetAddressType as syntax. Is this really correct.

It is correct, but the name is misleading.  They are the
(non-accessible) index values from the spdEndpointToGroupTable.  I
renamed it to spdIPEndpointAddType (and also spdIPInterfaceAddress
to spdIPEndpointAddress) here and in the notifications that use it.

    >> packet which triggered the notification in question is
    >> passing through."  ::= { spdNotificationVariables 2 }
    >>
    >>
    >> spdIPInterfaceAddress OBJECT-TYPE SYNTAX InetAddress
    >> MAX-ACCESS accessible-for-notify STATUS current DESCRIPTION
    >> "Contains the interface address for the interface that the
    >> packet which triggered the notification in question is
    >> passing through."

    HH> As with the previous object. Does this have any relationship
    HH> to the interface MIB.

They are related to the spdEndpointToGroupTable's index values for
the endpoint's address.  Since they are index values and are not
accessible in that table, we created notifications objects to
include them in a notification.

    >> ::= { spdNotificationVariables 3 }
    >>
    >>
    >> spdIPSourceType OBJECT-TYPE SYNTAX InetAddressType MAX-ACCESS
    >> accessible-for-notify STATUS current DESCRIPTION "Contains
    >> the source address type of the packet which triggered the
    >> notification in question."

    HH> 'in question' what does it add.

hmm, not much. Removed it here and in other notifications.

    >> ::= { spdNotificationVariables 4 }
    >>
    >>
    >> spdPacketDirection OBJECT-TYPE SYNTAX INTEGER { inbound(1),
    >> outbound(2) } MAX-ACCESS accessible-for-notify STATUS current
    >> DESCRIPTION "Indicates if the packet whic triggered the
    >> action in questions was inbound our outbound."  ::= {
    >> spdNotificationVariables 8 }

    HH>  From the previous source/destinations i get the feeling the
    HH>  direction
    HH> is already fixed. Why this object?

If everything is configured correctly, the direction should be
derivable from the source/destinations.  If there is a
misconfiguration and the incorrect action is being triggered, this
may not be the case (i.e. the difference between what should trigger
the action and what did trigger the action.).

    >>
    >>
    >> spdPacketPart OBJECT-TYPE SYNTAX OCTET STRING MAX-ACCESS
    >> accessible-for-notify STATUS current DESCRIPTION "Is the
    >> front part of the packet that triggered this notification.
    >> The size is determined by the value of 'SpdIPPacketLogging'
    >> or the size of the packet, whichever is smaller."  ::= {
    >> spdNotificationVariables 9 }

    HH> The value SpdIPPacketLogging is first of all an
    HH> TEXTUAL-CONVENTION, secondly the SYNTAX is "Integer32
    HH> (-1..65535)" and thus may become 65535 and does not fit in
    HH> the SNMP packet.  How will that be handled?

changed to:

   Is the front part of the packet that triggered this notification.
   The initial size limit is determined by the smaller of the size
   indicated by 'SpdIPPacketLogging' and the size of the triggering
   packet.

   The final limit is determined by the SNMP packet size when
   sending the notification.  The maximum size that can be included
   will be the smaller of the initial size given above and the
   length that will fit in a single SNMP notification packet after
   the rest of the notification's objects and any other necessary
   packet data (headers, encoding, etc...) has been included in the
   packet.

    >> -- spdRuleFilterCompliance MODULE-COMPLIANCE STATUS current
    >> DESCRIPTION "The compliance statement for SNMP entities that
    >> include an IPsec MIB implementation with Endpoint, Rules, and
    >> filters support."

    HH> Maybe it is a good idea to provide also a ReadOnly
    HH> Compliance statement.  I can immagine setups/implementations
    HH> that do not allow configuration via SNMP. (See for
    HH> intstance, the DIFF-SERV-MIB.

We never saw any interest by anyone to support this MIB ReadOnly so
we didn't think the work to support it was justifiable.

    >> MODULE -- This Module MANDATORY-GROUPS { spdEndpointGroup,
    >> spdGroupContentsGroup, spdRuleDefinitionGroup,
    >> spdIPHeaderFilterGroup, spdStaticFilterGroup,
    >> spdStaticActionGroup }
    >>
    >>
    >> GROUP spdIpsecSystemPolicyNameGroup DESCRIPTION "This group
    >> is mandatory for IPsec Policy implementations which support a
    >> system policy group name."

    HH> If mandatory, why not in the MANDATORY-GROUPS? (also others)

It is only mandatory for 'implementations which support a system
policy group name.'  In this way a system could be in compliance
with limits.  A system may not support system policy groups, but if
it does, it's mandatory to support spdIpsecSystemPolicyNameGroup

    >> OBJECT spdEndGroupRowStatus SYNTAX RowStatus { active(1),
    >> createAndGo(4), destroy(6)
    >> }
    >> DESCRIPTION
    >> "Support of the values notInService(2), notReady(3),
    >> and createAndWait(5) is not required."
    >>
    >>
    >> OBJECT spdEndGroupLastChanged MIN-ACCESS not-accessible
    >> DESCRIPTION "This object not required for compliance."

    HH> This may be not required for compliance, but it is defined
    HH> in a MANDATORY-GROUP. I guess you want to change the
    HH> grouping then.

The grouping is logical, but we didn't want to force compliance for
lastChanged objects.  Our belief was that it would be more confusing
to split table object's into multiple groups and instead put them
into one group with mentioned exceptions.

In looking at this though, the similarly used RowStatus values are
redundant.  The RowStatus TC already allows this behavior so these
have been excised from the MIB.

    >>
    >> spdEndpointGroup OBJECT-GROUP OBJECTS { spdEndGroupName,
    >> spdEndGroupLastChanged, spdEndGroupStorageType,
    >> spdEndGroupRowStatus
    >> }
    >> STATUS current
    >> DESCRIPTION
    >> "The IPsec Policy Endpoint Table Group."
    >> ::= { spdGroups 1 }

    HH> This counts also for the other descriptions, but it does not
    HH> add any value. I would suggest a more description text of
    HH> for instance why/what this group of objects are for.

change to (and others similarly):
"This group is made up of the objects from the IPsec Policy Endpoint
Table."

    >>
    >>
    >> spdActionNotificationGroup NOTIFICATION-GROUP NOTIFICATIONS {
    >> spdActionNotification, spdPacketNotification
    >> }
    >> STATUS current
    >> DESCRIPTION
    >> "Notifications."

    HH> This description adds nothing. It is equal as not having it.
    HH> WHat kind fo notifications are these?

changed to:

"This group is made up of all the Notifications for this MIB."

    >> ::= { spdGroups 14 }
    >>
    >>
    >>
    >> END
    >>
    >>
    >>
    >> 6.  Security Considerations
    >>
    >>
    >> 6.1 Introduction
    >>
    >>
    >> This document defines a MIB module used to configure IPsec
    >> policy services.  Since IPsec provides security services it
    >> is important that the IPsec configuration data be at least as
    >> protected as the IPsec provided security service.  There are
    >> two main threats you need to thwart when configuring IPsec
    >> devices.
    >>
    >>
    >> 1.  Malicious Configuration: only the official administrators
    >>     should
    >> be allowed to configure a device.  In other words,
    >> administrators' identities should be authenticated and their
    >> access rights checked before they are allowed to do device
    >> configuration.  The support for SET operations to the IPSP
    >> MIB in a non-secure environment, without proper protection,
    >> can have a negative effect on the security of the network
    >> traffic affected by the IPSP MIB.
    >>
    >>
    >> 2.  Disclosure of Configuration: Malicious parties should not
    >>     be
    >> able to read configuration data while the data is in network
    >> transit.  Any knowledge about a device's IPsec policy
    >> configuration could help an unfriendly party compromise that
    >> device and/or the network(s) it protects.  It is thus
    >> important to control even GET access to these objects and
    >> possibly to even encrypt the values of these objects when
    >> sending them over the network via SNMP.
    >>
    >>
    >> 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.
    >>
    >>
    >> Therefore, when configuring data in the IPSEC-SPD-MIB, you
    >> SHOULD use SNMP version 3.  The rest of this discussion
    >> assumes the use of SNMPv3.  This is a real strength, because
    >> it allows administrators the ability to load new IPsec
    >> configuration on a device and keep the conversation private
    >> and authenticated under the protection of SNMPv3 before any
    >> IPsec protections are available.  Once initial establishment
    >> of IPsec configuration on a device has been achieved, it
    >> would be possible to set up IPsec SAs to then also provide
    >> security and integrity services to the configuration
    >> conversation.  This may seem redundant at first, but will be
    >> shown to have a use for added privacy protection below.
    >>
    >>
    >> 6.2 Protecting against in-authentic access
    >>
    >>
    >> The current SNMPv3 User Security Model provides for key based
    >> user authentication.  Typically, keys are derived from
    >> passwords (but are not required to be), and the keys are then
    >> used in HMAC algorithms (currently MD5 and SHA-1 HMACs are
    >> defined) to authenticate all SNMP data.  Each SNMP device
    >> keeps a (configured) list of users and keys.  Under SNMPv3
    >> user keys may be updated as often as an administrator cares
    >> to have users enter new passwords.  But Perfect Forward
    >> Secrecy for user keys is not yet provided by standards track
    >> documents, although RFC2786 defines an experimental method of
    >> doing so.
    >>
    >>
    >> 6.3 Protecting against involuntary disclosure
    >>
    >>
    >> While sending IPsec configuration data to a PEP, there are a
    >> few critical parameters which MUST NOT be observed by third
    >> parties.

    HH> It would be wise to spell those objects out.

added:

   Specifically, except for public keys, keying information MUST NOT
   be allowed to be observed by third parties.

    >> These include IKE Pre-Shared Keys and possibly the private
    >> key of a public/private key pair for use in a PKI.  Were
    >> either of those parameters to be known to a third party, they
    >> could then impersonate your device to other IKE peers.  Aside
    >> from those critical parameters, policy administrators have an
    >> interest in not divulging any of their policy configuration.
    >> Any knowledge about a device's configuration could help an
    >> unfriendly party compromise that device.  SNMPv3 offers
    >> privacy security services, but at the time this document was
    >> written, the only standardized encryption algorithm supported
    >> by SNMPv3 is the DES encryption algorithm.  Support for other
    >> (stronger) cryptographic algorithms is in the works and may
    >> be done as you read this.  Policy administrators SHOULD use a
    >> privacy security service to configure their IPsec policy
    >> which is at least as strong as the desired IPsec policy.
    >> E.G., it is unwise to configure IPsec parameters implementing
    >> 3DES algorithms while only protecting that conversation with
    >> single DES.
    >>
    >>
    >> 6.4 Bootstrapping your configuration
    >>
    >> Hopefully vendors will not ship new products with a default
    >> SNMPv3 user/password pair, but it is possible.  Most SNMPv3
    >> distributions should hopefully require an out-of-band
    >> initialization over a trusted medium, such as a local console
    >> connection.

    HH> I understand the point, but what should a vendor do?

added:

   If a product does install with default user/password information,
   these values should be changed before connecting to a network.

    >> 8.1 Normative References

    HH> The normative refences for SNMP based RFC' can be cleaned up
    HH> I believe in accordance with the new boiler plate. be aware
    HH> that the MIB modules that are imported still must be
    HH> references as RFC.

    >>
    >> [RFC3585] Jason, J., Rafalow, L. and E. Vyncke, "IPsec
    >> Configuration Policy Information Model", RFC 3585, August
    >> 2003.
    >>
    >>
    >> 8.2 Informative References
    >>
    >>
    >> [RFCXXXX] Baer, M., Charlet, R., Hardaker, W., Story, R. and
    >> C.  Wang, "IPsec Security Policy IPsec Action MIB", December
    >> 2002.
    >>
    >>
    >> [RFCYYYY] Baer, M., Charlet, R., Hardaker, W., Story, R. and
    >> C.  Wang, "IPsec Security Policy IKE Action MIB", December
    >> 2002.

    HH> Add note for RFC editor to replace RFCXXX/YYY. Also you use
    HH> RFC XXXX for the dradft itself.

done. (it's in the xml doc)

    >>
    >> [IPPMWP] Lortz, V. and L. Rafalow, "IPsec Policy Model White
    >> Paper", November 2000.
    >>
    >>
    >>
    >> Authors' Addresses

    HH> I would put the addresses more compact. No need for filling
    HH> pages.

The spacing is an artifact of the xml2rfc application.

-- 
Michael Baer
[email-masked]

---

From: "Wijnen, Bert (Bert)" <[email-masked]>
Date: Thu, 13 Jan 2005 20:00:36 +0100

One thing that immediately caught my eye when quickly browsing this 

>                    = (ipAddressStatus   = 2)           -- deprecated
Pls, when you want to use example IPv4 addresses, stick to and use the ones 
assigned/reserved for that purpuse as per RFC3330:

   192.0.2.0/24 - This block is assigned as "TEST-NET" for use in
   documentation and example code.  It is often used in conjunction with
   domain names example.com or example.net in vendor and protocol
   documentation.  Addresses within this block should not appear on the
   public Internet.

And if you are going to do a revision (which is what it looks like), then
the following may be considered too:

IDNITs (as available at: http://ietf.levkowetz.com/tools/idnits/)
results looks reasonably OK:
--------
$ idnits draft-ietf-ipsp-spd-mib-01.txt
idnits 1.58

draft-ietf-ipsp-spd-mib-01.txt:

  Checking nits according to http://www.ietf.org/ID-Checklist.html :

  * The document seems to lack an IANA Considerations section.
    Checking conformance with RFC 3667/3668 boilerplate...
    the boilerplate looks good.

  Checking nits according to http://www.ietf.org/ietf/1id-guidelines.txt :

  - The page length should not exceed 58 lines per page, but there was 65 longer pages, the
    longest (page 63) being 70 lines
  - It seems as if not all pages are separated by form feeds - found 0 form feeds but 65 pages

  Miscellaneous warnings:

    None.

    Run idnits with the --verbose option for more detailed information.
----

reference checking:
---------
!! Missing citation for Normative reference:
  P062 L040:    [RFC3411]  Harrington, D., Presuhn, R. and B. Wijnen, "An

!! Missing citation for Normative reference:
  P062 L046:    [RFC3412]  Case, J., Harrington, D., Presuhn, R. and B. Wijnen,

!! Missing citation for Normative reference:
  P062 L052:    [RFC3413]  Levi, D., Meyer, P. and B. Stewart, "Simple Network

!! Missing citation for Normative reference:
  P062 L057:    [RFC3414]  Blumenthal, U. and B. Wijnen, "User-based Security Model

!! Missing citation for Normative reference:
  P062 L062:    [RFC3415]  Wijnen, B., Presuhn, R. and K. McCloghrie, "View-based
------------

Bert