Subject: [MIB-DOCTORS] MIB Doctor review: SYNTAX checks: draft-ietf-ipsp-ikeaction-mib-02.txt

From: "Wijnen, Bert \(Bert\)" <[email-masked]>
Date: Wed, 3 Jan 2007 14:15:19 +0100

SMICng (tools) tell me:

  $ smicmfm IPSEC-IKEACTION-MIB.mi2
  E: f(IPSEC-IKEACTION-MIB.mi2), (41,21) Date/time is wrong length
  ** 1 error and 0 warnings in parsing

This is cause by:
   ipiaMIB MODULE-IDENTITY
       LAST-UPDATED "20060905'"    -- 05 September 2006

which should be something like:
   ipiaMIB MODULE-IDENTITY
       LAST-UPDATED "200609050000Z"    -- 05 September 2006

Once I fix that, I get

  $ smicng IPSEC-IKEACTION-MIB.inc
  W: f(IPSEC-IKEACTION-MIB.mi2), (82,21) REVISION value "20060905'"
     is not a valid extended UTC time
sane problem as LAST-UPDATED
  W: f(IPSEC-IKEACTION-MIB.mi2), (1331,22) Row "ipiaIkeProposalEntry"
     does not have a consistent indexing scheme - index item
     ipiaIkeActPropName from base row ipiaIkeActionProposalsEntry
     is not defined as an index item
  W: f(IPSEC-IKEACTION-MIB.mi2), (1787,38) Row "ipiaIkeIdentityEntry"
     does not have a consistent indexing scheme - index item
     ipiaIkeActIdentityType from base row ipiaIkeActionEntry is not
     defined as an index item
  W: f(IPSEC-IKEACTION-MIB.mi2), (2299,18) Row
"ipiaRevokedCertificateEntry"
     does not have a consistent indexing scheme - index items must
     be in same order as used in INDEX clause for "base row"
ipiaCredMngCRLEntry

I suggest that the authors/editors review the last 3 warnings and 
make sure that this is their intended INDEXing scheme, regardless of
the errors. And pls explain why you index this way. It does look
weird to me too.

SMILINT tells me:

  C:\smi\mibs\work>smilint -l6 -s -m ./IPSEC-IKEACTION-MIB
  ./IPSEC-IKEACTION-MIB:41: [2] {date-length} date specification
    `20060905'' has an illegal length
  ./IPSEC-IKEACTION-MIB:82: [2] {date-length} date specification
    `20060905'' has an illegal length
  ./IPSEC-IKEACTION-MIB:40: [4] {module-identity-registration} 
    warning: uncontrolled MODULE-IDENTITY registration

The first two are the same as what SMICng reported on the date spec for
LAST-UPDATED and REVISION clauses.

The last one warns that you are registering the MIB under a branch 
that is not well controlled. You are doing:

       ::= { spdActions 2 }

I see in IPSEC-SPD-MIB that you have defined
   spdActions                OBJECT IDENTIFIER
        ::= { spdMIB 4 }

And that in its IANA considerations (rev 7 ID) you state:
   8.  IANA Considerations

   Only two IANA considerations exist for this document.  The first is
   just the node number allocation of the IPSEC-SPD-MIB itself.

   The IPSEC-SPD-MIB also allows for extension action MIB's.  Although
   additional actions are not required to use it, the node spdActions is
   allocated as a subtree under which IANA can define any additional
   actions.  IANA would be responsible for allocating any values under
   this node.  The only restriction is that additional nodes appended to
   spdACtions should be in reference to IPSEC-SPD-MIB actions.

So you don't even seem to ask that a spec is published for allocation?
I am somewhat surprised by this (and probably should have seen this 
earlier). The spdActions ultimately are under mib-2 which is standards 
track space. Oh well.

In any event, you do state that IANA assigns new values. So in your 
IPSEC-IKEACTION-MIB I would have expected:

       ::= { spdActions nnn } -- to be assigned by IANA. We suggest to
                              -- assign the value 2.

More review comments later.
Bert

_______________________________________________
MIB-DOCTORS mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/mib-doctors

---

From: "Wijnen, Bert \(Bert\)" <[email-masked]>
Date: Thu, 4 Jan 2007 16:04:43 +0100

[I have copied Keith, because I understand he did partially 
review this MIB module].

I have included Keith's comments (some are generic, some 
are specific T11 or IETF IMSS WG requirements and desires,
yet I think they make a lot of sense) as well, 
so you have a complete set. 

Also, Keith and I are not (yet) in complete agreement on
some aspects of MIB modules. I know that Keith has written
to you about (for example) the IPSEC-SPD-MIB after I had
done MIB doctor (and I guess at the time OPS AD review).
These are the two issues:

- I have advised, for objects of SYNTAX STorageType,
  to use some tekst aka

     For a storage type of permanent, none of the columns have
     to be writable."

  That is, I had interpreted the DESCRIPTION clause of the
  StorageType TC in RFC2579 to mandate that the use of the TC
  requires to specify which writable objects in the associated
  row must be writable (if any).

  Keith thinks this belongs in the MODULE-COMPLIANCE. We are
  still discussing this and so I cannot give any new advise yet.
  This explanation is to warn that I may revise my earlier
  advise as a result of our discussions (OPS AD will be 
  consulted as well before I want to change my earlier advise.
  In fact, Dan is at least being copied on our email exhanges).

