Subject: [Isms] AD review comments for draft-ietf-isms-dtls-tm-08

From: <[email-masked]>
Date: Thu, 25 Feb 2010 14:23:53 +0100

AD review comments for draft-ietf-isms-dtls-tm-08

I've now done my AD review for draft-ietf-isms-dtls-tm-08, and bunch
of comments. Most of them are quite small, and I've tried to suggest
possible text in most cases (but this is just to clarify what I'm
looking for; I'm not hung up on the exact wording):

Somewhat substantial first:

- Unless we have a normative dependency to
draft-ietf-tsvwg-dtls-for-sctp (which would delay this document for
unpredictable time), we need to specify which of the possible ways of
running DTLS over SCTP we're using in this document. Since this might
take some time, I would suggest just leaving out DTLS-over-SCTP from
this document.

- Currently, the document specifies how the TLS client authenticates/
authorizes the TLS server only when SNMP-TARGET-MIB is used.  But
clearly authenticating the TLS server is also important for
e.g. command generators. These may not have a MIB, but shouldn't we at
least recommend an implementation-specific management/configuration
interface that supports configuring the same details as
tlstmAddrTable?

(E.g. Syslog-over-TLS specifies that the management interface must be
able to configure this stuff, but that "management interface" could
be a configuration file, GUI, or even a proprietary MIB.)

- Overall: The details of exactly how (D)TLS needs to be used are
scattered over the document, and in some cases, not very clearly
written. Probably we should include a "(D)TLS usage" section that
explains what TLS features/etc. we're using and so on. Here's a first
sketch, replacing current 4.2:

4.2.  (D)TLS Usage

   (D)TLS MUST negotiate a cipher suite that uses X.509 certificates
   for authentication, and MUST authenticate both the client and the
   server. The mandatory-to-implement cipher suite is specified in the
   TLS specification [RFC5246].

   TLSTM verifies the certificates when the connection is opened (see
   Section 5.3). For this reason, TLS renegotiation with different
   certificates MUST NOT be done.  That is, implementations MUST
   either disable renegotiation completely (RECOMMENDED), or MUST
   present the same certificate during renegotiation (and MUST verify
   that the other end presented the same certificate).

   For DTLS over UDP, each SNMP message MUST be placed in a single UDP
   datagram; it MAY be split to multiple DTLS records. In other words,
   if a single datagram contains multiple DTLS application_data
   records, they are concatenated when received. The TLSTM
   implementation SHOULD return an error if the SNMP message does not
   fit in the UDP datagram, and thus cannot be sent.

   For DTLS over UDP, the DTLS server implementation MUST support DTLS
   cookies ([RFC4347] already requires that clients support DTLS
   cookies).  Implementations are not required to perform the cookie
   exchange for every DTLS handshake; however, enabling it by default
   is RECOMMENDED.

   For DTLS, replay protection MUST be used.

- Section 3.1.3. "As an implementation hint: although the
tlstmSessionID may be the same as the (D)TLS internal SessionID
caution must be exercised since the (D)TLS internal SessionID may
change over the life of the connection as seen by the TLSTM (for
example during renegotiation).  The tlstmSessionID identifier MUST NOT
change during the entire duration of the session from the TLSTM's
perspective even if the TLS internal session identifier does change."

This is not correct; it's possible to have several (D)TLS connections
(over different TCP connections, or different UDP five-tuples) that
have the same (D)TLS internal Session ID, so it does not uniquely
identify a TLSTM session. I would suggest just deleting this
paragraph.

Largely for the same reason, Section 5.1.1 is not quite correct;
here's an attempt to rephrase it:

5.1.1.  DTLS over UDP Processing for Incoming Messages

   For connection-oriented transport protocols, such as TCP, the
   transport protocol takes care of demultiplexing incoming packets to
   the right connection. Depending on the DTLS implementation, for
   DTLS over UDP, this demultiplexing may need to be done by the TLSTM
   implementation.

   Like TCP, DTLS over UDP uses the four-tuple <source IP, destination
   IP, source port, destination port> for identifying the connection
   (and relevant DTLS connection state). This means that when
   establishing a new session, implementations MUST use a different
   UDP source port number for each active connection to a remote
   destination IP-address/port-number combination to ensure the remote
   entity can disambiguate between multiple connections.

   If demultiplexing received UDP datagrams to DTLS connection state
   is done by the TLSTM implementation (instead of the DTLS
   implementation), the steps below describe one possible method to
   accomplish this.

   1)  The TLS Transport Model examines the UDP datagram, in an
       implementation-dependent manner.

   2)  The TLS Transport Model queries the LCD using the transport
       parameters (source and destination IP addresses and ports) to
       determine if a session already exists.

       If a matching entry in the LCD does not exist, create a new
       entry containing the transport parameters; a new DTLS
       connection handle (an implementation-dependent detail), and a
       new tlsTmSessionId.

   3)  Pass the UDP packet and DTLS connection handle to DTLS (in
       implementation-dependent manner). If processing does not return
       an incomingMessage and an incomingMessageLength then processing
       stops (this is the case when the UDP datagram contained DTLS
       handshake messages, for example).

   4)  Retrieve the incomingMessage and an incomingMessageLength from
       DTLS.  These results and the tlstmSessionID are used below in
       Section 5.1.2 to complete the processing of the incoming message.

