Subject: Re: [multipathtcp] WG Last Call for draft-ietf-mptcp-rfc6824bis
From: <[email-masked]>
To: <[email-masked]>
Cc: <[email-masked]>
Date: Sat, 14 Jul 2018 11:08:25 +0000
List-Id: Multi-path extensions for TCP <multipathtcp.ietf.org>

Hi,

Apart from the pure clarifications, there are a few comments from me and Yoshi that may be worth discussing:

--
2:  As we have removed AddrID from MP_PRIO, we have a certain problem in the situations like the followings.

Let's say a server has two address S1, S2 while the client is behind FW. Since the client is behind firewall, the server uses ADD_ADDR to send the info of S2 through S1, but it wants S2 to be used as a backup.

In this case, if MP_PRIO has addr id field, the server is able to specify S2 is a backup. However, since MP_PRIO doesn't have addr id any more, all the server can do is to wait until the client establishes the connection with S2. But, this connection doesn't look what the server wants. 
I guess using 1 bit in ADD_ADDR might solve the issue.
--

One other high-level comment:  it would be very good if the bis code, implemented in linux, could be released - to allow testing of the protocol changes in the bis, for instance to make sure that no unexpected issues have been introduced

--

Section 2.1 & 3.1 - can you remind me why we changed MP_CAPABLE so that Host A's key is sent only on the ACK (not on the SYN)? Is this so it's only sent when we know host B is MPTCP capable, which is therefore a slight security improvement?  (Just send a link to earlier emails, if that's easier)

--

Section 3.4.1 ... The 'E' bit exists to provide reliability for this option ...
Not sure - does this open security hole, where an attacker pretends an ADD-ADDR has been received? (the Echo doesn't include an hmac, so is easy to spoof)

--

We're waiting for this IPR to get added on the tracker
* one is inherited from draft-paasch-mptcp-syncookies (which got include in rfc6824bis) https://datatracker.ietf.org/ipr/2678/

--

-----Original Message-----
From: Alan Ford [mailto:[email-masked]] 
Sent: 11 July 2018 02:23
To: Eardley,PL,Philip,TUD1 R <[email-masked]>
Cc: [email-masked]
Subject: Re: [multipathtcp] WG Last Call for draft-ietf-mptcp-rfc6824bis

Thank you for your feedback, Phil. All proposed changes look good and we will wrap these into the next version.

@Everyone - we are however awaiting other WGLC feedback before doing a new revision!

Best regards,
Alan

> On 18 Jun 2018, at 14:59, [email-masked] wrote:
> 
> More comments:
> Section 3.5 -
> Option R (RST) - add (at the end of the bullet) "the Host A enters the 
> CLOSED state directly" (I think)
> 
> << If a host receives a packet with a valid MP_FASTCLOSE option, it
>   shall process it as follows :>>
> I think the text that follows (apart from the last bullet) is only relevant for Option A, where the MP_FASTCLOSE is sent on an ACK - it doesn't apply for option R (where it's sent on a RST). Anyway, things could be laid out slightly differently so the section is clearer. 
> 
> Section 3.6
> I wonder if it would be more logical for this section to be before Fast close?
> 
> This section could do with a read through. A few things:
> << As discussed in Section 3.5 above, the MP_FASTCLOSE option provides a
>   connection-level reset roughly analagous to a TCP RST. Regular TCP
>   RST options remain used to at the subflow-level to indicate the
>   receiving host has no knowledge of the MPTCP subflow or TCP
>   connection to which the packet belongs.>> Can this paragraph be 
> deleted? at least I found it a bit confusing that this text is not about this section.
> 
> "remain used to at" is ugly
> 
> << However, in MPTCP, there may be many reasons for rejecting the
>   opening of a subflow, but these semantics cannot be carried in a
>   standard TCP RST. It would be beneficial for a host to the reasons
>   why its subflow has been closed with a RST, and thus whether it
>   should try to re-establish the subflow immediately, later, or never
>   again.  These semantics are carried in the MP_TCPRST option that can
>   be included on a TCP RST packet.>>
> I presume the new MP_TCPRST can be added to a RST, whenever it's created - the sentence seems to imply it's only when it wants to reject the opening of a subflow (ie in response to MP_JOIN). 
> I assume inclusion of the MP_TCPRST option is optional?
> 
> potential better phrasing:
> A host sends a TCP RST in order to close a subflow or reject an attempt to open a subflow (MP_JOIN). In order to inform the receiving host why a subflow is being closed or rejected, the TCP RST packet MAY include the MP_TCPRST Option. The host MAY use this information to decide, for example, whether it tries to re-establish the subflow immediately, later, or never. 
> 
> <<   o  Unspecified error (code 0x0).  This is the default error implying
>      the subflow is not longer available.  The receiving host SHOULD
>      take account of the 'T' bit in deciding whether to re-estbalish
>      this subflow.  The presence of this option shows that the RST was
>      generated by a MPTCP-aware device.>>
> 
> " The receiving host SHOULD take account of the 'T' bit in deciding whether to re-estbalish this subflow."
> Only this sub-bullet mentions the T bit, which may suggest that it should not be taken account of with the other reason codes.  I think the sentence should be deleted (using the T flag is mentioned earlier). 
> 
> <<enough ressources>>  enough resources
> 
> Section 3.7
> I guess it's OK to refer to it as RST rather than TCP RST?
> 
> << The case described above is a specialized case of fallback, for when
>   the lack of MPTCP support is detected before any data is acknowledged
>   at the connection level on a subflow.  >> "The case above" - I'm not 
> sure which of the previous paragraph or paragraphs you're referring to.
> "specialised" - maybe "specific" [is it really specialised?] "for 
> when" -> when
> 
> S3.9.2
> << If the two communicating hosts immediately try to set up subflows
>   from all available addresses to all available addresses on the other
>   host, this could end up creating two subflows per path.  This is an
>   inefficient use of resources.>>
> is two subflows correct? Multiple I think " could end up creating two 
> subflows per path" -> "could create multiple subflows over the same path"
> 
> << If a host does not support TCP simultaneous open, it
>   is RECOMMENDED that some element of randomization is applied to the
>   time waited before opening new subflows, so that only one subflow
>   exists between a given address pair.  If, however, hosts signal
>   additional ports to use (for example, for leveraging ECMP on-path),
>   this heuristic need not apply.>>
> "time waited" - ugly phrase?
> "exists" -> is created
> "need not apply" -> is not appropriate
> 
> Section 5 - Security
> This section needs updating to reflect the changes in the bis.:
> 
> Add mention of ADD_ADDR, now with HMAC and reliability, Discuss the 
> revised MP_CAPABLE I'm not sure if the new fast close option (option 
> R) has any security impact?
> Discuss the MP_PRIO change (no off path address indication) Discuss 
> downgrade attacks (to v0) during connection initiation
> 
> Section 8 IANA
> << This document updates [RFC6824] and as such IANA is requested to
>   update the TCP option space registry to point to this document for
>   Multipath TCP>>
> This document obsoletes rather than updates...   I suspect this means all the MPTCP options need to be included in this document, but we can leave IANA to decide that. 
> 
> <<  |    C    |   Do not attempt to connect to   |    This document,    |
>   |         |          source address          |     Section 3.1      |
>>> 
> I don't think this is a good summary of flag C's meaning.
> 
> << The
>   contents of this sub-registry are to to this document, as follows:
>>> 
> Is this sentence needed?
> 
> Appendix B TCP Fast Open
> Maybe rephrae the title as "TCP Fast Open and MPTCP" or "Considerations about using TCP Fast Open and MPTCP"?
> 
> This section could do with a read through, it could do with a few 
> minor re-phrasings
> 
> 
> <<   TCP Fast Open (TFO) is an experimental TCP extension, described in
>   [RFC7413], which has been introduced with the objective of gaining
>   one RTT before transmitting data.>>
> " gaining one RTT"?  maybe: which allows data to be sent one RTT earlier than with regular TCP. 
> << It achieves this by sending the SYN-
>   segment together with data>>
> re-phrase (I think data is referring ot the cookie - but you havent 
> yet mentioned it - rather than user data)
> 
> "resp." = ?
> 
> << TFO and MPTCP can be combined provided that the total length of their
>   options does not exceed the maximum 40 bytes possible in TCP>> 
> "their" is wrong, as there may be other options. -> "all the"
> 
> << We also note that this does not entail a loss
>   of functionality, because TFO-data is always sent when only one path
>   is active.>>
> -> "TFO-data is only ever sent when only a single path is active"
> How do you guarantee this? Not sure I understand. 
> 
> << To solve this, the TFO data should not be considered part of the Data
>   Sequence Number space:>>
> Is this a SHOULD NOT?  Does it imply a change to rfc7413 (the tfo rfc)? I guess this is really a suggestion to the implementer?
> 
> Section B.3
> I assume this sub-section is an example (so no SHOULD or even MAY are 
> needed)
> 
> << client                                                         server
>     |                                                              |
>     |    S 0(0) <MP_CAPABLE>, <TFO cookie request>                 |
>     | -----------------------------------------------------------> |
>>> 
> Can you explain the S 0(0) nomenclature/ please.
> 
> 
> Appendix D Finite state machine
> I presume we don't need similar FSM for fastclose?
> 
> 
> Section 2:
> Need to add sub-sections on closing subflow & fast close & fallback
> 
> S2.7 - I think the third bullet about notable features could be 
> slightly altered, with enhanced security for ADD_ADDR and MP_CAPABLE. 
> You could also add a mention of rfc7430
> 
> 
> Section 2.1 & 3.1 - can you remind me why we changed MP_CAPABLE so 
> that Host A's key is sent only on the ACK (not on the SYN)? Is this so 
> it's only sent when we know host B is MPTCP capable, which is 
> therefore a slight security improvement?  (Just send a link to earlier 
> emails, if that's easier)
> 
> Thanks
> phil
> -----Original Message-----
> From: Eardley,PL,Philip,TUD1 R
> Sent: 14 June 2018 11:56
> To: [email-masked]' <[email-masked]>; 
> [email-masked]
> Subject: RE: WG Last Call for draft-ietf-mptcp-rfc6824bis
> 
> A few comments as I work through the document, basically minor so far
> 
> Section 3.1
> << Given the SYN exchange is different between v1 and v0
>   the exchange cannot be immediately downgraded, and therefore if the
>   far end has requested a lower version then the initiator SHOULD
>   respond with an ACK without any MP_CAPABLE option, to fall back to
>   regular TCP.>>
> I think it should say "to ensure the exchange cannot..."
> 
> 
> << The first 4 bits of the first octet in the MP_CAPABLE option
>   (Figure 4) define the MPTCP option subtype (see Section 8; for
>   MP_CAPABLE, this is 0), and the remaining 4 bits of this octet
>   specify the MPTCP version in use (for this specification, this is 1).
>>> 
> "The first 4 bits of the first octet in the MP_CAPABLE option" Fig 4 shows two earlier octets (Kind & Length) - I know these arent the MP-CAPABLE option, but I think it might be worth spelling out what they are. 
> 
> Would it be better strictly to have "0x0" rather than "0"? (ie match 
> the IANA registry)
> 
> Section 3.4.1
> <<A set of four flags are present after the subtype and before the
>   Address ID.  Only the rightmost bit - labelled 'E' - is assinged
>   today.  The other bits are currently unassigned and MUST be set to
>   zero by a sender and MUST be ignored by the receiver.
> 
>   The 'E' bit exists to provide reliability for this option.  Because
>   this option will often be sent on pure ACKs, there is no guarantee of
>   reliability.  Therefore, a receiver receiving a fresh ADD_ADDR option
>   (where E=0), will send the same option back to the sender, but not
>   including the HMAC, and with E=1.  The lack of this echo can be used
>   by the initial ADD_ADDR sender to retransmit the ADD_ADDR according
>   to local policy.
>>> 
> 4 comments:
> Assinged /assigned
> 
> The 'E' bit - better to say flag?
> 
> Section 2.3 needs to be amended to show the reply /echo message for ADD-ADDR. I suggest the pic in S2.3 also shows the E value. 
> 
> Not sure - does this open security hole, where an attacker pretends an 
> ADD-ADDR has been received? (the Echo doesn't include an hmac, so is 
> easy to spoof)
> 
> And in Fig 12
> |        Truncated HMAC (8 octets, if length > 10 octets)       |
> 
> Could this be made more straightforward? Ie spell out when length is >10 octets ie IPv6 (if I understand it). This also seems to mean that IPv4 doesn't include the HMAC, is this right??
> 
> Thanks
> phil
> 
> -----Original Message-----
> From: multipathtcp [mailto:[email-masked]] On Behalf Of 
> [email-masked]
> Sent: 05 June 2018 09:14
> To: [email-masked]
> Subject: [multipathtcp] WG Last Call for draft-ietf-mptcp-rfc6824bis
> 
> This starts a WG Last Call for draft-ietf-mptcp-rfc6824bis. Please send comments by the end of June. 
> 
> Please note there are three IPR disclosures (we're working on getting them added to the rfc6824bis page): 
> 
> * two are inherited from RFC6824  https://datatracker.ietf.org/ipr/search/?submit=draft&id=draft-ietf-mptcp-multiaddressed    
> * one is inherited from draft-paasch-mptcp-syncookies (which got 
> include in rfc6824bis) https://datatracker.ietf.org/ipr/2678/
> 
> Thanks,
> Phil & Yoshi
> _______________________________________________
> multipathtcp mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/multipathtcp
> 
> _______________________________________________
> multipathtcp mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/multipathtcp