Keep generated sessions alive across updates
This commit is contained in:
+50
-27
@@ -52,6 +52,7 @@ const STREAM_INITIAL_WINDOW: usize = 1024 * 1024;
|
||||
/// have accumulated since the last mirror. Keeps the atomic write off the
|
||||
/// per-packet hot path while bounding how much fresh output a crash can lose.
|
||||
const SCREEN_PERSIST_BYTES: usize = 4096;
|
||||
const IMPLICIT_EMPTY_SESSION_GRACE_SECS: u64 = 300;
|
||||
|
||||
/// Sentinel `target_host` sent to the client on a server-initiated `StreamOpen`
|
||||
/// that carries an SSH-agent connection (the client splices it into its local
|
||||
@@ -512,15 +513,16 @@ impl ServerState {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Whether a session named `name` should be backed by a persistent holder.
|
||||
/// Whether a session should be backed by a persistent holder.
|
||||
///
|
||||
/// With persistence enabled, explicitly named sessions get a detached
|
||||
/// holder. Implicit `term-<millis>-<pid>` sessions are one-off terminals
|
||||
/// created by `dosh host`; users cannot intentionally reattach to them by
|
||||
/// name, so persisting them only leaves stale holders that can be
|
||||
/// accidentally re-adopted after restarts.
|
||||
fn should_persist_session(&self, name: &str) -> bool {
|
||||
self.config.persist_sessions && !protocol::is_implicit_session_name(name)
|
||||
/// With persistence enabled, every interactive session gets a detached holder
|
||||
/// so an update/restart of `dosh-server` behaves like a transient network
|
||||
/// break. Named sessions can sit empty for the normal client timeout.
|
||||
/// Generated one-off sessions are also persisted, but cleanup gives them a
|
||||
/// shorter empty grace period so invisible abandoned holders do not linger
|
||||
/// indefinitely.
|
||||
fn should_persist_session(&self, _name: &str) -> bool {
|
||||
self.config.persist_sessions
|
||||
}
|
||||
|
||||
/// Spawn (or, in the persistent case, spawn-and-adopt) a PTY for a session.
|
||||
@@ -608,20 +610,6 @@ impl ServerState {
|
||||
if self.sessions.contains_key(&name) {
|
||||
continue;
|
||||
}
|
||||
if !self.should_persist_session(&name) {
|
||||
eprintln!(
|
||||
"discarding stale implicit persistent session {name} (shell pid {})",
|
||||
meta.shell_pid
|
||||
);
|
||||
persist::request_shutdown(&sessions_dir, &name, None);
|
||||
if meta.shell_pid > 0 {
|
||||
let _ = unsafe { libc::kill(meta.shell_pid, libc::SIGHUP) };
|
||||
let _ = unsafe { libc::kill(meta.shell_pid, libc::SIGTERM) };
|
||||
std::thread::sleep(Duration::from_millis(50));
|
||||
let _ = unsafe { libc::kill(meta.shell_pid, libc::SIGKILL) };
|
||||
}
|
||||
continue;
|
||||
}
|
||||
let (pty, control) = match self.adopt_persistent_pty(&name) {
|
||||
Ok(pair) => pair,
|
||||
Err(err) => {
|
||||
@@ -671,6 +659,7 @@ impl ServerState {
|
||||
fn insert_client(&mut self, session_name: &str, client_id: [u8; 16], client: ClientState) {
|
||||
if let Some(session) = self.sessions.get_mut(session_name) {
|
||||
session.clients.insert(client_id, client);
|
||||
session.empty_since = None;
|
||||
self.client_index
|
||||
.insert(client_id, session_name.to_string());
|
||||
}
|
||||
@@ -3842,9 +3831,9 @@ fn cleanup_disconnected_clients(state: &Arc<Mutex<ServerState>>) {
|
||||
.iter()
|
||||
.filter(|(name, session)| {
|
||||
!prewarm.contains(name.as_str())
|
||||
&& session
|
||||
.empty_since
|
||||
.is_some_and(|since| now.duration_since(since) >= timeout)
|
||||
&& session.empty_since.is_some_and(|since| {
|
||||
now.duration_since(since) >= empty_session_timeout(name, timeout)
|
||||
})
|
||||
})
|
||||
.map(|(name, _)| name.clone())
|
||||
.collect();
|
||||
@@ -3863,6 +3852,14 @@ fn cleanup_disconnected_clients(state: &Arc<Mutex<ServerState>>) {
|
||||
}
|
||||
}
|
||||
|
||||
fn empty_session_timeout(name: &str, configured_timeout: Duration) -> Duration {
|
||||
if protocol::is_implicit_session_name(name) {
|
||||
configured_timeout.min(Duration::from_secs(IMPLICIT_EMPTY_SESSION_GRACE_SECS))
|
||||
} else {
|
||||
configured_timeout
|
||||
}
|
||||
}
|
||||
|
||||
/// Select the client key (current or retained previous epoch) matching the
|
||||
/// packet header's `session_key_id`, so a packet sealed under the pre-rekey key
|
||||
/// during the grace window still decrypts while an expired/stale epoch is
|
||||
@@ -3909,7 +3906,7 @@ mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn persists_named_sessions_when_enabled() {
|
||||
fn persists_terminal_sessions_when_enabled() {
|
||||
let (pty_tx, _rx) = mpsc::unbounded_channel();
|
||||
let config = ServerConfig {
|
||||
persist_sessions: true,
|
||||
@@ -3919,7 +3916,7 @@ mod tests {
|
||||
let state = ServerState::new(config, [0u8; 32], pty_tx);
|
||||
assert!(state.should_persist_session("default")); // prewarmed
|
||||
assert!(state.should_persist_session("work")); // explicitly named
|
||||
assert!(!state.should_persist_session("term-1781470634216-76685")); // one-off terminal
|
||||
assert!(state.should_persist_session("term-1781470634216-76685")); // one-off update-survivable terminal
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -4074,6 +4071,12 @@ mod tests {
|
||||
Some((key, "work".to_string()))
|
||||
);
|
||||
assert!(state.client_mut(&client_id).is_some());
|
||||
state.sessions.get_mut("work").unwrap().empty_since = Some(Instant::now());
|
||||
state.insert_client("work", [7u8; 16], test_client_state([9u8; 32]));
|
||||
assert!(
|
||||
state.sessions["work"].empty_since.is_none(),
|
||||
"reattach must clear empty-since so cleanup cannot reap a live session"
|
||||
);
|
||||
|
||||
// Removing purges both the session map and the index.
|
||||
assert!(state.remove_client_everywhere(&client_id));
|
||||
@@ -4113,6 +4116,26 @@ mod tests {
|
||||
assert!(locked.lookup_client(&client_id).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn implicit_empty_session_timeout_is_bounded_for_update_reconnect() {
|
||||
let configured = Duration::from_secs(2_592_000);
|
||||
let implicit = protocol::generate_implicit_session_name();
|
||||
assert_eq!(
|
||||
empty_session_timeout(&implicit, configured),
|
||||
Duration::from_secs(IMPLICIT_EMPTY_SESSION_GRACE_SECS)
|
||||
);
|
||||
assert_eq!(
|
||||
empty_session_timeout("work", configured),
|
||||
configured,
|
||||
"named sessions keep the normal long timeout"
|
||||
);
|
||||
assert_eq!(
|
||||
empty_session_timeout(&implicit, Duration::from_secs(1)),
|
||||
Duration::from_secs(1),
|
||||
"tests/admins can still configure a shorter timeout"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn forged_plaintext_detach_does_not_remove_client() {
|
||||
let (pty_tx, _pty_rx) = mpsc::unbounded_channel();
|
||||
|
||||
Reference in New Issue
Block a user