From 9143b43637f8fce3b365a11a8ad879c99215e33b Mon Sep 17 00:00:00 2001 From: benya Date: Thu, 4 Jun 2026 16:55:06 +0300 Subject: [PATCH] Stop remote SSH sessions on close and timeout --- agent/openwrt/rmm-agent.sh | 79 ++++++++++++++++++++++++-- docs/api.md | 1 + docs/remote-access.md | 2 +- server/internal/httpapi/server.go | 25 ++++++-- server/internal/httpapi/server_test.go | 16 ++++++ 5 files changed, 113 insertions(+), 10 deletions(-) diff --git a/agent/openwrt/rmm-agent.sh b/agent/openwrt/rmm-agent.sh index 42df273..54bdc02 100644 --- a/agent/openwrt/rmm-agent.sh +++ b/agent/openwrt/rmm-agent.sh @@ -12,6 +12,7 @@ SPOOL_DIR="${SPOOL_DIR:-/tmp/rmm-agent-results}" BACKUP_DIR="${BACKUP_DIR:-/tmp/rmm-agent-backups}" CHECK_TARGETS="${CHECK_TARGETS:-1.1.1.1 8.8.8.8}" TUNNEL_IDENTITY_FILE="${TUNNEL_IDENTITY_FILE:-/etc/rmm-agent/tunnel_key}" +TUNNEL_STATE_DIR="${TUNNEL_STATE_DIR:-/tmp/rmm-agent-tunnels}" if [ -f "$CONFIG_FILE" ]; then # shellcheck disable=SC1090 @@ -418,6 +419,12 @@ safe_user_name() { [ -z "$clean" ] } +safe_session_id() { + [ -n "$1" ] || return 1 + clean="$(printf '%s' "$1" | tr -d 'A-Za-z0-9_-')" + [ -z "$clean" ] +} + package_manager() { if command -v apk >/dev/null 2>&1; then printf 'apk' @@ -555,7 +562,7 @@ remote_ssh_reverse_output() { [ -n "$server_user" ] || server_user="rmm-tunnel" [ -n "$duration_seconds" ] || duration_seconds="900" - if ! safe_host_name "$server_host" || ! safe_host_name "$local_host" || ! safe_user_name "$server_user"; then + if ! safe_session_id "$session_id" || ! safe_host_name "$server_host" || ! safe_host_name "$local_host" || ! safe_user_name "$server_user"; then printf 'remote tunnel host or user is invalid\n' return 2 fi @@ -565,6 +572,9 @@ remote_ssh_reverse_output() { fi log_file="/tmp/rmm-remote-${session_id:-session}.log" + mkdir -p "$TUNNEL_STATE_DIR" + pid_file="$TUNNEL_STATE_DIR/$session_id.pid" + remote_ssh_stop_session "$session_id" >/dev/null 2>&1 || true rm -f "$log_file" identity_args="" if [ -f "$TUNNEL_IDENTITY_FILE" ]; then @@ -572,18 +582,20 @@ remote_ssh_reverse_output() { fi if command -v ssh >/dev/null 2>&1; then # shellcheck disable=SC2086 - (ssh $identity_args -N -o StrictHostKeyChecking=accept-new -o ExitOnForwardFailure=yes -o ServerAliveInterval=15 -o ServerAliveCountMax=2 -R "$remote_port:$local_host:$local_port" -p "$server_port" "$server_user@$server_host"; printf 'ssh exited with code %s\n' "$?" >> "$log_file") >> "$log_file" 2>&1 & + ssh $identity_args -N -o StrictHostKeyChecking=accept-new -o ExitOnForwardFailure=yes -o ServerAliveInterval=15 -o ServerAliveCountMax=2 -R "$remote_port:$local_host:$local_port" -p "$server_port" "$server_user@$server_host" >> "$log_file" 2>&1 & elif command -v dbclient >/dev/null 2>&1; then # shellcheck disable=SC2086 - (dbclient $identity_args -N -y -R "$remote_port:$local_host:$local_port" -p "$server_port" "$server_user@$server_host"; printf 'dbclient exited with code %s\n' "$?" >> "$log_file") >> "$log_file" 2>&1 & + dbclient $identity_args -N -y -R "$remote_port:$local_host:$local_port" -p "$server_port" "$server_user@$server_host" >> "$log_file" 2>&1 & else printf 'remote ssh reverse requires ssh or dbclient on router\n' return 2 fi pid="$!" - (sleep "$duration_seconds"; kill "$pid" >/dev/null 2>&1 || true) >/dev/null 2>&1 & + printf '%s\n' "$pid" > "$pid_file" + (sleep "$duration_seconds"; remote_ssh_stop_pid "$session_id" "$pid") >/dev/null 2>&1 & sleep 2 if ! kill -0 "$pid" >/dev/null 2>&1; then + rm -f "$pid_file" printf 'remote ssh reverse failed to stay running\n' cat "$log_file" 2>/dev/null return 1 @@ -595,6 +607,61 @@ remote_ssh_reverse_output() { return 0 } +remote_ssh_stop_pid() { + session_id="$1" + expected_pid="$2" + pid_file="$TUNNEL_STATE_DIR/$session_id.pid" + [ -f "$pid_file" ] || return 0 + pid="$(cat "$pid_file" 2>/dev/null)" + [ "$pid" = "$expected_pid" ] || return 0 + case "$pid" in + ''|*[!0-9]*) + rm -f "$pid_file" + return 1 + ;; + esac + kill "$pid" >/dev/null 2>&1 || true + rm -f "$pid_file" +} + +remote_ssh_stop_session() { + session_id="$1" + remote_port="${2:-}" + if ! safe_session_id "$session_id"; then + printf 'remote session id is invalid\n' + return 2 + fi + pid_file="$TUNNEL_STATE_DIR/$session_id.pid" + if [ ! -f "$pid_file" ]; then + if safe_port "$remote_port"; then + for cmdline in /proc/[0-9]*/cmdline; do + [ -f "$cmdline" ] || continue + command_line="$(tr '\000' ' ' < "$cmdline" 2>/dev/null)" + case "$command_line" in + *ssh*" -R $remote_port:"*|*dbclient*" -R $remote_port:"*) + pid="$(printf '%s' "$cmdline" | cut -d/ -f3)" + kill "$pid" >/dev/null 2>&1 || true + printf 'remote ssh session stopped by port\n' + return 0 + ;; + esac + done + fi + printf 'remote ssh session is already stopped\n' + return 0 + fi + pid="$(cat "$pid_file" 2>/dev/null)" + remote_ssh_stop_pid "$session_id" "$pid" + printf 'remote ssh session stopped\n' +} + +remote_ssh_close_output() { + args="$1" + session_id="$(command_arg_string "$args" "session_id")" + remote_port="$(command_arg_string "$args" "remote_port")" + remote_ssh_stop_session "$session_id" "$remote_port" +} + run_command() { cmd_type="$1" args="$2" @@ -742,6 +809,10 @@ run_command() { remote_ssh_reverse_output "$args" return $? ;; + remote_ssh_close) + remote_ssh_close_output "$args" + return $? + ;; *) printf 'command type is not allowlisted\n' return 2 diff --git a/docs/api.md b/docs/api.md index fbd614e..b46ab9c 100644 --- a/docs/api.md +++ b/docs/api.md @@ -329,6 +329,7 @@ Allowed command types: - `uci_revert` - `uci_restore` - `remote_ssh_reverse` +- `remote_ssh_close` Package commands are package-manager aware on the agent. OpenWrt 25.12+ uses `apk`; older releases usually use `opkg`. Legacy `opkg_*` command names are still accepted for compatibility, but the UI uses `pkg_*`. diff --git a/docs/remote-access.md b/docs/remote-access.md index 7f00620..a2b1d79 100644 --- a/docs/remote-access.md +++ b/docs/remote-access.md @@ -17,7 +17,7 @@ Remote access is now implemented as an MVP reverse SSH session flow. The server 3. Server queues `remote_ssh_reverse` for the router. 4. Agent runs `ssh` or `dbclient` and requests `-R remote_port:127.0.0.1:22`. 5. Operator connects to the server endpoint shown by the UI. -6. Session expires automatically or is closed by the operator. +6. Session expires automatically on the router or is closed by an operator command. ## Docker/Linux Requirement diff --git a/server/internal/httpapi/server.go b/server/internal/httpapi/server.go index cea1b36..5638666 100644 --- a/server/internal/httpapi/server.go +++ b/server/internal/httpapi/server.go @@ -1020,7 +1020,7 @@ func (a *App) handleCloseRemoteSession(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusNotFound, "not found") return } - session, found, err := a.store.CloseRemoteSession(r.Context(), deviceID, sessionID) + session, found, err := a.store.GetRemoteSession(r.Context(), deviceID, sessionID) if err != nil { writeError(w, http.StatusInternalServerError, "failed to close remote session") return @@ -1029,9 +1029,24 @@ func (a *App) handleCloseRemoteSession(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusNotFound, "remote session not found") return } - _, _ = a.store.AddAuditEvent(r.Context(), "operator", "remote_session.close", deviceID, session.CommandID, mustJSON(map[string]string{ - "session_id": sessionID, - "request_id": requestID(r.Context()), + closeCommand, commandFound, err := a.store.CreateCommand(r.Context(), deviceID, "remote_ssh_close", mustJSON(map[string]string{ + "session_id": session.ID, + "remote_port": strconv.Itoa(session.RemotePort), + })) + if err != nil || !commandFound { + writeError(w, http.StatusInternalServerError, "failed to queue remote session close") + return + } + session, _, err = a.store.CloseRemoteSession(r.Context(), deviceID, sessionID) + if err != nil { + writeError(w, http.StatusInternalServerError, "failed to close remote session") + return + } + _, _ = a.store.AddAuditEvent(r.Context(), "operator", "remote_session.close", deviceID, closeCommand.ID, mustJSON(map[string]string{ + "session_id": sessionID, + "open_command_id": session.CommandID, + "close_command_id": closeCommand.ID, + "request_id": requestID(r.Context()), })) writeJSON(w, http.StatusOK, session) } @@ -1090,7 +1105,7 @@ func bearerToken(r *http.Request) (string, bool) { func AllowedCommandType(t string) bool { switch t { - case "ping", "traceroute", "route_show", "interfaces_show", "reboot", "service_restart", "pkg_list_installed", "pkg_update", "pkg_list_upgradable", "pkg_install", "pkg_remove", "opkg_list_installed", "opkg_update", "opkg_list_upgradable", "opkg_install", "opkg_remove", "uci_show", "uci_backup", "uci_preview", "uci_set", "uci_commit", "uci_commit_confirmed", "uci_revert", "uci_restore", "remote_ssh_reverse": + case "ping", "traceroute", "route_show", "interfaces_show", "reboot", "service_restart", "pkg_list_installed", "pkg_update", "pkg_list_upgradable", "pkg_install", "pkg_remove", "opkg_list_installed", "opkg_update", "opkg_list_upgradable", "opkg_install", "opkg_remove", "uci_show", "uci_backup", "uci_preview", "uci_set", "uci_commit", "uci_commit_confirmed", "uci_revert", "uci_restore", "remote_ssh_reverse", "remote_ssh_close": return true default: return false diff --git a/server/internal/httpapi/server_test.go b/server/internal/httpapi/server_test.go index 2949996..2a44f62 100644 --- a/server/internal/httpapi/server_test.go +++ b/server/internal/httpapi/server_test.go @@ -348,6 +348,22 @@ func TestAgentOperatorSmokeFlow(t *testing.T) { if remoteSession.Status != "closed" { t.Fatalf("expected closed remote session, got %#v", remoteSession) } + var commandsAfterClose struct { + Commands []struct { + Type string `json:"type"` + Args json.RawMessage `json:"args"` + } `json:"commands"` + } + requestJSON(t, http.MethodGet, srv.URL+"/api/devices/"+enrolled.DeviceID+"/commands", "operator-test", nil, http.StatusOK, &commandsAfterClose) + foundRemoteClose := false + for _, command := range commandsAfterClose.Commands { + if command.Type == "remote_ssh_close" && strings.Contains(string(command.Args), remoteSession.ID) { + foundRemoteClose = true + } + } + if !foundRemoteClose { + t.Fatalf("expected remote_ssh_close command, got %#v", commandsAfterClose.Commands) + } var devices struct { Devices []struct {