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

From: Harrie Hazewinkel <[email-masked]>
Date: Tue, 30 Nov 2004 10:13:22 +0100

Hi all,

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

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

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

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

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

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

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

(more detailed comments below)

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.

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

> 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.

The last part of the sentence sounds odd.

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

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

> 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].

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

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

But how does the MIB relate to the previous documents?

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

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

>       "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.

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

>    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.

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

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

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

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

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

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

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

Why partially? In that case this MIB is incomplete.

>  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

'of all the' -> ' of the'

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

Again 'partially'...

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

'contains different' -> 'contains the different'

>    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).

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.

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

>    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)

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

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

Add rfc as comment. (also other imports)

>    spdMIB MODULE-IDENTITY

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

Policy -> Policies

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

We are in the year 2004.

>          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

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

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

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

>    -- 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 }

Why do you not import them?

>    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) }

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

>    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.

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

Beyond that what is an SA??

>   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."

The examples roughly duplicate the text before and do not 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

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

>             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 }

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

>        ::= { 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."

The text does like to me partial repeating itself.

>        ::= { 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."

This text looks to me a duplicating. If the object has a value I would
indeed think it must be used. But how can you make sure that these
are used in the spdGroupContentsTable.
While thinkin of it, why not using the index of this table in the
spdGroupTable? Looks easier to me.

>        ::= { 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 }

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

>    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.

Is this table used for two purposes?

>  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."

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

>        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

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

>        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.

What is considered a sub-component?

>
>             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."

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

>        DEFVAL { spdTrueFilterInstance }

Why do you not use a zeroDotZero?

>        ::= { 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."

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

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

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

>        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.

'definable' -> 'defined'

>    spdRuleDefFilter OBJECT-TYPE

DEF-VAL?? It looks to me a simialr object as before.

>        ::= { 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 }

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

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

DEFVAL?

>    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 }

Before this was defined as "".

>
>    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 }

What does this add in the boolean logic?
>
>
>    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 }

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

>    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 }

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

>    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 }

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

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

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

>             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."

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

>        ::= { 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."

'in question' what does it add.

>        ::= { 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 }

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

>
>
>    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 }

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

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

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

>        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."

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

>            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."

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

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

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

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

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

>        ::= { 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.

It would be wise to spell those objects out.

>    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.

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

> 8.1  Normative References

The normative refences for SNMP based RFC' can be cleaned up I believe
in accordance with the new boiler plate. be aware that the MIB modules
that are imported still must be 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.

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

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

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