- I have advised to write text aka

     An attempt to set this object to a value that does not
     exist in the ipiaSaNegotiationParametersTable MUST result
     in an inconsistentValue error."

  Keith believes that this is incorrect, because sometimes an
  other error can be returned. That is if in the Elements of
  Procedure in RFC3416 (sect 4.2.5, page 19, but specifcally 
  pages 20 and 21) another error would be detected BEFORE you
  check the consistency of the value (i.e. step 10 on page 21).
  I had just assumed that that makes common sense (so if you 
  detect a noAccess as per step 1 on page 20), then OF COURSE
  you return a noAccess error. Again I am not sure that Keith
  and I yet agree on how to word this.

  There are also others in the MIB doctors team (I believe our
  current OPS AD is one of them) who think that we should NEVER
  list specific snmpErrorCodes in the DESCRIPTION clauses. This,
  because (at least in theory, we're not aware of any practice)
  a MIB could be used with non-SNMP management protocols.
  If that were the only issue, then we can solve it by using
  text aka

     An attempt to set this object to a value that does not
     exist in the ipiaSaNegotiationParametersTable would cause
     an inconsistency (and for example for SNMP could result
     in an inconsistentValue error)."

  Anyway, I again have to warn that I may change my earlier
  advise once this discussion closes on MIB doctors team.

General remark
I am not well enough versed/skilled in the security protocols
and mechanisms/algorithms that are being modeled in this MIB
module/document. So I have done a review with a SNMP/MIB view,
but I have not (and cannot) review it from a security-point
of view. In fact, after having spend more than a day on this
MIB module, I cannot say that I really understand it all.

So I am assuming that real security folk will do the securiy
review and check if this indeed correctly models the management
information as intended (or maybe already have done that). 
Also the grouping of Objects and the two MODULE-COMPLIANCE 
statements are outside my competency to determine if they 
make sense or not.

The MIB documents are nearly clean (from a SYNTAX point
of view) and also many of the issues I had brought up in 
the review if the IPSEC-SPD-MIB and also were rpesent in
an earlier version of this MIB document, have been resolved.
So a more detailed review made sense this time. Lots of
comments and questions still remain though.

Here are my comments on the IPSEC-IKEACTION-MIB module.

1. ipiaIkePhase1Filter and ipiaIkePhase2Filter
   are stated to be static filters. 
   So just by pointing to it the filter is activated?
   No matter what the value is?
   I see that in the IPSEC-SPD-MIB we made it a fixed
   value of 1. Would it be wise to be consistent and do
   that here as well?

   Same for ipiaRejectIKEAction and ipiaRejectIKEActionLog

2. I am confused by:

   ipiaCredFiltCredentialType OBJECT-TYPE
       SYNTAX      IpsaCredentialType
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "The credential type that is expected for this filter to
            succeed."
       DEFVAL { x509 }
       ::= { ipiaCredentialFilterEntry 2 }

   The reason is that the IpsaCredentialType TC states:

   IpsaCredentialType ::= TEXTUAL-CONVENTION
       STATUS   current
       DESCRIPTION
           "IpsaCredentialType identifies the type of credential
            contained in a corresponding IpsaIdentityFilter object."
       SYNTAX      INTEGER { reserved(0),
                             unknown(1),
                             sharedSecret(2),
                             x509(3),
                             kerberos(4) }

   So I am left wondering how I find the corresponding 
   IpsaIdentityFilter object ??

3. I see:

   ipiaCredFiltMatchFieldName OBJECT-TYPE

       SYNTAX      OCTET STRING (SIZE(0..256))
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "The piece of the credential to match against.  Examples:
            serialNumber, signatureAlgorithm, issuerName or
            subjectName.

            For credential types without fields (e.g. shared secret),
            this field SHOULD be left empty, and the entire credential
            will be matched against the ipiaCredFiltMatchFieldValue."
       ::= { ipiaCredentialFilterEntry 3 }

   So I wonder about a few things.

   - What is an empty field? I guess you mean the zero-length string.
     Pls be specific
   - What happens if some-one does not live up to the SHOULD
     requirement for exampel for a shared secret Credential Type?

4. I see:

   ipiaCredFiltAcceptCredFrom OBJECT-TYPE
       SYNTAX      OCTET STRING(SIZE(1..117))
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "This value is used to look up a row in the
            ipiaIpsecCredMngServiceTable for the Certificate Authority
            (CA) Information.  This value is empty if there is no CA
            used for this filter."
       ::= { ipiaCredentialFilterEntry 5 }

   And wonder what "empty" means here. Gievne the SIZE(1..117) it seems
   to me that it cannot mean zero-length string. So I have no idea
   how to determine if this "value" is "empty" ??

   I also wonder how to look up a row in the
ipiaIpsecCredMngServiceTable
   since in that table I see no column that has a similar SYNTAX. Well,
I
   see some of SYNTAX OCTET STRING, but they have different SIZE ranges.
   So I guess I am (at least) confused.

5. I see:

   ipiaCredFiltStorageType OBJECT-TYPE
       SYNTAX      StorageType
       MAX-ACCESS  read-create
       STATUS      current

       DESCRIPTION
           "The storage type for this row.  Rows in this table which
            were created through an external process MAY have a storage
            type of readOnly or permanent.

            For a storage type of permanent, none of the columns have
            to be writable."
       DEFVAL { nonVolatile }
       ::= { ipiaCredentialFilterEntry 7 }

  I do not see why you would use capitalized MAY. I assume one may also
  create readOnly or permanent rows via SNMP?
  This issue occurs in many (all) StorageType objects

  The last sentence in the DESCRIPTION clause probably better reads:

            For conceptual rows having the value of 'permanent', 
            none of the writable columns in the row need to allow 
            write-access."

  I.e. there is a difference of a column to has a MAX-ACCESS of 
  read-create and the "allowed to actually get write access to
  an instance".

  This issue occurs in many (all) StorageType objects

6. I see

  ipiaPeerIdFiltIdentityValue OBJECT-TYPE
      SYNTAX      IpsaIdentityFilter
      MAX-ACCESS  read-create
      STATUS      current
      DESCRIPTION
          "The string representation of the value that the peer ID
           payload value MUST match against. Wildcard mechanisms MUST
           be supported such that:

           - a ipiaPeerIdFiltIdentityValue of [email-masked]' will
             match a userFqdn ID payload of [email-masked]'

           - a ipiaPeerIdFiltIdentityValue of '*.example.com' will
             match a fqdn ID payload of 'WWW.EXAMPLE.COM'

  So it seems that matching is case-insensitive?
  If so, pls be specific about that.
  Is it true for all IpsecDoiIdentTypes ??

7. Not sure I understand this:

   ipiaIkeActThresholdDerivedKeys OBJECT-TYPE
       SYNTAX           Integer32 (0..100)
       MAX-ACCESS       read-create
       STATUS           current
       DESCRIPTION
           "ipiaIkeActThresholdDerivedKeys specifies what percentage
            of the derived key limit (see the LifetimeDerivedKeys
            property of IKEProposal) can expire before IKE SHOULD
            attempt to renegotiate the IKE phase 1 security
            association."
       DEFVAL           { 100 }
       ::= { ipiaIkeActionEntry 3 }

   For example, is zero a valid value? If so, does that not mean that
   the system would always be renegotiating? 
   And is this not making the system overly complex?
   You can see I am not a security expert,
   based on the last question I guess
   
8. I see:
   ipiaIkeActIdentityType OBJECT-TYPE
       SYNTAX      IpsecDoiIdentType
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "This column along with ipiaIkeActIdentityContext and

            endpoint information is used to refer an
            ipiaIkeIdentityEntry in the ipiaIkeIdentityTable."
       ::= { ipiaIkeActionEntry 6 }

   ipiaIkeActIdentityContext   OBJECT-TYPE
       SYNTAX           SnmpAdminString (SIZE(1..32))
       MAX-ACCESS       read-create
       STATUS           current
       DESCRIPTION
           "This column, along with ipiaIkeActIdentityType and endpoint
            information, is used to refer to an ipiaIkeIdentityEntry in
            the ipiaIkeIdentityTable."
       ::= { ipiaIkeActionEntry 7 }

   These two objects refer to the ipiaIkeIdentityTable, which I cannot
find.
   I do find a ipiaIkeIdentityTable (which I don't think is about
PEERs?)
   and a ipsaPeerIdentityTable (in the IPSEC-IPSECACTION-MIB). Possibly
the
   latter is intended? Pls correct or explain.

9. I see:
   ipiaIkeActPeerName OBJECT-TYPE
       SYNTAX      SnmpAdminString(SIZE(0..32))
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "This object indicates the peer id name of the IKE peer.
            This object can be used to look up the peer id value,
            address, credentials and other values in the
            ipiaPeerIdentityTable."
       ::= { ipiaIkeActionEntry 8 }

  Is it valid to have a zero-length string?
  If so, what does a zero length string mean?

10. I see:
   ipiaIkeActVendorId    OBJECT-TYPE
       SYNTAX           OCTET STRING (SIZE(0..65535))

       MAX-ACCESS       read-create
       STATUS           current
       DESCRIPTION
           "Vendor ID Payload.  A value of NULL means that Vendor ID
            payload will be neither generated nor accepted.  A non-NULL
            value means that a Vendor ID payload will be generated
            (when acting as an initiator) or is expected (when acting
            as a responder)."
       DEFVAL { "" }
       ::= { ipiaIkeActionEntry 11 }

   I think I would rename this object to ipiaIkeActVendorIdPayload.
   I believe that VendorId is often understood to be an
enterprise-number
   and so the descriptor here may confuse people.

   More importantly, the "NULL" value is unclear to me. I think you
   mean "zero-length string" ??

   Similar suggestion to rename ipiaIpsecActVendorId to
   ipiaIpsecActVendorGroupId or some such.

   Similar issue with ipiaIkePropVendorId

11. I see:
   ipiaIpsecActPeerGatewayIdName OBJECT-TYPE
       SYNTAX      OCTET STRING (SIZE(0..116))
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "This object indicates the peer id name of the peer
            gateway.  This object can be used to look up the peer id
            value, address and other values in the
            ipiaPeerIdentityTable.  This object is used when initiating
            a tunnel SA.  This object is not used for transport SAs.
            If no value is set and ipiaIpsecActMode is tunnel, the peer
            gateway is determined from the source or destination
            address of the packet."
       ::= { ipiaIpsecActionEntry 7 }

   Pls be specific in "If no value is set". I guess you mean the 
   zero-length octet string.

   It also mentions ipiaPeerIdentityTable, which I cannot find. 
   Which table is intended? Is it ipsaPeerIdentityTable ??

   Same for DESCRIPTION clause of ipiaIpsecActPeerGatewayIdName

12. In
   ipiaIpsecActGranularity OBJECT-TYPE
       SYNTAX      INTEGER { subnet(1), address(2), protocol(3),
                             port(4) }

       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "This object specifies how the proposed selector for the
            security association will be created.  The selector is
            created by using the FilterList information.  The selector
            can be subnet, address, porotocol, or port."
       ::= { ipiaIpsecActionEntry 9 }

   You may want to be a bit more specific as to what comprises the
   "FilterList" !!??

13. For

   ipiaSaNegParamMinLifetimeSecs OBJECT-TYPE
       SYNTAX      Unsigned32
       UNITS       "seconds"
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "ipiaSaNegParamMinLifetimeSecs specifies the minimum seconds
            lifetime that will be accepted from the peer."
       ::= { ipiaSaNegotiationParametersEntry 2 }

   ipiaSaNegParamMinLifetimeKB OBJECT-TYPE
       SYNTAX      Unsigned32
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "ipiaSaNegParamMinLifetimeKB specifies the minimum kilobyte
            lifetime that will be accepted from the peer."
       ::= { ipiaSaNegotiationParametersEntry 3 }

  I wonder if the values zero mean anything special?
  Or are they not valid?

14. For
   ipiaIkePropCipherKeyLength OBJECT-TYPE
       SYNTAX      Unsigned32
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "This object specifies, in bits, the key length for
            the cipher algorithm used in IKE Phase 1 negotiation."
       ::= { ipiaIkeProposalEntry 3 }

   Don't you want to say anything about KeyLength requirements?
   Is a value of zero valid at all? If so, what does that mean?

15. For ipiaIkePropCipherKeyRounds
   Is zero a valid value? Are there any other constraints?

16. For
   ipiaIkePropMaxLifetimeSecs OBJECT-TYPE
       SYNTAX      Unsigned32
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "ipiaIkePropMaxLifetimeSecs specifies the maximum amount of
            time to propose a security association remain valid.

            A value of 0 indicates that the default lifetime of
            8 hours SHOULD be used."
       ::= { ipiaIkeProposalEntry 10 }
   It would be good to add a UNIX clause "seconds" I guess.

   I am a bit surprised that if the default is 8 hours, why you would
   represent that by 0. But I can live with it. However is anything
   wrong using 
     DEFVAL { 0 }
   Or possibly even DEFVAL { 28880 } -- 8 hours

17. For ipiaIkePropMaxLifetimeKB, is a value of zero valid?
    If so what does it mean? Any other constraints?

18. For
   ipiaIpsecPropProtocolId OBJECT-TYPE
       SYNTAX      IpsecDoiSecProtocolId
       MAX-ACCESS  not-accessible
       STATUS      current
       DESCRIPTION
           "The protocol Id for the transforms for this proposal.  The
            protoIsakmp(1) value is not valid for this object.  This
            object, along with the ipiaIpsecPropTransformsName, is the
            index into the ipiaIpsecTransformsTable."
       ::= { ipiaIpsecProposalsEntry 3 }

   I guess that the value zero is also no valid.
   In that case (assuming all other potential values are valid), one 
   could do a
       SYNTAX      IpsecDoiSecProtocolId(2..255)
   So that it is also machine-readable that protoIsakmp is not valid.
   If zero is also a valid value, then one can do
       SYNTAX      IpsecDoiSecProtocolId(0 | 2..255)

  Same for ipiaIpsecTranType
  
19. I see:
     ipiaAutoIkePriority  OBJECT-TYPE
       SYNTAX       Integer32 (0..65535)
    Since this is an INDEX object, I recommend to make it
       SYNTAX       Unsigned (0..65535)
20. For ipiaAutoIkeAddressType I wonder if DNS is a valid
     type. If so, then you MUST describe at what time name
     resolution is to take place. See RFC4001.
     Mmm... I see in the MODULE-COMPLIANCE that only IPv4/v6
     addresses need to be supported. But does that mean that
     dns cannot be supported?

21. When I see
   ipiaAutoIkeSourcePort OBJECT-TYPE
       SYNTAX        InetPortNumber
       MAX-ACCESS    read-create
       STATUS        current
       DESCRIPTION
           "The property ipiaAutoIkeSourcePort specifies the port
            number for the source port for auotstarting IKE SA's.

            The value of 0 for this object is illegal."
       ::= { ipiaAutostartIkeEntry 5 }

   Then I would use a SYNTAX InertPortNumber (1..65545)
   So that the illegal value is also detectable in machine
   readable form.

22. For 
 ipiaAutoIkeProtocol OBJECT-TYPE
     SYNTAX      Unsigned32 (0..255)
     MAX-ACCESS  read-create
     STATUS      current
     DESCRIPTION
         "The property Protocol specifies the protocol number used in
          comparing with policy filter entries and used in any phase
          2 negotiations."
     ::= { ipiaAutostartIkeEntry 8 }

  Are these protocol number maintained by IANA some place maybe?
  If so it woul dbe good to point to it.

23. I see
   ipiaIcmsPolicyStatement OBJECT-TYPE
       SYNTAX      OCTET STRING (SIZE(0..1024))
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "This Value represents the Credential Management Service
            Policy Statement, or a reference describing how to obtain
            it (e.g., a URL).  If one doesn't exist, this value can be
            left blank"
       ::= { ipiaIpsecCredMngServiceEntry 3 }

  I am not sure what a 'blank value' is for an OCTET STRING.
  I could see/assume that it is the zero-length string. 
  If so, better be specific/explicit about it to avoid confusion.
 
  The use of 'empty' and 'blanl' for OCTET STRINGS (or TCs based
  on OCTET STRINGs) occurs many times. Pls check them all

24. So for
ipiaIcmsCredentialName OBJECT-TYPE
    SYNTAX      SnmpAdminString (SIZE(0..32))
    MAX-ACCESS  read-create
    STATUS      current
    DESCRIPTION
        "This value is used as an index into the
         ipiaCredentialFilterTable to look up the actual credential
         value."
    ::= { ipiaIpsecCredMngServiceEntry 5 }

  what does a zero length string mean?
  Pls add that to the DESCRIPTION clause

25. I see:
   ipiaCmcDistributionPoint OBJECT-TYPE
       SYNTAX      OCTET STRING (SIZE(0..256))
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "This Value represents a Distribution Point for a Credential
            Revocation List. It can be relative to the Credential
            Management Service or a full name (URL, e-mail, etc...)."
 
   So I wonder what a zero length string means in this object??

26. I see:
   ipiaCmcThisUpdate OBJECT-TYPE
       SYNTAX      OCTET STRING (SIZE(0..32))
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "This value is the issue date of this CRL. This
            SHOULD be in utctime or generalizedtime."

   It would be good to have a REFERENCE clause to the
utctime/generalized
   time, and how you expect this OCTET-STRING to be formatted?
   This occurs multiple times in this MIB module.

27. Keith had asked:

   - ipiaIpsecCredMngServiceTable 
   - ipiaCredMngCRLTable 

   Please could they have consistent descriptors, i.e.,
   remove "Ipsec" from "ipiaIpsecCredMngServiceTable" 
   so that they are:

   - ipiaCredMngServiceTable 
   - ipiaCredMngCRLTable 

   I think that makes sense. I do not see why the string "Ipsec" 
   is needed in the ipiaIpsecCredMngServiceTable; But maybe I am
   missing something.

28. Keith had suggested (and for the IPSEC-SPD-MIB this have been
    applied) that

   xxxLastChanged OBJECT-TYPE
       SYNTAX      TimeStamp
       MAX-ACCESS  read-only
       STATUS      current
       DESCRIPTION
           "The value of sysUpTime when this row was last modified
            or created either through SNMP SETs or by some other
            external means."

   Would be changed to something like:

   xxxLastChanged OBJECT-TYPE
       SYNTAX      TimeStamp
       MAX-ACCESS  read-only
       STATUS      current
       DESCRIPTION
           "The value of sysUpTime when this row was last modified
            or created either through SNMP SETs or by some other
            external means.

            If this row has not been modified since the last
            re-initialization of the network management subsystem,
            this object SHOULD have a zero value."

   I think that makes sense, and so should be applid to the IKEACTION
   MIB as well. I would even change the SHOULD (you use that capitalize
   2119 termonolo0gy too often). SO I would even say:

   xxxLastChanged OBJECT-TYPE
       SYNTAX      TimeStamp
       MAX-ACCESS  read-only
       STATUS      current
       DESCRIPTION
           "The value of sysUpTime when this row was last modified
            or created either through SNMP SETs or by some other
            external means.

            If this row has not been modified since the last
            re-initialization of the network management subsystem,
            this object has a zero value."

29. Comments from Keith (he did send them to (at least a subset) of
    you all.
    I support these comments; I think I also have some of the
    same comments, either above, or below in my NITS section):

  Substantive Comments:
 
  1. None of the MODULE-COMPLIANCE statements apply to FC-SP, but that's
OK.
  Instead, FC-SP can specify its requirement for compliance to the 
  relevant subsets of both IPSP MIBs in the MODULE-COMPLIANCE statements

  in FC-SP's own MIB modules.  However, in order to do that, the IPSP 
  MIB modules need to define object groups with only the relevant 
  subsets, i.e., not containing other objects.  Specifically,
  
  a) the ipiaCredentialFilterGroup group, which contains the objects 
  from the ipiaIpsecCredMngServiceTable & ipiaCredMngCRLTable tables, 
  also contains objects from the ipiaCredentialFilterTable and 
  ipiaRevokedCertificateTable tables.  I request that the 
  ipiaCredentialFilterGroup be split up so that there is one object 
  group per table.
  
  b) the ipsaSharedGroup group which contains the objects in the 
  ipsaCredentialTable & ipsaCredentialSegmentTable tables also contains

  objects from the ipsaAhTransformTable, ipsaEspTransformTable, 
  ipsaIpcompTransformTable & ipsaPeerIdentityTable tables.  I request 
  that the ipsaSharedGroup be split up so that there is one object group

  per table.
 
  2. T11.5 has discussed the usefulness of a management application 
  being able to use managed objects in these MIB(s) to determine whether

  a CRL is still current, and that the simplest way to determine this is

  to compare the "CRL signature".  To facilitate this, I request that an

  object be added to the ipiaCredMngCRLTable to contain the 
  CRL-signature.  (Note that while such an extension could be defined in

  a FC-SP MIB module, it would seem wrong to do so because the extension

  is not specific to FC-SP.)
 
  3. It is necessary that the DESCRIPTION for the 
  ipiaIcmsPolicyStatement object explain how the value is formatted 
  within the OCTET STRING.
 
  4. A SIZE clause is needed on the SYNTAX of ipsaCredSegValue.

  Clarification/Editorial Comments
 
  5. It is helpful for a MIB module to contain REFERENCE clauses.
  These MIB modules contain REFERENCE clauses only for TC definitions, 
  none for any MIB objects.  To pick just one of many potential 
  examples, shouldn't ipiaIcmsPolicyStatement's definition have a 
  REFERENCE clause pointing to RFC 3280 ??
 
  6. ipiaCredMngCRLEntry's DECSRIPTION needs to specify (explicitly) 
  what is represented by the value of ipiaIcmsName in its INDEX clause.
  
  7. ipiaIpsecCredMngServiceTable's DESCRIPTION says that it is:
 
            "A table of Credential Management Service values.  This
             table is usually used for credential/certificate values
             that are used with a management service (e.g. Certificate
             Authorities)."
 
  it would be clearer if it explained (explicitly) that each row in this

  table represents one Credential Management Service.
  
  8. ipiaCmcDistributionPoint's DESCRIPTION says:
  
            "This Value represents a Distribution Point for a Credential
             Revocation List. It can be relative to the Credential
             Management Service or a full name (URL, e-mail, etc...)."
 
  a) the format needs to be specified,
  b) "it can be relative to ..." needs to be expanded to specify how to 
  determine if, and if so, to which of the cited examples it is
