Subject: [Isms] review of draft-ietf-isms-transport-security-model-00.txt

From: Juergen Schoenwaelder <[email-masked]>
Date: Sun, 5 Nov 2006 01:11:09 +0100

I have reviewed <draft-ietf-isms-transport-security-model-00.txt> and
below are my comments (as a technical contributor).

/js

01: Page 5: "self- contained" -> "self-contained"

02: Page 5: "User Security Model" -> "User-based Security Model"

03: Page 6: [todo]: I think this [todo] can simply be removed.

04: Page 7: ", s the" -> ", as the"

05: Page 8: "secruity" -> "security"

06: Page 8: I am not sure how useful it will ever be to run USM over
    SSH (second bullet). If we like to discuss such cases, it might be
    worth to note, however, that in such a configuration the
    securityName would be derived from the USM userName and the
    securityName provided by the secure Transport Model would be
    ignored.

07: Page 8/9: [discuss] I do not think that the secure transport model
    must determine the security model. I do not think we want this
    binding and "overwrite" the msgSecurityModel field in the SNMPv3
    message. However, the transport security model must check that the
    transport actually did provide an appropriate tmStateReference and
    drop any messages where this is not the case (so that plain SNMP
    over TCP messages do not go pass the transport security model).

07: Page 8: The more interesting case in term of coexistance that IMHO
    should be mentioned in section 2.3. is the combination of a secure
    transport model with a community-based security model [RFC 3584].
    It should be made clear that in this combination you won't get an
    authenticated securityName since the community-based security
    model derives the securityName from the community string which is
    not authenticated.

08: Page 9: "WholeMsg" -> "wholeMsg"

09: Page 10: It should probably be noted somewhere that the fact that
    there is only a single Transport Security Model for potentially
    many secure transports implies that you cannot have different
    access control rules for say SNMPv3/TMS/SSH and SNMPv3/TMS/TLS
    since the isAccessAllowed primitive only takes the securityModel
    as input but not the transport model. I am not saying this is a
    problem - just that we should be clear about this.

10: Page 11: "how Transport" -> "how the Transport"

11: Page 12: ". if the" -> ". If the"

12: Page 13, section 3.1.3: Why do we not simply state that the
    msgSecurityParameters are a zero-length OCTET STRING? Section 6.6
    of RFC 3412 states that this field is opaque from the message
    processor's point of view and just passed to the security model.
    Since the Transport Security Model does not have any data to put
    there, we should IMHO just set this field to a zero-length OCTET
    STRING. Defining and ASN.1 SEQUENCE which contains a zero-length
    OCTET STRING and serializing this using BER and putting the result
    (0x10020400) there does not really add any value.

13: Page 14: "to imatch" -> "to match"

14: Page 15: I do not understand the text about the LCD, the MIB
    module and lookups. In fact, I do not see why there is a MIB with
    a user table in the first place (see below). I believe this text
    is not needed an can be discarded.

15: Page 16: Is the discussion of the prepareOutgoingMessage() ASI
    needed? This seems to be unchanged from RFC 3411.

16: Page 17: The text states that the Transport Subsystem
    architectural extension did add parameters to the
    generateRequestMsg() and generateResponseMsg() ASIs. I checked the
    ID and could not find this. So this needs to be added to the
    architectural extension document together with a discussion why
    the new parameters are needed.

    <soap>
      The community-based security model has the capability to filter
      messages based on the IP addresses. It seems this never really
      worked according to the architecture because the ASIs never
      passed the transport address to the security model. So it seems
      that this work actually makes the community-based security fit
      into the architecture. ;-)
    </soap>

17: Page 18: It would be nice if the elements of procedure could
    actually be highlighted by putting them into a subsubsection.

18: Page 18: "Subystem" -> "Subsystem"

19: Page 19: Item 4): Fill the securityParameters with a zero-length
    OCTET STRING (see 12 above).

20: Page 19: It seem subsection 4.3 is empty.

21: Page 19: The subsection structure is confusing. It seems that the
    section header "4.4. Prepare ..." should be dropped in order to be
    consistent.

22: Page 20: "is::" -> "is:"

23: Page 20: It would be nice if the elements of procedure could
    actually be highlighted by putting them into a subsubsection.

24: Page 20: Replace 

     1) If the received securityParameters is not the serialization of an
     OCTET STRING formatted according to the transportSecurityParameters,
     and the contained OCTET STRING is not empty, then the
     snmpInASNParseErrs counter [RFC3418] is incremented, and an error
     indication (parseError) is returned to the calling module.

    with:

     1) If the received securityParameters is not a zero length
     string, then the snmpInASNParseErrs counter [RFC3418] is
     incremented, and an error indication (parseError) is returned to
     the calling module.

    (according to 12 above).

25: Page 20: I do not understand step 2). If you want to check that
    the security level reported via the tmStateReference is at least
    as good as the securityLevel indicated in the message, just do it
    and discard the packet otherwise. I do not understand the text
    about securityModel and securityName. SecurityName is an OUT
    parameter and the securityModel was used to call us; was your idea
    to check for "surprising" combinations of security models with
    transport models? I believe we should not do this.

26: Page 20: Before step 3) (mistakenly called 2) again), we have to
    make sure a valid tmStateReference has been provided. If it is
    missing (e.g. the packet was received via a plain TCP/UDP
    transport), then we should discard the packet and increment a
    suitable counter.

27: Page 21: The transportStats subtree is empty - drop 5.3. The
    transportState subtree is empty - drop 5.4.

28: Page 22: Transport-Subsystem-MIB will be renamed - updated needed
    here to be consistent.

29: Page 22: The name of the MIB module should be changed to
    SNMP-TRANSPORT-SM-MIB to be consistend with other SNMPv3
    specifications and I suggest to register the module below
    snmpModules.

30: Page 22-27: Almost all definitions should be removed since they
    are not needed. What is missing are two counters: one for messages
    dropped due to the security level in the message being
    inconsistent with the security level provided by the transport
    (step 2) in the elements of procedure for incoming messages) and
    one counter for messages dropped due to a missing tmStateReference.

31: Page 28: The MIB boilerplate security text needs to be updated.

32: Page 29: IANA considerations must be updated if we register under
    snmpModules.

33: Page 30: I believe reference [RFC3419] is not needed.

34: Appendix A: Some of the tags need to be updated. If we drop the
    user table in the MIB module, we can also drop section
    A.1. Section A.2 depends on the SSH transport model which I think
    we should avoid. It might make more sense to let the SSH transport
    model depend on the Tranport Security Model and to move the
    notification example to the SSH document. (We should definately
    keep the example since this is useful.)

-- 
Juergen Schoenwaelder		    International University Bremen
<http://www.eecs.iu-bremen.de/>	    P.O. Box 750 561, 28725 Bremen, Germany

_______________________________________________
Isms mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/isms

---

From: Jeffrey Hutzelman <[email-masked]>
Date: Wed, 15 Nov 2006 17:45:29 -0500

>     securityName provided by the secure Transport Model would be

Right.  That is, if USM runs over any transport, then it should run over 
any transport, including SSH.  I'm not sure this will ever have any 
operational value, but it's good for thinking about whether the abstraction 
is good enough.

>     problem - just that we should be clear about this.

That's an interesting point.  Of course, it's not clear to me how the 
securityName would be derived with a TLS transport; if the namespaces 
happened to be disjoint, this wouldn't be an issue.