Subject: [multipathtcp] 6824bis -11 review

From: Rahul Arvind Jadhav <[email-masked]>
Date: Sat, 28 Jul 2018 07:38:00 +0000

Hi,

As promised  in IETF102, here is my review for 6824bis-11. As Phil had suggested, I didn't take the trouble to check if my comments overlap with any of the previous discussions/comments. 

1. Section 3.6: Subflow Reset
[RJ]: Is MP_TCPRST a SHOULD or MUST to be included along with the TCP_RST ? Can this be stated explicitly in section 3.6?

2. Section 3.4.2. Remove Address
   "The sending and receipt (if no keepalive response was received) of
   this message SHOULD trigger the sending of RSTs by both hosts on the
   affected subflow(s) "
[RJ]: Since a MP_TCPRST should be associated with this RST, what should be the corresponding Reason Code that can be used in MP_TCPRST for this case? I checked existing reason codes in section 3.6 and none of those made sense (may be except for 0x0 ?).

3. Section 3.4.1. Address Advertisement 
   "A host can send an ADD_ADDR message with an already assigned Address
   ID, but the Address MUST be the same as previously assigned to this
   Address ID, and the Port MUST be different from one already in use
   for this Address ID."
[RJ]: Didn't understand the rationale for allowing reuse of Address ID only for port change and not for IP change.

4. Section 3.4.1. Address Advertisement
[RJ]: For long-lived MPTCP connection, the ADD_ADDR needs to be resent if the sub-flow is closed and if it needs to be reopened in the future. Is this correct? The reason for me to believe this is, if REMOVE_ADDR is sent and if the sub-flow is active/functioning, then the REMOVE_ADDR will be ignored. This means that when the sub-flow is closed, the corresponding Address ID is removed. Thus it means to establish a new sub-flow from the peer an ADD_ADDR has to be sent.

5. passive opener
[RJ]: The term "passive opener" is used several times in the document. This term needs to be clarified in Terminology section.

6. Section 3 Figure 3
[RJ]: Length field in figure 3 encompasses Kind, Length and Option payload. This should be explicitly stated.

7. Section 3.1 says: "it is expected that A will have generated a key locally before the initial SYN" 
[RJ]: Is there any advantage of generating the key in advance? The key generation can be avoided by A if node B responds without MP_CAPABLE. Can this be left to the implementation?

8. Section 3.2 says: The MP_JOIN option includes an "Address ID".  This is an identifier that only has significance within a single connection,
[RJ]: The significance of "Address ID" is only within the connection and __only on the peer__ who generates the ID. Is this correct? 

9. Section 3.3 says: M = Data Sequence Number (DSN), Subflow Sequence Number (SSN), Data-Level Length, and Checksum present
[RJ]: and Checksum present (if negotiated)

10. [RJ]: Can an attacker spoof MP_FAIL option to the receiver with DSN=0 (infinite mapping)? This will cause the receiver to discard all the data following DSN and the failed data won't be DATA_ACKed. Is there any way to protect this?

11. [RJ]: I was hoping to find a section in the introduction which summarizes changes in v1. This could be of help for someone who aims to migrate/understand briefly the implications of switching to v1. 

------------Editorials:
Section 2.7:
Currently: "To meet the threats identified in ..."
Should be: "To address the threats identified in ..."

Section 2.7:
Currently: "keys are sent in the clear in the MP_CAPABLE messages"
Should be: "keys are sent in clear in the MP_CAPABLE messages"

Section 3.1:
Spellfix: requsted -> requested
Spellfix: wishs -> wishes

Section 3.1:
Currently: "The key MUST be hard to guess, and it MUST be unique for the sending host at any one time."
Should be: "The key MUST be hard to guess, and it MUST be unique for the sending host across all MPTCP connections."

Section 3.1:
Currently: "there is not a collision"
Should be: "there is no collision"

Section 3.1:
Currently: "The receiver will acknowledge this data a the connection"
Should be: "The receiver will acknowledge this data at the connection"