relative.
  c) the DESCRIPTION also needs to explain what it means to have the 
  zero-length string as the value of this object.
 
  9. ipiaCmcNextUpdate's DESCRIPTION says:
 
            "This value indicates the date the next version of this CRL
             will be issued. This SHOULD be in utctime or
             generalizedtime."
 
  a) REFERENCEs are needed for "utctime" and "generalizedtime",
  b) how is it determined which format is being used: utctime or
     generalizedtime?
  c) perhaps, it would be better to use the IETF-standardized
DateAndTime
     as the syntax ??  
  d) similarly for ipiaCmcThisUpdate.
 
  10. Several DESCRIPTIONs use the ambiguous terms: "blank", "null
string"
  and "empty".  One use of "empty" is clarified by appending "(0
length)".
  One use of "null string" is also clarified by appending "(0 length)".
  If "(0 length)" is the intended meaning for all such cases, then *all*

  such usages should be changed to be "zero-length string" (with the 
  words "blank", "null" and "empty" deleted).
 
  11. ipiaIcmsCredentialName's DESCRIPTION defines only how its value is

  used.  From this descripiton of its usage, the definition of what the 
  value actually means has to be infered.  This needsx to be changed so 
  that both the definition of what the value means and how it can be 
  used are explicitly defined.  Further, its syntax allows the value to 
  be the zero-length string which cannot be used in the prescribed
