[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: MIB/PIB doctor review for draft-ietf-ipsp-ipsecpib-09.txt - part 2
Hi Bert,
Thanks for your comments and no need to worry since I am working on the draft. Will submit a new version when I finish.
Best regards
Man Li
-----Original Message-----
From: ext Wijnen, Bert (Bert) [mailto:bwijnen@xxxxxxxxxx]
Sent: Friday, March 26, 2004 8:59 AM
To: Li Man.M (Nokia-NRC/Boston)
Cc: 'smb@xxxxxxxxxxxxxxxx'; 'ho@xxxxxxxxxxxx'; 'lsanchez@xxxxxxxxxxx';
'avri@xxxxxxxxxxxxxx'; 'ipsec-policy@xxxxxxxx'
Subject: RE: MIB/PIB doctor review for draft-ietf-ipsp-ipsecpib-09.txt -
part 2
I have to be very careful here (cause it took me many months
before I finished my review; for which I again appologize).
But once I have done the review, these things are kind of "fresh" in
my mind/memory. If it takes a long time before you get to respond or
send a new revision... things get swapped out, sometimes into a
sort of off-site store...
Thanks,
Bert
> -----Original Message-----
> From: Wijnen, Bert (Bert)
> Sent: dinsdag 16 maart 2004 15:49
> To: Man.M.Li@xxxxxxxxx; bwijnen@xxxxxxxxxx
> Cc: smb@xxxxxxxxxxxxxxxx; ho@xxxxxxxxxxxx; lsanchez@xxxxxxxxxxx;
> avri@xxxxxxxxxxxxxx; ipsec-policy@xxxxxxxx
> Subject: MIB/PIB doctor review for draft-ietf-ipsp-ipsecpib-09.txt -
> part 2
>
>
> Steve, you may want to pay specific attention to items 34-37.
>
> Addtional things I found when revieing revision 9
>
> Comments 1-8 were in part 1 email of the review
>
> 9. Doing s smilint syntax check, I get the following:
>
> C:\smi\pibs\ietf>smilint -l 6 -s -m -inamelength-32
> ./IPSEC-POLICY-PIB
> /IPSEC-POLICY-PIB:83: [5] {type-without-format} warning: type
> `Unsigned16TC' has no format specification
> simple to fix by adding a DISPLAY-HINT
>
> /IPSEC-POLICY-PIB:19: [5] {import-unused} warning: identifier
> `zeroDotZero' imported from module `SNMPv2-SMI's never used
>
> simple to fix by removing zeroDotZero from the imports
>
> 10. I see a lot of acronyms in the document without expansion
> (when used
> for the first time. These maybe well know to security
> folk, but not
> to new readers. RFC-Editor wants them to be exanded first
> time they
> are used. Pls note that when PIB module is extracted,
> previous text is
> gone, and so it is probably wise to always expand in PIB
> DESCRIPTION
> clauses.
>
> 11. All RFCs that contain PIB or MIB modules from which you
> IMPORT must
> be normatively referenced. The RFC-Editor recently also
> wanted such
> references to have citations. The notatation we agreed on is
>
> IMPORTS
> Unsigned32, Unsigned64, MODULE-IDENTITY,
> OBJECT-TYPE, TEXTUAL-CONVENTION, MODULE-COMPLIANCE,
> OBJECT-GROUP, pib
> FROM COPS-PR-SPPI -- [RFC3159]
> TruthValue
> FROM SNMPv2-TC -- [RFC2579]
>
> and so on. But.. your references seem to be just numbered.
> I Do not find
>
> OBJECT-GROUP, pib
> FROM COPS-PR-SPPI -- [14]
>
> very acceptable. This is because when the PIB module gest
> extracted,
> then a "citation" of [14] does not help much. A Citation
> of [RFC3159]
> would still be clear for the reader (or so I think).
>
> 12. I see various ENUMERATIONS (e.g. INTEGERS) that probably would be
> better represented as a TC.
> For example:
>
> SYNTAX INTEGER {
> in(1),
> out(2),
> bi-directional(3)
> }
>
> and also
> SYNTAX INTEGER {
> copy(1),
> set(2),
> clear(3)
> }
> there are others. Those are just 2 examples.
>
> 13.I wonder if this is wise:
>
> Unsigned16TC ::= TEXTUAL-CONVENTION
> STATUS current
> DESCRIPTION
> "An unsigned 16 bit integer."
> SYNTAX Unsigned32 (0..65535)
>
> In some places its use seems OK. In many places, it is used
> to indicate an "order" as in:
>
> ipSecRuleOrder OBJECT-TYPE
> SYNTAX Unsigned16TC
> STATUS current
> DESCRIPTION
> "Specifies the precedence order of the rule within
> all the rules
> associated with {IfName, Roles}. A smaller value indicates a
> higher precedence order. "
> ::= { ipSecRuleEntry 9 }
>
> I wonder if it would not be better to define a TC aka:
>
> IpSecOrderTC ::= TEXTUAL-CONVENTION
> STATUS current
> DESCRIPTION "An unsigned 16 bit integer that
> defines the order
> of a Rule or Set. A smaller value indicates a
> higher precedence order."
> SYNTAX Unsigned32 (0..65535)
>
> And then use that TC for all "order" objects.
>
> In the case of
> ipSecAssociationDhGroup OBJECT-TYPE
> SYNTAX Unsigned16TC
>
> I also wonder if a IpSecDhGroupTC would not be a wise thing.
>
>
> 14.I see citations in DESCRIPTION clauses, as in:
>
> ipSecRuleIfName OBJECT-TYPE
> SYNTAX SnmpAdminString
> STATUS current
> DESCRIPTION
> "The interface capability set to which this IPsec rule applies.
> The interface capability name specified by this attribute MUST
> exist in the frwkCapabilitySetTable [9] prior to
> association with
> an instance of this class."
> ::= { ipSecRuleEntry 2 }
>
> The [9] of course means nothing anymore when the PIB module gets
> extracted. If you used a notation ala [RFCxxxx] then it would still
> mean something in an extracted module.
>
> 15.In the ipSecXxxTable DESCRIPTION clauses you use text aka:
> "This table ...."
> In all other PIB modules that the IETF has published sofar, we did
> not talk about a table but either of a PRC (Policy Rule Class) or
> just class. See the DIFFSERV-PIB and the FRAMEWORK-PIB for example.
> I think that changing this so as to be consistent is a GOOD THING.
>
> 16.In the DESCRIPTION clause of:
> ipSecActionSetDoPacketLogging OBJECT-TYPE
> SYNTAX TruthValue
> STATUS current
> DESCRIPTION
> "Specifies whether to log when the resulting security
> association
> is used to process a packet. For ipSecStaticActions, a
> log message
> is to be generated when the IPsecBypass, IpsecDiscard
> or IKEReject
> actions are executed."
> ::= { ipSecActionSetEntry 5 }
>
> I wonder if "IPsecBypass, IpsecDiscard or IKEReject" are meant to
> mean the same thing as the enumerations in:
>
> ipSecStaticActionAction OBJECT-TYPE
> SYNTAX INTEGER {
> byPass(1),
> discard(2),
> ikeRejection(3),
> preConfiguredTransport(4),
> preConfiguredTunnel(5)
> }
> STATUS current
>
> If so, then you may want to sync up the labels.
>
> 17.I see:
> ipSecAssociationUseKeyExchangeGroup OBJECT-TYPE
> SYNTAX TruthValue
> STATUS current
> DESCRIPTION
> "Specifies whether or not to use the same GroupId for
> phase 2 as
> was used in phase 1. If UsePFS is false, then this
> attribute is
> ignored.
>
> A value of true indicates that the phase 2 GroupId
> should be the
> same as phase 1. A value of false indicates that the
> group number
> specified by the ipSecSecurityAssociationDhGroup
> attribute SHALL
> be used for phase 2. "
> ::= { ipSecAssociationEntry 7 }
>
> And wonder if ipSecSecurityAssociationDhGroup is mean to point to:
>
> ipSecAssociationDhGroup OBJECT-TYPE
> SYNTAX Unsigned16TC
> STATUS current
> DESCRIPTION
> "Specifies the key exchange group to use for phase 2 when the
> property ipSecSecurityAssociationUsePfs is true and
> the property
> ipSecSecurityAssociationUseKeyExchangeGroup is false."
> ::= { ipSecAssociationEntry 8 }
>
> If so, then better sync up the descriptors.
> If not, then where is ipSecSecurityAssociationDhGroup defined?
> By the way, in the last object, there are also a few descriptors
> that start with ipSecSecurityAssociation.... and that I
> cannot find.
> Again, probably s/Security// ??
>
> 18.I see:
> ipSecAhTransformReplayPreventionWindowSize OBJECT-TYPE
> SYNTAX Unsigned32
> STATUS current
> DESCRIPTION
> "Specifies, in bits, the length of the sliding window
> used by the
> replay prevention detection mechanism. The value of
> this property
> is ignored if UseReplayPrevention is false. It is
> assumed that the
> window size will be power of 2."
> ::= { ipSecAhTransformEntry 5 }
> Might be good to add a UNITS clause?
> But I also wonder what "window size will be power of 2" means?
>
> 19.ipSecEspTransformCipherKeyLength,
> ipSecEspTransformReplayPreventionWindowSize
> and may be others can also use a UNITS clause
>
> 20.In Table/Class ipSecCompTransformTable I see
> ipSecCompTransformAlgorithm
> and also ipSecCompTransformPrivateAlgorithm. Canthey both
> be specified
> for the same instance? If so, what happens? If not, can you make
> that clear? If yes, you may want to explain also what happens.
>
> 21.ipSecIkeRuleTable is probably talking about IKEv1 ??
> Should that be made clear?
> What sort of change do we expect for IKEv2?
> WOuld it make sense to define this "optional" class in a separate
> document?
> In any event, you do not put in the DESCRIPTION clause
> that this table/class
> is optional. That is expressed (and even in machine
> readable form) in the
> MODULE-COMPLIACNE.
>
> 22.I am really wondering and worried about some of the
> UNIQUENESS clauses
> that require many columns/attributes to define uniqueness.
> But ipSecIkeAssociationEntry really seems excessive to me.
> It states
> that ALL attributes need to be checked for unqiueness !!???
> Is that really intended?
> You may want to check all tables/classes in fact.
>
> 23.ipSecIkeAssociationVendorId is an OCTET STRING and the
> descripiton clause
> talks about a vlaue of NULL. That is not clear! Better is
> to talk about
> a zero length OCTET STRING. That is clear!
>
> 24.I see:
> ipSecIkeProposalPrfAlgorithm OBJECT-TYPE
> SYNTAX Unsigned16TC
> STATUS current
> DESCRIPTION
> "Specifies the Psuedo-Random Function (PRF) to propose
> for the IKE
> association."
> ::= { ipSecIkeProposalEntry 7 }
> So where do I find what any value means?
> And would it not be better to have a separate TC fo such algorithm
> values? And pls add REFERENCE clause to point to it.
>
> 24.Similar question for ipSecIkeProposalIkeDhGroup
> And pls add REFERENCE clause to point to it.
>
> 25.IN ipSecIkePeerEndpointIdentityValue you are using IP addresses as
> example. But you must use the ange of addresses that has been
> allocated for examples as per ID-NITs
> http://www.ietf.org/ID-nits.html
> which states:
>
> Addresses used in examples should prefer use of fully
> qualified
> domain names to literal IP addresses, and prefer use
> of example
> fqdn's such as foo.example.com to real-world fqdn's.
> See RFC 2606
> for example domain names that can be used. There is
> also a range
> of IP addresses set aside for this purpose. These are
> 192.0.2.0/24
> (see RFC 3330). Private addressess that would be used
> in the real
> world should be avoided in examples.
>
> 26.Object ipSecCredentialFieldsValue is defined as an OCTET STRING.
> However, reading the DESCRIPTION clause, it feels as if this is
> a string that will be used/read by human beings. In that case, it
> must be a UTF-8 string. Possibly a SnmpAdminString or some other
> TC that defines UTF-8 text.
>
> 27.When I read the ipSecSelectorTable and related tables,
> then I really
> wonder if you guys have really evaluated the possible re-use of the
> frwkIpFilterTable in the FRAMEWORK-PIB !!??
> I thought that that framework filter table was meant to be re-used
> by many (if not all) filter/selector uses.
>
> 28.I have trouble understanding the ipSecIpsoFilterSetTable
> I bet it is cause by the very short DESCRIPTION clauses that do not
> mean much to me (and so probably do not mean much to other
> non-expert
> readers).
>
> 29.When I see ipSecRuleTimePeriodTimePeriod defined as an OCTET STRING
> then I wonder. It claims to be RFC2445 format (as RFC3060 also
> prescribes). There they talk about a string.
> I think it is US-ASCII, possibly represented as UTF-8 ??
> I need to go and check to be sure. In any event, if you
> say OCTET STRING
> and specify that THISANDPRIOR has special meaning, then it
> needs to be
> clear what encoding needs to be used for that text.
> A SIZE specification may also make sense (or so I think).
> This is a generic Policy construct, which probably even
> warrants a TC.
>
> 30.I do not understand
> ipSecRuleTimePeriodMonthOfYearMask OBJECT-TYPE
> SYNTAX OCTET STRING
> STATUS current
> DESCRIPTION
> "An octet string that specifies which months the policy is valid
> for. The octet string is structured as follows:
>
> - a 4-octet length field, indicating the length of the entire
> octet string; this field is always set to 0x00000006 for this
> property;
>
> - a 2-octet field consisting of 12 bits identifying
> the 12 months
> of the year, beginning with January and ending with December,
> followed by 4 bits that are always set to '0'. For
> each month,
> the value '1' indicates that the policy is valid for
> that month,
> and the value '0' indicates that it is not valid.
>
> If this property is omitted, then the policy rule is treated as
> valid for all twelve months."
> ::= { ipSecRuleTimePeriodEntry 3 }
>
> My questions/thinking:
> - if indeed the length is ALWAYS 6, with only 2 octets of
> information,
> then why not define it as: OCTET STRING (SIZE(2))
> - But since you need to represent BITS, I wonder why you do not
> use the BITS construct aka:
> ipSecRuleTimePeriodMonthOfYearMask OBJECT-TYPE
> SYNTAX BITS {
> january(0),
> february(1),
> march(2),
> ...
> december(11)
> }
> In fact I would make it a TC. It is a generic construct for Policy
> information. As you probably know, BITS does translate to an
> octet string of the proper size on the wire.
>
> 31.Similar questions for ipSecRuleTimePeriodDayOfMonthMask.
> ipSecRuleTimePeriodDayOfWeekMask I have similar questions
>
> 32.For ipSecRuleTimePeriodTimeOfDayMask I ahve similar questions as
> in point 29 above
>
> 33.ipSecRuleTimePeriodLocalOrUtcTime
> I can live with it. It does not always result in a 16bit unint
> as per RFC3060 sect 6.5.6, but can easily be converted to that.
> Agin, it would be good if defined as a TC.
>
> 34.The reason I suggest to define TCs for points 29-33, is that the
> same TCs can/should be used by the MIB defintion. Since the MIB
> wants to (and can) go stds track, there probably should be a
> separate POLICY-TC-MIB module that defines those TCs.
>
> 35.In fact, the whole TimePeriod... stuff seems so generic to any
> policy, that I would think it would also be in a separate module
> so that it can be re-used much easier.
> In fact the RFC3060 (and possibly RFC3460) should be separate
> modules that can be re-used by technology specific specifications
> and/or modules. I thought that that was the whole concept/idea of
> doing a PCIM and PCIMe. They would be CORE definitions upon which
> technology specific definitions (like IPsec and QoS) could/would
> build. Have we completely lost this idea/concept?
>
> 36.If I see it correctly, then every table has its own OBJECT-GROUP
> with a description clause of "Objects from the xxxTable."
> Is that the most logical grouping? I doubt it very much.
> Has any thought gone into this?
>
> 37.I will leave it to your security ADs to tell you about the security
> considerations section. But it seems to me that it is inadequate.
>
> 38.RFC-Editor considerations is incorrect. There are many more
> normative references. In any event, [9] has been published as
> RFC3318, as you indicate yoruself in ref section. ANdf ref [12]
> has been published as RFC3385, as you can see on your WG
> charter page.
>
> 39.IANA Considerations section
> - the 1st para is fine, except I would add a ptr to the
> iana web page
> that has the registry. Makes it easier for everyone
> - the 2nd para need to be formulated in the same way as 1st para.
> So that it is also readable after IAN has done the assignment.
> Pls also add ptr to the cirrect iana web page.
>
> Since this will result in a new revision, I propose to also add the
> proper IPR statements as per the new RFCs 3667/8/9
>
> Bert
>