[dtn] AD review of draft-ietf-dtn-tcpclv4-12

[Magnus Westerlund <[e-mail]>]

Hi,

 

I have now performed my AD review of draft-ietf-dtn-tcpclv4-12. I think most
are minor comments, however the TLS and security related ones may be more
problematic to resolve. I will now be on vacation so you know you will not
receive any quick replies from me before the end of the month.

 

1.	Section 1: What are the applicability of TCPCLv4 to BPv6? I wonder
as RFC5050 is referenced. 


2.	Section 1.1: Session State Changed. Some editorial issues. Missing
":" initially. Then double ".." on Terminated. 


3.	Section 1.1: 
   Session Idle Changed  The TCPCL supports indication when the live/idle
sub-state changes.  This occurs only when the top-level session state is
Established.  Because TCPCL transmits serially over a TCP connection, it
suffers from "head of queue blocking" this indication provides information
about when a session is available for immediate transfer start.

So in which direction are this change indicated/reported, both or only in
one of them, or any as implied by Section 2.1's definition of Live Session? 


4.	Section 1.1: Transmission Intermediate Progress: Segment is not
defined prior to this. Maybe a forward pointe? Or should maybe the whole
subsection (1.1) be moved to after definitions? 


5.	Section 2: Is there a point to use the RFC 8174 language that makes
only capital words have special meaning? 


6.	Section 3.1: "One of these parameters is a singleton endpoint
identifier for each node (not the singleton Endpoint Identifier (EID) of any
application running on the node) to denote the bundle-layer identity of each
DTN node."

The above quote does imply something that at least BPBis isn't making clear
that a particular application agent would have its own EID. It is not clear
that there are a one-to-one relationship between bundle nodes and
application agents. Can you please clarify what the relationship is and lets
figure out if that needs to be clarified back to BPbis. 


7.	Section 3.1: "Bundle interleaving can be accomplished by
fragmentation at the BP layer or by establishing multiple TCPCL sessions
between the same peers."

Are there clear rules established for how many TCPCL sessions in parallel
that may be established? By the end of my reading this is unanswered. 


8.	Section 3.1: XFER_REFUSE does that indicate that the bundle has
already been received. How else does one separate other reasons for refusing
a bundle versus that one have received it prior?


9.	Section 3:1:    Once a session is established established, TCPCL is
a symmetric protocol between the peers. 
Double established


10.	Figure 4: "Close message" is this TCP level message, if that is the
case can that be clarified by prefix with TCP?


11.	Section 3.2: 
   Notes on Established Session states: 
      Session "Live" means transmitting or reeiving over a transfer
      stream.

      Session "Idle" means no transmission/reception over a transfer
      stream.

     Session "Closing" means no new transfers will be allowed.

Note that "Closing" is not used in Figure 4, it is called ending. Note
spelling error on "live" receving.


12.	Section 3.2: Figure 5 and 6 uses PCH without explanation. Figure 5
could probably also benefit by expanding CH as Contact Header.


13.	Figure 8 and 10: Uses [SESSTERM] is this the same as using the
SESS_TERM message, or some other procedure. Please clarify. 


14.	Section 3.3: "   Many other policies can be established in a TCPCL
network between these two extremes."

The list above includes three items, so the two extremes needs to be
enumerated. 


15.	Section 4.1: Can TCPCLv3 and TCPCLv4 coexist on Port 4556? Based on
9.1 the answer is yes, please clarify here.


16.	Section 4.1: "Therefore, the entity MUST retry the connection setup
no earlier than some delay time from the last attempt, and it SHOULD use a
(binary) exponential backoff mechanism to increase this delay in case of
repeated failures."

Any recommended upper limit for the backoff?


17.	Section 4.2:    Version:  A one-octet field value containing the
value 4 (current version of the protocol). 

I think the use of "the protocol" is unclear, maybe call it "the TCP
convergence layer". This to avoid confusion with BPv7. 


18.	Section 4.2: Please define how to set and ignore reserved bits in
the Flags field so that it may be extended in the future. 