manner.
  Presumably, having the zero-length string as its value is appropriate 
  when the value is not intended to be looked up, as will be the case 
  for FC=SP.  Thus, the DESCRIPTION needs to be expanded to define the 
  meaning of the value and explicitly describe the case where the value 
  is the zero-length string, e.g., similarly to how 
  ipiaIkeIdCredentialName's DESCRIPTION does (but with "blank" changed 
  to "zero-length string".)
 
  12. The DESCRIPTION of ipsaCredMngName says:
 
            "This value is used as an index into the
             ipsaIpsecCredMngServiceTable.  For IDs that have no
             credential management service, this value is left blank."
 
  Each row in this table is (I think) for a "credential" -- it would be 
  helpful to explain the relationship between "IDs" and a row's 
  credential.
 
  13. The DECSRIPTION of ipsaCredRemoteID says:
 
           "This object represents the Identification (e.g. user name)
            of the user of the key information on the remote site.  If
            there is no ID associated with this credential, the value
            of this object SHOULD be the null string."
 
  Is "ID" a reference to "the Identification (e.g. user name) ... on the

  remote site", or is it a reference to the same "IDs" as in
ipsaCredMngName ??
 
  14. "allowble" in ipiaIcmsMaxChainLength's DESCRIPTION has a typo.

  Other Comments
 
  15. ipiaCredFiltAcceptCredFrom's DESCRIPTION says:
 
     ... This value is empty if there is no CA used for this filter.
 
   but its syntax is:
 
      SYNTAX      OCTET STRING(SIZE(1..117))
 
  Assuming the word "empty" will be rephrased as "zero-length string", 
  then the SYNTAX needs to allow a length of zero.

