Re: [dtn] Mirja Kühlewind's Discuss on draft-ietf-dtn-tcpclv4-18: (with DISCUSS and COMMENT)

[Brian Sipos <[e-mail]>]

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

On Thu, 2020-02-20 at 05:06 -0800, Mirja Kühlewind via Datatracker wrote:
> Mirja Kühlewind has entered the following ballot position for
> draft-ietf-dtn-tcpclv4-18: Discuss
> 
> 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/
> 
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> Thanks for this well-written document. I have a couple of thing in the comment
> section below that should be clarified. But there is one point which does not
> seem correct to me and therefore I'm raising it at thee discuss level:
> 
> Sec 5.1.1:
> "Both sides SHALL send a KEEPALIVE message whenever the negotiated interval
>    has elapsed with no transmission of any message (KEEPALIVE or other).
> 
>    If no message (KEEPALIVE or other) has been received in a session
>    after some implementation-defined time duration, then the node SHALL
>    terminate the session by transmitting a SESS_TERM message (as
>    described in Section 6.1) with reason code "Idle Timeout". "
> 
> It is not necessary that both endpoints send keepalives when TCP is used
> underneath as data is transmitted reliably. If one end sends keepalives and
> transmission fails it will close the TCP connection no matter what. Therefore
> the case where no keepalive is received, can only happen if no keepalive was
> send by the application, however, if the own keepalives are send successfully
> it is also received and the TCP connection is alive. If this is only to test
> liveness of the TCP connection, why don't you use TCP keepalives instead?
> 
> Further what happens when a keepalive fails? Should one endpoint try to
> reconnect, immediately or later when new data is available?
> 
BS: You are correct that it is not strictly necessary that both sides transmit
KEEPALIVE messages to keep the TCP connection alive. The current KEEPALIVE
behavior is inherited from TCPCLv3 and has not had any (positive or negative)
discussion in the WG so it was just kept as-is.
It's not explicitly stated, but there are no restrictions on when to re-
establish a session to an entity which has lost a session due to Idle Timeout.
Depending on local BP agent policy a preemptive session may be reinstated or the
agent may wait until a transfer is needed.

> And one more small thing:
> sec 6.1:
> "However, an entity MUST always send
>    the contact header to its peer before sending a SESS_TERM message."
> This normative requirement seems contradicting the way version failures are
> handled earlier on in the doc.
> 
BS: The contact header is always the first thing sent from either side of a new
TCP connection intended to establish a TCPCL session. A passive entity can
choose to close the TCP connection rather than sending a contact header if some
security policy dictates that, but an entity cannot send anything before a
contact header.
The graceful way to refuse a connection at the start is to send a contact header
(with some desired version number, but it really doesn't matter) and then either
close the TCP connection or send a SESS_TERM. This is described in section 4.3
"Contact Validation and Negotiation". I don't see the discrepancy between these
sections.

> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> 1) Sec 4.1:
> "Therefore, the entity MUST retry the
>    connection setup no earlier than some delay time from the last
>    attempt."
> It would be good to provide a recommended value or at least a minimum value.
> 
BS: This would seem to be quite network dependent. I can see making a
recommendation for a minimum initial delay of 1 second, but that's quite
arbitrary.

> 2) Similar here in sec 4.1:
> " If the
>    passive entity does not receive a Contact Header after some
>    implementation-defined time duration after TCP connection is
>    established, the entity SHALL close the TCP connection."
> It would be good to provide some default value or at least some more
> discussion
> about ranges of values. In any case this value should be larger than X times
> the RTT as TCP segments can get lost any might need to be transmitted. I guess
> something in the order of second would be appropriate?
> 
BS: For an earlier comment I added a very wide bound for this:
Entities SHOULD choose a Contact Header reception timeout interval no 
longer than 10 minutes (600 seconds).

