Subject: [Isms] Responses to Robert Story's about the TLSTM draft

From: Wes Hardaker <[email-masked]>
Date: Mon, 25 Jan 2010 14:11:18 -0800

Robert,

Thanks for your comments about the document.  They've definitely been
helpful in improving readability.  Although some of them were duplicates
of others, there are other good points that no one else caught (like
duplicate paragraphs!)

Below are my responses.  Note that because your comments were sent to me
outside the WG many working group members may not have seen these yet,
but I don't think they're continuous.  I'd still encourage other WG
participants to read them and my responses though.

My responses are prefixed below with "WH:" if you want to search for
them.  The status of each item is as follows:

  DONE:     Suggestion was acted on
  WONTDO:   Nothing was done; refer to the WH: response for reasons why
  DISCUSS:  There is an outstanding question back to you
  ANSWERED: A question was answered but had no action resulting from it

A note of disclosure: Robert and I do work for the same employer.  He
has not had the opportunity before recently to read the draft, however,
and send the comments to me after the end of the last WG last call but
before the last 19th cut-off date so I decided to act on them just as I
had everyone else's comments to the list.

* DONE section 1, in diagram, 'LCD' is used, but not expanded 
  until section 5.1.1. LCD is not used as an acronym in 5590 or
  5591.
  + WH: removed LCD from the diagram; I don't think it's needed.

* DONE section 1.1, 
  
  + DONE paragraphs starting with "Authentication in this document 
    typically ..."
     and "The terms "manager" and "agent" are not used in this
     document ..." are both repeated (though slightly differently worded).
    
     + WH: good catch; thanks.  (the first set of wording was
       copied from 5592 and the second set should have been removed).
    
  + DONE paragraph starting with "Throughout this document, the terms 
    "client" and "server" ..." talks about ends of connections, then
    says 'either SNMP entity'... 3411 defines more than 2 SNMP
    entities...
    + WH: oddly, 5592 has the same issue.  However, our text was
      already fixed due to other suggested text replaced.

* DONE section 3.2 "e.g..," should be "e.g.". "transport- related" should be 
  "transport-related".

* DONE section 4.3.1, 
  
  + DONE destTransportDomain description contains and extraneous quote after 
    snmpDTLSSCTPDomain.
    
  + DONE delete 'handle/' from tmStateReference description. 5590 consistenly 
    uses 'reference'.
    
* DONE section 4.3.2 
  
  + DONE destTransportDomain description contains and extraneous quote after 
    snmpDTLSSCTPDomain.
    
  + DONE "incomingMessage:  The whole SNMP message after being processed by 
    (D)TLS and removed of the (D)TLS transport layer data." "removed of the"
    isn't grammatically correct. reword.
    
  + DONE delete 'handle/' from tmStateReference description. 5590 consistenly 
    uses 'reference'.
    
* DONE section 5, paragraph 2, "...should be overwritten appropriately first 
  prior..." is awkward... delete 'first'.
  
* DONE 5.1.1 
  
  + DONE 5.1.1 nitpick?. "When establishing a new session 
    implementations MUST use a different UDP source port number
    for each connection to a remote destination IP-
    address/port-number combination".. this implies that UDP
    source ports can't be reused once a session is closed. maybe
    say '... for each active connection ...'?  I haven't seen
    anything yet about closing sessions - is that an explicit
    action? i.e. if the client must/should close a session, then
    the server should know when a packet from the same source
    port is a new session..
    + WH: added "active"; good suggestion, thanks.
    
  + WONTDO paragraph 2 "... although any implementation-dependent 
    method should be suitable as long as the results are
    deterministic." is a bit wishy-wash. seems like there should
    be a MUST there (e.g results MUST be deterministic).
    + WH: I'd need a specific proposal.
    
  + WONTDO step 2) first paragraph: "to determine if a session 
    already exists and its tlstmSessionID." is awkward. "to
    determine the tlstmSessionID of an existing session." or "
    to determine if a session already exists, and if so, to
    retrieve its tlstmSessionID."
    + WH: actually, it was truncated because someone else
      pointed out that the sessionID extraction already appears
      below.
    
  + WONTDO second paragraph: "If DTLS returns success then stop processing 
    of this message.  If DTLS returns an error then increment
    the snmpTlstmSessionNoSessions counter and stop processing
    the message."  I'm confused by the 'stop processing of
    this message' in BOTH cases. Shouldn't success continue
    processing? I'd think that success meant that a new
    tlstmSessionID would now be in the LCD for use in step 3.
    + WH: No, actually it should stop either way.  If we didn't
      have a tlstmSessionID then there won't be a SNMP message
      to act on after this step.
    
  + WONTDO Note: maybe section 5.3 on establishing a session should be 
    moved before processing incoming and outgoing messages? seems
    like it'd be less confusing.
    + WH: Actually, no...  It's historical.  5592 does it in
      that order so we're mirroring it.     
    
  + DONE third paragraph: "Note that an entry would already exist if the 
    client and server's session establishment procedures had been
    successfully completed previously (as described both above and in
    Section 5.3)"  I note that section 5.3 makes no mention of
    putting anything in the LCD.
    + WH: Good point.  Added: 
    
      The tlstmSessionID is also stored in the LCD for later
      lookup during processing of incoming messages (<xref
      target="snmpinprocessing" />).
    