More comments as a result of comments I saw from Keith on the 
draft-ietf-ipsp-spd-mib-xx.txt, some of it has been applied
to rev 7 of that doc, some not. Anyway....

Keith worried about text in sect 4 (SPD MIB, also for sect 4
IKEACTION-MIB):

  This MIB module is a task specific derivation (i.e. an SMIv2
instantiation)
  of the IPCP's IPsec configuration model for use with SNMPv3.

I think it is better to 
   s/for use with SNMPv3/that can be used with SNMP/ 
Your security Considerations section takes care of any
possible use with SNMPv2c or SNMPv1, but certainly 
any future SNMPvx migth be able to support it (NO, I am not 
predicting or advocating an SNMPv4). Also an ISMS based usage
is foreseable. 

You may want to re-check the use of SYNTAX Integer32. According to
RFC4181 in several case use of Unsigned32 is recommended. I don't
think that your use of Integer32 is fatally flawed, but it is better
to follow the guidelines/recommendations.

-------- NITS and admins and questions

a. I see that the textual conventions are prefixed with Ike.
   I think I would prefer them to be prefixed with Ipia (same
   as mib module). But it is just a nit. I can live with it.
b. I see that IpsecDoiSecProtocolId TC does not follow the
   Ike prefix naming. Why is this TC not moved to the
   IPSEC-IPSECACTION-MIB so it sits together with the other 
   OpsecDoiXxx TCs?
