Re: [secdir] [Last-Call] Secdir last call review of draft-ietf-dtn-tcpclv4-15

[Brian Sipos <[e-mail]>]

Christopher,
A new draft has been posted [1] which should address all of your comments on the -15 draft. Most of the changes [2] are to either fix typos and inconsistencies or to add requirements supporting TLS use and threat modeling.
Our AD has requested a re-review of the new draft to verify that there are no more issues present. Would you be able to review the new draft version?
Thank you,
Brian S.

[1] https://tools.ietf.org/html/draft-ietf-dtn-tcpclv4-16
[2] https://www.ietf.org/rfcdiff?url1=draft-ietf-dtn-tcpclv4-15&url2=draft-ietf-dtn-tcpclv4-16&difftype=--html
________________________________
From: Brian Sipos <[e-mail]>
Sent: Wednesday, November 13, 2019 10:53
To: Christopher Wood <[e-mail]>; [e-mail] <[e-mail]>
Cc: [e-mail] <[e-mail]>; [e-mail] <[e-mail]>; [e-mail] <[e-mail]>
Subject: Re: [Last-Call] Secdir last call review of draft-ietf-dtn-tcpclv4-15

Christopher,
Thank you for these comments. My responses are inline below with prefix "BS: ". I have been making edits to a future draft which is not yet uploaded.

________________________________
From: Christopher Wood <[e-mail]>
Sent: Wednesday, November 6, 2019 20:57
To: [e-mail] <[e-mail]>
Cc: [e-mail] <[e-mail]>; [e-mail] <[e-mail]>; [e-mail] <[e-mail]>
Subject: Re: [Last-Call] Secdir last call review of draft-ietf-dtn-tcpclv4-15

Oops. Datatracker butchered the formatting. Let's try that again:

~~~

Reviewer: Christopher Wood
Review result: Has Issues

Comments:
- Section 1.1, fourth bullet: I assume certificate revocation is also out of scope. If so, listing this explicitly might be useful. If not, why not? Relatedly, is pre-shared key authentication supported? If so, perhaps we should mention that this too is out of scope?
BS: I added explicit out-of-scope indication for both revocation and PSK (and other non-certificate) use.

- If TLS sits below TCPCL and if resumption is used, what is the definition of a session's lifetime? Is it still bound to the underlying TCP connection lifetime? I would suggest that the definition be generalized to accommodate TLS, e.g., perhaps the lifetime is bound to the underlying transport state lifetime. Relatedly, is TLS session resumption supported (or encouraged)? When discussing TLS session establishment in Section 4, this seems to be omitted.
BS: I added text to Section 4 to indicate both the lifetime of TLS sessions and the ability to attempt to resume sessions.

- Section 3.2, first paragraph: What does "initiate TLS security" mean, exactly? Does this mean the initiator sends a TLS ClientHello upon TCP connection establishment, or only after some other TCPCL headers are exchanged? Similarly, is the Node ID transferred used in authenticating such a TLS connection? If so, how? Is the Node ID sent as the TLS SNI, as hinted at in Section 4.4.1 and 4.4.2, is it included in the responder's certificate SAN list, etc? I think specific details are needed here, perhaps with forward pointers to Section 4 as needed.
BS: I rephrased the first paragraph and added a reference to Section 4. The text in Section 3 is supposed to be higher-level and informative, where Section 4 is requirements.

- Section 3.2: It's not clear to me how SESS_TERM translates to graceful TLS termination (to avoid truncation attacks). The state machine diagram outlined in Section 3.3 suggests that SESS_TERM, once negotiated, does imply the end of the data transfer. It therefore seems possible for an attacker to truncate data sent between receipt of SESS_TERM and TCP FIN by simply closing the TCP connection. It would be good to require use of a TLS closure alert when finished to avoid this type of truncation. (Maybe BP prevents this by marking data transfer lengths. However, even if that's the case, proper use of TLS seems prudent.)
​BS: I added a requirement to send Closure Alert before shutting down the TLS session. The TCPCL messages are all fixed-length or self-indicated length so there is no possibility of data truncation.

- Section 4, first paragraph: If entities are encouraged to keep sessions alive for as long as possible, guidance on how to update TLS keying material (via key updates or explicitly tearing down the connection and starting anew) seems prudent. TLS has a bound on how much data can be encrypted under one key.
BS: I added text to Security Considerations section to include a reference about key use limits and key updates.