- Section 7, tlstmCertSANDNSName: I thought we agreed in IETF76 that
this should be lowercased?

- Section 7, tlstmCertSANIpAddress: I thought we agreed in IETF76 that
IPv6 addresses need to be in lowercase?

- Section 7, tlstmCertCommonName: "[..] by directly passing the value
without any transformations" -> "[..]. The value is converted to UTF-8
encoding, but no other transformation are done."

- Section 7, tlstmParamsClientFingerprint: we should probably allow an
empty value to accommodate the common case where the client has just
one certificate/private key?

- Section 7, tlstmAddrTable DESCRIPTION is a bit vague on some of the
details, and in its present form, would need a normative reference to
draft-saintandre-tls-server-id-check (which we might want to avoid so
this document is not delayed). Suggested rephrasing:

   "This table is used by a (D)TLS client when a (D)TLS connection is
   being set up using an entry in the SNMP-TARGET-MIB.  It extends the
   SNMP-TARGET-MIB's snmpTargetAddrTable so that the client can verify
   that the correct server has been reached.

   This verification can use either a certificate fingerprint, or an
   identity authenticated via certification path validation.

   If there is a row in this table corresponding to the entry in the
   SNMP-TARGET-MIB that was used to establish the session (and that
   row is active), and the row's tlstmAddrServerFingerprint column has
   non-empty value, the server's presented certificate is compared
   with the fingerprint (and the tlstmAddrServerIdentity column is
   ignored). If the fingerprint matches, the verification has
   succeeded.

   If the server's presented certificate has passed certification path
   validation [RFC5280] to a configured trust anchor, and a row exists
   with a zero-length tlstmAddrServerFingerprint value, then the
   tlstmAddrServerIdentity column contains the expected host name.
   This expected host name is then compared against the server's
   certicate as follows:

   - Implementations MUST support matching the expected host name
   against a dNSName in the subjectAltName extension field and SHOULD
   support checking the name against the common name portion of the
   subject distinguished name.

   - The '*' (ASCII 42) wildcard character is allowed in the dNSName
   of the subjectAltName extension (and in common name, if used to
   store the host name), but only as the left-most (least significant)
   DNS label in that value.  This wildcard matches any left-most DNS
   label in the server name.  That is, the subject *.example.com
   matches the server names a.example.com and b.example.com, but does
   not match example.com or a.b.example.com.  Implementations MUST
   support wildcards in certificates as specified above, but MAY
   provide a configuration option to disable them.

   - If the locally configured name is an internationalized domain
   name, conforming implementations MUST convert it to the ASCII
   Compatible Encoding (ACE) format for performing comparisons, as
   specified in Section 7 of [RFC5280].

   If none of the rows in this table match, then the connection MUST
   be closed (except if the server can be authorized by other,
   implementation-dependent means)."

- Section 7, "tlstmAddrServerIdentity", remove the last sentence
(about wildcards) and REFERENCE, and change DEFVAL to "" (the wildcard
cannot be used by the procedure as currently described, since the
identity column is looke at only if fingerprint is zero-length, and if
the fingerprint is zero-length, wildcard is not allowed...)

- Section 5.3.1, step 4, rephrase 2nd paragraph as:

   If the connection is being established from configuration based on
   SNMP-TARGET-MIB configuration, then the tlstmAddrTable DESCRIPTION
   clause describes how the verification is done (using either a
   certificate fingerprint, or an identity authenticated via
   certification path validation).