c. I do not see why you would use capitalized MAY in the
   DESCRIPTION clause of ipiaCredentialFilterTable. A lower
   case "may" seems more than adequate to me.
d. In general I think the capitalize RFC2119 terms are OVERUSED 
   in various DESCRIPTION clauses.
e. For ipiaIkeActExchangeMode, I guess you could add some text
   (or better a REFERENCE clause to a document) explaining what
   'main' and 'aggressive' mean?
f. Various DESCRIPTION clause are "empty" or useless. For
   example:

   ipiaCredMngCRLEntry OBJECT-TYPE
       SYNTAX      IpiaCredMngCRLEntry
       MAX-ACCESS  not-accessible
       STATUS      current
       DESCRIPTION
           "A row in the ipiaCredMngCRLTable."

   Sure it is a row in this table. Can you say something about what
   sort of information is recorded/managed in such a row?

g. I see no need for the following. They are just empty placeholders
   --
   --
   -- Notification objects information
   --
   --

   ipiaNotificationVariables OBJECT IDENTIFIER ::=
      { ipiaNotificationObjects 1 }

   ipiaNotifications OBJECT IDENTIFIER ::=
      { ipiaNotificationObjects 0 }

   But... they do not harm, so if you want to keep them, fine.
   It just clutters the MIB module (which is big enough anyway).

------------- TYPOs   
  (appology that they are not always in order as they appear
   in the MIB module; I entered them here as I ran into them
   while jumping around to find references/pointers etc.)

  ipiaAutostartIkeTable OBJECT-TYPE
      SYNTAX      SEQUENCE OF IpiaAutostartIkeEntry
      MAX-ACCESS  not-accessible
      STATUS      current
      DESCRIPTION
          "The parameters in the autostart IKE Table are used to
           automatically initiate IKE phaes I and II (i.e. IPsec)

s/phaes/phases/

   ipiaIcmsMaxChainLength OBJECT-TYPE
       SYNTAX      Integer32 (0..255)
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "This value is the maximum length of the chain allowble from

s/allowble/allowable/

   ipiaIKECompliance MODULE-COMPLIANCE
       STATUS      current
       DESCRIPTION
           "The compliance statement for SNMP entities that include an
            IPsec MIB implementation and supports IKE actions.

            -- OBJECT ipiaAutoIkeAddressType
            -- SYNTAX InetAddreessType { ipv4(1), ipv6(2) }

s/InetAddreessType/InetAddressType/

   ipiaIkeActAgressiveModeGroupId OBJECT-TYPE
       SYNTAX           IkeGroupDescription
       MAX-ACCESS       read-create
       STATUS           current
       DESCRIPTION
           "The values to be used for Diffie-Hellman exchange."
       ::= { ipiaIkeActionEntry 5 }

s/values/value/ ?? I think in the instantiation of one object,
it represents only one value, no?

  ipiaIpsecActDoPacketLogging OBJECT-TYPE
      SYNTAX      SpdIPPacketLogging
      MAX-ACCESS  read-create
      STATUS      current
      DESCRIPTION

          "ipiaIpsecActDoPacketLogging specifies whether or not an
           audit message SHOULD be logged and if there is logging, how
           many bytes of the packet to place in the notification."

s/notification/log record/ ??

   ipiaIkeActDoPacketLogging OBJECT-TYPE
       SYNTAX      SpdIPPacketLogging
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "ikeDoPacketLogging specifies whether or not an audit
            message SHOULD be logged and if there is logging, how many
            bytes of the packet to place in the notification."

s/notification/log record/ ??

Bert

---

From: "Wijnen, Bert \(Bert\)" <[email-masked]>
Date: Thu, 4 Jan 2007 18:06:56 +0100

One more comment 

            -- OBJECT ipiaAutoIkeAddressType
            -- SYNTAX InetAddreessType { ipv4(1), ipv6(2) }
            -- DESCRIPTION
            -- Only support for global IPv4 and IPv6 address
            -- types is required.
            --
            -- OBJECT ipiaAutoIkeSourceAddress
            -- SYNTAX InetAddress (SIZE(4|16))
            -- DESCRIPTION
            -- Only support for global IPv4 and IPv6 address
            -- types is required.
            -- OBJECT ipiaAutoIkeDestAddress
            -- SYNTAX InetAddress (SIZE(4|16))
            -- DESCRIPTION
            -- Only support for global IPv4 and IPv6 address
            -- types is required.
            --"

  The above notation is done for INDEX objects (they cannot be
   lisyed in a real OBJECT clause, and so we put them as commnets
   in a DESCRIPTION clasue. For these objects though you can
   (and should use) real OBJECT clauses, not just comments.

Bert

> -----Original Message-----
> From: Wijnen, Bert (Bert) 
> Sent: donderdag 4 januari 2007 16:05
> To: [email-masked]; [email-masked]; 
> [email-masked]; [email-masked]; [email-masked]
> Cc: Russ Housley; [email-masked]; MIB Doctors 
> \\\(E-mail\\\); Keith McCloghrie
> Subject: MIB Doctor review: mib content: 
> draft-ietf-ipsp-ikeaction-mib-02.txt
> 
> I have included Keith's comments (some are generic, some are 
> specific T11 or IETF IMSS WG requirements and desires, yet I 
> think they make a lot of sense) as well, so you have a complete set. 
> 
> Also, Keith and I are not (yet) in complete agreement on some 
> aspects of MIB modules. I know that Keith has written to you 
> about (for example) the IPSEC-SPD-MIB after I had done MIB 
> doctor (and I guess at the time OPS AD review).
> module/document. So I have done a review with a SNMP/MIB 
> view, but I have not (and cannot) review it from a 
> security-point of view. In fact, after having spend more than 
> a day on this MIB module, I cannot say that I really 
> understand it all.
> 
> review and check if this indeed correctly models the 
> management information as intended (or maybe already have done that). 
> The MIB documents are nearly clean (from a SYNTAX point of 
> view) and also many of the issues I had brought up in the 
> review if the IPSEC-SPD-MIB and also were rpesent in an 
> earlier version of this MIB document, have been resolved.
>    since in that table I see no column that has a similar 
> SYNTAX. Well, I
>    These two objects refer to the ipiaIkeIdentityTable, which 
> I cannot find.
>    I do find a ipiaIkeIdentityTable (which I don't think is 
> about PEERs?)
>    and a ipsaPeerIdentityTable (in the 
> IPSEC-IPSECACTION-MIB). Possibly the
>   1. None of the MODULE-COMPLIANCE statements apply to FC-SP, 
> but that's OK.
>   determine if, and if so, to which of the cited examples it 
> is relative.
>   10. Several DESCRIPTIONs use the ambiguous terms: "blank", 
> "null string"
>   and "empty".  One use of "empty" is clarified by appending 
> "(0 length)".
>   be the zero-length string which cannot be used in the 
> prescribed manner.
>   This MIB module is a task specific derivation (i.e. an 
> SMIv2 instantiation)

