Subject: [Isms] Review of draft-ietf-isms-dtls-tm-01.txt

From: Alan Luchuk <[email-masked]>
Date: Fri, 20 Nov 2009 15:38:37 -0500 (EST)

Hello,

I have reviewed  draft-ietf-isms-dtls-tm-01.txt; my comments are included
below.  My apology for my late review of this document.

My overall comment is that the document is in generally good shape, could 
be advanced, and that my company is interested in it advancing.  My thanks 
to the author and reviewers; their hard work shows!   I reviewed a much 
earlier draft, and appreciate the additional technical completeness and 
clarification in this latest version.  I especially appreciate the 
"implementation hints" and the well-thought-out and thorough specification 
of mapping X.509 certficates to SNMP security names.  The engineering 
effort shows.

Most of what I noted were minor typograpical errors and possible wording
changes.  There are a places where more clarification might be helpful.
I suggest changes to the MIB that are NOT ESSENTIAL to the correct opera-
tion or implementability of SNMP over (D)TLS, but the presence or absence
of the suggested changes may affect the utility of othe MIB objects.

My company has not implemented SNMP over (D)TLS, but we are interested in
doing so.  We have customer demand for SNMP over (D)TLS.

The specific suggestions are listed in the order in which they appear in
the draft.  I've catagorized the changes below as "typo", "wording", 
"formatting", "clarification", or "content error" changes.

Section 1:  Page 6, only sentence:  (wording)
---------------------------------------------

Suggest changing  "equally as legitimate."  to  "equally valid."

Section 2.1 Page 8, first bulleted item:  (clarification)
---------------------------------------------------------

The text reads:  

    The TLS Transport Model SHOULD always use authentication of both 
     the server and the client.

An explanation of why this is true would be helpful for implementors
who do not understand security as thoroughly as those on the ISMS WG.

Section 2.1 Page 8:  (formatting/typo)
-------------------------------------- 

There is a straggling item number 3 that appears to be the heading
"How the TLSTM fits into the Transport Subsystem".  Suggest fixing
this.

Section 2.1 Page 9:  (formatting)
---------------------------------

If possible, fixing the page breaks so that the diagram appears
on a single page.  This applies to other diagrams as well.

Section 3.1.1 Page 10, item number 1:  (wording)
------------------------------------------------

The text reads:  

   1.  Modification of Information - The modification threat is the
       danger that some unauthorized entity may alter in-transit SNMP
                    ^^^^

Suggest change:  

   1.  Modification of Information - The modification threat is the
       danger that an unauthorized entity may alter in-transit SNMP
                    ^^

Section 3.1.1 Page 12, item number 5, second to last paragraph:  (typo)
-----------------------------------------------------------------------

The text reads:

   Implementations are not required to perform the stateless cookie
   exchange for every DTLS handshakes but in environments where
                                    ^
Suggest removing the "s" from "handshakes".

Section 4.1.1 Page 15, second bulleted text, fourth paragraph:  (typo)
----------------------------------------------------------------------

The text reads:

   o  Identifies a certificate issuer's fingerprint and allows a child
      certificate's subjectAltName or CommonName to be mapped to the
      tmSecurityNome."
                 ^

Suggest changing  "tmSecurityNome."  to  "tmSecurityName."

Section 4.3.1 Page 17, first paragraph, last sentence:  (wording)
-----------------------------------------------------------------

The text reads:

   This document specifies three TLS and DTLS based Transport Domains for 
   use: the snmpTLSDomain, the snmpDTLSUDPDomain and the snmpDTLSSCTPDomain.

Suggested text change:

   This document specifies the snmpTLSDomain, the snmpDTLSUDPDomain and 
   the snmpDTLSSCTPDomain" transport domains.

There are a few other places in the document where this suggested change
could be made.

Section 4.3.1 Page 17, second paragraph:  (wording)
---------------------------------------------------

The text reads:

   destTransportAddress:  The transport address of the destination TLS
      Transport Model in a format specified by the SnmpTLSAddress, the
      SnmpDTLSUDPAddress or the SnmpDTLSSCTPAddress TEXTUAL-CONVENTIONs.

Question:  Does the format into the API really matter?  Would omitting
the format clause be better, or suggesting the intended format?

Section 4.3.2 Page 17:  (content error)
---------------------------------------

The text reads:

      statusInformation =
      receiveMessage(
      IN   transportDomain               -- origin transport domain
      IN   transportAddress              -- origin transport address
      IN   incomingMessage               -- the message received
      IN   incomingMessageLength         -- its length
      IN   tmStateReference              -- reference to transport state
       )

