Hi,

I came up a while ago with a patchset for MPTCP support for HAProxy also, see https://github.com/haproxy/haproxy/issues/1028

Back then I also discussed some ideas with Willy how to implement it more elegantly in HAProxy. It's still somewhere on my todo list but unfortunately I didn't catch that up since then. As I had a good real-world usecase recently for MPTCP I though of looking into this again also.

Björn

On 25.04.24 00:12, Willy Tarreau wrote:
Hi!

On Wed, Apr 24, 2024 at 05:45:03PM +0200, Nicolas CARPi wrote:
Hello,

On 24 Apr, Dorian Craps wrote:
This attached patch uses MPTCP by default instead of TCP on Linux.
The backward compatibility of MPTCP is indeed a good point toward
enabling it by default. Nonetheless, I feel your message should include
a discussion on the security implications of this change.

As you know, v0 had security issues. v1 addresses them, and we can
likely consider that new attacks targeting this protocol will pop up as
it becomes widespread.

In fact, that's already the case:

See: CVE-2024-26708: mptcp: really cope with fastopen race
or CVE-2024-26826: mptcp: fix data re-injection from stale subflow
or CVE-2024-26782 kernel: mptcp: fix double-free on socket dismantle

The three CVEs above are all from April 2024.

Given that MPTCP v1 is relatively new (2020), and that we do not have
real assurances that it is at least as secure as plain TCP, I would
humbly suggest inverting the logic, and making it an opt-in option.

This way, a vulnerability impacting MPTCP would only impact users that
enabled it, instead of 100% of HAProxy users. In a few years, making it
the default could be reconsidered.

Please note that I'm simply voicing my concern as a user, and the core
dev team might have a very different view about these aspects.

It sounds good to have MPTCP enabled by default
Except when looking at it through the prism of the increased attack
surface! ;)

IPPROTO_MPTCP is defined just in case old libC are being used and
don't have the ref.
Shouldn't it be defined with a value, as per
https://www.mptcp.dev/faq.html#why-is-ipproto_mptcp-not-defined ?
(sorry if it's a dumb remark, I'm not a C dev)

Without going into all the details and comments above, I just want to
say that I'll study this after 3.0 is released, since there's still a
massive amount of stuff to adjust for the release and we're way past
a feature freeze. I *am* interested in the feature, which has been
floating around for a few years already. However I tend to agree with
Nicolas that, at least for the principle of least surprise, I'd rather
not have this turned on by default. There might even be security
implications that some are not willing to take yet, and forcing them
to guess that they need to opt out of something is not great in general
(it might change in the future though, once everyone turns it on by
default, like we did for keep-alive, threads, and h2 for example).

I'm also concerned by the change at the socket layer that will make all
new sockets cause two syscalls on systems where this is not enabled,
and I'd really really prefer that we use the extended syntax for
addresses that offers a lot of flexibility. We can definitely have
"mptcp@address" and probably mptcp as a variant of tcp. Regarding how
we'd deal with the fallback, we'll see, but overall, thinking that
someone would explicitly configure mptcp and not even get an error if
it cannot bind is problematic to me, because among the most common
causes of trouble are things that everyone ignores (module not loaded,
missing secondary IP, firewall rules etc) that usually cause problems.
Those we can discover at boot time should definitely be reported. But
let's discuss this in early June instead (I mean, feel free to discuss
the points together, but I'm not going to invest more time on this at
this moment).

Thanks!
Willy



Reply via email to