[3/3] SSH: do not send spoofable TCP keep alive messages

Message ID 09688ff4-d262-d136-0d1f-9102732a5e0d@ipfire.org
State Accepted
Commit 7a981d94cb2c3e48ecaf07c506c8353a2c839d79
Headers
Series [1/3] OpenSSH: Update to 9.0p1 |

Commit Message

Peter Müller April 18, 2022, 8:40 p.m. UTC
  By default, both SSH server and client rely on TCP-based keep alive
messages to detect broken sessions, which can be spoofed rather easily
in order to keep a broken session opened (and vice versa).

Since we rely on SSH-based keep alive messages, which are not vulnerable
to this kind of tampering, there is no need to double-check connections
via TCP keep alive as well.

This patch thereof disables using TCP keep alive for both SSH client and
server scenario. For usability reasons, a timeout of 5 minutes (10
seconds * 30 keep alive messages = 300 seconds) will be used for both
client and server configuration, as 60 seconds were found to be too
short for unstable connectivity scenarios.

Signed-off-by: Peter Müller <peter.mueller@ipfire.org>
---
 config/ssh/ssh_config  | 12 ++++++++----
 config/ssh/sshd_config |  8 +++++---
 2 files changed, 13 insertions(+), 7 deletions(-)
  

Comments

Michael Tremer April 19, 2022, 10:17 a.m. UTC | #1
Hello,

Thanks for this.

I would personally like a longer timeout than 60 seconds.

If a DSL modem loses sync, or DFS kicks in and the WiFi has to change channels, 60 seconds is not a long time. There cannot be any security reason for keeping it that low, so I would like to ask if there is any other reason that I missed.

-Michael

> On 18 Apr 2022, at 21:40, Peter Müller <peter.mueller@ipfire.org> wrote:
> 
> By default, both SSH server and client rely on TCP-based keep alive
> messages to detect broken sessions, which can be spoofed rather easily
> in order to keep a broken session opened (and vice versa).
> 
> Since we rely on SSH-based keep alive messages, which are not vulnerable
> to this kind of tampering, there is no need to double-check connections
> via TCP keep alive as well.
> 
> This patch thereof disables using TCP keep alive for both SSH client and
> server scenario. For usability reasons, a timeout of 5 minutes (10
> seconds * 30 keep alive messages = 300 seconds) will be used for both
> client and server configuration, as 60 seconds were found to be too
> short for unstable connectivity scenarios.
> 
> Signed-off-by: Peter Müller <peter.mueller@ipfire.org>
> ---
> config/ssh/ssh_config  | 12 ++++++++----
> config/ssh/sshd_config |  8 +++++---
> 2 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/config/ssh/ssh_config b/config/ssh/ssh_config
> index ee0954d5c..85c069dda 100644
> --- a/config/ssh/ssh_config
> +++ b/config/ssh/ssh_config
> @@ -5,7 +5,7 @@
> 
> # Set some basic hardening options for all connections
> Host *
> -        # Disable Roaming as it is known to be vulnerable
> +        # Disable undocumented roaming feature as it is known to be vulnerable
>         UseRoaming no
> 
>         # Only use secure crypto algorithms
> @@ -13,15 +13,19 @@ Host *
>         Ciphers chacha20-poly1305@openssh.com,aes256-gcm@openssh.com,aes128-gcm@openssh.com,aes256-ctr,aes192-ctr,aes128-ctr
>         MACs hmac-sha2-512-etm@openssh.com,hmac-sha2-256-etm@openssh.com,umac-128-etm@openssh.com,hmac-sha2-512,hmac-sha2-256,umac-128@openssh.com
> 
> -        # Always visualise server host keys (but helps to identify key based MITM attacks)
> +        # Always visualise server host keys (helps to identify key based MITM attacks)
>         VisualHostKey yes
> 
>         # Use SSHFP (might work on some up-to-date networks) to look up host keys
>         VerifyHostKeyDNS yes
> 
> -        # send keep-alive messages to connected server to avoid broken connections
> +        # Send SSH-based keep alive messages to connected server to avoid broken connections
>         ServerAliveInterval 10
> -        ServerAliveCountMax 6
> +        ServerAliveCountMax 30
> +
> +	# Disable TCP keep alive messages since they can be spoofed and we have SSH-based
> +	# keep alive messages enabled; there is no need to do things twice here
> +	TCPKeepAlive no
> 
>         # Ensure only allowed authentication methods are used
>         PreferredAuthentications publickey,keyboard-interactive,password
> diff --git a/config/ssh/sshd_config b/config/ssh/sshd_config
> index 456556540..76c9b3eb1 100644
> --- a/config/ssh/sshd_config
> +++ b/config/ssh/sshd_config
> @@ -46,11 +46,13 @@ AllowTcpForwarding no
> AllowAgentForwarding no
> PermitOpen none
> 
> -# Detect broken sessions by sending keep-alive messages to clients via SSH connection
> +# Send SSH-based keep alive messages to connected clients to avoid broken connections
> ClientAliveInterval 10
> +ClientAliveCountMax 30
> 
> -# Close unresponsive SSH sessions which fail to answer keep-alive
> -ClientAliveCountMax 6
> +# Since TCP keep alive messages can be spoofed and we have the SSH-based already,
> +# there is no need for this to be enabled as well
> +TCPKeepAlive no
> 
> # Add support for SFTP
> Subsystem	sftp	/usr/lib/openssh/sftp-server
> -- 
> 2.34.1
  
