Mailing List Archive

[bug][sshd][security] ClientAliveInterval multiplied twice
Hello,

My team discovered that starting from openssh v9.2 a ClientAliveInterval param from /etc/ssh/sshd_config
is interpreted as multiplied twice value.

----
Component: sshd
Version: 9.2p1 and higher
Hardware: ARM64
OS:Linux
----

sshd_config
----
ClientAliveInterval 5
ClientAliveCountMax 3
---

Debug logs
----
[2023-05-29 12:00:16] debug1: client_input_channel_req: channel 0 rtype keepalive@openssh.com reply 1
[2023-05-29 12:00:26] debug1: client_input_channel_req: channel 0 rtype keepalive@openssh.com reply 1
[2023-05-29 12:00:36] debug1: client_input_channel_req: channel 0 rtype keepalive@openssh.com reply 1
[2023-05-29 12:00:46] debug1: client_input_channel_req: channel 0 rtype keepalive@openssh.com reply 1
----

Looks like the problem was introduced with commit d478cdc7ad6edd4b1bcd1e86fb2f23194ff33d5a
With a new ptimeout api sshd server can double a client probing interval time.

commit d478cdc7ad6edd4b1bcd1e86fb2f23194ff33d5a
Author: djm@openbsd.org <djm@openbsd.org>
Date: Fri Jan 6 02:38:23 2023 +0000

upstream: replace manual poll/ppoll timeout math with ptimeout API

feedback markus / ok markus dtucker

OpenBSD-Commit-ID: c5ec4f2d52684cdb788cd9cbc1bcf89464014be2


@@ -251,19 +237,18 @@ wait_until_can_do_something(struct ssh *ssh,
*conn_in_readyp = (*pfdp)[0].revents != 0;
*conn_out_readyp = (*pfdp)[1].revents != 0;

+ /* ClientAliveInterval probing */
if (client_alive_scheduled) {
time_t now = monotime();
-
- /*
- * If the ppoll timed out, or returned for some other reason
- * but we haven't heard from the client in time, send keepalive.
- */
- if (ret == 0 || (last_client_time != 0 && last_client_time +
- options.client_alive_interval <= now)) {
+ if (ret == 0 &&
+ now > last_client_time + options.client_alive_interval) {
+ /* ppoll timed out and we're due to probe */
client_alive_check(ssh);
last_client_time = now;
- } else if (*conn_in_readyp)
+ } else if (ret != 0 && *conn_in_readyp) {
+ /* Data from peer; reset probe timer. */
last_client_time = now;
+ }
}
}


This bug can be easily fixed by changing client_alive_check() condition.
Proposed soulution/Fix:

From 6cf45ca16c51cc99b5210dea708e22e4dec8b2b0 Mon Sep 17 00:00:00 2001
From: Dawid Majchrzak <dawid.majchrzak@nokia.com>
Date: Wed, 31 May 2023 18:51:53 +0200
Subject: [PATCH] serverloop: Fix ClientAliveInterval probing condition

Commit d478cdc7ad6edd4b1bcd1e86fb2f23194ff33d5a introduced
ClientAliveInterval parameter regression that can double a probing
interval time.

Signed-off-by: Dawid Majchrzak <dawid.majchrzak@nokia.com>
---
serverloop.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/serverloop.c b/serverloop.c
index de5fa2e3..f160550e 100644
--- a/serverloop.c
+++ b/serverloop.c
@@ -253,7 +253,7 @@ wait_until_can_do_something(struct ssh *ssh,
/* ClientAliveInterval probing */
if (client_alive_scheduled) {
if (ret == 0 &&
- now > last_client_time + options.client_alive_interval) {
+ now >= last_client_time + options.client_alive_interval) {
/* ppoll timed out and we're due to probe */
client_alive_check(ssh);
last_client_time = now;
--
2.25.1


BR, Dawid
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev