Skip to content

fix(server): answer keepalives during quiet Unix sessions #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gl-agnx merged 2 commits into from
Oct 14, 2026
Merged

fix(server): answer keepalives during quiet Unix sessions #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
58bcd8b
fix: answer global keepalive requests during active SSH sessions
gl-agnx Oct 14, 2026
a0a364f
fix: drain channel close before tearing down SSH transport
gl-agnx Oct 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 111 additions & 24 deletions common/noxssh_server.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1245,6 +1245,26 @@ static netnox_return_t srv_send_global_request_failure(netnox_ssh_server_t * s)
return srv_send_packet(s, p, 1u);
}

/* Unsupported global requests still need a reply when requested. In
* particular, OpenSSH ServerAlive probes run while exec/shell owns the
* receive loop; REQUEST_FAILURE is a valid liveness reply, not permission
* to enable forwarding or another unsupported operation. */
static netnox_return_t srv_handle_global_request(netnox_ssh_server_t *s,
const uint8_t *payload,
uint32_t payload_len)
{
uint32_t off = 1u, request_len;
if(s == NULL || payload == NULL || payload_len < 6u ||
payload[0] != NETNOX_SSH_MSG_GLOBAL_REQUEST) return NETNOX_RETURN_BAD_PARAM;
/* Subtract before comparing: an adversarial u32 string length must not
* wrap an offset and manufacture an in-bounds want-reply byte. */
if(netnox_ssh_payload_read_u32(payload,payload_len,&off,&request_len) != NETNOX_RETURN_SUCCESS ||
request_len == 0u || request_len >= payload_len - off) return NETNOX_RETURN_FAILED;
off += request_len;
if(payload[off] != 0u) return srv_send_global_request_failure(s);
return NETNOX_RETURN_SUCCESS;
}

static netnox_return_t srv_send_channel_open_confirmation(netnox_ssh_server_t * s,
uint32_t client_sender_channel)
{
Expand Down Expand Up @@ -1358,6 +1378,37 @@ static netnox_return_t srv_send_channel_close(netnox_ssh_server_t * s)
return srv_send_packet(s, payload, payload_len);
}

#ifndef _WIN32
/* EOF is a half-close, never connection termination (RFC4254 section5.3).
* channel_open stays set until CLOSE; completion already sent our CLOSE
* before clearing it, so a late peer CLOSE must not generate a duplicate. */
static netnox_return_t srv_handle_channel_end(netnox_ssh_server_t *s,
const uint8_t *payload, uint32_t len)
{
uint32_t off = 1u, recipient;
if(!s || !payload || len != 5u ||
(payload[0] != NETNOX_SSH_MSG_CHANNEL_EOF &&
payload[0] != NETNOX_SSH_MSG_CHANNEL_CLOSE) ||
netnox_ssh_payload_read_u32(payload,len,&off,&recipient) != NETNOX_RETURN_SUCCESS ||
recipient != s->local_channel_id) return NETNOX_RETURN_BAD_PARAM;
if(payload[0] == NETNOX_SSH_MSG_CHANNEL_CLOSE && s->channel_open) {
if(srv_send_channel_close(s) != NETNOX_RETURN_SUCCESS) return NETNOX_RETURN_FAILED;
s->channel_open = 0u;
}
return NETNOX_RETURN_SUCCESS;
}

static netnox_return_t srv_begin_close_wait(uint64_t *deadline)
{
uint64_t now;
if(*deadline) return NETNOX_RETURN_SUCCESS; // Late packets cannot extend it.
now = srv_now_ms();
if(now == UINT64_MAX || now > UINT64_MAX - 10000u) return NETNOX_RETURN_FAILED;
*deadline = now + 10000u;
return NETNOX_RETURN_SUCCESS;
}
#endif

static netnox_return_t srv_maybe_window_adjust(netnox_ssh_server_t * s)
{
uint8_t payload[64];
Expand All @@ -1380,6 +1431,25 @@ static netnox_return_t srv_maybe_window_adjust(netnox_ssh_server_t * s)
}

#ifndef _WIN32
// 1=readable, 2=interrupted/retry, 0=peer hangup/drain expired, -1=error.
// Drain only after child completion/CLOSE, so the peer can send its final
// CLOSE/DISCONNECT before TCP teardown. This bounds the first-byte wait;
// complete packet receipt still uses its existing callback/packet deadlines.
static int srv_wait_close_packet(netnox_ssh_server_t *s, uint64_t deadline)
{
uint64_t now = srv_now_ms();
struct pollfd p = {s->transport_fd, POLLIN, 0};
int rc;
if(now == UINT64_MAX || s->transport_fd < 0) return -1;
if(now >= deadline) return 0;
rc = poll(&p,1,(int)(deadline-now));
if(rc < 0) return errno == EINTR ? 2 : -1;
if(!rc) return 0;
if(p.revents & POLLIN) return 1;
if(p.revents & POLLHUP) return 0;
return -1;
}

static netnox_return_t srv_send_exit_status(netnox_ssh_server_t *s, uint32_t status)
{
uint8_t p[64]; uint32_t len = 0u;
Expand Down Expand Up @@ -1409,7 +1479,7 @@ static int srv_write_child(int fd, const uint8_t *data, uint32_t len, int pty)

static netnox_return_t srv_run_session_unix(netnox_ssh_server_t *s, const char *cmd, int shell)
{
int pair[2] = {-1, -1}, child_fd = -1, status = 0, child_reaped = 0, output_eof = 0;
int pair[2] = {-1, -1}, child_fd = -1, status = 0, child_reaped = 0, output_eof = 0, peer_closed = 0;
pid_t pid;
struct winsize ws;
uint8_t payload[NETNOX_SSH_MAX_PACKET_LEN], output[4096];
Expand Down Expand Up @@ -1469,7 +1539,9 @@ static netnox_return_t srv_run_session_unix(netnox_ssh_server_t *s, const char *
uint32_t off = 1u, channel, amount;
const uint8_t *data; uint32_t data_len;
if(srv_recv_packet(s, payload, sizeof(payload), &len) != NETNOX_RETURN_SUCCESS || !len) break;
if(payload[0] == NETNOX_SSH_MSG_CHANNEL_DATA) {
if(payload[0] == NETNOX_SSH_MSG_GLOBAL_REQUEST) {
if(srv_handle_global_request(s,payload,len) != NETNOX_RETURN_SUCCESS) break;
} else if(payload[0] == NETNOX_SSH_MSG_CHANNEL_DATA) {
if(netnox_ssh_payload_read_u32(payload,len,&off,&channel) != NETNOX_RETURN_SUCCESS || channel != s->local_channel_id ||
netnox_ssh_payload_read_string_view(payload,len,&off,&data,&data_len) != NETNOX_RETURN_SUCCESS ||
off != len || data_len > s->local_window_size || srv_write_child(child_fd,data,data_len,s->want_pty)) break;
Expand All @@ -1481,9 +1553,14 @@ static netnox_return_t srv_run_session_unix(netnox_ssh_server_t *s, const char *
channel != s->local_channel_id || amount > UINT32_MAX - s->remote_window_size) break;
s->remote_window_size += amount;
} else if(payload[0] == NETNOX_SSH_MSG_CHANNEL_EOF) {
if(srv_handle_channel_end(s,payload,len) != NETNOX_RETURN_SUCCESS) break;
if(!s->want_pty) shutdown(child_fd, SHUT_WR);
else { const uint8_t eot = 4u; (void)srv_write_child(child_fd,&eot,1u,1); }
} else if(payload[0] == NETNOX_SSH_MSG_CHANNEL_CLOSE || payload[0] == NETNOX_SSH_MSG_DISCONNECT) break;
} else if(payload[0] == NETNOX_SSH_MSG_CHANNEL_CLOSE) {
if(srv_handle_channel_end(s,payload,len) != NETNOX_RETURN_SUCCESS) break;
peer_closed = 1;
break;
} else if(payload[0] == NETNOX_SSH_MSG_DISCONNECT) break;
else if(payload[0] == NETNOX_SSH_MSG_CHANNEL_REQUEST && s->want_pty) {
const uint8_t *req; uint32_t req_len; uint32_t cols, rows, width, height;
if(netnox_ssh_payload_read_u32(payload,len,&off,&channel) != NETNOX_RETURN_SUCCESS ||
Expand Down Expand Up @@ -1515,8 +1592,9 @@ static netnox_return_t srv_run_session_unix(netnox_ssh_server_t *s, const char *
if(!child_reaped && waitpid(pid,&status,WNOHANG) == pid) child_reaped = 1;
if(output_eof && child_reaped) {
uint32_t exit_code = WIFEXITED(status) ? (uint32_t)WEXITSTATUS(status) : 128u + (uint32_t)WTERMSIG(status);
(void)srv_send_exit_status(s, exit_code);
(void)srv_send_channel_eof(s); (void)srv_send_channel_close(s);
if(srv_send_exit_status(s, exit_code) != NETNOX_RETURN_SUCCESS ||
srv_send_channel_eof(s) != NETNOX_RETURN_SUCCESS ||
srv_send_channel_close(s) != NETNOX_RETURN_SUCCESS) break;
s->channel_open = 0u; close(child_fd); return NETNOX_RETURN_SUCCESS;
}
}
Expand All @@ -1527,7 +1605,7 @@ static netnox_return_t srv_run_session_unix(netnox_ssh_server_t *s, const char *
if(waitpid(pid,&status,WNOHANG)==0) { kill(-pid,SIGKILL); waitpid(pid,&status,0); }
}
s->channel_open = 0u;
return NETNOX_RETURN_FAILED;
return peer_closed ? NETNOX_RETURN_SUCCESS : NETNOX_RETURN_FAILED;
}
#else
static netnox_return_t srv_run_exec_win32(netnox_ssh_server_t * s, const char * cmd)
Expand Down Expand Up @@ -1785,6 +1863,9 @@ netnox_return_t netnox_ssh_server_serve_one(netnox_ssh_server_t * s)
{
uint8_t payload[NETNOX_SSH_MAX_PACKET_LEN];
uint32_t payload_len = 0u;
#ifndef _WIN32
uint64_t close_deadline_ms = 0u;
#endif

if(s == NULL || s->send_cb == NULL || s->recv_cb == NULL) {
return NETNOX_RETURN_BAD_PARAM;
Expand All @@ -1796,6 +1877,13 @@ netnox_return_t netnox_ssh_server_serve_one(netnox_ssh_server_t * s)
}

for(;;) {
#ifndef _WIN32
if(close_deadline_ms) {
int ready = srv_wait_close_packet(s,close_deadline_ms);
if(ready == 2) continue;
if(ready <= 0) return ready == 0 ? NETNOX_RETURN_SUCCESS : NETNOX_RETURN_FAILED;
}
#endif
if(srv_recv_packet(s, payload, (uint32_t)sizeof(payload), &payload_len) != NETNOX_RETURN_SUCCESS) {
SRVDBG("receive packet failed");
break;
Expand All @@ -1818,23 +1906,8 @@ netnox_return_t netnox_ssh_server_serve_one(netnox_ssh_server_t * s)
case NETNOX_SSH_MSG_EXT_INFO:
continue;
case NETNOX_SSH_MSG_GLOBAL_REQUEST: {
uint32_t off = 1u;
const uint8_t * req = NULL;
uint32_t req_len = 0u;
uint8_t want = 0u;
if(payload_len < 3u) {
continue;
}
if(netnox_ssh_payload_read_string_view(payload, payload_len, &off, &req, &req_len) != NETNOX_RETURN_SUCCESS) {
continue;
}
if(off >= payload_len) {
continue;
}
want = payload[off];
if(want != 0u) {
(void)srv_send_global_request_failure(s);
}
if(srv_handle_global_request(s,payload,payload_len) != NETNOX_RETURN_SUCCESS)
return NETNOX_RETURN_FAILED;
continue;
}
case NETNOX_SSH_MSG_SERVICE_REQUEST: {
Expand Down Expand Up @@ -1872,6 +1945,9 @@ netnox_return_t netnox_ssh_server_serve_one(netnox_ssh_server_t * s)
}
continue;
case NETNOX_SSH_MSG_CHANNEL_OPEN: {
#ifndef _WIN32
if(close_deadline_ms) return NETNOX_RETURN_FAILED;
#endif
uint32_t off = 1u;
const uint8_t * typ = NULL;
uint32_t typ_len = 0u;
Expand Down Expand Up @@ -1973,6 +2049,7 @@ netnox_return_t netnox_ssh_server_serve_one(netnox_ssh_server_t * s)
if(srv_run_session_unix(s, cmdbuf, 0) != NETNOX_RETURN_SUCCESS) {
return NETNOX_RETURN_FAILED;
}
if(srv_begin_close_wait(&close_deadline_ms) != NETNOX_RETURN_SUCCESS) return NETNOX_RETURN_FAILED;
#else
if(srv_run_exec_win32(s, cmdbuf) != NETNOX_RETURN_SUCCESS) {
return NETNOX_RETURN_FAILED;
Expand All @@ -1985,6 +2062,7 @@ netnox_return_t netnox_ssh_server_serve_one(netnox_ssh_server_t * s)
#ifndef _WIN32
if(want_reply && srv_send_channel_success(s) != NETNOX_RETURN_SUCCESS) return NETNOX_RETURN_FAILED;
if(srv_run_session_unix(s, NULL, 1) != NETNOX_RETURN_SUCCESS) return NETNOX_RETURN_FAILED;
if(srv_begin_close_wait(&close_deadline_ms) != NETNOX_RETURN_SUCCESS) return NETNOX_RETURN_FAILED;
#else
if(want_reply) (void)srv_send_channel_failure(s);
#endif
Expand Down Expand Up @@ -2039,9 +2117,18 @@ netnox_return_t netnox_ssh_server_serve_one(netnox_ssh_server_t * s)
continue;
}
case NETNOX_SSH_MSG_CHANNEL_EOF:
case NETNOX_SSH_MSG_CHANNEL_CLOSE:
case NETNOX_SSH_MSG_CHANNEL_CLOSE: {
#ifndef _WIN32
if(srv_handle_channel_end(s,payload,payload_len) != NETNOX_RETURN_SUCCESS)
return NETNOX_RETURN_FAILED;
if(payload[0] == NETNOX_SSH_MSG_CHANNEL_CLOSE &&
srv_begin_close_wait(&close_deadline_ms) != NETNOX_RETURN_SUCCESS) return NETNOX_RETURN_FAILED;
continue;
#else
s->channel_open = 0u;
return NETNOX_RETURN_SUCCESS;
#endif
}
default:
SRVDBG("unhandled msg %u", (unsigned)payload[0]);
continue;
Expand Down
100 changes: 100 additions & 0 deletions docs/server.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,103 @@ Evidence under `E:/MPUNoxV_build_artifacts/ethernet_qualification_20261013`:
RX busy/buffer drops were recorded. These tests prove working SSH over
physical Ethernet, not line-rate, zero-drop operation, complete link-loss
recovery, power-cycle persistence or autonomous FPGA flash configuration.

### Quiet exec keepalive correction (source candidate)

The Unix exec/PTY session has its own receive loop. Source through `2e294de`
answered unsupported global requests in the outer connection loop but
dropped them inside a running session. OpenSSH sends a global
`keepalive@openssh.com` request with a requested reply and counts missing
replies toward its `ServerAliveCountMax` timeout. A failure reply is valid
liveness evidence and does not grant the unsupported operation. See the
[OpenSSH 9.5 client implementation](https://github.com/openssh/openssh-portable/blob/V_9_5_P1/clientloop.c#L444-L483).

The source fix uses one validated global-request handler in both loops.
It sends no reply for want-reply zero and `SSH_MSG_REQUEST_FAILURE` for a
nonzero flag. Truncated headers, missing boolean and overflowing string
lengths fail closed; transport send failures propagate. Request-specific
arguments for unsupported requests remain opaque, and forwarding stays
unsupported. No authentication, credential, host seed, crypto or client
keepalive setting changes are part of this correction. The correction is
for the Unix server, not the unused Windows one-shot `_popen` implementation.

Offline test:

```sh
python3 -B scripts/test_session_keepalive.py --fixture-parent <private-scratch>
```

This compiles the actual parser and both actual dispatch blocks with
`-Wall -Wextra -Werror`, using a simulated send transport. It checks repeated
quiet-session probes, want-reply zero/one/nonzero, unsupported request
arguments, send failure, and malformed/truncated/overflow requests. It
does not establish encrypted transport, a Unix child or RV64 execution.

Root must compile the full Linux server and run the existing isolated-account
`scripts/ci_smoke_sshd.sh`. That smoke now includes a quiet 45-second command
with stdout still open and a quiet 45-second output-EOF child returning 9,
both with `ServerAliveInterval=10` and `ServerAliveCountMax=3`. The former
11-second smoke without keepalive probes did not cover this failure.

On 2026-10-14, the board operator reported a read-only
`sleep 45; printf QUIET_COMMAND_COMPLETED` reproducer exiting 255 after
45.391 seconds with no stdout under those same keepalive settings. This is
pre-fix reproduction, not validation of a new binary. After source and
binary qualification, root should repeat that exact command through the
existing trusted-host/public-key SSH policy and require exit 0 plus the
exact marker; also check the output-EOF case returns 9 rather than 255.
The root-owned updater must not resume from an ambiguous timeout without
its separate checkpoint/card-state review.

### Fast PTY close correction (source candidate)

The longer keepalive CI smoke exposed an older close-order bug on the fast
Linux host: after the PTY child completed, the outer loop received a queued
stdin `CHANNEL_EOF` and immediately returned, closing TCP before the client
finished its CLOSE/DISCONNECT exchange. The log ended at packet96 and
OpenSSH reported `client_loop: send disconnect: Broken pipe`, exit255.
The daemon's180-second timeout had not expired. This failure is not ignored.

[RFC4254 section5.3](https://www.rfc-editor.org/rfc/rfc4254#section-5.3)
defines EOF as a half-close and requires a reciprocal CLOSE unless already
sent. [OpenSSH9.5](https://github.com/openssh/openssh-portable/blob/V_9_5_P1/clientloop.c#L1526-L1536)
sends a final transport DISCONNECT and treats a failed send as fatal.
The correction validates EOF/CLOSE framing and recipient, keeps EOF from
terminating the connection, acknowledges CLOSE exactly once, and preserves
the transport briefly for the final client packets. Successful child
completion now also requires successful exit-status, EOF and CLOSE sends.
An active client's intentional CLOSE still ends its child session and
receives the required CLOSE reply, without manufacturing a child exit code.

The post-close read wait uses a fixed10-second absolute deadline that late
EOF, duplicate CLOSE, IGNORE or keepalive packets cannot extend. No new
channel opens while that completed connection drains. It ends promptly on
client DISCONNECT/hangup or expiry; there is no added unconditional sleep.
The existing packet reader checks its10-second deadline before each receive
callback; the POSIX daemon configures each blocking callback with a10-second
socket timeout. A callback entered just before the packet deadline can still
wait another10 seconds. Therefore the bound is the10-second first-byte poll
plus up to roughly20 seconds of packet receipt, excluding crypto processing;
it is **not a hard10-second whole-drain deadline**. This bound assumes the
daemon's socket timeout was installed and does not apply to arbitrary blocking
caller-supplied transport callbacks. No authentication, keys or SSH keepalive
policy changes are part of this correction. The unused Windows one-shot
server retains its original EOF/CLOSE path.

```sh
python3 -B scripts/test_channel_close.py --fixture-parent <private-scratch>
```

The native fixture compiles the actual channel-end, completion and outer
dispatch blocks plus clock/poll helpers with simulated transport. It checks
EOF alone keeps the connection open; EOF/CLOSE/DISCONNECT ordering;
reply-once and send failures; framing/channel identity; fixed deadlines,
readable-with-hangup, interruption/errors; and exit-status9/EOF/CLOSE order.
It is not actual Unix/fork/PTY or encrypted SSH integration. The Linux smoke
retains the original PTY check and adds20 fast-close repetitions plus a PTY
returning9. All markers must occur as standalone output lines, so an echoed
input command cannot satisfy the check; any exit255 remains a failure. Native/full Linux builds,
CI and root-owned physical retests must qualify this successor separately
from the prior keepalive binary. Where VM sudo is unavailable, use the
unprivileged native fixtures/build/CTest and root's authorized existing-account
temporary-daemon tests; no new account or credential change is required.
Loading
Loading