---

From: "Wijnen, Bert \(Bert\)" <[email-masked]>
Date: Tue, 9 Jan 2007 22:52:07 +0100

[I have copied Keith, because I understand he did partially review this
MIB module].

I have included Keith's comments in the review posting for the
ikeaction-mib.
Pls check there.

Also, Keith and I are not (yet) in complete agreement on some aspects of
MIB modules. I know that Keith has written to you about (for example)
the IPSEC-SPD-MIB after I had done MIB doctor (and I guess at the time
OPS AD review).
These are the two issues:

Please seem my comment on ikeaction-mib for details.
The point is that I again have to warn that I may change my
earlier advise once this discussion closes on MIB doctors team.

General remark
I am not well enough versed/skilled in the security protocols and
mechanisms/algorithms that are being modeled in this MIB
module/document. So I have done a review with a SNMP/MIB view, but I
have not (and cannot) review it from a security-point of view. In fact,
after having spend more than a day on this MIB module, I cannot say that
I really understand it all.

So I am assuming that real security folk will do the securiy review and
check if this indeed correctly models the management information as
intended (or maybe already have done that). 
Also the grouping of Objects and the two MODULE-COMPLIANCE statements
are outside my competency to determine if they make sense or not.

The MIB documents are nearly clean (from a SYNTAX point of view) and
also many of the issues I had brought up in the review if the
IPSEC-SPD-MIB and also were rpesent in an earlier version of this MIB
document, have been resolved.
So a more detailed review made sense this time. Lots of comments and
questions still remain though.

Here are my comments on the IPSEC-IPSECACTION-MIB module.

1. I think this one looks better and is in better shape than the
   ikeaction-mib. This is good.

2. Several of my comments in the ikeaction-mib review apply,
   examples are:
   - use of "empty", "blank" or "NULL" for OCTET STRINGS
     you probably mean "zero-length string".
   - Use of capitalized terms from RFC2119 seem not
     always appropriate.
   - Use more REFERENCE clauses to point to 
     supporting/explaing/background documents
   - In various places where you use a SnmpAdminString TC
     with a SIZE (0..xx), pls make sure that you explain what
     the zero-length string means. Also make sure that a 
     zero-length string is always valid, if not, pls change
     the SIZE spec.
   - empty placeholders for NotificationVariables and Notifications.
   - possibly others, pls check   

2. I see
   ipsaSaPreActActionLifetimeSec OBJECT-TYPE
       SYNTAX      Unsigned32
       UNITS       "seconds"
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "ipsaSaPreActActionLifetimeSec specifies how long in seconds
            the security association derived from this action is used.

            The default lifetime is 8 hours.

            Note: the actual lifetime of the preconfigured SA will be
            the lesser of the value of this object and of the value of
            the MaxLifetimeSecs property of the associated transform.

            A value of 0 indicates no time limit on the lifetime
            of the SA."
       DEFVAL      { 28800 }

   Which is good, because in a similar object in IKEACTION MIB, you used
   the value of zero to mean the default of 8 hours. I have suggested
   in my IPSEC-IKEACTION-MIB review that you use 28800 as the DEFAULT.
   Whatever you do, pls be consistent. And the spec here looks better to
   me than in IPSEC-IKEACTION-MIB.

3. ipsaSaPreActDFHandling
   You also have a DFHandling in ikeaction-mib.
   Maybe better to use a common TC ??

4. in 
   ipsaSaPreActESPTransformName OBJECT-TYPE
       SYNTAX      SnmpAdminString (SIZE(0..32))
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "This object is the name of the ESP transform to use as an
            index into the ESPTransformTable.  A zero length value
            indicates no transform of this type is used."
       ::= { ipsaSaPreconfiguredActionEntry 14 }

   I see mention of ESPTransformTable. I cannot find that table.
   You probably/possibly mean ipsaEspTransformTable

5. ipsaSaPreActESPEncSecretName and ipsaSaPreActESPAuthSecretName
   claim to be usable as an index into ipsaCredentialTable.
   So Does a zero length string value mean there is no such info?
   Pls explain in DESCRIPTION clauses. 

6. For
     ipsaSaPreActIPCompSPI OBJECT-TYPE
         SYNTAX      Integer32
   I wonder of an SPI is not better represented as an Unsigned32.
   After all, the RFCs claim that SPI is an arbitrary 32bit value.
7. For these 2 objects:
     ipsaAhTranMaxLifetimeSec OBJECT-TYPE
      SYNTAX      Unsigned32
      UNITS       "seconds"
      MAX-ACCESS  read-create
      STATUS      current
      DESCRIPTION
          "ipsaAhTranMaxLifetimeSec specifies how long in seconds the
           security association derived from this transform SHOULD be
           used.

           A value of 0 indicates that the default lifetime of
           8 hours SHOULD be used."
      ::= { ipsaAhTransformEntry 2 }

     ipsaAhTranMaxLifetimeKB OBJECT-TYPE
      SYNTAX      Unsigned32
      MAX-ACCESS  read-create

      STATUS      current
      DESCRIPTION
          "ipsaAhTranMaxLifetimeKB specifies how long in kilobytes the
           security association derived from this transform SHOULD be
           used."
      ::= { ipsaAhTransformEntry 3 }

  I really get confused by the DESCRIPTION clause vs the descriptor
  (object name). 
  The name seem to indicate these are "maximum" life times.
  The DESCRIPTION (specifically since it uses capitalized SHOULD) 
  seems to tell me that they mmust be used at least this long (or
  may be exactly this long) ??
  I will admit that  English is not my native languange.

  The following is better in this respect:

    ipsaEspTranMaxLifetimeKB OBJECT-TYPE
       SYNTAX      Unsigned32
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "ipsaEspTranMaxLifetimeKB specifies how long in kilobytes
            the security association derived from this transform is
            used."
       ::= { ipsaEspTransformEntry 3 }

   However now it seems to say (in the DESCRIPTION clause) it is to
   be exactly this value, where as I think you mean max so many