I _think_ the ASI arguments all specify items that are returned to the
ASI caller, so I _think_ it should be:

      statusInformation =
      receiveMessage(
      OUT   transportDomain               -- origin transport domain
      OUT   transportAddress              -- origin transport address
      OUT   incomingMessage               -- the message received
      OUT   incomingMessageLength         -- its length
      OUT   tmStateReference              -- reference to transport state
       )

Now, in a programming language, one would pass pointers into an API, which
would then be populated by the API, then returned to the caller.  Thus, if
receiveMessage() specified an _API_ (not an _ASI_), then the original 
_might_ be correct.  But the original obscures the fact that the informa-
tion items specified by the arguments are _returned_ by the ASI.  Thus, the 
original seems to obscure the information flow.

I note that this diagram comes from RFC 5590, section 6.3, on page 25,
which I _think_ is incorrect also.

Am I missing something here?

Section 4.3.2 Page 18, first paragraph:  (wording)
--------------------------------------------------

The text reads:

   transportAddress:  The transport address of the source of the
      received message in a format specified by the SnmpTLSAddress, the
      SnmpDTLSUDPAddress or the SnmpDTLSSCTPAddress TEXTUAL-CONVENTION.

Question:  Does the format into the API really matter?  Would omitting
the format clause be better, or suggesting the intended format?

Section 4.4 Page 18, first paragraph:  (typo/content error)
-----------------------------------------------------------

The text reads:

   This section describes the services provided by the (D)TLS Transport
   Model with their inputs and outputs."               ^^^

Suggested change:

   This section describes the services provided by the TLS Transport
   Model with their inputs and outputs.

Section 4.4 Page 18, first paragraph:  (clarification)
------------------------------------------------------

The text reads:

  The following sections describe services for establishing and closing 
  a session and for passing messages between the (D)TLS transport

Suggested change:

  The following sections describe services for establishing and closing 
  a (D)TLS session and for passing messages between the (D)TLS transport
    ^^^^^^

Section 4.4.1 Page 19, sixth paragraph:  (clarification/wording)
----------------------------------------------------------------

The text reads:

   Neither DTLS or UDP provides a session de-multiplexing mechanism and
   it is possible that implementations will only be able to identify a
   unique session based on a unique combination of source address,
   destination address, source UDP port number and destination UDP port
   number.  Because of this, when establishing a new sessions
   implementations MUST use a different UDP source port number for each
   connection to a given remote destination IP-address/port-number
   combination to ensure the remote entity can properly disambiguate
   between multiple sessions from a host to the same port on a server.
   TLS and DTLS over SCTP provide session de-multiplexing so this
   restriction is not needed for TLS or DTLS over SCTP implementations.

Suggested changes:

   Neither DTLS or UDP provides a session de-multiplexing mechanism and
   it is possible that implementations will only be able to identify a
   unique DTLS session based on a unique combination of source address,
          ^^^^
   destination address, source UDP port number and destination UDP port
   number.  Because of this, when establishing a new sessions with
   different security parameters,                             ^^^^
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   implementations MUST use a different UDP source port number for each
   connection to a given remote destination IP-address/port-number
   combination to ensure the remote entity can properly differentiate
                                                        ^^^^^^^^^^^^^
   between multiple sessions from a host to the same port on a server.
   TLS and DTLS over SCTP provide session de-multiplexing so this
   restriction is not needed for TLS or DTLS over SCTP implementations.

I _think_ "disambiguate" is jargon, not a real word.

Section 4.4.1 Page 19, last paragraph, second line:  (typo)
-----------------------------------------------------------

Add a comma after the clause "if the process was successful".

Section 4.4.2 Page 19, first paragraph:  (clarification)
--------------------------------------------------------

The text reads:

   When the TLS Transport Model invokes the (D)TLS record layer to
   verify proper security for the incoming message, it must use the
   following ASI:

Suggested change:

   The TLS Transport Model invokes the tlsRead ASI to verify proper
   security for the incoming message:

Section 4.4.2 Page 20, third paragraph:  (attaboy)
--------------------------------------------------

Thanks for the implementation hint in the paragraph about the "tlsSessionID"!

Section 5.1.1 Page 22, second paragraph:  (typo)
------------------------------------------------

Add a hyphen between "implementation dependent" in the fourth line.

Section 5.1.1 Page 22, second paragraph:  (wording)
---------------------------------------------------

The text reads:

   accomplish this, although any implementation dependent method should
   be suitable as long as the results are consistently deterministic.

