Disable TCP simultaneous connect

[madaidan]

This creates a net.ipv4.tcp_simult_connect sysctl to enable/disable this. The default is to disable it.

TCP simultaneous connect adds a weakness in Linux's implementation of TCP that allows two clients to connect to each other without either entering a listening state. The weakness allows an attacker to easily prevent a client from connecting to a known server provided the source port for the connection is guessed correctly.

TCP simultaneous connect is rarely used and Linux is one of the only OSes to even support it.

Attempts to upstream this functionality in the past have been unsuccessful.

This is based on GRKERNSEC_NO_SIMULT_CONNECT.

[XHDR]

Is there any reason for the extra check in tcp_ecn_rcv_syn? There doesn't seem to be any code path that would execute it, given the prior check at if (th->syn && sysctl_tcp_simult_connect) {.

[madaidan]

Ah, you're right. Grepping the source shows nothing else executes it so the check can be removed. I only added it originally as it was part of the original grsec patch:

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 7727ffe..9488999 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -288,11 +288,13 @@ static void tcp_ecn_rcv_synack(struct tcp_sock *tp, const struct tcphdr *th)
        tp->ecn_flags &= ~TCP_ECN_OK;
 }

+#ifndef CONFIG_GRKERNSEC_NO_SIMULT_CONNECT
 static void tcp_ecn_rcv_syn(struct tcp_sock *tp, const struct tcphdr *th)
 {
    if ((tp->ecn_flags & TCP_ECN_OK) && (!th->ece || !th->cwr))
        tp->ecn_flags &= ~TCP_ECN_OK;
 }
+#endif

 static bool tcp_ecn_rcv_ecn_echo(const struct tcp_sock *tp, const struct tcphdr *th)
 {

[anthraxx]

This looks like something useful to include.

However, I believe it would make sense to also document implications of that knob f.e. related to TCP STUNT and NAT-less configurations behind stateful firewalls like in IPv6.

Also it would be great to be able to specify the default of this knob via kconfig like TCP_SIMULT_CONNECT_DEFAULT_ON with a default of 0 aka not set

[madaidan]

Also it would be great to be able to specify the default of this knob via kconfig like TCP_SIMULT_CONNECT_DEFAULT_ON with a default of 0 aka not set

Where should that be? net/ipv4/Kconfig? security/Kconfig?

[tsautereau-anssi]

net/ipv4/Kconfig

[madaidan]

Done. Anything else?

[tsautereau-anssi]

Thanks a lot @madaidan.

Now I'm just wondering about the default value that should be used (this old thread is a good start for people that would like to think a bit more about that matter). Typically in CLIP OS we will surely disable TCP simultaneous connect, but I'm not sure @anthraxx is gonna want to do the same for Arch Linux's linux-hardened kernel if it actually ends up annoying many users...

[anthraxx]

Hm yeah, basically I'm still thinking what the default value for TCP_SIMULT_CONNECT_DEFAULT_ON should be. In Arch Linux i will most likely enable it by default as I believe the majority of users has that on desktop systems. However, as this could potentially lead to regressions in userland maybe a better default value is keeping TCP_SIMULT_CONNECT_DEFAULT_ON on. Normally I really prefer to opt-out of security features instead of opt-on but maybe this would have too many problems on average desktop systems so keeping it on by default is a better choice? :thinking:

[madaidan]

Disabling this can break things like WebRTC which is commonly used so it'd probably best to enable it by default https://github.com/madaidan/linux-hardened/commit/758aa6d0c9d2778fd299db124df8d0f55089e957

Anyone who doesn't want simultaneous connect can easily change the sysctl to disable it.

[anthraxx]

This patch is considered to be accepted, however I'm still considering what to do with the default value. Maybe I will pull this in as is for now and consider swapping to simult off by default later.

[anthraxx]

applied via 9c8dad758536bd42792bcc15ba87609a9ad01033

[madaidan]

@anthraxx is this going to be added to the LTS branch too?

[anthraxx]

Yes, was field testing it for a while, the next releases will contain this patch from master