* DONE 5.2 
  
  + WONTDO step 5; "..suitable to send the message over." seems 
    awkward. I'd just end the sentence with "... if there is a
    corresponding entry in the LCD."
    + WH: while I agree it may be a bit awkward, it think it's
      important to leave the concept of "suitable" in since it
      reminds the reader that authNoPriv messages can go over an
      authPriv channel.
    
  + DONE step 5 has sub-steps 4a and 4b. Probably should be 5a and 5b. 
    
  + DONE step 5b (aka step 4b) typo "tmStateReferenc" -> "tmStateReference" 
    
  + DONE step 6 references a tmSessionID 'resulting from a previous step (3 
    or 4)", I think a step was added, and this should be step 4 or 5.

* WONTDO 5.3, step 3b. paragraph 3: "If the connection is being 
  established for other reasons then configuration and
  procedures outside the scope of this document should be
  followed." I'm not clear on what 'other reasons' means, since
  there haven't been any reasons mentioned yet.
  + WH: the TARGET-MIB was the reason mentioned.  Other reasons
    would be a non TARGET-MIB based instance (ie, snmpget ...).
    You should take the configuration given to you from the
    command line options here.
  
* DONE one where you seem to be violating the requirements of a 
  referenced draft:
  
    "tlstmAddrServerIdentity mentions wildcards for domain
    names, but the specified reference
    [draft-saintandre-tls-server-id-check] specifically states
    that 'in the case of domain names, the reference identity
    MUST NOT contain the wildcard character ...' (section 2,
    conventions)."
  
    + WH: Yep; this has been changed.
  
        one that makes me think working is wrong or additional counters are
        needed:
  
* WONTDO snmpTlstmSession(Opens|Closes) both qualify themselves with 'as an 
  (D)TLS client'. To me, this implies that Opens/Closes counters are
  only incremented when originating a message. I was expecting to find
  later counters for a (D)TLS server, to count Opens/Closes for
  incoming connections to a server. There is
  snmpTlstmSessionInvalidClientCertificates for failed (D)TLS
  server sessions, but nothing for successful connections.
  + WH: The complexities of detecting server side connections
    are difficult (see the list discussion).  Client side is
    much easier, and although it would be helpful it has been
    decided by the WG both previously and recently that only the
    client side connections could be easily counted.
* DONE snmpDTLSUDPDomain description uses 'dudp' as the prefix, but the 
  second rfc ed note says "replace 'dudp' ... if 'dtls' is not
  assigned..." suspect 'dtls' should be 'dudp'
  
* DONE snmpDTLSSCTPDomain description uses 'dsct' as the prefix, but the 
  second rfc ed note says "replace 'dsct' ... if 'dtls' is not
  assigned..." suspect 'dtls' should be 'dudp'
  
  + WH: you meant 'dsct', but done.
  
* DONE tlstmIdentities.* could use a comment block to state that these 
  'Identities' are actually identity types to be used in the
  tlstmCertToTSNTable. Personally, I would have used an enum for the
  various types, instead of a long OID string.
  + WH: the OID solution was based on working group consensus in
    order to allow for easy future extensibility.  This is
    necessary to support 3rd-party definitions of otherName
    subjectAltName mappings.  Comment added though:
  
  -- Identities for use in the tlstmCertToTSNTable
  
* DONE tlstmCertSANRFC822Name typo in example ("... is mapped to 
  tmSecurityName: [email-masked]" (s/exmap/examp/).
  
* DONE tlstmCertSANAny: "... certificate any of the above types 
  .." should probably be "... certificate _of_ any of the above
  types ..."
  
* DONE snmpTlstmSessionUnknownServerCertificate: "... to known certificate 
  authority." should be "... to _a_ knowns certificate authority."
  
* DONE snmpTlstmSessionInvalidServerCertificates: any real reason to mention 
  'cryptographic validation' twice?.  suggest striking "even if the
  fingerprint or expected validation path was known.  I.E., a
  cryptographic validation occurred during certificate validation
  processing."
  + because there is other types of certificate processing, I
    think it's necessary to say this counts validation processing.
  
* DONE tlstmAddrTable 
  
  + DONE : "... expect when establishing a new (D)TLS connections." 
    should either be 'establishing a new .. connection.' or
    'establishing new .. connections.' (number agreement)
    + WH: this text was largely rewritten (by Tom) and doesn't
      contain that issue any longer.
    
  + DONE this object is the first use of 'trust anchor' in the 
    document. the rest of the document (including earlier in
    this object) use something like 'certificate validation
    path'.
    + WH: changed to the following, which completely removes the
      only instance of "trust anchor" in the document.
      tlstmAddrEntry changed as well.
    
         If there is no row in this table corresponding to an
         entry in the SNMP-TARGET-MIB and another certificate
         validation path algorithm (such as the path validation
         procedures defined in [RFC5280]) can be used, then the
         connection SHOULD still proceed."
    
* DONE tlstmServerCertificateUnknown: "...this may result because..." is 
  awkward, if not outright wrong. mabye "... this may be because...".
  
* DONE section 9 'RECOMMENDEDs' (RECOMMENDs?) 'that implementers also 
  support this cookie exchange, but 3.1.1 items 5 sates that "DTLS
  Transport Model server implementations MUST support DTLS cookies." 
  + Changed to talk about the users and changed the text so the
    2119 keywords work:
  
    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.
  
* WONTDO 9.3, "Even if the network itself is secure (for example 
  by using IPsec), even then,", suggest deleting second use of
  even ('even then').
  + WH: This is copied from 5592.  Every time I've tried to fix
    text that was copied from the sibling document I've gotten
    in trouble ;-)

-- 
Wes Hardaker
Cobham Analytic Solutions