From 55d001d9f89d7fb6ed257ef04f326e2c53bb1a0e Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Tue, 10 Feb 2026 16:12:41 -0600 Subject: [PATCH] Add fork()-based process isolation for SNMP operations Each SNMP GET/WALK now runs in a forked child process. If libnetsnmp triggers a SIGSEGV or other fatal signal, only the child dies - the parent agent logs the crash and continues operating for all other devices. Key changes: - C helper: snmp_get_isolated() and snmp_walk_isolated() using fork+pipe pattern with 60s alarm watchdog and mutex-serialized forks - Rust: IsolationMode enum (Fork/Direct) controlled by TOWEROPS_SNMP_ISOLATION env var, defaults to Fork - New CrashRecovered error variant with signal info and logging - Device poller logs crash recovery events at error level - Startup logs active isolation mode Set TOWEROPS_SNMP_ISOLATION=direct to disable isolation for debugging. --- .cargo/config.toml | 7 + build.rs | 1 + native/snmp_helper.c | 417 +++++++++++++++++++++++++++++ native/snmp_helper.h | 86 ++++++ src/main.rs | 3 + src/snmp/client.rs | 539 +++++++++++++++++++++++++++++++++++++- src/snmp/device_poller.rs | 44 +++- src/snmp/mod.rs | 2 +- src/snmp/types.rs | 16 ++ 9 files changed, 1097 insertions(+), 18 deletions(-) create mode 100644 .cargo/config.toml diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 0000000..b370d4b --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,7 @@ +# Limit test parallelism to avoid macOS fork() issues with Objective-C runtime. +# Our SNMP isolation uses fork() for crash protection, which conflicts with +# macOS's multi-threaded fork safety checks when many threads fork concurrently. +# On Linux (production target), this limit is not needed but is harmless. +[env] +OBJC_DISABLE_INITIALIZE_FORK_SAFETY = "YES" +RUST_TEST_THREADS = "2" diff --git a/build.rs b/build.rs index 80a4e68..dd8aaf8 100644 --- a/build.rs +++ b/build.rs @@ -6,6 +6,7 @@ fn main() { cc::Build::new() .file("native/snmp_helper.c") .include("native") + .define("SNMP_HELPER_TEST", None) .compile("snmp_helper"); // Link against netsnmp library diff --git a/native/snmp_helper.c b/native/snmp_helper.c index 7d32334..fc0222a 100644 --- a/native/snmp_helper.c +++ b/native/snmp_helper.c @@ -4,6 +4,11 @@ #include #include #include +#include +#include +#include +#include +#include static pthread_once_t init_once = PTHREAD_ONCE_INIT; @@ -476,3 +481,415 @@ int snmp_walk( return 0; } + +/* --- Process-isolated (fork-based) operations --- */ + +/** + * Write exactly `len` bytes to fd, retrying on EINTR. + * Returns 0 on success, -1 on error. + */ +static int write_full(int fd, const void* buf, size_t len) { + const uint8_t* p = (const uint8_t*)buf; + size_t remaining = len; + while (remaining > 0) { + ssize_t n = write(fd, p, remaining); + if (n < 0) { + if (errno == EINTR) continue; + return -1; + } + p += n; + remaining -= (size_t)n; + } + return 0; +} + +/** + * Read exactly `len` bytes from fd, retrying on EINTR. + * Returns 0 on success, -1 on error/EOF. + */ +static int read_full(int fd, void* buf, size_t len) { + uint8_t* p = (uint8_t*)buf; + size_t remaining = len; + while (remaining > 0) { + ssize_t n = read(fd, p, remaining); + if (n < 0) { + if (errno == EINTR) continue; + return -1; + } + if (n == 0) return -1; /* unexpected EOF */ + p += n; + remaining -= (size_t)n; + } + return 0; +} + +/** + * Reset signal handlers to defaults in the child process. + * This ensures any crash handler installed by the parent doesn't interfere. + */ +static void child_reset_signals(void) { + signal(SIGSEGV, SIG_DFL); + signal(SIGBUS, SIG_DFL); + signal(SIGABRT, SIG_DFL); + signal(SIGPIPE, SIG_DFL); +} + +/* + * Serialize fork operations to avoid issues with concurrent forks + * in multi-threaded processes. On macOS, concurrent fork() from + * multiple threads can trigger Objective-C runtime crashes (SIGKILL). + * On Linux this is still beneficial as it prevents resource exhaustion. + */ +static pthread_mutex_t fork_mutex = PTHREAD_MUTEX_INITIALIZER; + +#ifdef __APPLE__ +/* + * On macOS, the Objective-C runtime kills forked children from + * multi-threaded parents by default. Our children never use Objective-C + * and _exit() after SNMP work, so this is safe to disable. + * Set before main() to ensure it's in place before any threads start. + */ +__attribute__((constructor)) +static void disable_objc_fork_safety(void) { + setenv("OBJC_DISABLE_INITIALIZE_FORK_SAFETY", "YES", 0); +} +#endif + +void snmp_get_isolated( + const char* ip_address, + uint16_t port, + const char* community, + int version, + int64_t timeout_us, + int retries, + const snmp_v3_config_t* v3_config, + const char* oid_str, + snmp_isolated_get_result_t* result +) { + /* Initialize result to error state */ + memset(result, 0, sizeof(*result)); + result->status = -1; + + pthread_mutex_lock(&fork_mutex); + + int pipefd[2]; + if (pipe(pipefd) != 0) { + snprintf(result->error_buf, sizeof(result->error_buf), + "pipe() failed: %s", strerror(errno)); + pthread_mutex_unlock(&fork_mutex); + return; + } + + pid_t pid = fork(); + if (pid < 0) { + close(pipefd[0]); + close(pipefd[1]); + snprintf(result->error_buf, sizeof(result->error_buf), + "fork() failed: %s", strerror(errno)); + pthread_mutex_unlock(&fork_mutex); + return; + } + + if (pid == 0) { + /* === CHILD PROCESS === */ + close(pipefd[0]); /* close read end */ + child_reset_signals(); + alarm(60); /* watchdog: kill child if stuck */ + + /* Initialize net-snmp fresh in child */ + init_snmp("towerops-child"); + netsnmp_ds_set_int(NETSNMP_DS_LIBRARY_ID, + NETSNMP_DS_LIB_OID_OUTPUT_FORMAT, + NETSNMP_OID_OUTPUT_NUMERIC); + + snmp_isolated_get_result_t child_result; + memset(&child_result, 0, sizeof(child_result)); + child_result.status = -1; + + /* Open session */ + char error_buf[512] = {0}; + void* sess = snmp_open_session(ip_address, port, community, version, + timeout_us, retries, v3_config, + error_buf, sizeof(error_buf)); + if (!sess) { + snprintf(child_result.error_buf, sizeof(child_result.error_buf), + "%s", error_buf); + write_full(pipefd[1], &child_result, sizeof(child_result)); + close(pipefd[1]); + _exit(1); + } + + /* Perform GET */ + int value_type = 0; + int ret = snmp_get(sess, oid_str, + child_result.value_buf, sizeof(child_result.value_buf), + &value_type, child_result.error_buf, + sizeof(child_result.error_buf)); + snmp_close_session(sess); + + child_result.status = ret; + child_result.value_type = value_type; + write_full(pipefd[1], &child_result, sizeof(child_result)); + close(pipefd[1]); + _exit(0); + } + + /* === PARENT PROCESS === */ + close(pipefd[1]); /* close write end */ + + /* Unlock after fork so other threads can proceed */ + pthread_mutex_unlock(&fork_mutex); + + /* Try to read the result from the child */ + snmp_isolated_get_result_t pipe_result; + memset(&pipe_result, 0, sizeof(pipe_result)); + int read_ok = read_full(pipefd[0], &pipe_result, sizeof(pipe_result)); + close(pipefd[0]); + + /* Wait for child to exit */ + int wstatus = 0; + pid_t waited; + do { + waited = waitpid(pid, &wstatus, 0); + } while (waited < 0 && errno == EINTR); + + if (waited < 0) { + snprintf(result->error_buf, sizeof(result->error_buf), + "waitpid() failed: %s", strerror(errno)); + result->status = -1; + return; + } + + if (WIFSIGNALED(wstatus)) { + /* Child was killed by a signal (crash) */ + result->status = -2; + result->child_signal = WTERMSIG(wstatus); + snprintf(result->error_buf, sizeof(result->error_buf), + "SNMP child process killed by signal %d", result->child_signal); + return; + } + + if (read_ok != 0) { + /* Could not read from pipe but child exited normally - unexpected */ + result->status = -1; + snprintf(result->error_buf, sizeof(result->error_buf), + "Failed to read result from child (exit code %d)", + WEXITSTATUS(wstatus)); + return; + } + + /* Successfully read result from child */ + memcpy(result, &pipe_result, sizeof(*result)); +} + +void snmp_walk_isolated( + const char* ip_address, + uint16_t port, + const char* community, + int version, + int64_t timeout_us, + int retries, + const snmp_v3_config_t* v3_config, + const char* oid_str, + snmp_isolated_walk_header_t* header, + snmp_walk_result_t* results, + size_t max_results +) { + /* Initialize header to error state */ + memset(header, 0, sizeof(*header)); + header->status = -1; + + pthread_mutex_lock(&fork_mutex); + + int pipefd[2]; + if (pipe(pipefd) != 0) { + snprintf(header->error_buf, sizeof(header->error_buf), + "pipe() failed: %s", strerror(errno)); + pthread_mutex_unlock(&fork_mutex); + return; + } + + pid_t pid = fork(); + if (pid < 0) { + close(pipefd[0]); + close(pipefd[1]); + snprintf(header->error_buf, sizeof(header->error_buf), + "fork() failed: %s", strerror(errno)); + pthread_mutex_unlock(&fork_mutex); + return; + } + + if (pid == 0) { + /* === CHILD PROCESS === */ + close(pipefd[0]); /* close read end */ + child_reset_signals(); + alarm(60); /* watchdog */ + + /* Initialize net-snmp fresh in child */ + init_snmp("towerops-child"); + netsnmp_ds_set_int(NETSNMP_DS_LIBRARY_ID, + NETSNMP_DS_LIB_OID_OUTPUT_FORMAT, + NETSNMP_OID_OUTPUT_NUMERIC); + + snmp_isolated_walk_header_t child_header; + memset(&child_header, 0, sizeof(child_header)); + child_header.status = -1; + + /* Open session */ + char error_buf[512] = {0}; + void* sess = snmp_open_session(ip_address, port, community, version, + timeout_us, retries, v3_config, + error_buf, sizeof(error_buf)); + if (!sess) { + snprintf(child_header.error_buf, sizeof(child_header.error_buf), + "%s", error_buf); + write_full(pipefd[1], &child_header, sizeof(child_header)); + close(pipefd[1]); + _exit(1); + } + + /* Allocate results buffer in child */ + snmp_walk_result_t* child_results = (snmp_walk_result_t*)calloc( + max_results, sizeof(snmp_walk_result_t)); + if (!child_results) { + snprintf(child_header.error_buf, sizeof(child_header.error_buf), + "Failed to allocate walk results buffer"); + snmp_close_session(sess); + write_full(pipefd[1], &child_header, sizeof(child_header)); + close(pipefd[1]); + _exit(1); + } + + /* Perform WALK */ + size_t num_results = 0; + int ret = snmp_walk(sess, oid_str, child_results, max_results, + &num_results, child_header.error_buf, + sizeof(child_header.error_buf)); + snmp_close_session(sess); + + child_header.status = ret; + child_header.num_results = (uint32_t)num_results; + + /* Write header first */ + write_full(pipefd[1], &child_header, sizeof(child_header)); + + /* Write each result individually (each < PIPE_BUF) */ + for (size_t i = 0; i < num_results; i++) { + write_full(pipefd[1], &child_results[i], sizeof(snmp_walk_result_t)); + } + + free(child_results); + close(pipefd[1]); + _exit(0); + } + + /* === PARENT PROCESS === */ + close(pipefd[1]); /* close write end */ + + /* Unlock after fork so other threads can proceed */ + pthread_mutex_unlock(&fork_mutex); + + /* Read header from child */ + snmp_isolated_walk_header_t pipe_header; + memset(&pipe_header, 0, sizeof(pipe_header)); + int read_ok = read_full(pipefd[0], &pipe_header, sizeof(pipe_header)); + + uint32_t results_read = 0; + if (read_ok == 0 && pipe_header.status >= 0 && pipe_header.num_results > 0) { + /* Read individual results, capping at max_results */ + uint32_t to_read = pipe_header.num_results; + if (to_read > (uint32_t)max_results) { + to_read = (uint32_t)max_results; + } + for (uint32_t i = 0; i < to_read; i++) { + if (read_full(pipefd[0], &results[i], sizeof(snmp_walk_result_t)) != 0) { + break; + } + results_read++; + } + } + close(pipefd[0]); + + /* Wait for child to exit */ + int wstatus = 0; + pid_t waited; + do { + waited = waitpid(pid, &wstatus, 0); + } while (waited < 0 && errno == EINTR); + + if (waited < 0) { + snprintf(header->error_buf, sizeof(header->error_buf), + "waitpid() failed: %s", strerror(errno)); + header->status = -1; + return; + } + + if (WIFSIGNALED(wstatus)) { + /* Child was killed by a signal (crash) */ + header->status = -2; + header->child_signal = WTERMSIG(wstatus); + snprintf(header->error_buf, sizeof(header->error_buf), + "SNMP child process killed by signal %d", header->child_signal); + return; + } + + if (read_ok != 0) { + header->status = -1; + snprintf(header->error_buf, sizeof(header->error_buf), + "Failed to read header from child (exit code %d)", + WEXITSTATUS(wstatus)); + return; + } + + /* Successfully read from child */ + memcpy(header, &pipe_header, sizeof(*header)); + header->num_results = results_read; +} + +#ifdef SNMP_HELPER_TEST +int snmp_test_crash_in_child(int* child_signal) { + if (!child_signal) return -1; + *child_signal = 0; + + int pipefd[2]; + if (pipe(pipefd) != 0) return -1; + + pid_t pid = fork(); + if (pid < 0) { + close(pipefd[0]); + close(pipefd[1]); + return -1; + } + + if (pid == 0) { + /* Child: close pipe ends and deliberately crash */ + close(pipefd[0]); + close(pipefd[1]); + child_reset_signals(); + + /* Trigger SIGSEGV by writing to a null pointer */ + volatile int* null_ptr = NULL; + *null_ptr = 42; + _exit(99); /* should not reach here */ + } + + /* Parent */ + close(pipefd[0]); + close(pipefd[1]); + + int wstatus = 0; + pid_t waited; + do { + waited = waitpid(pid, &wstatus, 0); + } while (waited < 0 && errno == EINTR); + + if (waited < 0) return -1; + + if (WIFSIGNALED(wstatus)) { + *child_signal = WTERMSIG(wstatus); + return 0; + } + + return -1; /* child didn't crash as expected */ +} +#endif diff --git a/native/snmp_helper.h b/native/snmp_helper.h index cc3f936..11adc9f 100644 --- a/native/snmp_helper.h +++ b/native/snmp_helper.h @@ -108,4 +108,90 @@ int snmp_walk( size_t error_buf_len ); +/** + * Result from isolated (fork-based) SNMP GET operation. + * status >= 0: value_len (success), -1: error, -2: child crash + */ +typedef struct { + int status; + int value_type; + int child_signal; + char error_buf[512]; + uint8_t value_buf[1024]; +} snmp_isolated_get_result_t; + +/** + * Header for isolated SNMP WALK result stream. + * status 0: success, -1: error, -2: child crash + */ +typedef struct { + int status; + uint32_t num_results; + int child_signal; + char error_buf[512]; +} snmp_isolated_walk_header_t; + +/** + * Perform an SNMP GET in a forked child process for crash isolation. + * + * @param ip_address IP address of the device + * @param port UDP port (usually 161) + * @param community Community string for SNMPv1/v2c + * @param version SNMP version: 1 (SNMPv1), 2 (SNMPv2c), 3 (SNMPv3) + * @param timeout_us Timeout in microseconds + * @param retries Number of retries + * @param v3_config SNMPv3 configuration (NULL for v1/v2c) + * @param oid_str OID string to GET + * @param result Output result structure + */ +void snmp_get_isolated( + const char* ip_address, + uint16_t port, + const char* community, + int version, + int64_t timeout_us, + int retries, + const snmp_v3_config_t* v3_config, + const char* oid_str, + snmp_isolated_get_result_t* result +); + +/** + * Perform an SNMP WALK in a forked child process for crash isolation. + * + * @param ip_address IP address of the device + * @param port UDP port (usually 161) + * @param community Community string for SNMPv1/v2c + * @param version SNMP version: 1 (SNMPv1), 2 (SNMPv2c), 3 (SNMPv3) + * @param timeout_us Timeout in microseconds + * @param retries Number of retries + * @param v3_config SNMPv3 configuration (NULL for v1/v2c) + * @param oid_str Starting OID string to WALK + * @param header Output header (status, num_results, error) + * @param results Output buffer for walk results + * @param max_results Maximum number of results + */ +void snmp_walk_isolated( + const char* ip_address, + uint16_t port, + const char* community, + int version, + int64_t timeout_us, + int retries, + const snmp_v3_config_t* v3_config, + const char* oid_str, + snmp_isolated_walk_header_t* header, + snmp_walk_result_t* results, + size_t max_results +); + +#ifdef SNMP_HELPER_TEST +/** + * Test helper: deliberately crashes (SIGSEGV) in a forked child. + * Returns 0 on success (child crashed as expected), -1 on error. + * child_signal receives the signal that killed the child. + */ +int snmp_test_crash_in_child(int* child_signal); +#endif + #endif // SNMP_HELPER_H diff --git a/src/main.rs b/src/main.rs index 9b9b06c..9aeff5b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -130,6 +130,8 @@ fn install_crash_handler() { let msg = format!( "\n*** FATAL: {} (signal {})\n\ *** This is likely a bug in C FFI code (libnetsnmp).\n\ + *** SNMP process isolation should prevent most crashes from reaching here.\n\ + *** If this persists, check TOWEROPS_SNMP_ISOLATION env var.\n\ *** Set RUST_BACKTRACE=1 for more info.\n", name, sig ); @@ -195,6 +197,7 @@ async fn async_main() { } tracing::info!("Towerops agent starting"); + tracing::info!("SNMP isolation mode: {:?}", snmp::isolation_mode()); // Check for newer Docker image version version::check_for_updates(); diff --git a/src/snmp/client.rs b/src/snmp/client.rs index f8959b0..d5bd065 100644 --- a/src/snmp/client.rs +++ b/src/snmp/client.rs @@ -2,10 +2,53 @@ use super::types::{SnmpError, SnmpResult, SnmpValue}; use netsnmp_sys::*; use std::ffi::{c_char, CStr, CString}; use std::ptr; +use std::sync::OnceLock; use zeroize::{Zeroize, Zeroizing}; type SecretString = Zeroizing; +/// Controls whether SNMP operations run in forked child processes (crash isolation) +/// or directly in the current process. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum IsolationMode { + /// Each SNMP operation forks a child process. A crash in libnetsnmp + /// only kills the child; the parent agent survives. + Fork, + /// SNMP operations run directly (legacy behavior). Use for debugging. + Direct, +} + +/// Returns the configured isolation mode. Reads `TOWEROPS_SNMP_ISOLATION` +/// once; defaults to `Fork`. +pub fn isolation_mode() -> IsolationMode { + static MODE: OnceLock = OnceLock::new(); + *MODE.get_or_init( + || match std::env::var("TOWEROPS_SNMP_ISOLATION").as_deref() { + Ok("direct") => IsolationMode::Direct, + _ => IsolationMode::Fork, + }, + ) +} + +// FFI structs matching C definitions in snmp_helper.h + +#[repr(C)] +struct SnmpIsolatedGetResult { + status: i32, + value_type: i32, + child_signal: i32, + error_buf: [c_char; 512], + value_buf: [u8; 1024], +} + +#[repr(C)] +struct SnmpIsolatedWalkHeader { + status: i32, + num_results: u32, + child_signal: i32, + error_buf: [c_char; 512], +} + #[cfg(not(test))] const SNMP_TIMEOUT_SECS: i64 = 10; #[cfg(not(test))] @@ -69,6 +112,35 @@ extern "C" { error_buf: *mut c_char, error_buf_len: usize, ) -> i32; + fn snmp_get_isolated( + ip_address: *const c_char, + port: u16, + community: *const c_char, + version: i32, + timeout_us: i64, + retries: i32, + v3_config: *const SnmpV3ConfigC, + oid_str: *const c_char, + result: *mut SnmpIsolatedGetResult, + ); + fn snmp_walk_isolated( + ip_address: *const c_char, + port: u16, + community: *const c_char, + version: i32, + timeout_us: i64, + retries: i32, + v3_config: *const SnmpV3ConfigC, + oid_str: *const c_char, + header: *mut SnmpIsolatedWalkHeader, + results: *mut SnmpWalkResult, + max_results: usize, + ); +} + +#[cfg(test)] +extern "C" { + fn snmp_test_crash_in_child(child_signal: *mut i32) -> i32; } /// SNMPv3 configuration @@ -107,13 +179,29 @@ pub struct SnmpClient; impl SnmpClient { pub fn new() -> Self { - // Initialize libnetsnmp via C helper - unsafe { - snmp_init_library(); + // In fork mode, skip init - each child initializes its own copy. + // In direct mode, initialize once in the parent process. + if isolation_mode() == IsolationMode::Direct { + unsafe { + snmp_init_library(); + } } Self } + /// Parse SNMP version string to integer. + fn parse_version(version: &str) -> SnmpResult { + match version { + "1" | "v1" => Ok(1), + "2c" | "v2c" | "2" => Ok(2), + "3" | "v3" => Ok(3), + _ => Err(SnmpError::RequestFailed(format!( + "Unsupported SNMP version: {}", + version + ))), + } + } + /// Perform an SNMP GET operation pub async fn get( &self, @@ -124,16 +212,28 @@ impl SnmpClient { oid: &str, v3_config: Option, ) -> SnmpResult { - // Clone data for the blocking task let ip_address = ip_address.to_string(); let community = community.to_string(); let version = version.to_string(); let oid = oid.to_string(); - // Run SNMP operation in blocking thread pool tokio::task::spawn_blocking(move || { - let session = SnmpSession::new(&ip_address, port, &community, &version, v3_config)?; - session.get(&oid) + if isolation_mode() == IsolationMode::Fork { + let version_num = Self::parse_version(&version)?; + get_isolated( + &ip_address, + port, + &community, + version_num, + SNMP_TIMEOUT_SECS * 1_000_000, + SNMP_RETRIES, + v3_config.as_ref(), + &oid, + ) + } else { + let session = SnmpSession::new(&ip_address, port, &community, &version, v3_config)?; + session.get(&oid) + } }) .await .map_err(|e| SnmpError::RequestFailed(format!("Task join error: {}", e)))? @@ -149,16 +249,28 @@ impl SnmpClient { oid: &str, v3_config: Option, ) -> SnmpResult> { - // Clone data for the blocking task let ip_address = ip_address.to_string(); let community = community.to_string(); let version = version.to_string(); let oid = oid.to_string(); - // Run SNMP operation in blocking thread pool tokio::task::spawn_blocking(move || { - let session = SnmpSession::new(&ip_address, port, &community, &version, v3_config)?; - session.walk(&oid) + if isolation_mode() == IsolationMode::Fork { + let version_num = Self::parse_version(&version)?; + walk_isolated( + &ip_address, + port, + &community, + version_num, + SNMP_TIMEOUT_SECS * 1_000_000, + SNMP_RETRIES, + v3_config.as_ref(), + &oid, + ) + } else { + let session = SnmpSession::new(&ip_address, port, &community, &version, v3_config)?; + session.walk(&oid) + } }) .await .map_err(|e| SnmpError::RequestFailed(format!("Task join error: {}", e)))? @@ -548,6 +660,311 @@ impl Drop for SnmpSession { } } +/// Parse a raw SNMP value buffer + type into an SnmpValue. +/// Shared by both direct and isolated code paths. +fn parse_snmp_value(value_buf: &[u8], value_len: usize, value_type: i32) -> SnmpResult { + let buf = &value_buf[..value_len]; + match value_type as u8 { + ASN_OCTET_STR => match String::from_utf8(buf.to_vec()) { + Ok(s) => Ok(SnmpValue::String(s)), + Err(_) => Ok(SnmpValue::OctetString(buf.to_vec())), + }, + ASN_OPAQUE => Ok(SnmpValue::OctetString(buf.to_vec())), + ASN_IPADDRESS => { + if value_len == 4 { + Ok(SnmpValue::IpAddress(format!( + "{}.{}.{}.{}", + buf[0], buf[1], buf[2], buf[3] + ))) + } else { + Ok(SnmpValue::OctetString(buf.to_vec())) + } + } + ASN_OBJECT_ID => match String::from_utf8(buf.to_vec()) { + Ok(s) => Ok(SnmpValue::Oid(s)), + Err(_) => Ok(SnmpValue::OctetString(buf.to_vec())), + }, + ASN_INTEGER | ASN_COUNTER | ASN_GAUGE | ASN_TIMETICKS | ASN_UINTEGER => { + if value_len >= std::mem::size_of::() { + let value = unsafe { (buf.as_ptr() as *const i64).read_unaligned() }; + Ok(SnmpValue::Integer(value)) + } else { + Err(SnmpError::RequestFailed("Invalid integer size".into())) + } + } + ASN_COUNTER64 => { + if value_len >= 8 { + let high = u32::from_ne_bytes([buf[0], buf[1], buf[2], buf[3]]); + let low = u32::from_ne_bytes([buf[4], buf[5], buf[6], buf[7]]); + Ok(SnmpValue::Counter64((high as u64) << 32 | low as u64)) + } else { + Err(SnmpError::RequestFailed("Invalid counter64 size".into())) + } + } + ASN_NULL => Ok(SnmpValue::Null), + _ => Err(SnmpError::RequestFailed(format!( + "Unsupported type: {}", + value_type + ))), + } +} + +/// Parse a walk result entry into (oid_string, SnmpValue). +fn parse_walk_result(res: &SnmpWalkResult) -> Option<(String, SnmpValue)> { + if res.value_len == 0 { + return None; + } + let oid_str = unsafe { + CStr::from_ptr(res.oid.as_ptr() as *const c_char) + .to_string_lossy() + .to_string() + }; + parse_snmp_value(&res.value, res.value_len, res.value_type) + .ok() + .map(|v| (oid_str, v)) +} + +/// Helper to build SNMPv3 C config and keep CStrings alive. +struct V3CStrings { + _username: CString, + _auth_pass: Option, + _priv_pass: Option, + _auth_proto: Option, + _priv_proto: Option, + _sec_level: CString, + config: SnmpV3ConfigC, +} + +impl V3CStrings { + fn new(v3: &V3Config) -> SnmpResult { + let username = CString::new(v3.username.as_str()) + .map_err(|_| SnmpError::RequestFailed("Invalid username".into()))?; + let auth_pass = v3 + .auth_password + .as_ref() + .map(|p| CString::new(p.as_str())) + .transpose() + .map_err(|_| SnmpError::RequestFailed("Invalid auth password".into()))?; + let priv_pass = v3 + .priv_password + .as_ref() + .map(|p| CString::new(p.as_str())) + .transpose() + .map_err(|_| SnmpError::RequestFailed("Invalid priv password".into()))?; + let auth_proto = v3 + .auth_protocol + .as_ref() + .map(|p| CString::new(p.as_str())) + .transpose() + .map_err(|_| SnmpError::RequestFailed("Invalid auth protocol".into()))?; + let priv_proto = v3 + .priv_protocol + .as_ref() + .map(|p| CString::new(p.as_str())) + .transpose() + .map_err(|_| SnmpError::RequestFailed("Invalid priv protocol".into()))?; + let sec_level = CString::new(v3.security_level.as_str()) + .map_err(|_| SnmpError::RequestFailed("Invalid security level".into()))?; + + let config = SnmpV3ConfigC { + username: username.as_ptr(), + auth_password: auth_pass + .as_ref() + .map(|c| c.as_ptr()) + .unwrap_or(ptr::null()), + priv_password: priv_pass + .as_ref() + .map(|c| c.as_ptr()) + .unwrap_or(ptr::null()), + auth_protocol: auth_proto + .as_ref() + .map(|c| c.as_ptr()) + .unwrap_or(ptr::null()), + priv_protocol: priv_proto + .as_ref() + .map(|c| c.as_ptr()) + .unwrap_or(ptr::null()), + security_level: sec_level.as_ptr(), + }; + + Ok(Self { + _username: username, + _auth_pass: auth_pass, + _priv_pass: priv_pass, + _auth_proto: auth_proto, + _priv_proto: priv_proto, + _sec_level: sec_level, + config, + }) + } +} + +/// Perform an SNMP GET in a forked child process for crash isolation. +#[allow(clippy::too_many_arguments)] +fn get_isolated( + ip_address: &str, + port: u16, + community: &str, + version: i32, + timeout_us: i64, + retries: i32, + v3_config: Option<&V3Config>, + oid: &str, +) -> SnmpResult { + let ip_cstr = CString::new(ip_address) + .map_err(|_| SnmpError::RequestFailed("Invalid IP address".into()))?; + let comm_cstr = CString::new(community) + .map_err(|_| SnmpError::RequestFailed("Invalid community string".into()))?; + let oid_cstr = + CString::new(oid).map_err(|_| SnmpError::InvalidOid(format!("Invalid OID: {}", oid)))?; + + let v3_strings = v3_config.map(V3CStrings::new).transpose()?; + let v3_ptr = v3_strings + .as_ref() + .map(|s| &s.config as *const _) + .unwrap_or(ptr::null()); + + let mut result = SnmpIsolatedGetResult { + status: -1, + value_type: 0, + child_signal: 0, + error_buf: [0; 512], + value_buf: [0; 1024], + }; + + unsafe { + snmp_get_isolated( + ip_cstr.as_ptr(), + port, + comm_cstr.as_ptr(), + version, + timeout_us, + retries, + v3_ptr, + oid_cstr.as_ptr(), + &mut result, + ); + } + + match result.status { + -2 => Err(SnmpError::CrashRecovered { + signal: result.child_signal, + message: unsafe { + CStr::from_ptr(result.error_buf.as_ptr()) + .to_string_lossy() + .to_string() + }, + }), + -1 => { + let err_msg = unsafe { + CStr::from_ptr(result.error_buf.as_ptr()) + .to_string_lossy() + .to_string() + }; + if err_msg.contains("timeout") || err_msg.contains("Timeout") { + Err(SnmpError::Timeout) + } else if err_msg.contains("Failed to parse OID") { + Err(SnmpError::InvalidOid(err_msg)) + } else if err_msg.contains("Unknown host") || err_msg.contains("Connection refused") { + Err(SnmpError::NetworkUnreachable) + } else { + Err(SnmpError::RequestFailed(err_msg)) + } + } + value_len => parse_snmp_value(&result.value_buf, value_len as usize, result.value_type), + } +} + +/// Perform an SNMP WALK in a forked child process for crash isolation. +#[allow(clippy::too_many_arguments)] +fn walk_isolated( + ip_address: &str, + port: u16, + community: &str, + version: i32, + timeout_us: i64, + retries: i32, + v3_config: Option<&V3Config>, + start_oid: &str, +) -> SnmpResult> { + let ip_cstr = CString::new(ip_address) + .map_err(|_| SnmpError::RequestFailed("Invalid IP address".into()))?; + let comm_cstr = CString::new(community) + .map_err(|_| SnmpError::RequestFailed("Invalid community string".into()))?; + let oid_cstr = CString::new(start_oid) + .map_err(|_| SnmpError::InvalidOid(format!("Invalid OID: {}", start_oid)))?; + + let v3_strings = v3_config.map(V3CStrings::new).transpose()?; + let v3_ptr = v3_strings + .as_ref() + .map(|s| &s.config as *const _) + .unwrap_or(ptr::null()); + + const MAX_RESULTS: usize = 10000; + let mut header = SnmpIsolatedWalkHeader { + status: -1, + num_results: 0, + child_signal: 0, + error_buf: [0; 512], + }; + let mut results_buf: Vec = vec![ + SnmpWalkResult { + oid: [0; 256], + value: [0; 1024], + value_len: 0, + value_type: 0, + }; + MAX_RESULTS + ]; + + unsafe { + snmp_walk_isolated( + ip_cstr.as_ptr(), + port, + comm_cstr.as_ptr(), + version, + timeout_us, + retries, + v3_ptr, + oid_cstr.as_ptr(), + &mut header, + results_buf.as_mut_ptr(), + MAX_RESULTS, + ); + } + + match header.status { + -2 => Err(SnmpError::CrashRecovered { + signal: header.child_signal, + message: unsafe { + CStr::from_ptr(header.error_buf.as_ptr()) + .to_string_lossy() + .to_string() + }, + }), + -1 => { + let err_msg = unsafe { + CStr::from_ptr(header.error_buf.as_ptr()) + .to_string_lossy() + .to_string() + }; + if err_msg.contains("Failed to parse OID") { + Err(SnmpError::InvalidOid(err_msg)) + } else { + Err(SnmpError::RequestFailed(err_msg)) + } + } + _ => { + let parsed: Vec<(String, SnmpValue)> = results_buf + .iter() + .take(header.num_results as usize) + .filter_map(parse_walk_result) + .collect(); + Ok(parsed) + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -755,6 +1172,106 @@ mod tests { } } + #[test] + fn test_struct_layout_get_result() { + // Verify SnmpIsolatedGetResult matches C layout + // C struct: int status (4) + int value_type (4) + int child_signal (4) + // + char error_buf[512] + uint8_t value_buf[1024] + // With padding: 4+4+4 = 12, then 512 + 1024 = 1548 total + assert_eq!( + std::mem::size_of::(), + 4 + 4 + 4 + 512 + 1024 + ); + } + + #[test] + fn test_struct_layout_walk_header() { + // C struct: int status (4) + uint32_t num_results (4) + // + int child_signal (4) + char error_buf[512] + assert_eq!( + std::mem::size_of::(), + 4 + 4 + 4 + 512 + ); + } + + #[test] + fn test_fork_crash_recovery() { + // Verify that a SIGSEGV in a forked child does NOT kill our process + let mut child_signal: i32 = 0; + let ret = unsafe { snmp_test_crash_in_child(&mut child_signal) }; + assert_eq!(ret, 0, "snmp_test_crash_in_child should succeed"); + assert_eq!( + child_signal, + libc::SIGSEGV, + "child should have died from SIGSEGV" + ); + } + + #[test] + fn test_isolation_mode_default() { + // Default isolation mode should be Fork (unless TOWEROPS_SNMP_ISOLATION=direct) + let mode = isolation_mode(); + // We can't guarantee env var state in tests, just verify it returns a valid value + assert!( + matches!(mode, IsolationMode::Fork | IsolationMode::Direct), + "isolation_mode() should return Fork or Direct" + ); + } + + #[test] + fn test_isolated_get_unreachable_host() { + // An isolated GET to an unreachable host should return an error, not crash + let result = get_isolated( + "192.0.2.1", + 161, + "public", + 2, + SNMP_TIMEOUT_SECS * 1_000_000, + SNMP_RETRIES, + None, + "1.3.6.1.2.1.1.1.0", + ); + assert!(result.is_err()); + } + + #[test] + fn test_isolated_get_invalid_oid() { + let result = get_isolated( + "127.0.0.1", + 161, + "public", + 2, + SNMP_TIMEOUT_SECS * 1_000_000, + SNMP_RETRIES, + None, + "not-a-valid-oid", + ); + assert!(result.is_err()); + match result.unwrap_err() { + SnmpError::InvalidOid(_) => {} + e => panic!("Expected InvalidOid, got: {:?}", e), + } + } + + #[test] + fn test_isolated_walk_unreachable_host() { + let result = walk_isolated( + "192.0.2.1", + 161, + "public", + 2, + SNMP_TIMEOUT_SECS * 1_000_000, + SNMP_RETRIES, + None, + "1.3.6.1.2.1.1", + ); + // Should either error or return empty, not crash + match result { + Err(_) => {} + Ok(results) => assert!(results.is_empty()), + } + } + #[test] fn test_c_helpers_session_lifecycle() { // Test session open/close lifecycle (doesn't require actual SNMP device) diff --git a/src/snmp/device_poller.rs b/src/snmp/device_poller.rs index 0edb2a7..ac8dc1a 100644 --- a/src/snmp/device_poller.rs +++ b/src/snmp/device_poller.rs @@ -243,8 +243,7 @@ fn perform_get( config: &DeviceConfig, oid: &str, ) -> SnmpResult { - // Use the thread-local runtime to execute async C FFI call - runtime.block_on(async { + let result = runtime.block_on(async { client .get( &config.ip, @@ -255,7 +254,24 @@ fn perform_get( config.v3_config.clone(), ) .await - }) + }); + + if let Err(SnmpError::CrashRecovered { + signal, + ref message, + }) = result + { + tracing::error!( + "SNMP GET crash recovered for {}:{} OID {} (signal {}): {}", + config.ip, + config.port, + oid, + signal, + message + ); + } + + result } /// Perform SNMP WALK using C FFI @@ -265,8 +281,7 @@ fn perform_walk( config: &DeviceConfig, base_oid: &str, ) -> SnmpResult> { - // Use the thread-local runtime to execute async C FFI call - runtime.block_on(async { + let result = runtime.block_on(async { client .walk( &config.ip, @@ -277,7 +292,24 @@ fn perform_walk( config.v3_config.clone(), ) .await - }) + }); + + if let Err(SnmpError::CrashRecovered { + signal, + ref message, + }) = result + { + tracing::error!( + "SNMP WALK crash recovered for {}:{} OID {} (signal {}): {}", + config.ip, + config.port, + base_oid, + signal, + message + ); + } + + result } #[cfg(test)] diff --git a/src/snmp/mod.rs b/src/snmp/mod.rs index a2305fa..0a7d510 100644 --- a/src/snmp/mod.rs +++ b/src/snmp/mod.rs @@ -4,7 +4,7 @@ mod poller_registry; pub mod trap; mod types; -pub use client::{SnmpClient, V3Config}; +pub use client::{isolation_mode, SnmpClient, V3Config}; pub use device_poller::DeviceConfig; pub use poller_registry::PollerRegistry; pub use trap::{SnmpTrap, TrapListener, DEFAULT_TRAP_PORT}; diff --git a/src/snmp/types.rs b/src/snmp/types.rs index e074a73..339dd88 100644 --- a/src/snmp/types.rs +++ b/src/snmp/types.rs @@ -4,6 +4,7 @@ pub enum SnmpError { InvalidOid(String), Timeout, NetworkUnreachable, + CrashRecovered { signal: i32, message: String }, } impl std::fmt::Display for SnmpError { @@ -13,6 +14,9 @@ impl std::fmt::Display for SnmpError { Self::InvalidOid(oid) => write!(f, "Invalid OID: {}", oid), Self::Timeout => write!(f, "Timeout"), Self::NetworkUnreachable => write!(f, "Network unreachable"), + Self::CrashRecovered { signal, message } => { + write!(f, "SNMP crash recovered (signal {}): {}", signal, message) + } } } } @@ -78,6 +82,18 @@ mod tests { ); } + #[test] + fn test_crash_recovered_display() { + let err = SnmpError::CrashRecovered { + signal: 11, + message: "child killed by SIGSEGV".to_string(), + }; + assert_eq!( + format!("{}", err), + "SNMP crash recovered (signal 11): child killed by SIGSEGV" + ); + } + #[test] fn test_snmp_error_is_error() { let error: &dyn std::error::Error = &SnmpError::Timeout;