- Section 10, rephrase items 1..6 as:

  1. Two TCP/UDP/SCTP port numbers from the "Registered Ports" range
  of the Port Numbers registry, with keywords "snmptls" and
  "snmptls-trap".  These are the default ports for receipt of SNMP
  command messages (snmptls) and SNMP notification messages
  (snmptls-trap) over a TLS Transport Model as defined in this
  document.

  And delete both paragraphs starting with "If possible, ..." later
  in this section (if we ask for the same port, we will get it).

- C.1. I think we need an example here. Here's a first attempt
   (based on RFC 5591, Appendix A):

   For notification originators performing authorization checks, the
   server's certificate must be verified against the expected
   certificate before proceeding to send the notification.  This is
   done by configuring the tlstmAddrTable accordingly. For example,
   if the verification is done via certification path validation
   (to a trust anchor configured in implementation dependent manner),
   the entries could look like:

          snmpTargetAddrTable row:
             snmpTargetAddrName              toCRAddr
         *   snmpTargetAddrTDomain           snmpTLSTCPDomain
         *   snmpTargetAddrTAddress          192.0.2.1:5162
             snmpTargetAddrTimeout           1500
             snmpTargetAddrRetryCount        3
             snmpTargetAddrTagList           toCRTag
             snmpTargetAddrParams            toCR   (MUST match below)
             snmpTargetAddrStorageType       nonVolatile
             snmpTargetAddrColumnStatus      createAndGo

         snmpTargetParamsTable row:
             snmpTargetParamsName            toCR
             snmpTargetParamsMPModel         SNMPv3
         *   snmpTargetParamsSecurityModel   TransportSecurityModel
             snmpTargetParamsSecurityName    "blueberry"
             snmpTargetParamsSecurityLevel   authPriv
             snmpTargetParamsStorageType     nonVolatile
             snmpTargetParamsRowStatus       createAndGo

          snmpTargetAddrTable row:
             snmpTargetAddrName              toCRAddr
         *   snmpTargetAddrTDomain           snmpTLSTCPDomain
         *   snmpTargetAddrTAddress          192.0.2.1:5162
             snmpTargetAddrTimeout           1500
             snmpTargetAddrRetryCount        3
             snmpTargetAddrTagList           toCRTag
             snmpTargetAddrParams            toCR   (MUST match above)
             snmpTargetAddrStorageType       nonVolatile
             snmpTargetAddrColumnStatus      createAndGo

          tlstmAddrTable row:
             snmpTargetAddrName              toCRAddr
             tlstmAddrServerFingerprint      ""
             tlstmAddrServerIdentity         "server.example.org"
             tlstmAddrStorageType            nonVolatile
             tlstmAddrRowStatus              createAndGo

Semi-editorial/editorial:

- Section 1, "While (D)TLS supports multiple authentication
mechanisms, this document only discusses X.509 certificate based
authentication.  Although other forms of authentication are possible
they are outside the scope of this specification." sounds a bit
cumbersomet. Perhaps "While [..], this document [..]; other forms of
authentication are outside the scope of this specification" would
be more fluent?

- Section 1: "connectionless (e.g.  UDP or SCTP) transport":
SCTP is connection-oriented, not connectionless.