Suggested change:

   accomplish this, although any implementation dependent method should
   be suitable as long as the results are consistent.

The phrase "consistently deterministic" is redundant.

Section 5.1.1 Page 23, item 5:  (typo)
--------------------------------------

Change "tlsWholeMsg" to "wholeTlsMsg".

Section 5.1.2 Page 23, heading:  (clarification)
------------------------------------------------

The text reads:

    5.1.2.  Transport Processing for Incoming Messages

Should it read:

    5.1.2.  Transport Processing for Incoming SNMP Messages

There are a few other places in the text of this section where adding a
clue about whether the message is SNMP might clarify the text.

Section 5.1.2 Page 24, third paragraph:  (wording)
--------------------------------------------------

The text reads:

   tmTransportAddress  = The address the message originated from,
      determined in an implementation dependent way.

Suggested change:

   tmTransportAddress  = The address the message originated from.

Does the phrase "determined in an implementation dependent way" matter?

Section 5.2 Page 25, heading:  (clarification)
----------------------------------------------

The text reads:

    5.2.  Procedures for an Outgoing Message

Should it read:

    5.2.  Procedures for an Outgoing SNMP Message

There are a few other places in the text of this section where adding a
clue about the message is SNMP might clarify the text.

Section 5.2 Page 25, bulleted item 4b:  (typo)
----------------------------------------------

The text reads:

            same cache entry.  If an error is returned from
            OpenSession(), then discard the message, increment the
            ^

Suggested change:

            same cache entry.  If an error is returned from
            openSession(), then discard the message, increment the
            ^

I _think_ this refers to the "openSession() ASI.

Section 5.3 Page 27, bulleted item 3:  (wording)
------------------------------------------------

The text reads:

   3)  Once a (D)TLS secured session is established and both sides have 
       performed any appropriate certificate authentication verification 

Suggested change: 

   3)  Once a (D)TLS secured session is established and both sides have 
       verified the authenticity of the peer's certificate

The original phrase "certificate authentication verification" sounds confusing.

Section 5.4 Page 28, bulleted item 2:  (wording)
------------------------------------------------

The text reads:

   2)  If there is no session open associated with the tmStateReference,
                      ^^^^^^^^^^^^
       then closeSession processing is completed.

Suggested change:

   2)  If there is no open session associated with the tmStateReference,
                      ^^^^^^^^^^^^
       then closeSession processing is completed.

Section 6.3 Page 29:  (typo)
----------------------------

Change "statical" to "statistical".

Section 6.3 Page 29:  (wording)
-------------------------------

Change "feedback" to "information".

Section 7 Page 34 sixth paragraph under "SnmpTLSAddress":  (typo)
-----------------------------------------------------------------

Change the case of "snmpTLSAddress" to "SnmpTLSAddress".

Section 7 Page 36 sixth paragraph under "SnmpDTLSUDPAddress":  (typo)
---------------------------------------------------------------------

Change the case of "snmpDTLSUDPAddress" to "SnmpDTLSUDPAddress".

Section 7 Page 37 sixth paragraph under "SnmpDTLSSCTPAddress":  (typo)
----------------------------------------------------------------------

Change the case of "snmpDTLSSCTPAddress" to "SnmpDTLSSCTPAddress".

Section 7 Page 39 under tlstmSessionInvalidClientCertificates:  (wording)
-------------------------------------------------------------------------

The text reads:

    DESCRIPTION
        "The number of times an incoming session was not established
        on an (D)TLS server because the presented client certificate was
        invalid.  Reasons for invalidation includes, but is not
                                                  ^      ^^
        limited to, cryptographic validation failures and lack of a
                                                      ^^^
        suitable mapping row in the tlstmCertToSNTable."

Suggested change:

    DESCRIPTION
        "The number of times an incoming session was not established
        on an (D)TLS server because the presented client certificate was
        invalid.  Reasons for invalidation include, but are not
                                                        ^^^
        limited to, cryptographic validation failures, or a lack of a
                                                     ^ ^^
        suitable mapping row in the tlstmCertToSNTable."

Section 7 Page 39 under tlstmSessionInvalidServerCertificates:  (wording)
-------------------------------------------------------------------------

Same change in the DESCRIPTION clause as described above for the
tlstmSessionInvalidClientCertificates DESCRIPTION clause.

Section 7 Page 40:  (technical)
-------------------------------

I suggest adding:

tlstmCertToSNTableLastChangedEngineBoots  OBJECT-TYPE
    SYNTAX       INTEGER (1..2147483647)
    MAX-ACCESS   read-only
    STATUS       current
    DESCRIPTION
        "The value of snmpEngineBoots.0 when the tlstmCertToSNTable
        was last modified through any means."

just above the "tlstmCertToSNTableLastChanged" object.  

Having an engine boots and "tlstmCertToSNTableLastChanged" values would 
simplify the reliable detection of changes to the tlstmCertToSNTable.
Without an engine boots value, one cannot reliably detect changes to
the tlstmCertToSNTable.

I also suggest adding analogous engine boots values before the
"tlstmParamsTableLastChanged" and "tlstmAddrTableLastChanged" objects.

Section 7 Page 42 under "tlstmCertToSNFingerprint":  (clarification)
--------------------------------------------------------------------

How is the actual Fingerprint of a certificate generated, and how is this
MIB object populated?  I _assume_ the Fingerprint is generated by reading 
in, then hashing, the certificate.  Or is the Fingerprint already _in_ the 
certificate and just needs to be extracted?

A similar clarification under "tlstmParamsClientFingerprint", and
"tlstmAddrServerFingerprint" would be nice.  Or just providing the
clarification under the "Fingerprint" TEXTUAL-CONVENTION might be
sufficient.

Section 8.1 Page 54 first paragraph:  (clarification)
------------------------------------------------------

The text reads:

   use separate ports for Notification sessions and for Command
   sessions.  If this implementation recommendation is followed, (D)TLS

Are these "listen" ports, or "connect" ports?

Section 8.1 Page 54 first paragraph:  (typo)
--------------------------------------------

Should REQUEST and RESPONSE be in lowercase?

Change "Request- Response" to "request-response"

Section 8.2 Page 54:  (attaboy)
-------------------------------

Thanks for the nice explanation!

Section 9.1 Page 56 second paragraph:  (clarification)
------------------------------------------------------

The text reads:

   The instructions found in the DESCRIPTION clause of the
   tlstmCertToSNTable object must be followed exactly.  It is also
   important that the rows of the table be searched in prioritized order
   starting with the row containing the lowest numbered tlstmCertToSNID
   value.

Why _must_ the procedure be followed exactly, and why is it important
that the rows of the table be searched in prioritized order?

Section 11 Page 59 second paragraph:  (typo)
--------------------------------------------

Add a second space after the colon after the phrase "(in alphabetical order):"

Section A.1 Page 62 third bulleted item:  (typo/clarification)
--------------------------------------------------------------

The text reads:

   o  Messages are protected against replay.  (D)TLS uses explicit
      sequence numbers and integrity checks.  DTLS uses a sliding window
      to protect against replay of messages within a session.

Are (D)TLS and DTLS used properly here?

Section B Page 63 first paragraph:  (typo)
------------------------------------------

Jettison the extra space between "challenge- response".

Section B Page 64 last two paragraphs:  (clarification)
-------------------------------------------------------

The very last paragraph discusses "decrypted information", but no explan-
ation of where the "encrypted information" originated.  I _believe_ I 
understand this, but persons new to PKIX might not.  The understanding of 
how/where the certificate hashes are encrypted and stored, then decrypted 
and compared, is critical to the understanding of PKIX.

Section C.1 Page 65 first paragraph:  (typo)
--------------------------------------------

Change "Notification Generator's" to "Notification Generators".

Section C.2 Page 66 second to the last paragraph:  (typo)
---------------------------------------------------------

Change "issuing certificate" to "issuing certificate".

Change "1 to 1" to "one-to-one".

Hope this review and comments are useful to the WG.

Regards,
--Alan

 ------------------------------------------------------------------------------
 Alan Luchuk               SNMP Research, Inc.          Voice:  +1 865 573 1434
 Senior Software Engineer  3001 Kimberlin Heights Road  FAX:    +1 865 573 9197
 [email-masked]           Knoxville, TN  37920-9716    http://www.snmp.com/
 ------------------------------------------------------------------------------

---

From: Wes Hardaker <[email-masked]>
Date: Fri, 20 Nov 2009 14:25:36 -0800

AL> I have reviewed  draft-ietf-isms-dtls-tm-01.txt; my comments are included
AL> below.  My apology for my late review of this document.

Alan,

I greatly appreciate the review and will respond later at length about
your questions and concerned as I process through the list.

AL> My company has not implemented SNMP over (D)TLS, but we are
AL> interested in doing so.  We have customer demand for SNMP over
AL> (D)TLS.

I'm certainly glad to hear that!
-- 
Wes Hardaker
Cobham Analytic Solutions