Section 3.4.1:
Spellfix: assinged -> assigned

Section 3.6:
Currently: "Regular TCP RST options remain used to at the subflow-level"
Should be: "Regular TCP RST options continue to be used at the subflow-level"

Section 3.6:
Spellfix: re-estbalish -> re-establish
Spellfix: ressources -> resources

Section 3.6:
Currently: "It would be beneficial for a host to the reasons ..."
Should be: "It would be beneficial for a host to provide the reasons ..."

---

From: Alan Ford <[email-masked]>
Date: Sat, 28 Jul 2018 18:19:16 +0100

Hi Rahul,

Thank you very much indeed for this review.

> On 28 Jul 2018, at 08:38, Rahul Arvind Jadhav <[email-masked]> wrote:
> 
> Hi,
> 

It’s a MAY :)  We say:

"These semantics are carried in the MP_TCPRST option that can be included on a TCP RST packet.”

We should change that to “MAY be included”.

> [RJ]: Since a MP_TCPRST should be associated with this RST, what should be the corresponding Reason Code that can be used in MP_TCPRST for this case? I checked existing reason codes in section 3.6…

That’s an interesting one. There may be some benefit (maybe for middleboxes, or maybe for security) to sharing this information in the RST with an explicit reason code. We could add one - but I would be interested in hearing other opinions.

> [RJ]: Didn't understand the rationale for allowing reuse of Address ID only for port change and not for IP change.

I cannot recall the rationale here either.

Furthermore - as written - this would seem to prevent retransmission. Which is crazy.

If nobody can recall the reason for this I would suggest we remove it.

> [RJ]: For long-lived MPTCP connection, the ADD_ADDR needs to be resent if the sub-flow is closed and if it needs to be reopened in the future. Is this correct?

No. Unless an Address + ID has been explicitly removed, it remains valid.

> The reason for me to believe this is, if REMOVE_ADDR is sent and if the sub-flow is active/functioning, then the REMOVE_ADDR will be ignored. This means that when the sub-flow is closed, the corresponding Address ID is removed. Thus it means to establish a new sub-flow from the peer an ADD_ADDR has to be sent.

Hmm. That’s not the intention, but I appreciate things are not completely clear. We say:

   The receipt of REMOVE_ADDR SHOULD first trigger
   the sending of a TCP keepalive [RFC1122] on the path, and if a
   response is received the path SHOULD NOT be removed.

So if REMOVE_ADDR is received for an already-active path, the intention is that it should be silently ignored.

I appreciate this does not explicitly state the logical next step from this. My first thought was that the Address ID and address in the table are still active, so if the subflow later was terminated, the far end would be legitimately able to re-establish it. But that might not be best.

I’m not immediately sure what’s best here for clarification. Either:
a) state that if the REMOVE_ADDR is ignored, the address remains active in the table for the remainder of the MPTCP connection;
b) state that if the REMOVE_ADDR is ignored, the subflow remains active but the address is removed from the table for any future connections.

This is to cover a corner case which should never happen, of course. I think option b is probably best.

> 5. passive opener
> [RJ]: The term "passive opener" is used several times in the document. This term needs to be clarified in Terminology section

Thank you. We got told off for “server” in the early days, hence why that appeared. You are right we need to be consistent.

> [RJ]: Length field in figure 3 encompasses Kind, Length and Option payload. This should be explicitly stated.

That is explicitly stated in the RFC793 definition of TCP options. However, I see no harm in repeating it here.

> [RJ]: Is there any advantage of generating the key in advance? The key generation can be avoided by A if node B responds without MP_CAPABLE. Can this be left to the implementation?

I think so yes. This is probably just a hangover from MPTCP v0, and is definitely an implementation detail.

> [RJ]: The significance of "Address ID" is only within the connection and __only on the peer__ who generates the ID. Is this correct? 

Yes. We can clarify.

> [RJ]: and Checksum present (if negotiated)

Thank you.

> 10. [RJ]: Can an attacker spoof MP_FAIL option to the receiver with DSN=0 (infinite mapping)? This will cause the receiver to discard all the data following DSN and the failed data won't be DATA_AC…

MP_FAIL DSN must be within the DSN window.

> 11. [RJ]: I was hoping to find a section in the introduction which summarizes changes in v1. This could be of help for someone who aims to migrate/understand briefly the implications of switching t…

One has been circulated on the list; it could be included in an Appendix.

> Should be: "It would be beneficial for a host to provide the reasons …"

Thank you again for all this feedback.

Best regards,
Alan

---

From: Alan Ford <[email-masked]>
Date: Sat, 28 Jul 2018 19:09:00 +0100

Addendum...

> On 28 Jul 2018, at 18:19, Alan Ford <[email-masked]> wrote:
> 

Having looked at the text again here, I think I can recall the rationale. But we need to consider whether this is actually the intended behaviour.

Address ID was originally designed to provide a mechanism whereby a client behind a NAT could refer to its address without using the IP address, which would be altered by the NAT. This would allow a client, whose wifi interface had disappeared, to tell the far end over its cellular interface that it had gone, by referring to the Address ID it had used in the MP_JOIN, in the REMOVE_ADDR.

There is a secondary use here which I realise we haven’t explicitly stated. If a client declared its private address in ADD_ADDR, but also sent a MP_JOIN with the same address ID, but naturally a different address, the far end would say “ah, these addresses don’t match, so I should not bother attempting to connect to the Address in the ADD_ADDR”. This is an optimisation but one that should probably be mentioned.

So the point is that Address ID refers purely to the address, and not to the address + port pair. Therefore a new port can be advertised in a new ADD_ADDR using the same Address ID.

So we should clarify this:

a) retransmitting an ADD_ADDR with same ID, address, and port is fine
b) this could permit the far end to trigger another connection attempt
c) sending a new ADD_ADDR with same ID + address and different port is fine
d) to reassign an address ID you must first REMOVE_ADDR

Regards,
Alan

---

From: Rahul Arvind Jadhav <[email-masked]>
Date: Sun, 29 Jul 2018 03:17:24 +0000

Thanks Alan for clarifying.
But I’m still not clear about this. Please find my comments [RJ] inline.

On 28 Jul 2018, at 18:19, Alan Ford <[email-masked]<mailto:[email-masked]>> wrote:

[RJ] The far end here matches the Address ID, identifies the ADD_ADDR context, and uses the received source address .. May be the far end should keep this newly learnt IP address within the ADD_ADDR context for future MP_JOIN once this sub-flow closes? As I understand, Address ID alone is the primary-key for matching anyways.

[RJ] I’m not sure about this interpretation. My thinking was that Address ID refers purely to the sub-flow possibility on an interface without any dependence on address + port. Address and port are parameters for the Address ID which might possibly change in the future.

a) retransmitting an ADD_ADDR with same ID, address, and port is fine
b) this could permit the far end to trigger another connection attempt
[RJ] a & b is clear from existing text

c) sending a new ADD_ADDR with same ID + address and different port is fine
[RJ] again this is what the text already says, but I’m not still clear about this. Why ID + address has to match? Only ID should be enough.

d) to reassign an address ID you must first REMOVE_ADDR
[RJ] This also I don’t get. Why should it be necessary to do a REMOVE_ADDR to change an IP address for an existing ID? If a ADD_ADDR refers to an existing ID with a new Address then can’t this simply override the IP address for that ID?

Another way to put this question, In which case a receiver after matching the Address ID (for MP_JOIN or ADD_ADDR) will depend on matching against the address field and what is achieved by matching the address field?

By allowing to override an IP address in the existing ID, a REMOVE_ADDR + ADD_ADDR cycle which requires at-least one RTT would be saved. Secondly, the implementation becomes lot more simpler (for me this is the major motivation).

Thanks,
Rahul