19.	Section 4.3 and 4.4: Due to how the Contact Header relate to TLS
there is clear risk for a TLS stripping attack where the CAN_TLS flag is
cleared. I think there need to be some thought about mitigation of this
weakness. Depending on the expected mix of entities and their capabilities
one can either have policy for a deployment where one mandates TLS being
used, thus preventing the bid-down by not being according to policy. It is
more difficult to mitigate in a deployment where one have some entities that
doesn't support TLS, unless one can some way securely learn which entities
support it or not and thus can detect the manipulation. One can potentially
also first attempt to do a TLS handshake for the best version one supports.
Then run the CH inside the TLS to prevent TCPCL version and other flags to
be manipulated. But that doesn't solve the down-bid. I did note the
negotiation in Section 4.7 and the relation to Security Policy. Maybe the
solution is to write some text on the risk of TLS striping in Section 8 and
add forward pointers in 4.7 to that risk. 


20.	Section 4.4: Dealing with new TLS versions. BCP 195 does not appear
to me to define how to deal with newer versions. However, as TLS 1.3 already
exist I think this is from the start a relevant question. 


21.	Section 4.4: So what about entity authentication? Will the TCPCL
entity have a name / identity that can be authenticated so that one know
that one are talking to the right entity. And is the solution for this a
classical PKI, or something else? Also does the passive entity expect the
active (TLS client) to authenticate itself also? 


22.	Section 4.8: Please define how to set and ignore the reserved bits.


23.	Section 4.5: Based on that many later sections just refer to the
Message Header, shouldn't the section title for section 4.5 be Message
Header? Now the first mentioning of "Message Header" are in Figure 16's
title. 


24.	Section 6.1:
   After sending a SESS_TERM message, an entity MAY continue a possible
   in-progress transfer in either direction.  After sending a SESS_TERM
   message, an entity SHALL NOT begin any new outgoing transfer (i.e.
   send an XFER_SEGMENT message) for the remainder of the session.
   After receving a SESS_TERM message, an entity SHALL NOT accept any
   new incoming transfer for the remainder of the session. 

To me it seems that the above paragraph contains one contradiction. The
parenthesis in the second sentence (i.e. send an XFER_SEGMENT message)
appears to be false. Because as the beginning of the sentence implies that
it can't start a new transfer. However SESS_TERM is allowed to be inserted
in between XFER_SEGEMENT messages for a transfer ID, i.e.
XFER_SEGMENT(ID=7), SESS_TERM, XFER_SEGMENT(ID=7, end) is an allowed
sequence. Can you please clarify.


25.	Section 8. 
If this identifier is used outside of a TLS-secured session or 
   without further verification as a means to determine which bundles
   are transmitted over the session, then the node that has falsified
   its identity would be able to obtain bundles that it otherwise would
   not have.

I don't see how a entity could trust the in SESS_INIT provided EID more just
because it is in TLS unless there are some mechanism for binding the EID to
the TLS session endpoint (client or server). Some type of authentication is
needed to prove the identity. 


26.	Section 8.
Therefore, an entity SHALL NOT use the EID value of an
unsecured contact header to derive a peer node's identity unless it
   can corroborate it via other means.

The EID value is not part of the CH, only SESS_INIT. Is this a left over
from TCPCLv3 or earlier versions?


27.	Section 8. Needs text on  TLS stripping attack due to the optional
TLS usage.


28.	Section 9: "In this section, registration procedures are as defined
in [RFC8126]." . defined as in .


29.	Section 9: "Some of the registries below are created new for TCPCLv4
but share code values with TCPCLv3."

I don't think "share" is the right word here. Maybe "Some of the registries
have been defined as version specific to TCPCLv4, and imports some or all
codepoints from TCPCLv3."


30.	Section 9.3: Is RFC Required unnecessary strict? Considering the
quite large name space, I would think that specification required would be a
suitable policy? Just trying to understand what you think is gained by
having someone publish the extension as an RFC, in any stream including the
independent. 


31.	Section 9.4: Same question as for 30)


32.	Section 9.5: Here RFC required may be suitable. However, does it
make sense to allocate 2 or 4 points also here for Private/Experiments?


33.	Section 9.6: Here I would consider Expert Review with a
specification requirement. I also do not understand why so much is reserved,
a reason for that? 


34.	I would expect that the policy for 9.6-9.8 to be aligned so may
require change if change is decided on 33. 


35.	A big question I have after having read this document is how one
discover / determine the IP+port(s) to connect to for a given EID. Is this
currently completely deployment specific? And how bound is this to Bundle
routing? 

What may be missing is the section that tells the intended implementor that
in addition to follow this specification you do need a solution to the
following things, like for example authentication framework that allows one
to verify the TLS connects to EID mappings. Or an address resolution
protocol that maps EID to IP address and port pairs for cases when routing
simply give you Forward this bundle to EID using TCPCLv4. 

 

Cheers

 

Magnus Westerlund