> 3) Sec 4.3:
> " To prevent a flood
>    of repeated connections from a misconfigured application, an entity
>    MAY elect to hold an invalid connection open and idle for some time
>    before ending it."
> Not sure why you need to hold it open...? All you need is to not accept but
> ignore new connections from that peer/IP address for some time. And more
> questions:
>   - When kept open and you suddenly received a valid message, should you
>   process it? - And when  you finally decide to close the connection, how do
>   you do that? TCP RST, or FIN? - Do you send (TCP) keepalives?
> 
BS: This requirement was a hold over from TCPCLv3 and I'm not exactly sure the
rationale either. It does make sense to handle this as a firewall-type rule to
disallow new connections from that peer. I don't know that this warrants a TCPCL
requirement though, that seems more generic firewall activity.

> 4) Also 4.3:
> "   The first negotiation is on the TCPCL protocol version to use.  The
>    active entity always sends its Contact Header first and waits for a
>    response from the passive entity.  The active entity can repeatedly
>    attempt different protocol versions..."
> If you read on in this section it seems like the receiver always sends a
> SESS_TERM if the version is not compatible. So I assume you mean the active
> entity can open a TCP and try again. And I assume it should do it immediately
> after the SESS_TERM is received. I believe that's what the following
> paragraphs
> say but this part confused me a bit. So might be only an editorial issue.
> Please clarify! However, if there would be any attempt to send another CH on
> the same TCP connection, that could lead to ambiguity and would need to be
> further specified. Also more point, the text does not specify what the
> receiver's response to the CH is. The figure shows that it will also send a
> CH,
> however, that should also be spelled out in the text!
> 
BS: I edited this paragraph from an earlier comment and clarified that a contact
header send is one-for-one with a TCP connection. The Section 4.1 was also
updated to have specific sequencing SHALL statements.
Here is the new Section 4.3 text for reference:

The first negotiation is on the TCPCL protocol version to use.
The active entity always sends its Contact Header first and waits for
a response from the passive entity.
During contact initiation, the active TCPCL node SHALL send the highest TCPCL
protocol
version on a first session attempt for a TCPCL peer.
If the active entity receives a Contact Header with a lower
protocol version than the one sent earlier on the TCP connection,
the TCP connection SHALL be closed.
If the active entity receives a SESS_TERM message with reason of
"Version Mismatch", that node MAY attempt further TCPCL sessions with the
peer using earlier protocol version numbers in decreasing order.
Managing multi-TCPCL-session state such as this is an implementation matter.

> 5) sec 4.4.1:
> "When present, the SNI SHALL contain the same host name
>       used to establish the TCP connection."
> Not sure what this means... how do you use an host name in an TCP connection?
> Do you mean the same host name that has been used in name resolution to get
> the
> IP address that is used to establish the TCP connection? Or something else?
> 
BS: Yes, I've combined the two requirements here to read:

When a resolved host name was used to establish the TCP connection, the TLS
ClientHello SHOULD include a Server Name Indication (SNI) in accordance with
[RFC6066] containing that host name (of the passive entity) which was resolved.

> 6) sec 5.1.2:
> What is the entity receiving the MSG_REJECT supposed to do?
> 
BS: The intent of this message is for troubleshooting only. A likely use of
MSG_REJECT is that the message type was unknown to the peer and the reject
sender has already closed the TCP connection.

> 7) sec 5.2.3: General question on the ACK mechanism: As you say correctly the
> fact that you don't get a notification is not a TCP protocol matter but only
> an
> interface matter. E.g. if taps would be used, this information would be
> available. Was it considered to make the ACK mechanism optional, e.g. by using
> a flag in the XFER_SEGMENT to request an ACK per segment? Also more questions:
>  - Why are the message flags reflected?
>  - Why is the Acknowledged length field needed if there needs to be one ACK
>  (send in order) for each segment anyway?
> 
BS: The use of XFER_ACK is supposed to indicate a stronger state than just "the
peer received these messages on the TCP stream. It's supposed to indicate that
the segment of data has been fully processed, which includes indication to the
BP agent (and potentially persistence in the agent for reactive fragmentation
purposes).

There was some discussion about negotiating the use of ACK or similar parameters
but the WG consensus was to keep the protocol with as few negotiated behaviors
as possible. The Segment MRU parameter and associated XFER_SEGMENT data size
determine how much data each XFER_ACK covers so that is in a way negotiation of
"when to use XFER_ACK". As stated elsewhere in the document, it is possible to
send an entire transfer in a single segment which is the least overhead for
XFER_SEGMENT and the least number of XFER_ACK per transfer (exactly one).