- Section 1: We shouldn't have a figure longer than one page.
Here's an attempt to clean the figure slightly so it fits on
one page:

 +---------------------------------------------------------------------+
 |                              Network                                |
 +---------------------------------------------------------------------+
     ^                     |            ^               |
     |Notifications        |Commands    |Commands       |Notifications
 +---|---------------------|-------+ +--|---------------|--------------+
 |   |                     V       | |  |               V              |
 | +------------+  +------------+  | | +-----------+   +----------+    |
 | |  (D)TLS    |  |  (D)TLS    |  | | | (D)TLS    |   | (D)TLS   |    |
 | |  (Client)  |  |  (Server)  |  | | | (Client)  |   | (Server) |    |
 | +------------+  +------------+  | | +-----------+   +----------+    |
 |       ^             ^           | |       ^              ^          |
 |       |             |           | |       |              |          |
 |       +-------------+           | |       +--------------+          |
 | +-----|------------+            | | +-----|------------+            |
 | |     V            |            | | |     V            |            |
 | | +--------+       |   +-----+  | | | +--------+       |   +-----+  |
 | | | TLS TM |<--------->|Cache|  | | | | TLS TM |<--------->|Cache|  |
 | | +--------+       |   +-----+  | | | +--------+       |   +-----+  |
 | |Transport Subsys. |      ^     | | |Transport Subsys. |      ^     |
 | +------------------+      |     | | +------------------+      |     |
 |    ^                      |     | |    ^                      |     |
 |    |                      +--+  | |    |                      +--+  |
 |    v                         |  | |    V                         |  |
 | +-----+ +--------+ +-------+ |  | | +-----+ +--------+ +-------+ |  |
 | |     | |Message | |Securi.| |  | | |     | |Message | |Securi.| |  |
 | |Disp.| |Proc.   | |Subsys.| |  | | |Disp.| |Proc.   | |Subsys.| |  |
 | |     | |Subsys. | |       | |  | | |     | |Subsys. | |       | |  |
 | |     | |        | |       | |  | | |     | |        | |       | |  |
 | |     | | +----+ | | +---+ | |  | | |     | | +----+ | | +---+ | |  |
 | |    <--->|v3MP|<--> |TSM|<--+  | | |    <--->|v3MP|<--->|TSM|<--+  |
 | |     | | +----+ | | +---+ |    | | |     | | +----+ | | +---+ |    |
 | |     | |        | |       |    | | |     | |        | |       |    |
 | +-----+ +--------+ +-------+    | | +-----+ +--------+ +-------+    |
 |    ^                            | |    ^                            |
 |    |                            | |    |                            |
 |    +-+------------+             | |    +-+----------+               |
 |      |            |             | |      |          |               |
 |      v            v             | |      v          V               |
 | +-------------+ +-------------+ | | +-------------+ +-------------+ |
 | |   COMMAND   | | NOTIFICAT.  | | | |  COMMAND    | | NOTIFICAT.  | |
 | |  RESPONDER  | | ORIGINATOR  | | | | GENERATOR   | | RECEIVER    | |
 | | application | | application | | | | application | | application | |
 | +-------------+ +-------------+ | | +-------------+ +-------------+ |
 |                     SNMP entity | |                     SNMP entity |
 +---------------------------------+ +---------------------------------+

- Section 1.1 says 'The terms "manager" and "agent" are not used in
this document'; however, the rest of the document occasionally uses
the word 'manager' to refer to the person who's configuring
stuff. Should these be changed to 'administrator'?

- Section 1.1: "this document (unless otherwise specified) is referring
to TLSTM's specific session and not directly to the (D)TLS protocol's
session".  Actually, even when this document *does* specify otherwise
and explicitly says "(D)TLS session", it's still usually talking
about D(TLS) connections, not sessions. Proposed rephrasing here:

   "The (D)TLS protocols also have an internal notion of a session and
   although these two concepts of a session are related, this document
   is referring to the TLSTM's specific session and not directly to
   the (D)TLS protocol's session."

- Section 3: "to close a (D)TLS session" -> "to close the session"
(this is *not* about (D)TLS sessions, but TLSTM sessions)

- Section 3.1.1, item 2: "The TLS Transport Model MUST support
authentication of both the server and the client." This sentence is
very confusing; it seems to be targeted at the writer of this
specification, not implementor. I would just suggest deleting this
sentence.

- Section 3.1.1, item 4: "The TLS Transport Model SHOULD support the
message encryption to protect sensitive data from eavesdropping
attacks." This sentence is very confusing; it seems to be targeted at
the writer of this specification, not implementor. Or if it's targeted
at the implementor, is it trying to say that "implementations SHOULD
support at least one cipher suite that does encryption?" (that's not
needed; the mandatory-to-implement cipher suite in TLS RFCs is quite
enough) I would just suggest deleting this sentence.

