From da764550896201e2c13697a8521a3f7f67469a42 Mon Sep 17 00:00:00 2001 From: Valentin Haudiquet Date: Sun, 4 Oct 2026 21:18:47 +0200 Subject: [PATCH] test(namespace): generate hostnames through a shared helper The hostname tests reimplemented the generation algorithm inline, so they kept passing if the real code drifted; check the actual generate_hostname path instead, and drop the tautological check_user_namespace test. Extract utils::random_suffix and replace the misleading HOSTNAME_SUFFIX_BITS constant (it held the 2^24 entropy space, not a bit count, and the suffix length was derived through a log2()/4 round-trip) with HOSTNAME_SUFFIX_LEN. The QEMU VM hostname generator duplicated the algorithm with different mixing; both callers now share the helper. --- crates/ecr/src/namespace.rs | 126 ++++++++---------------------------- crates/ecr/src/qemu_vm.rs | 13 +--- crates/ecr/src/utils.rs | 30 +++++++-- 3 files changed, 55 insertions(+), 114 deletions(-) diff --git a/crates/ecr/src/namespace.rs b/crates/ecr/src/namespace.rs index 5c20665..d2eb7b8 100644 --- a/crates/ecr/src/namespace.rs +++ b/crates/ecr/src/namespace.rs @@ -381,35 +381,21 @@ fn get_subgid_range(gid: nix::unistd::Gid) -> Result<(u32, u32)> { )) } +/// Generate a unique hostname for a chroot run: +/// `ecr--` (see [`crate::utils::random_suffix`]). +fn generate_hostname(distro: &str) -> String { + format!( + "ecr-{}-{}", + distro, + crate::utils::random_suffix(crate::utils::HOSTNAME_SUFFIX_LEN) + ) +} + /// Set hostname in UTS namespace pub fn set_hostname(distro: &str) -> Result<()> { use nix::unistd::sethostname; - use std::collections::hash_map::DefaultHasher; - use std::hash::{Hash, Hasher}; - // Seed from both the current time and the PID so each invocation gets a - // distinct suffix even when called in rapid succession. Time alone is - // not sufficient: truncating nanoseconds to u8 in a tight loop produces - // the same byte every iteration. - let mut hasher = DefaultHasher::new(); - std::time::SystemTime::now().hash(&mut hasher); - std::process::id().hash(&mut hasher); - let mut state = hasher.finish(); - - let chars = b"abcdefghijklmnopqrstuvwxyz0123456789"; - // Use HOSTNAME_SUFFIX_BITS for entropy (6 hex chars = 24 bits) - let suffix_len = (crate::utils::HOSTNAME_SUFFIX_BITS as f64).log2() as usize / 4; - let random_suffix: String = (0..suffix_len) - .map(|_| { - // Knuth multiplicative LCG — each step advances the full 64-bit state. - state = state - .wrapping_mul(6364136223846793005) - .wrapping_add(1442695040888963407); - chars[(state >> 33) as usize % chars.len()] as char - }) - .collect(); - - let hostname = format!("ecr-{}-{}", distro, random_suffix); + let hostname = generate_hostname(distro); sethostname(&hostname).with_context(|| format!("Failed to set hostname to {}", hostname))?; @@ -419,90 +405,30 @@ pub fn set_hostname(distro: &str) -> Result<()> { #[cfg(test)] mod tests { use super::*; - use std::hash::{Hash, Hasher}; + use std::collections::HashSet; #[test] - fn test_check_user_namespace_returns_ok() { - // This test verifies the function runs without panicking - // On most modern Linux systems with user namespaces enabled, this should pass - let result = check_user_namespace(); - // We can't assert success because it depends on system configuration - // But we can verify it doesn't panic and returns a Result - assert!(result.is_ok() || result.is_err()); - } - - #[test] - fn test_hostname_format() { - // Test that hostname generation produces valid format - use std::collections::HashSet; - - let mut hostnames = HashSet::new(); - for _ in 0..100 { - let mut hasher = std::collections::hash_map::DefaultHasher::new(); - std::time::SystemTime::now().hash(&mut hasher); - std::process::id().hash(&mut hasher); - let mut state = hasher.finish(); - - let chars = b"abcdefghijklmnopqrstuvwxyz0123456789"; - let suffix_len = (crate::utils::HOSTNAME_SUFFIX_BITS as f64).log2() as usize / 4; - let random_suffix: String = (0..suffix_len) - .map(|_| { - state = state - .wrapping_mul(6364136223846793005) - .wrapping_add(1442695040888963407); - chars[(state >> 33) as usize % chars.len()] as char - }) - .collect(); - - let hostname = format!("ecr-test-{}", random_suffix); - - // Verify hostname format + fn hostname_format() { + for _ in 0..10 { + let hostname = generate_hostname("test"); assert!(hostname.starts_with("ecr-test-")); - assert!(hostname.len() > 9); // "ecr-test-" + at least 1 char + assert_eq!( + hostname.len(), + "ecr-test-".len() + crate::utils::HOSTNAME_SUFFIX_LEN + ); assert!(hostname .chars() .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-')); - - hostnames.insert(hostname); } - - // With 100 iterations and good entropy, we should get many unique hostnames - assert!( - hostnames.len() > 50, - "Expected many unique hostnames, got {}", - hostnames.len() - ); } #[test] - fn test_set_hostname_uniqueness() { - // Verify that rapid consecutive calls produce different hostnames - use std::collections::HashSet; - - let mut hostnames = Vec::new(); - for _ in 0..10 { - // Simulate the hostname generation logic - let mut hasher = std::collections::hash_map::DefaultHasher::new(); - std::time::SystemTime::now().hash(&mut hasher); - std::process::id().hash(&mut hasher); - let mut state = hasher.finish(); - - let chars = b"abcdefghijklmnopqrstuvwxyz0123456789"; - let suffix_len = (crate::utils::HOSTNAME_SUFFIX_BITS as f64).log2() as usize / 4; - let random_suffix: String = (0..suffix_len) - .map(|_| { - state = state - .wrapping_mul(6364136223846793005) - .wrapping_add(1442695040888963407); - chars[(state >> 33) as usize % chars.len()] as char - }) - .collect(); - - hostnames.push(format!("ecr-test-{}", random_suffix)); - } - - let unique: HashSet<_> = hostnames.iter().collect(); - // Most hostnames should be unique (high entropy) - assert!(unique.len() >= 8, "Expected mostly unique hostnames"); + fn hostname_generation_is_unique_across_calls() { + let hostnames: HashSet = (0..100).map(|_| generate_hostname("test")).collect(); + assert!( + hostnames.len() > 50, + "expected mostly unique hostnames, got {} distinct out of 100", + hostnames.len() + ); } } diff --git a/crates/ecr/src/qemu_vm.rs b/crates/ecr/src/qemu_vm.rs index 0ad462c..fafe75a 100644 --- a/crates/ecr/src/qemu_vm.rs +++ b/crates/ecr/src/qemu_vm.rs @@ -76,17 +76,10 @@ pub fn launch_qemu(config: QemuConfig, progress: &dyn Progress) -> Result<()> { let shell = crate::utils::detect_shell(&config.rootfs_path); // Generate a unique hostname like "ecr-vm-a1b2c3" - // Use VM_HOSTNAME_SUFFIX_BITS constant for entropy - let hostname_suffix = format!( - "{:x}", - (std::process::id() as u64).wrapping_mul( - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap_or_default() - .as_nanos() as u64 - ) % crate::utils::VM_HOSTNAME_SUFFIX_BITS + let hostname = format!( + "ecr-vm-{}", + crate::utils::random_suffix(crate::utils::HOSTNAME_SUFFIX_LEN) ); - let hostname = format!("ecr-vm-{}", hostname_suffix); // Build kernel command line // For initramfs boot, use rdinit= instead of init= diff --git a/crates/ecr/src/utils.rs b/crates/ecr/src/utils.rs index 9e556a5..407ec4c 100644 --- a/crates/ecr/src/utils.rs +++ b/crates/ecr/src/utils.rs @@ -11,11 +11,33 @@ pub const CHILD_STACK_SIZE: usize = 1024 * 1024; /// Buffer size for reading error messages from pipes (4 KiB, typical page size) pub const ERROR_BUFFER_SIZE: usize = 4096; -/// Maximum entropy bits for hostname suffix (24 bits = 6 hex chars) -pub const HOSTNAME_SUFFIX_BITS: u64 = 0x1000000; +/// Length of the random hostname suffix (36⁶ ≈ 2×10⁹ distinct values) +pub const HOSTNAME_SUFFIX_LEN: usize = 6; -/// Maximum entropy bits for VM hostname suffix (24 bits = 6 hex chars) -pub const VM_HOSTNAME_SUFFIX_BITS: u64 = 0x1000000; +/// Random lowercase-alphanumeric suffix for hostnames. Seeded from the +/// current time and the PID so calls in rapid succession — including from +/// separate processes — produce different values; time alone truncated to +/// the suffix space repeats within a tight loop. +pub fn random_suffix(len: usize) -> String { + use std::collections::hash_map::DefaultHasher; + use std::hash::{Hash, Hasher}; + + let mut hasher = DefaultHasher::new(); + std::time::SystemTime::now().hash(&mut hasher); + std::process::id().hash(&mut hasher); + let mut state = hasher.finish(); + + const CHARS: &[u8] = b"abcdefghijklmnopqrstuvwxyz0123456789"; + // Knuth multiplicative LCG — each step advances the full 64-bit state. + (0..len) + .map(|_| { + state = state + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + CHARS[(state >> 33) as usize % CHARS.len()] as char + }) + .collect() +} // ============================================================================ // Architecture handling