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

From: Marius Feldmann <[email-masked]>
Date: Sun, 12 Nov 2017 17:21:34 +0100

Hello,

below you will find some further comments about TCPCLv4 as described in [1].

Regards

Marius

[1] https://tools.ietf.org/html/draft-ietf-dtn-tcpclv4-02

----

Abstract:
* "Bundle Protocl" -> "Bundle Protocol"
* "...updates to the Bundle Protocol contents, encodings, and
convergence layer requirements in Bundle Protocl Version 7." - This
statement is not precise in my opinion. The current BP 7 draft only
provides general requirements in regards to the CL. Which precise
requirements does this statement point to?

Page 1:
* Page numbers provided in the table of contents are not correct
(increment by 1 to correct them).

Page 2:
* Maybe TCPCL could be motivated a bit better (currently it is just
stated that BP needs a CL)? Which precise problem(s) does TCPCL solve?

Page 3:
* "The locations of the TCPCL and the BP in the Internet model protocol
stack are shown in Figure 1." - Which protocol stack model is shown in
figure 1? Usually, we should stay with the one provided in RFC 1122
(which does not mention a physical layer and a presentation layer). It
seems this figure shows a combination of different models.

Page 4:
* Capitalized "Session" - should be written with lower case "s".
* Long form of "MRU" should be stated.
* Maybe the section "3.  General Protocol Description" should be
restructured into a section "3.1 Overview" (including the current
section 3.1) and "3.2 Example Message Exchange". Currently, the
structure is a bit strange with a long section introduction and a short
section 3.1. - in my opinion this could be merged to an "overview"
section 3.1.
* "XFER_INIT" may be mentioned in the section "3. General Protocol
Description" due to its appearance in figure 2.

Page 6:
* "The following figure visually depicts..." - Remove "visually". A
figure is a visual entity by definition, isn't it?

Page 7:
* Maybe figure 2 should also point out the parameters used in the
contact header?

Page 8:
* "Destination port number 4556 has been assigned by IANA as the
well-known port number for the TCP convergence layer." - Ports between
1024 and 49151 are named "Registered Ports", not "well-known", see RFC 6335.

Page 10:
* I was wondering if the "CAN_TLS" flag (which is actually the only
supported flag) could not be expressed as header extension item. If
expressed as header extension item it is also rendered possible to
differentiate between optional or enforced TLS.

Page 11:
* "reason code" is mentioned without introducing the concept globally,
maybe it should be introduced before or at least a reference should be
provided.
* "Each type This specification does..." - "Each type" should be
removed, I guess.

Page 13:
* "The message types defined in this specificaiton are listed in Table
3." - "specificaiton"; furthermore the information about table 3 is
redundant with the sentence before.

Page 21:
* "receving" -> "receiving", "DATA_SEGEMNT" -> "DATA_SEGMENT"

Page 22:
* "As bundles can be large, the TCPCL supports an optional mechanism by
which a receiving node MAY indicate to the sender that it does not want
to receive the corresponding bundle." - As it is stated in the next
sections, there are more reasons for transfer refusal besides the "size
of the bundle". Thus, the section may not be introduced in this way.

Page 26:
* "After receiving a SHUTDOWN message in response, both sides MAY close
the TCP connection." - Why is it "MAY", not "MUST"? The only reason may
be a reuse of the connection for a further TCPCL session. Before it is
mentioned that a contact header is sent via a new TCP connection (e.g.
page 9 "Once a TCP connection is established, both parties exchange a
contact header."). Which sence does it make to keep the TCP connection?
Either the "MAY" should be transformed to a "MUST" or the state in which
a contact header may be sent should be clarified.

----

On 19.07.2017 14:50, Brian Sipos 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
> _______________________________________________
> dtn mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/dtn

---

From: Brian Sipos <[email-masked]>
Date: Sun, 26 Nov 2017 10:47:32 +0000

Marius,

Thank you for these comments. I've been working through addressing them as well as I can.

Two of these need some further discussion though:

  *   Your comment about the USE_TLS flag is valid, but there had been discussion at an earlier IETF of whether the TLS behavior of TCPCLv4 is an integral part of the protocol and the conclusion was that it is integral to TCPCL and is to be mandatory-to-implement (but not mandatory-to-use). For this reason it's treated differently than any other extension which may come down the road.
  *   The comment about SHUTDOWN sequencing brings to light a possible consistency issue with the current spec and also with TCPCLv3. As it currently stands there is really no hard condition for closing a TCP connection after sending or receiving a TCPCL SHUTDOWN message. Since as currently stated (which is a carry-over from TCPCLv3) a node may send a SHUTDOWN but then continue to send a huge number of remaining XFER_SEGMENT messages, the receiver has no good logic to distinguish between a shutdown-but-wait-for-data behavior vs. a shutdown-and-close-connection behavior. I suppose the TCPCLv4 spec could explicitly mention both of these options as valid choices which would explain the "MAY" condition of Page 26.

________________________________
From: dtn <[email-masked]> on behalf of Marius Feldmann <[email-masked]>
Sent: Sunday, November 12, 2017 11:21
To: [email-masked]
Subject: Re: [dtn] draft-ietf-dtn-tcpclv4-02 comments

Hello,

Regards

Marius

----

Abstract:

Page 1:

Page 2:

Page 3:

Page 4:

Page 6:

Page 7:

Page 8:

Page 10:

Page 11:

Page 13:

Page 21:

Page 22:

Page 26:

----

>> Thank you for your work on the DTN WG, and for considering my
dtn Info Page - Internet Engineering Task Force<https://www.ietf.org/mailman/listinfo/dtn>
www.ietf.org
"This list is for discussions related to the formation of a Delay Tolerant Networking (DTN) working group. The IRTF DTNRG research group has worked on the particular ...

_______________________________________________
dtn mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/dtn
dtn Info Page - Internet Engineering Task Force<https://www.ietf.org/mailman/listinfo/dtn>
www.ietf.org
"This list is for discussions related to the formation of a Delay Tolerant Networking (DTN) working group. The IRTF DTNRG research group has worked on the particular ...

---

From: Victoria Pritchard <[email-masked]>
Date: Tue, 28 Nov 2017 00:04:48 +0000

Hi Brian, everyone,

Sorry for the delay, as I promised this a few days after the DTN meeting in
Singapore. Here's my comments on the TCPCLv4 draft, covering my main issues
with the draft and questions about the protocol....it's based on version 2
of the draft - you may have addressed some of these points already...

In general I think there's a lot of detailed explanation in the overview
sections which I think would be better moved into the more specific
sections later. There is some crossover in later sections too. Having info
in different places ends up causing confusion as they can seem to
contradict each other. For example, I think there’s a lot of scattered info
about shutdown and closing of sessions/connections. Also in Section 3, as a
general overview, you don’t need that much information – it could be
simplified a lot by taking out the MUSTs and MAYs, and leave the detail on
exactly how each feature works in the relevant sections later in the draft.

In Section 3 you might also want to mention TLS, signals to/from other
processes, the fact a shutdown can include a reason.

There are a few places where this draft mentions activities I don’t think
are the responsibility of a CL, or if they are, doesn't give enough
information. Should be clear about the scope of TCPCL, discussing in the
early part of the draft, and being clear throughout. Particularly regarding
bundle fragmentation, routing, mapping endpoint ID to IP address, and
reporting failures. For example, Section 3 mentions “routing and forwarding
messages” and fragmentation (also in 4.1). Section 4 mentions “find an
alternate route”. And also be clear about interactions with other processes.

I had a lot of questions around acknowledgements….If all the acks are
received for a particular transfer, do we tell BP that the bundle was
delivered? If not, do we tell BP the transfer failed? How long do we wait
for acknowledgements? What action is taken for unacknowledged packets? Why
ack every single segment? Could we get into a situation where we started
sending a second bundle with a new transfer ID but then realised part of
the first one was not received? The draft says you can't interleave data
from different bundles...

The draft mentions that a transfer can be interrupted and resumed (to avoid
re-sending the already transmitted part), but doesn't describe how it is
resumed. Does the BP just give a smaller bundle the next time, omitting any
part it knew was already received (does it get told how much was received)?
What I took from reading the 5th paragraph in section 3 completely changed
when I read the rest of the draft. Might be good to include possible ways
the transfer might be interrupted to help explain this.

It wasn't clear to me if the TCPCL session should end when the bundles
given by BP have all been sent, or if it should be maintained with
keepalives in case further bundles need to be sent at a later time. Both
ways are described in different places.

The draft isn’t always clear about the meaning of idle, and the details
surrounding keepalives. Under Section 4.1 Keepalive, from the description,
it's not clear if this is the dead timer or the send interval - "longest
allowable interval" makes it sound like if a keepalive is not received in
this time then the session would be considered down. I think elsewhere it
says 2 x keepalive interval? So assuming this should be the maximum time
between sending keepalives, the description should be updated. Also, since
this is the "longest allowable interval", would you ever send keepalives
more frequently than this, and why? Being really picky, saying this is the
longest allowable interval between keepalives would mean you had to send a
keepalive even if other messages were being sent, but elsewhere it says the
keepalive is only sent if the session is idle, so this definition is
incorrect. Is it worth just removing "longest allowable"? Also in 5.2.1,
keepalives sound optional - "SHOULD send a KEEPALIVE message", and "MAY
terminate the session". Is that intentional? The word "allowable" was used
earlier which sounds more like a must. If no keepalive/other message is
received, is "Idle Timeout" the correct error code? Would think idle
timeout should relate to bundle transmission, i.e. having no bundles to
transfer so the session is idle apart from keepalives. The missing
keepalives would be a session failure though, not an idle timeout. Section
6.2 describes “idle” as I would expect. Would like to see all this
information collected in one section so that it is clear.

I'm unclear about the EID that gets sent in the contact header. Information
on this is scattered through the draft. It looks like some other process
outside TCPCL may find it useful? How do you determine which EID to
include? Would you ever want to include multiple EIDs? I would expect this
to be described in Section 4.1 (Contact Header).

I wondered why TLS is optional...Also think TLS should be moved to before
the existing Section 5.1, since it would happen immediately after the
contact header exchange. With it being an AND, one side can just say it
doesn’t want to use TLS, to force a non-TLS session. Should the flag
instead indicate the peer will only accept a TLS session? If you make it an
OR, then if either side requests TLS, do not begin a session without it? Or
make it default to use, since the contact header must be re-exchanged
anyway to ensure no man-in-the-middle. Could a man-in-the-middle not change
the TLS indicator in the unsecured contact header? In Section 7, it says
“When TCPCL session security is mandatory," ...but it's not mandatory,
because if either side indicates it is not using TLS, the session does not
use TLS. Also, in the RFC referenced, it says that a STARTTLS capability
could be removed from the unsecured message, so should the security
considerations here mention if a man-in-the-middle attacker could force a
non-TLS session? Also the text about the unsecured contact header might fit
nicely with the rest of the TLS text - or at least a mention of it and a
reference to the security considerations where it is described?

A message rejection includes the header of the rejected message, but the
header is only made up of the type field....is that enough to be useful?
What does TCPCL do upon receiving a message rejection? Also how would you
choose to use the unknown and unsupported reason codes? Are these
effectively the same thing?

How is segment size chosen? And why or when would it vary?

In Section 4.1, regarding Transfer MRU, if we learn the maximum bundle size
here in the contact header, can this be used in bundle fragmentation before
we get to a point where we get a refuse message back with the code "No
resources" indicating fragmentation is needed?

How are the extensions to be defined? In extra drafts? Informally as
experiments?

Also, let me know if you want all my fussy comments on typos and wording
etc...

Kind regards,
Victoria.