- Section 3.1.1, item 5: replace all three paragraphs with

   5.  Denial of Service - the RFC 3411 architecture [RFC3411] states
   that denial of service (DoS) attacks need not be addressed by an
   SNMP security protocol.  However, connectionless transports (like
   DTLS over UDP are susceptible to a variety of denial of service
   attacks because they are more vulnerable to spoofed IP
   addresses. See Section 4.2 for details how the cookie mechanism is
   used. This mechanism, though, does not provide any defense against
   denial of service attacks mounted from valid IP addresses.

- Section 3.1.2: "(D)TLS session provides" -> "(D)TLS connection
provides"

- Section 3.1.3: "D(TLS) Sessions" -> "D(TLS) Connections"

- Section 3.3, "(D)TLS sessions may be initiated" -> "(D)TLS
connections may be initiated

- Section 3.3, "locally held certificate" -> "locally held certificate
(and the corresponding private key)"

- Section 4.1.1, OLD
     Authentication using (D)TLS will require that SNMP entities are
     provisioned with certificates, which are signed by trusted
     certificate authorities (possibly the certificate itself).
  NEW:
     Authentication using (D)TLS will require that SNMP entities have
     certificates, either signed by trusted certification authorities,
     or self-signed.

- Section 4.1.1, "tmpSecurityName" -> "tmSecurityName"

- Section 4.3.1 and 4.3.2: SnmpTLSDomain -> SnmpTLSTCPDomain

- Section 4.4.1.1, "Transport Models SHOULD generate a predictable
tmSecurityName so operators will know what to use when configuring MIB
modules that use securityNames derived from tmSecurityNames."
Setting requirements for all transport models (not just DTLSTM)
doesn't seem to belong to this document...

- Section 4.4.1.1, "through an existing (D)TLS session" -> "through an
existing (D)TLS connection"

- Section 4.4.1.2, "D(TLS) session" -> "D(TLS) connection" (both
instances)

- Section 5.1, "for all (D)TLS-based sessions" -> " for
all(D)TLS-based connections"

- Section 5.1: "TLS and DTLS/SCP" -> "TLS/TCP and DTLS/SCTP"

- Section 5.1: "5.1.2describes" -> "5.1.2 describes"

- Section 5.1.2, "lifetime of the (D)TLS session" -> "lifetime of session"

- Section 5.3 and 5.3.1 and 5.3.2, "(D)TLS session" -> "(D)TLS connection"
(all instances)

- Section 5.4, "Have (D)TLS close the specified session" ->
"Have (D)TLS close the specified connection"

- Section 6.3, "(D)TLS session usage" -> "session usage"

- Section 7, snmpTlstmSessionAccepts DESCRIPTION: TODO

- Section 7, tlstmParamsTable and tlstmAddrTable: "(D)TLS session" ->
"(D)TLS connection"

- Section 7, tlstmParamsClientFingerprint: "locally held X.509
certificate" -> "locally held X.509 certificate (and the corresponding
private key)".

- Section 8.1, " A session is discussed throughout this document as
meaning a security association between the (D)TLS client and the
(D)TLS server."  -> "A session is discussed throughout this document
as meaning a security association between two TLSTM instances."

- Section 9: OLD
     DTLS adds to the security considerations of TLS only because it
     is more vulnerable to denial of service attacks.  A random cookie
     exchange was added to the handshake to prevent anonymous denial
     of service attacks.  RFC 4347 recommends that the cookie exchange
     is utilized for all handshakes.  It is also RECOMMENDED by this
     specification that users enable this cookie exchange.
  NEW:
     When run over UDP, DTLS is more vulnerable to denial of service
     attacks from spoofed IP addresses; see Section 4.2 for details
     how the cookie exchange is used to address this issue.

- Appendix A and B: Please just delete these -- the details aren't
quite right (e.g. DTLS sessions...), and fixing them is not worth the
effort needed.

- C.1, would suggest adding "In case the notification originator has
multiple certificates, ..." to the beginning of the 2nd para.

- C.2, "(D)TLS session" -> "D(TLS) connection"

- C.2: suggest replacing "HASH" with an example hash. If I understand
the Fingerprint DISPLAY-HINT correctly, that might be something like
"02:68b9389e49a815aa4a79f40fcfce7eb8cd10669c"

- C.2: OLD
     tlstmCertToTSNMapType      = 1      (specified)
     tlstmCertToTSNSecurityName = "blueberry"
  NEW:
     tlstmCertToTSNMapType      = tlstmCertSpecified
     tlstmCertToTSNData         = "blueberry"

- C.2: OLD
     tlstmCertToTSNMapType     = 2       (bySubjectAltName)
     tlstmCertToTSNSANType     = 1       (any)
  NEW:
     tlstmCertToTSNMapType     = tlstmCertSANAny
     tlstmCertToTSNData        = ""      (not used)

- Overall: "certificate authority" -> "certification authority"

Best regards,
Pasi

---

From: Wes Hardaker <[email-masked]>
Date: Thu, 25 Feb 2010 18:52:53 -0800

PE> I've now done my AD review for draft-ietf-isms-dtls-tm-08, and bunch
PE> of comments.

Thanks for the comments Pasi.  I'll review them and respond to them
hopefully on Monday.
-- 
Wes Hardaker
Cobham Analytic Solutions