Re: [dtn] Roman Danyliw's No Objection on draft-ietf-dtn-tcpclv4-18: (with COMMENT)

[Brian Sipos <[e-mail]>]

Roman,
I'm including responses below with prefix "BS: "

On Wed, 2020-02-19 at 12:13 -0800, Roman Danyliw via Datatracker wrote:
> Roman Danyliw has entered the following ballot position for
> draft-ietf-dtn-tcpclv4-18: No Objection
> 
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
> 
> 
> Please refer to 
> https://www.ietf.org/iesg/statement/discuss-criteria.html
> 
> for more information about IESG DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-dtn-tcpclv4/
> 
> 
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Thanks for all of the changes made in response to the LC SECDIR review.  Also,
> thank you for the LC SECDIR review, Chris (Wood)!
> 
> A reference implementation (as noted in [github-dtn-bpbis-tcpcl]) that
> includes
> a Wireshark dissector is phenomenal (and a bar we should all strive towards
> when making a new protocol)
> 
> ** Section 2.1.  In the definition of the TCPCL Session, “there are two
> possible transfer streams; one in each direction, with one stream from each
> entity being the outbound stream and the other being the inbound stream.”,
> what
> does this imply about the underlying TCP connections?  It would be worth being
> clearer on the relationship between a given stream and the TCP connection.
> 
BS: I will add clarification to this, but a single TCPCL session is a single TCP
connection and always one-to-one in TCPCL.

> ** Section 3.1. Is the list of provided convergence layer services enumerated
> in this section unique to the TCPCL, or would it be expected that all CLs
> would
> implement it?  If the latter, then why isn’t it in the draft-ietf-dtn-bpbis?
> 
BS: This was a discussion topic in the WG and it was decided that the various
CLs will have unique enough set of BP agent interfaces that there isn't a
universal set of services other than "attempt to send this bundle" and "this
bundle was just received". Even then some CLs may be unidirectional and only
support one of those services (per-instance).

> ** Section 3.2. Per “Each transfer is performed by an sequence of logical
> segments of data …”, what is the relationship between a “logical segment” and
> a
> “transfer segment” (defined in Section 2.1)?
> 
BS: I changed the wording on this for clarity to read:
Each transfer is performed by segmenting the transfer data into 
one or more XFER_SEGMENT messages.

> ** Section 3.3. Figure 4.  What is the state transition from “Established
> Session Live” to “Established Session Ending”?  Wouldn’t a session go from
> live
> to idle when the transfer is done (and then session ending)?  Is the Session
> live to Session ending perhaps due to an interrupt or termination request
> while
> the transfer is underway?
> 
BS: You are correct that a session can change from Live to Idle when a transfer
finishes. The purpose of the Ending state is simply to inhibit start of new
transfers. The Ending state is entered solely by sending a SESS_TERM message. I
clarified the logic of the Ending state based on earlier comments to read:
  Once a SESS_TERM message is sent the state of that TCPCL session
  changes to Ending.
  While the session is in the Ending state, an entity MAY finish an in-progress
  transfer in either direction.
  While the session is in the Ending state, an entity SHALL NOT begin any new
outgoing
  transfer for the remainder of the session.
  While the session is in the Ending state, an entity SHALL NOT accept any new
incoming
  transfer for the remainder of the session.

> ** Section 4.2.  Given that this new version of the convergence layer is a
> “green field” (judging from the list if implementations), why not require TLS
> v1.3 as the minimum (rather than TLS v1.2)?
> 
BS: Based on other review comments and AD comment, I am updating this spec to
use TLS 1.3 as the baseline. The only real required compatibility behavior is
the ClientHello message which is identical in form between the TLS versions.

> ** Section 8.6. (as suggested by Chris in his telechat SECDIR review) This
> section isn’t clear on the threat.  It seems to cover material relevant to
> validation better aligned with Section 4.4.2.  In particular, the reference to
> RFC5280 and reminder to check CRLs would be needed guidance to added to the
> Section 4.4.2 paragraph, “Any certificate received during TLS handshake SHALL
> be validated up to one or more trusted certificate authority (CA) certificates
> …”
> 
BS: I clarified the threat text with the following statements at the top:
Even when TLS itself is operating properly an attacker can attempt to exploit
vulnerabilities within certificate check algorithms or configuration 
to establish a secure TCPCL session using an invalid certificate.
A BP agent treats the peer Node ID within a TCPCL session as authoritative and
an invalid certificate exploit could lead to bundle data leaking and/or denial
of service to the Node ID being impersonated.

> ** Editorial
> Section 2.1. Typo. s/entitys/entities/
> 
> Section 2.1. Typo. s/passivley/passively/
> 
> Section 2.1. Typo. s/trasnfer/transfer/
> 
> Section 2.1. Typo. s/Inititated/Initiated/g
> 
> Section 3.1. Typo. s/defintion/definition/
> 
> Section 3.1. Typo. s/reciver/receiver/
> 
> Section 3.1. Typo. s/transmssion/transmission/
> 
> Section 3.2. Typo. s/negligable/negligible/
> 
> Section 3.2. Typo. s/by an sequence/by a sequence/
> 
> Section 3.3. Typo. s/reeiving/receiving/
> 
> Section 3.3. Typo. s/Recevied/Received/
> 
> Section 3.4.  Typo. s/polcy/policy/
> 
> Section 4.2. Typo. s/contanct/contact/
> 
> Section 4.2. Typo. s/behavor/behavior/
> 
> Section 4.4.1. Typo. s/assoicated/associated/
> 
> Section 4.4.2.  Typo. s/unauthticate/unauthenticated/
> 
> Section 4.4.3.  s/Connnection/Connection/
> 
> Section 4.6. Typo. s/ mulitplicity/ multiplicity/
> 
> Please run a spell-check on the document.  Additional typos were found past
> Section 4.6 but were not documented here
> 
BS: I did run a spellcheck on the document and hope that I caught all of these
and some others.