Peter Müller April 19, 2022, 10:40 a.m. UTC | #2
Hello Michael,

thanks for your reply.

> Hello,
> 
> Thanks for this.
> 
> I would personally like a longer timeout than 60 seconds.
> 
> If a DSL modem loses sync, or DFS kicks in and the WiFi has to change channels, 60 seconds is not a long time. There cannot be any security reason for keeping it that low, so I would like to ask if there is any other reason that I missed.

Um, actually, this patch features a timeout of five minutes (10 seconds * 30 keep-alive's = 300
seconds = 5 minutes) before a dangling SSH connection is being terminated. Or did I misunderstand
you?

Thanks, and best regards,
Peter Müller

> 
> -Michael
> 
>> On 18 Apr 2022, at 21:40, Peter Müller <peter.mueller@ipfire.org> wrote:
>>
>> By default, both SSH server and client rely on TCP-based keep alive
>> messages to detect broken sessions, which can be spoofed rather easily
>> in order to keep a broken session opened (and vice versa).
>>
>> Since we rely on SSH-based keep alive messages, which are not vulnerable
>> to this kind of tampering, there is no need to double-check connections
>> via TCP keep alive as well.
>>
>> This patch thereof disables using TCP keep alive for both SSH client and
>> server scenario. For usability reasons, a timeout of 5 minutes (10
>> seconds * 30 keep alive messages = 300 seconds) will be used for both
>> client and server configuration, as 60 seconds were found to be too
>> short for unstable connectivity scenarios.

This was precisely your concern about the first attempt of this patch, which
is why I raised this to 300 seconds instead of 60.

>>
>> Signed-off-by: Peter Müller <peter.mueller@ipfire.org>
>> ---
>> config/ssh/ssh_config  | 12 ++++++++----
>> config/ssh/sshd_config |  8 +++++---
>> 2 files changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/config/ssh/ssh_config b/config/ssh/ssh_config
>> index ee0954d5c..85c069dda 100644
>> --- a/config/ssh/ssh_config
>> +++ b/config/ssh/ssh_config
>> @@ -5,7 +5,7 @@
>>
>> # Set some basic hardening options for all connections
>> Host *
>> -        # Disable Roaming as it is known to be vulnerable
>> +        # Disable undocumented roaming feature as it is known to be vulnerable
>>         UseRoaming no
>>
>>         # Only use secure crypto algorithms
>> @@ -13,15 +13,19 @@ Host *
>>         Ciphers chacha20-poly1305@openssh.com,aes256-gcm@openssh.com,aes128-gcm@openssh.com,aes256-ctr,aes192-ctr,aes128-ctr
>>         MACs hmac-sha2-512-etm@openssh.com,hmac-sha2-256-etm@openssh.com,umac-128-etm@openssh.com,hmac-sha2-512,hmac-sha2-256,umac-128@openssh.com
>>
>> -        # Always visualise server host keys (but helps to identify key based MITM attacks)
>> +        # Always visualise server host keys (helps to identify key based MITM attacks)
>>         VisualHostKey yes
>>
>>         # Use SSHFP (might work on some up-to-date networks) to look up host keys
>>         VerifyHostKeyDNS yes
>>
>> -        # send keep-alive messages to connected server to avoid broken connections
>> +        # Send SSH-based keep alive messages to connected server to avoid broken connections
>>         ServerAliveInterval 10
>> -        ServerAliveCountMax 6
>> +        ServerAliveCountMax 30
>> +
>> +	# Disable TCP keep alive messages since they can be spoofed and we have SSH-based
>> +	# keep alive messages enabled; there is no need to do things twice here
>> +	TCPKeepAlive no
>>
>>         # Ensure only allowed authentication methods are used
>>         PreferredAuthentications publickey,keyboard-interactive,password
>> diff --git a/config/ssh/sshd_config b/config/ssh/sshd_config
>> index 456556540..76c9b3eb1 100644
>> --- a/config/ssh/sshd_config
>> +++ b/config/ssh/sshd_config
>> @@ -46,11 +46,13 @@ AllowTcpForwarding no
>> AllowAgentForwarding no
>> PermitOpen none
>>
>> -# Detect broken sessions by sending keep-alive messages to clients via SSH connection
>> +# Send SSH-based keep alive messages to connected clients to avoid broken connections
>> ClientAliveInterval 10
>> +ClientAliveCountMax 30
>>
>> -# Close unresponsive SSH sessions which fail to answer keep-alive
>> -ClientAliveCountMax 6
>> +# Since TCP keep alive messages can be spoofed and we have the SSH-based already,
>> +# there is no need for this to be enabled as well
>> +TCPKeepAlive no
>>
>> # Add support for SFTP
>> Subsystem	sftp	/usr/lib/openssh/sftp-server
>> -- 
>> 2.34.1
>
  
