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

["Christopher Wood" <[e-mail]>]

Thanks for the updates, Brian! And apologies for the delay in my response. I reviewed the diff and the changes look good to me. I only have one nit: I might rename the title of section 8.4 to "Active Transport Security Stripping" to distinguish this from the previous passive attacks.

Best,
Chris

On Mon, Nov 25, 2019, at 1:39 PM, Brian Sipos wrote:
>  
>  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.
>