The repetition of the flags is not strictly necessary but is a holdover from
TCPCLv3. The ACK length is also a holdover and is redundant as you mention, but
it is helpful from a troubleshooting perspective.

> 8) sec 5.2.4: Can you maybe explain why the XFER_REFUSE is a message/feature
> of
> the CL and not the BP?
> 
BS: It's really just to allow interrupting a transfer at a lower layer than the
BP agent for data handling issues like insufficient storage. It also allows a
large bundle transfer to be interrupted if the receiver won't accept it for
routing or other reasons after the bundle header is received and decoded (but
the rest of the bundle payload is not yet received).

> 9) sec 5.2.4:
> "If a sender receives a XFER_REFUSE message, the sender
>    MUST complete the transmission of any partially sent XFER_SEGMENT
>    message.  There is no way to interrupt an individual TCPCL message
>    partway through sending it."
> Not sure if use of normative language is appropriate here, because I believe
> what you mean is that as soon as the data/message is given to the TCP
> connection, you can't call it back. That's just a fact and nothing the sender
> may or can enforce. However, it could reset the TCP connection effectively but
> that probably not what is desired. Please clarify or remove normative
> language!
> 
BS: This text was a holdover from earlier TCPCLv3. I agree that this is not
strictly a protocol requirement, but it is an important implementation concern
that once the header portion of a message is sent the entire message needs to
follow so that the receiver can properly decode the subsequent messages.
Would simply replacing the "MUST" with a "has to" be reasonable?

> 10) sec 5.2.5.1: Can you further explain what the use case ifs for the
> Transfer
> Length Extension? When do you expect the actual length to be different?
> 
BS: The purpose is not to provide redundant data but to indicate the total
length to the receiver at the start of transfer so that the receiver may
validate or pre-allocate storage for the transfer.
In a trivial implementation each transfer is a single segment and the total
length is implied by the segment data length.
This was made into an optional extension because there are some uses of TCPCL
which either use single-segment transfers or which don't need to pre-allocate
storage.

> 11) sec 6.1:
> "   After sending a SESS_TERM message, an entity MAY continue a possible
>    in-progress transfer in either direction."
> Why is this necessary? Why can the entity not just send the SESS_TERM after
> the
> end of the transfer? Please clarify in the doc!
> 
BS: I'm adding text to Section 6 to clarify, but here is the paragraph:

This section describes the procedures for terminating a TCPCL session.
The purpose of terminating a session is to allow transfers to complete
before the session is closed but not allow any new transfers to start.
A session state change is necessary for this to happen because transfers
can be in-progress in either direction (transfer stream) within a session.
Waiting for a transfer to complete in one direction does not control
or influence the possibility of a transfer in the other direction.
Either peer of a session can terminate an established session at any time.

> 12) 6.1:
> " When performing an unclean shutdown, a receiving node
>    SHOULD acknowledge all received data segments before closing the TCP
>    connection."
> What do you mean here? TCP acknowledgements? If so, this should not be
> normative as TCP will do that not matter what. However, you can not send any
> new application data/CL ACK after a TCP FIN was sent. Please clarify!
> 
BS: I clarified this to "... SHOULD acknowledge all received XFER_SEGMENTs with
an XFER_ACK ..." Does this make sense?

> 13) sec 6.1:
> "   If a session is to be terminated before a protocol message has
>    completed being sent, then the node MUST NOT transmit the SESS_TERM
>    message but still SHALL close the TCP connection. "
> Not sure how this case is supposed to happen. When give a message to your TCP
> stack it will send it. If sending fails on the TCP level, the connection will
> be closed automatically. Or do I misunderstand the intended scenario here?
> 
BS: This requirement really applies to a situation where an entity really really
needs to end a session promptly and doesn't have time to wait for an earlier
message to complete and the SESS_TERM handshake to finish. Maybe the word
"terminated" here needs to be replaced by some other term? "closed"?