- Section 4: This text:

  Once a TCP connection is established, each entity MUST immediately
  transmit a contact header over the TCP connection.

suggests that TLS does *not* proceed as normal upon TCP connection establishment. This is quite problematic, since any active attacker can simply muck with the ContactHeader CAN_TLS bit to disable TLS, right? Is this threat not considered in scope? Relatedly, what is the threat model? (SSL stripping is mentioned in Section 8, but without mention of a threat model.) "Secure" sessions subject to active downgrade do not offer much in the way of security, and the document should acknowledge that in the Security Considerations. Concretely, how about the following text to replace the second paragraph in Section 8?

   TCPCL does not protect against active network attackers. In particular, an
   active man-in-the-middle attackers to set the CAN_TLS flag to 0 on either
   side of a TCPCL ContactHeader exchange.  This leads to the "SSL Stripping"
   attack described in [RFC7457].

   If TLS is desired for use on any TCPCL network, it is strongly encouraged that
   the security policy disallow use of TCPCL when "Enable TLS" is
   negotiated to false.  This requires that the TLS handshake occurs,
   regardless of the policy-driven parameters of the handshake and
   policy-driven handling of the handshake outcome.
BS: Your statements are definitely true. The purpose of the CAN_TLS flag has been clarified in Section 4.2 "Contact Header", and Section 8 is updated to have a stronger recommendation for policy to require TLS use when TLS is available.
The purpose of the CAN_TLS flag is to allow the use of TCPCL on entities which simply do not have a TLS implementation available. This was a stated goal of the DTN WG due to the desire to use TCPCL in environments where the entities are constrained (TLS is unavailable) but the network is trusted.
Regarding the threat model, this is something I need to look into and find some examples of in other RFCs.

- Section 4.4.2: Why is the recommendation that entities "SHOULD terminate the session" if the peer's certificate is untrusted, rather than "MUST terminate the session"? In what circumstances would an entity not want to terminate the connection? (Later text mentions that this may be allowed by "security policy," in which case we should mention that here.)
BS: I updated these requirements to SHALL and made sure they are all conditioned on policy rules.

- Section 4.4.2, fourth paragraph: Why is host name validation done *after* TLS completes, rather than during the connection? This seems wrong, though I suspect I'm misunderstanding the details.
BS: I updated the text to read "Either during or immediately after the TLS handshake..." The only reason for this is some TLS implementations allow interrupting the handshake but some only allow inspecting the results of the handshake.

- 4.7, Enable TLS: If security policy allows the absence of TLS, why not just always use TLS and have that policy tune TLS peer authentication? (See https://tools.ietf.org/html/rfc7858#section-4.1 for an example of this.) It seems strange that opportunistic security is supported (and desired as a feature?) yet not always used.
BS: Per the earlier response to Section 4 text, the reason to allow non-TLS sessions is purely for constrained entities. The text in Section 4.2 and Section 8 are updated to clarify the CAN_TLS flag to indicate the presence of TLS in the network stack, not really a configuration parameter.

- Section 8: I see no reason why one would want to use TLS for authentication without any form of confidentiality. I would remove text referencing this use case.
BS: I removed this text to avoid confusion.

- Section 8: In describing volumetric DoS attacks, it might help to consider the "opposite" sort of attack, e.g., similar to what the HTTPT/2 data dribble attack exploited? (https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-9511)
BS: This is a good catch, I added Section 8 text to warn against accepting too-small Segment MRU or Transfer MRU parameters.

Nits:
- Section 2.1, TCP Connection: typo in "and other states association" (should be associated?)
BS: Fixed this typo.
- Section 2.1, Transmission Intermediate Progress: typo "transferr" (and elsewhere)
BS: Fixed this typo.
- Inconsistent notation of SESS_TERM (it's referred to as SESSTERM in lots of places)
BS: The "SESS_TERM" was a message while a "SESSTERM" was an activity. I renamed the activity to "ST" to be consistent with other abbreviated activity names.
- Section 3.4, last paragraph: typo "from from"
BS: Fixed this typo.
- Section: typo "negotating"
BS: Fixed several of this typo.