---

From: <[email-masked]>
Date: Mon, 30 Jul 2018 08:35:24 +0000

Rahul,

Thanks very much for the detailed review, it is much appreciated
phil

-----Original Message-----
From: Rahul Arvind Jadhav [mailto:[email-masked]] 
Sent: 28 July 2018 08:38
To: [email-masked]
Cc: Eardley,PL,Philip,TUD1 R <[email-masked]>; [email-masked]
Subject: 6824bis -11 review

Hi,

5. passive opener

6. Section 3 Figure 3

------------Editorials:
Section 2.7:

Section 2.7:

Section 3.1:

Section 3.1:

Section 3.1:

Section 3.1:

Section 3.4.1:

Section 3.6:

Section 3.6:

Section 3.6:

---

From: Alan Ford <[email-masked]>
Date: Mon, 30 Jul 2018 16:55:53 +0100

Hi Rahul,

Comments inline...

> On 29 Jul 2018, at 04:17, Rahul Arvind Jadhav <[email-masked]> wrote:
> 
> On 28 Jul 2018, at 18:19, Alan Ford <[email-masked] <mailto:[email-masked]>> wrote:
> 
> 

Yes, the intention is that the source address from a MP_JOIN can be used for future connections, creating or updating the mapping to the Address ID. Which makes me realise we probably need the ‘C’ bit on the MP_JOIN. But that’s an aside.

> [RJ] I’m not sure about this interpretation. My thinking was that Address ID refers purely to the sub-flow possibility on an interface without any dependence on address + port. Address and port are…

For the purposes of MPTCP, I’m not sure how interface and address are different here.

> [RJ] again this is what the text already says, but I’m not still clear about this. Why ID + address has to match? Only ID should be enough. 

Consider:

a) ADD_ADDR comes in saying ID=2, addr=10.0.0.1 (private)
b) MP_JOIN comes in saying ID=2, addr=8.9.10.11 (public)
c) ADD_ADDR comes in saying ID=2, addr=192.168.0.1

What does this mean? Is (c) an implicit REMOVE+ADD? And thus does this mean the subflow created in (b) is now no longer valid?

By keeping the address / ID mapping, if an address is lost, then so is the subflow.

> By allowing to override an IP address in the existing ID, a REMOVE_ADDR + ADD_ADDR cycle which requires at-least one RTT would be saved. Secondly, the implementation becomes lot more simpler (for m…

Again, see the example above. It is my view that it is removes ambiguity for a sender to have a unique ID to address mapping.

Best regards,
Alan

---

From: Rahul Arvind Jadhav <[email-masked]>
Date: Tue, 31 Jul 2018 00:21:14 +0000

Consider:

[RJ2] Rather than viewing ( c) as an implicit REMOVE+ADD, can we see it as an UPDATE? Because ID remains same. Subflow created in (b) remains valid (not sure if I’m missing anything here, I see no reason to lose this subflow).
In step ( c), the receiver can override Address field in context to ID=2 to the new addr (192.168.0.1) ……   8.9.10.11 is kept separately as the learnt actual rcvd IP (from MP_JOIN) in the same context ID=2. We have to agree that an implementation needs to maintain this IP separately in context to ID=2 anyways.
So if we do an explicit REMOVE+ADD, the end result is the same as this UPDATE (but with much less overhead & impl ease).

---

From: Alan Ford <[email-masked]>
Date: Tue, 31 Jul 2018 13:07:06 +0100

Hi Rahul, all,

> On 31 Jul 2018, at 01:21, Rahul Arvind Jadhav <[email-masked]> wrote:
>  

I would be interested in hearing from other implementers here… Is there an issue with “updating” using ADD_ADDR?

Given we say that REMOVE_ADDR should not remove a subflow if it’s still alive, we have precedent for implying a subflow can exist to an address which does not have an Address ID - to - IP address mapping.

So given that, does updating, as Rahul suggests, cause a problem?

Best regards,
Alan