kilobytes.

8. Is this inconsistency (w.r.t. DEFVAL and/or default and the meaning
   of zero) done on purpose:

     ipsaSaPreActActionLifetimeSec

             .. snip ..

            The default lifetime is 8 hours.

             .. snip ..

            A value of 0 indicates no time limit on the lifetime
            of the SA."
       DEFVAL      { 28800 }

     ipsaAhTranMaxLifetimeSec OBJECT-TYPE

            .. snip ..

     ipsaEspTranMaxLifetimeSec OBJECT-TYPE

            .. snip ..

     ipsaIpcompTranMaxLifetimeSec OBJECT-TYPE
   
            .. snip ..

9. IN 
   ipsaEspTranCipherKeyLength OBJECT-TYPE
       SYNTAX      Unsigned32
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "This object specifies, in bits, the key length for
            the ESP cipher algorithm."

   I wonder if the value can really be zero. If so, what does that mean?
   And can it be weak key lengths like 1,2,3 and such?

10. ipsaEspTranReplayWindowSize 
   Is a zero value valid? I guess in case    ipsaEspTranReplayPrevention
   is 'false' that a zero value would be OK. Bit what does it mean
   (or is it allowed) when 'true' ??

   Same for ipsaAhTranReplayWindowSize (and any other xxxWindowSize)

11. In
      ipsaSaPreActAHSPI OBJECT-TYPE
        SYNTAX      Integer32
        MAX-ACCESS  read-create
        STATUS      current
        DESCRIPTION
          "This object represents the SPI value for the AH SA."

    And in other places where an SPI is defined, I wonder if
    Integer32 is the best representations.
    RFC4301 states:

      Security Parameters Index (SPI)

      An arbitrary 32-bit value that is used by a receiver to identify
      the SA to which an incoming packet should be bound.  For a unicast
      SA, the SPI can be used by itself to specify an SA, or it may be
      used in conjunction with the IPsec protocol type.  Additional IP
      address information is used to identify multicast SAs.  The SPI is
      carried in AH and ESP protocols to enable the receiving system to
      select the SA under which a received packet will be processed.  An
      SPI has only local significance, as defined by the creator of the
      SA (usually the receiver of the packet carrying the SPI); thus an
      SPI is generally viewed as an opaque bit string.  However, the
      creator of an SA may choose to interpret the bits in an SPI to
      facilitate local processing.

    So I'd say that an Unsigned32 is probably better. Or maybe even 
    an OCTET STRING SIZE(4). Not a fatal flaw though to use Interger32.
    I also found (based on a ptr from Sam Hartman) that in appendix
    C of RFC4301 it seems to be defined as an interger as well. Oh well.

11 Things about LiftimeSecs, LifetimeKB, KeyLength, and such of course
apply
   to all occurences in all tables.

12. When I read

   ipsaCredRemoteID OBJECT-TYPE
       SYNTAX      OCTET STRING(SIZE(0..256))
       MAX-ACCESS  read-create
       STATUS      current
       DESCRIPTION
           "This object represents the Identification (e.g. user name)
            of the user of the key information on the remote site.  If
            there is no ID associated with this credential, the value
            of this object SHOULD be the null string."

   Then I get the impression that this object is intended for human
   consumption (or representing humans). I'd think that a UTF-8 based
   string would be better so that a user name can be properly
represented
   also for non US-ASCII names.

13.
   ipsaCredSegIndex OBJECT-TYPE
       SYNTAX      Integer32 (1..65535)
       MAX-ACCESS  not-accessible
       STATUS      current

   RFC4181 strongly RECOMMENDs to use Unsigned32 for INDEX objects.

14.
   ipsaCredSegStorageType OBJECT-TYPE
       SYNTAX      StorageType
       MAX-ACCESS  read-only
       STATUS      current
       DESCRIPTION
           "The storage type for this row.  This object is
            read-only. Rows in this table have the same value as the
            ipsaCrendStorageType for the corresponding row in the
            ipsaCredentialTable.

            For a storage type of permanent, none of the columns have
            to be writable."
       DEFVAL { nonVolatile }
       ::= { ipsaCredentialSegmentEntry 4 }

  I suggest one could get rid of this object and just state the
persistency
  behaviour in the DESCRIPTION clause of ipsaCredentialSegmentEntry

15. I think I already mentioned in my IPSEC-IKEACTION-MIB review...
  I get completely confused by the 3 TCs:

    IpsaIdentityFilter, IpsaCredentialType and IpsecDoiIdentType

  I got confused again when I compared and tried to understand how these
  are used in the ipsaCredentialTable and ipsaPeerIdentityTable.
  Are those usesnon-consistent? If they are intended as used/specified, 
  pls try to help me clear my confusion.

16.

  ipsaNotificationVariables OBJECT IDENTIFIER ::=
     { ipsaNotificationObjects 1 }

  ipsaNotifications OBJECT IDENTIFIER ::=
     { ipsaNotificationObjects 0 }

  Seems empty and useless. Can be deleted as far as I am concerned.
  However, not a fatal flaw or serious error.
  I find Notifications to be defined under NotificationObjects
  somehwat strange as well.

17. MODULE-COMPLIANCE
  I see:

           -- OBJECT ipsaPeerIdAddressType
           -- SYNTAX InetAddressType { ipv4(1), ipv6(2) }
           -- DESCRIPTION
           -- Only support for global IPv4 and IPv6 address
           -- types is required.
           --
           -- OBJECT ipsaPeerIdAddress
           -- SYNTAX InetAddress (SIZE(4|16))
           -- DESCRIPTION
           -- Only support for global IPv4 and IPv6 address
           -- types is required.
           --"
   The above notation is done for INDEX objects (they cannot be
   lisyed in a real OBJECT clause, and so we put them as commnets
   in a DESCRIPTION clasue. For these objects though you can
   (and should use) real OBJECT clauses, not just comments.

   I now notice that this is also the case for the IKEACTION-MIB.
   I have added that comment to my IKEACTION-MIB review.

Bert