Michael Tremer April 19, 2022, 10:41 a.m. UTC | #3
Oh, maybe I misread your first email. Sorry. 5 minutes > 60s. Cool.

> On 19 Apr 2022, at 11:40, Peter Müller <peter.mueller@ipfire.org> wrote:
> 
> Hello Michael,
> 
> thanks for your reply.
> 
>> Hello,
>> 
>> Thanks for this.
>> 
>> I would personally like a longer timeout than 60 seconds.
>> 
>> If a DSL modem loses sync, or DFS kicks in and the WiFi has to change channels, 60 seconds is not a long time. There cannot be any security reason for keeping it that low, so I would like to ask if there is any other reason that I missed.
> 
> Um, actually, this patch features a timeout of five minutes (10 seconds * 30 keep-alive's = 300
> seconds = 5 minutes) before a dangling SSH connection is being terminated. Or did I misunderstand
> you?
> 
> Thanks, and best regards,
> Peter Müller
> 
>> 
>> -Michael
>> 
>>> On 18 Apr 2022, at 21:40, Peter Müller <peter.mueller@ipfire.org> wrote:
>>> 
>>> By default, both SSH server and client rely on TCP-based keep alive
>>> messages to detect broken sessions, which can be spoofed rather easily
>>> in order to keep a broken session opened (and vice versa).
>>> 
>>> Since we rely on SSH-based keep alive messages, which are not vulnerable
>>> to this kind of tampering, there is no need to double-check connections
>>> via TCP keep alive as well.
>>> 
>>> This patch thereof disables using TCP keep alive for both SSH client and
>>> server scenario. For usability reasons, a timeout of 5 minutes (10
>>> seconds * 30 keep alive messages = 300 seconds) will be used for both
>>> client and server configuration, as 60 seconds were found to be too
>>> short for unstable connectivity scenarios.
> 
> This was precisely your concern about the first attempt of this patch, which
> is why I raised this to 300 seconds instead of 60.
> 
>>> 
>>> Signed-off-by: Peter Müller <peter.mueller@ipfire.org>
>>> ---
>>> config/ssh/ssh_config | 12 ++++++++----
>>> config/ssh/sshd_config | 8 +++++---
>>> 2 files changed, 13 insertions(+), 7 deletions(-)
>>> 
>>> diff --git a/config/ssh/ssh_config b/config/ssh/ssh_config
>>> index ee0954d5c..85c069dda 100644
>>> --- a/config/ssh/ssh_config
>>> +++ b/config/ssh/ssh_config
>>> @@ -5,7 +5,7 @@
>>> 
>>> # Set some basic hardening options for all connections
>>> Host *
>>> - # Disable Roaming as it is known to be vulnerable
>>> + # Disable undocumented roaming feature as it is known to be vulnerable
>>> UseRoaming no
>>> 
>>> # Only use secure crypto algorithms
>>> @@ -13,15 +13,19 @@ Host *
>>> Ciphers chacha20-poly1305@openssh.com,aes256-gcm@openssh.com,aes128-gcm@openssh.com,aes256-ctr,aes192-ctr,aes128-ctr
>>> MACs hmac-sha2-512-etm@openssh.com,hmac-sha2-256-etm@openssh.com,umac-128-etm@openssh.com,hmac-sha2-512,hmac-sha2-256,umac-128@openssh.com
>>> 
>>> - # Always visualise server host keys (but helps to identify key based MITM attacks)
>>> + # Always visualise server host keys (helps to identify key based MITM attacks)
>>> VisualHostKey yes
>>> 
>>> # Use SSHFP (might work on some up-to-date networks) to look up host keys
>>> VerifyHostKeyDNS yes
>>> 
>>> - # send keep-alive messages to connected server to avoid broken connections
>>> + # Send SSH-based keep alive messages to connected server to avoid broken connections
>>> ServerAliveInterval 10
>>> - ServerAliveCountMax 6
>>> + ServerAliveCountMax 30
>>> +
>>> +	# Disable TCP keep alive messages since they can be spoofed and we have SSH-based
>>> +	# keep alive messages enabled; there is no need to do things twice here
>>> +	TCPKeepAlive no
>>> 
>>> # Ensure only allowed authentication methods are used
>>> PreferredAuthentications publickey,keyboard-interactive,password
>>> diff --git a/config/ssh/sshd_config b/config/ssh/sshd_config
>>> index 456556540..76c9b3eb1 100644
>>> --- a/config/ssh/sshd_config
>>> +++ b/config/ssh/sshd_config
>>> @@ -46,11 +46,13 @@ AllowTcpForwarding no
>>> AllowAgentForwarding no
>>> PermitOpen none
>>> 
>>> -# Detect broken sessions by sending keep-alive messages to clients via SSH connection
>>> +# Send SSH-based keep alive messages to connected clients to avoid broken connections
>>> ClientAliveInterval 10
>>> +ClientAliveCountMax 30
>>> 
>>> -# Close unresponsive SSH sessions which fail to answer keep-alive
>>> -ClientAliveCountMax 6
>>> +# Since TCP keep alive messages can be spoofed and we have the SSH-based already,
>>> +# there is no need for this to be enabled as well
>>> +TCPKeepAlive no
>>> 
>>> # Add support for SFTP
>>> Subsystem	sftp	/usr/lib/openssh/sftp-server
>>> -- 
>>> 2.34.1
  

