Re: [dtn] draft-ietf-dtn-tcpclv4-02 comments

[Brian Sipos <[e-mail]>]

Matt,

I plan on adding some concrete requirements regarding protocol version negotation, e.g. "A connecting TCPCL endpoint shall always provide the highest TCPCL protocol version on a first session attempt for a listing peer. If a connecting endpoint receives a Version Mismatch shutdown reason, that endpoint may attempt further TCPCL sessions for earlier protocol version numbers in decreasing order."


Do you think this would be sufficient to cover these kinds of situations. Currently, the protocol does not define a unit of connectivity larger than a single session, so there is no concept of a single endpoint memorizing the supported version(s) of its peers and using those memorized versions for later sessions. To me this is starting to get into neighbor discovery territory, which I would like to avoid.

________________________________
From: Matt Wronkiewicz <[e-mail]>
Sent: Friday, July 21, 2017 2:33:08 AM
To: Brian Sipos
Cc: [e-mail]
Subject: Re: draft-ietf-dtn-tcpclv4-02 comments

Brian,

Regarding version mismatch, the reconnection mechanism you described
provides backward compatibility for TCPCLv3. I haven't thought of
anything better for that scenario. Reconnecting with assumptions about
the data sent by the listener could fail in certain, admittedly
pathological, cases.

If the header length is the next field after the version byte and this
part of the structure is maintained across versions, there is another
method for allowing version negotiation post-v4. Each side can send
"dtn!", a header block for every version it supports, and an end tag
(0xff?). Both sides are able to skip incompatible headers because they
can at least determine the header length. Then the session begins
using the highest version that both support. TCPCLv4-only
implementations would just need to skip incompatible header blocks
until they hit the end tag.

Matt

On Wed, Jul 19, 2017 at 5:50 AM, Brian Sipos <[e-mail]> wrote:
> Matt,
> This is some great feedback, I've in-lined responses below and also
> copied the DTN mailing list for their consideration.
>
> On Tue, 2017-07-18 at 00:28 -0700, Matt Wronkiewicz wrote:
>> DTN WG team,
>>
>> I read your draft for TCPCLv4 and look forward to implementing it.
>> Listed are some comments on the version at
>> <https://tools.ietf.org/html/draft-ietf-dtn-tcpclv4-02>:
>>
>>
>> 1. The contact header does not specify its total length. This may
>> cause a problem with header extensions because it is unclear at what
>> point a node should stop waiting for additional data, set the session
>> parameters, and start sending messages or upgrade security. The
>> contact header should include a length field that counts the
>> remaining
>> bytes including extensions.
>>
>> If it does not include a length field, there are some additional
>> ambiguities.
>
> This was definitely an oversight and needs to be added in. The contact
> header is supposed to be fully self-contained data, which right now it
> is not. We could add in an extension item count or an all-extension
> octet size. The size may be more helpful.
> I also didn't catch this earlier because I added extension handling to
> the spec but did not include this in the test implementation; oops!
>
>>
>> 1.a. A critical header extension with no other flags could be
>> misinterpreted as a XFER_SEGMENT message, and vice versa. An
>> implementation must reject XFER_SEGMENT messages that do not follow
>> XFER_INIT with the same transfer id. It must not send them before
>> sending XFER_INIT. This should be clarified in 5.4.3. Data
>> Transmission (XFER_SEGMENT).
>>
>> 1.b. In Table 2: Header Extension Item Flags, several reserved bits
>> must be zero, or the header extension would collide with other
>> message
>> types that are allowed after the session is established.
>>
>>
>> 2. In Section 4.2. Validation and Parameter Negotiation, 3rd
>> paragraph, if two nodes send different version numbers in the contact
>> header, it is specified that one of the nodes must send a SHUTDOWN
>> with a version mismatch. There is no opportunity for the other side
>> to
>> downgrade to the older version. There is also no way for the older
>> version to negotiate a session, because it has not received the
>> connection flags and cannot interpret the remote EID.
>>
> I can add some guidance to this section. There is no mechanism for in-
> TCP-connection downgrade, so the recommended behavior is to re-connect
> on the same TCP port but then provide a TCPCLv3 contact header and see
> if the listening agent accepts that instead. A similar procedure could
> be use to try TCPCLv3 first then v4 after. The listener (TCP server)
> has discretion to look at the start of the contact header before
> sending its only reply header.
>
>>
>> 3. The inability for two nodes supporting multiple TCPCL versions to
>> handshake is concerning. There are very few TCPCLv3 installations, so
>> backward compatibility is less of a worry. But if TCPCLv4 is
>> successful it will be widely deployed when there is a need for a new
>> version. It would be good to provide some mechanism for nodes that
>> implement both TCPCLv4 and v5 to establish a connection with a node
>> that only implements v4.
>>
>> One way to allow forward compatibility would be to require v4 nodes
>> to
>> send a VERSION_MISMATCH in response to a newer version, after which
>> the newer node may send a compatible contact header or disconnect.
>>
> Similar to above, do you think the one-version-per-connection-attempt
> is acceptable? Some additional informational language could be added to
> explain this situation.
>
>>
>> 4. Nodes implementing TCPCLv4 will presumably already have functions
>> for CBOR encoding and decoding. Restructuring the messages to use it
>> would remove a lot of padding. It would also provide more flexibility
>> and robustness for adding header extensions and new protocol
>> versions.
>> I recognize this is a drastic change that you have probably already
>> discussed.
>>
> We definitely had discussed the original proposed SDNV vs. CBOR vs.
> fixed-length-fields and settled on current fixed-length due to the
> convergence layer being a lower-level protocol than BP itself. This was
> due to concern about complexity in a TCPCL implementation. There is
> also the possibility, I believe, of a pure bundle forwarder which
> speaks TCPCL but does not actually examine the bundle contents and
> simply forwards bundle, as opaque data, to some actual BP node.
>
>>
>> 5. Table 13: SHUTDOWN Reason Codes is missing "Resource Exhaustion",
>> listed in Table 8: SHUTDOWN Reason Codes.
>>
>>
>> 6. In Table 8: SHUTDOWN Reason Codes, in the description for Resource
>> Exhaustion, the word "resource" is misspelled.
>>
> These will be corrected.
>
>>
>> 7. The shutdown procedure is not specified when one node requires TLS
>> and the other does not set USE_TLS. It is unclear whether the
>> strict-TLS node should send SHUTDOWN with "Contact Failure" or "TLS
>> Failure". A more descriptive "TLS Required" failure code could be
>> added.
>>
> I agree that we should distinguish between "TLS Agreement Failure" when
> the endpoint policy disallows the agreed Enable TLS state, and "TLS
> Handshake Failure" where TLS is enabled and attempted but the handshake
> itself fails.
>
>>
>> 8. In section 4.1.1. Header Extension Items, in the entry for Flags,
>> the word "unknown" is misspelled.
>>
> This will be corrected.
>
>>
>> Thank you for your work on the DTN WG, and for considering my
>> feedback.
>>
>> Matt Wronkiewicz