Patch

diff --git a/config/ssh/ssh_config b/config/ssh/ssh_config
index ee0954d5c..85c069dda 100644
--- a/config/ssh/ssh_config
+++ b/config/ssh/ssh_config
@@ -5,7 +5,7 @@ 
 
 # Set some basic hardening options for all connections
 Host *
-        # Disable Roaming as it is known to be vulnerable
+        # Disable undocumented roaming feature as it is known to be vulnerable
         UseRoaming no
 
         # Only use secure crypto algorithms
@@ -13,15 +13,19 @@  Host *
         Ciphers chacha20-poly1305@openssh.com,aes256-gcm@openssh.com,aes128-gcm@openssh.com,aes256-ctr,aes192-ctr,aes128-ctr
         MACs hmac-sha2-512-etm@openssh.com,hmac-sha2-256-etm@openssh.com,umac-128-etm@openssh.com,hmac-sha2-512,hmac-sha2-256,umac-128@openssh.com
 
-        # Always visualise server host keys (but helps to identify key based MITM attacks)
+        # Always visualise server host keys (helps to identify key based MITM attacks)
         VisualHostKey yes
 
         # Use SSHFP (might work on some up-to-date networks) to look up host keys
         VerifyHostKeyDNS yes
 
-        # send keep-alive messages to connected server to avoid broken connections
+        # Send SSH-based keep alive messages to connected server to avoid broken connections
         ServerAliveInterval 10
-        ServerAliveCountMax 6
+        ServerAliveCountMax 30
+
+	# Disable TCP keep alive messages since they can be spoofed and we have SSH-based
+	# keep alive messages enabled; there is no need to do things twice here
+	TCPKeepAlive no
 
         # Ensure only allowed authentication methods are used
         PreferredAuthentications publickey,keyboard-interactive,password
diff --git a/config/ssh/sshd_config b/config/ssh/sshd_config
index 456556540..76c9b3eb1 100644
--- a/config/ssh/sshd_config
+++ b/config/ssh/sshd_config
@@ -46,11 +46,13 @@  AllowTcpForwarding no
 AllowAgentForwarding no
 PermitOpen none
 
-# Detect broken sessions by sending keep-alive messages to clients via SSH connection
+# Send SSH-based keep alive messages to connected clients to avoid broken connections
 ClientAliveInterval 10
+ClientAliveCountMax 30
 
-# Close unresponsive SSH sessions which fail to answer keep-alive
-ClientAliveCountMax 6
+# Since TCP keep alive messages can be spoofed and we have the SSH-based already,
+# there is no need for this to be enabled as well
+TCPKeepAlive no
 
 # Add support for SFTP
 Subsystem	sftp	/usr/lib/openssh/sftp-server