From e2952b53547314f9f6a742243c5732537c361058 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Thu, 20 Aug 2020 15:39:57 +0100 Subject: [PATCH 1/6] main: Simplify version handling Print a simple version string rather than delaying the output to display a structured version string. The structured output is potentially more useful but: - This output is not consistent with other components. - Delaying the output makes `--version` unusable in some environments (since a lot of setup is called before the version string can be output). Signed-off-by: James O. D. Hunt --- src/agent/src/main.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/agent/src/main.rs b/src/agent/src/main.rs index 79ed6753d9..ba21f53f5c 100644 --- a/src/agent/src/main.rs +++ b/src/agent/src/main.rs @@ -105,6 +105,19 @@ fn announce(logger: &Logger) { fn main() -> Result<()> { let args: Vec = env::args().collect(); + + if args.len() == 2 && args[1] == "--version" { + println!( + "{} version {} (api version: {}, commit version: {}, type: rust)", + NAME, + version::AGENT_VERSION, + version::API_VERSION, + env::var("VERSION_COMMIT").unwrap_or("unknown".to_string()) + ); + + exit(0); + } + if args.len() == 2 && args[1] == "init" { rustjail::container::init_child(); exit(0); @@ -177,13 +190,6 @@ fn main() -> Result<()> { announce(&logger); - if args.len() == 2 && args[1] == "--version" { - // force logger to flush - drop(logger); - - exit(0); - } - // This "unused" variable is required as it enables the global (and crucially static) logger, // which is required to satisfy the the lifetime constraints of the auto-generated gRPC code. let _guard = slog_scope::set_global_logger(logger.new(o!("subsystem" => "rpc"))); From bac79eeef0a082eda0bf4245e06e68816fe1a4e0 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Thu, 20 Aug 2020 15:42:04 +0100 Subject: [PATCH 2/6] main: Display config in announce Add the configuration details in the announcement log call. Signed-off-by: James O. D. Hunt --- src/agent/Cargo.lock | 79 +++++++++++++++++++++++++++++++++++++++++++ src/agent/src/main.rs | 5 +-- 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index 590e1e71e5..a81f4f3ab5 100644 --- a/src/agent/Cargo.lock +++ b/src/agent/Cargo.lock @@ -30,6 +30,18 @@ version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b585a98a234c46fc563103e9278c9391fde1f4e6850334da895d27edb9580f62" +[[package]] +name = "arrayref" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4c527152e37cf757a3f78aae5a06fbeefdb07ccc535c980a3208ee3060dd544" + +[[package]] +name = "arrayvec" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cff77d8686867eceff3105329d4698d96c2391c176d5d03adc90c7389162b5b8" + [[package]] name = "autocfg" version = "1.0.0" @@ -49,12 +61,29 @@ dependencies = [ "rustc-demangle", ] +[[package]] +name = "base64" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b41b7ea54a0c9d92199de89e20e58d49f02f8e699814ef3fdf266f6f748d15c7" + [[package]] name = "bitflags" version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" +[[package]] +name = "blake2b_simd" +version = "0.5.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d8fb2d74254a3a0b5cac33ac9f8ed0e44aa50378d9dbb2e5d83bd21ed1dc2c8a" +dependencies = [ + "arrayref", + "arrayvec", + "constant_time_eq", +] + [[package]] name = "byteorder" version = "1.3.4" @@ -95,6 +124,12 @@ dependencies = [ "time", ] +[[package]] +name = "constant_time_eq" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "245097e9a4535ee1e3e3931fcfcd55a796a44c643e8596ff6566d68f09b87bbc" + [[package]] name = "crc32fast" version = "1.2.0" @@ -125,6 +160,26 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "dirs" +version = "3.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "142995ed02755914747cc6ca76fc7e4583cd18578746716d0508ea6ed558b9ff" +dependencies = [ + "dirs-sys", +] + +[[package]] +name = "dirs-sys" +version = "0.3.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e93d7f5705de3e49895a2b5e0b8855a1c27f080192ae9c32a6432d50741a57a" +dependencies = [ + "libc", + "redox_users", + "winapi", +] + [[package]] name = "errno" version = "0.2.5" @@ -531,6 +586,17 @@ version = "0.1.56" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2439c63f3f6139d1b57529d16bc3b8bb855230c8efcc5d3a896c8bea7c3b1e84" +[[package]] +name = "redox_users" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09b23093265f8d200fa7b4c2c76297f47e681c655f6f1285a8780d6a022f7431" +dependencies = [ + "getrandom", + "redox_syscall", + "rust-argon2", +] + [[package]] name = "regex" version = "1.3.7" @@ -564,6 +630,18 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cabe4fa914dec5870285fa7f71f602645da47c486e68486d2b4ceb4a343e90ac" +[[package]] +name = "rust-argon2" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2bc8af4bda8e1ff4932523b94d3dd20ee30a87232323eda55903ffd71d2fb017" +dependencies = [ + "base64", + "blake2b_simd", + "constant_time_eq", + "crossbeam-utils", +] + [[package]] name = "rustc-demangle" version = "0.1.16" @@ -575,6 +653,7 @@ name = "rustjail" version = "0.1.0" dependencies = [ "caps", + "dirs", "error-chain", "lazy_static", "libc", diff --git a/src/agent/src/main.rs b/src/agent/src/main.rs index ba21f53f5c..de61de035b 100644 --- a/src/agent/src/main.rs +++ b/src/agent/src/main.rs @@ -86,7 +86,7 @@ lazy_static! { use std::mem::MaybeUninit; -fn announce(logger: &Logger) { +fn announce(logger: &Logger, config: &agentConfig) { let commit = match env::var("VERSION_COMMIT") { Ok(s) => s, Err(_) => String::from(""), @@ -100,6 +100,7 @@ fn announce(logger: &Logger) { "agent-version" => version::AGENT_VERSION, "api-version" => version::API_VERSION, + "config" => format!("{:?}", config), ); } @@ -188,7 +189,7 @@ fn main() -> Result<()> { // Recreate a logger with the log level get from "/proc/cmdline". let logger = logging::create_logger(NAME, "agent", config.log_level, writer); - announce(&logger); + announce(&logger, &config); // This "unused" variable is required as it enables the global (and crucially static) logger, // which is required to satisfy the the lifetime constraints of the auto-generated gRPC code. From 1b2fe4a5bed07d0187695a97ce59781edec9ea9b Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Thu, 20 Aug 2020 15:43:13 +0100 Subject: [PATCH 3/6] agent: Refactor main function Move the sandbox creation into a new function. Signed-off-by: James O. D. Hunt --- src/agent/src/main.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/agent/src/main.rs b/src/agent/src/main.rs index de61de035b..9349dcfa0f 100644 --- a/src/agent/src/main.rs +++ b/src/agent/src/main.rs @@ -195,6 +195,14 @@ fn main() -> Result<()> { // which is required to satisfy the the lifetime constraints of the auto-generated gRPC code. let _guard = slog_scope::set_global_logger(logger.new(o!("subsystem" => "rpc"))); + start_sandbox(&logger, &config)?; + + let _ = log_handle.join(); + + Ok(()) +} + +fn start_sandbox(logger: &Logger, config: &agentConfig) -> Result<()> { let shells = SHELLS.clone(); let debug_console_vport = config.debug_console_vport as u32; @@ -262,8 +270,6 @@ fn main() -> Result<()> { server.shutdown(); - let _ = log_handle.join(); - if config.debug_console { shell_handle.join().unwrap(); } From d5fbba3b0a8b768f44a92c0a405f41ceda2ac5ac Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Fri, 21 Aug 2020 11:42:09 +0100 Subject: [PATCH 4/6] main: Remove commented out and redundant code Remove confusing commented out code and some stray testing code. Signed-off-by: James O. D. Hunt --- src/agent/src/main.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/agent/src/main.rs b/src/agent/src/main.rs index 9349dcfa0f..8ffb68f87a 100644 --- a/src/agent/src/main.rs +++ b/src/agent/src/main.rs @@ -246,12 +246,6 @@ fn start_sandbox(logger: &Logger, config: &agentConfig) -> Result<()> { //vsock:///dev/vsock, port let mut server = rpc::start(sandbox.clone(), VSOCK_ADDR, VSOCK_PORT); - /* - let _ = fs::remove_file("/tmp/testagent"); - let _ = fs::remove_dir_all("/run/agent"); - let mut server = grpc::start(sandbox.clone(), "unix:///tmp/testagent", 1); - */ - let handle = thread::spawn(move || { // info!("Press ENTER to exit..."); // let _ = io::stdin().read(&mut [0]).unwrap(); @@ -259,10 +253,6 @@ fn start_sandbox(logger: &Logger, config: &agentConfig) -> Result<()> { let _ = rx.recv().unwrap(); }); - // receive something from destroy_sandbox here? - // or in the thread above? It depneds whether grpc request - // are run in another thread or in the main thead? - // let _ = rx.wait(); let _ = server.start().unwrap(); @@ -274,8 +264,6 @@ fn start_sandbox(logger: &Logger, config: &agentConfig) -> Result<()> { shell_handle.join().unwrap(); } - let _ = fs::remove_file("/tmp/testagent"); - Ok(()) } From 572de288f0e003256e1cc9496205f6338434caf1 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Fri, 21 Aug 2020 11:43:25 +0100 Subject: [PATCH 5/6] sandbox: Remove unnecessary thread Don't create a thread to wait for the ttRPC server to end - it isn't required as the operation should be blocked on. Signed-off-by: James O. D. Hunt --- src/agent/src/main.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/agent/src/main.rs b/src/agent/src/main.rs index 8ffb68f87a..c124dfa5b9 100644 --- a/src/agent/src/main.rs +++ b/src/agent/src/main.rs @@ -246,17 +246,9 @@ fn start_sandbox(logger: &Logger, config: &agentConfig) -> Result<()> { //vsock:///dev/vsock, port let mut server = rpc::start(sandbox.clone(), VSOCK_ADDR, VSOCK_PORT); - let handle = thread::spawn(move || { - // info!("Press ENTER to exit..."); - // let _ = io::stdin().read(&mut [0]).unwrap(); - // thread::sleep(Duration::from_secs(3000)); - - let _ = rx.recv().unwrap(); - }); - let _ = server.start().unwrap(); - handle.join().unwrap(); + let _ = rx.recv().map_err(|e| format!("{:?}", e)); server.shutdown(); From d12f920b3fe287fe3d0805851a8fa70d57c11458 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Fri, 21 Aug 2020 11:40:19 +0100 Subject: [PATCH 6/6] console: Fix crash if debug console disabled The logic for the debug console meant that if the debug console was _disabled_, the agent was guaranteed to crash on function exit due to the unsafe code block. Fixed by simplifying the code to use the standard `Option` idiom for optional values. Fixes: #554. Signed-off-by: James O. D. Hunt --- src/agent/src/main.rs | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/agent/src/main.rs b/src/agent/src/main.rs index c124dfa5b9..91c9409d1d 100644 --- a/src/agent/src/main.rs +++ b/src/agent/src/main.rs @@ -47,7 +47,7 @@ use std::os::unix::io::AsRawFd; use std::path::Path; use std::sync::mpsc::{self, Sender}; use std::sync::{Arc, Mutex, RwLock}; -use std::{io, thread}; +use std::{io, thread, thread::JoinHandle}; use unistd::Pid; mod config; @@ -84,8 +84,6 @@ lazy_static! { Arc::new(RwLock::new(config::agentConfig::new())); } -use std::mem::MaybeUninit; - fn announce(logger: &Logger, config: &agentConfig) { let commit = match env::var("VERSION_COMMIT") { Ok(s) => s, @@ -195,32 +193,37 @@ fn main() -> Result<()> { // which is required to satisfy the the lifetime constraints of the auto-generated gRPC code. let _guard = slog_scope::set_global_logger(logger.new(o!("subsystem" => "rpc"))); - start_sandbox(&logger, &config)?; + start_sandbox(&logger, &config, init_mode)?; let _ = log_handle.join(); Ok(()) } -fn start_sandbox(logger: &Logger, config: &agentConfig) -> Result<()> { +fn start_sandbox(logger: &Logger, config: &agentConfig, init_mode: bool) -> Result<()> { let shells = SHELLS.clone(); let debug_console_vport = config.debug_console_vport as u32; - let shell_handle = if config.debug_console { + let mut shell_handle: Option> = None; + if config.debug_console { let thread_logger = logger.clone(); - thread::spawn(move || { - let shells = shells.lock().unwrap(); - let result = setup_debug_console(shells.to_vec(), debug_console_vport); - if result.is_err() { - // Report error, but don't fail - warn!(thread_logger, "failed to setup debug console"; + let builder = thread::Builder::new(); + + let handle = builder + .spawn(move || { + let shells = shells.lock().unwrap(); + let result = setup_debug_console(shells.to_vec(), debug_console_vport); + if result.is_err() { + // Report error, but don't fail + warn!(thread_logger, "failed to setup debug console"; "error" => format!("{}", result.unwrap_err())); - } - }) - } else { - unsafe { MaybeUninit::zeroed().assume_init() } - }; + } + }) + .map_err(|e| format!("{:?}", e))?; + + shell_handle = Some(handle); + } // Initialize unique sandbox structure. let mut s = Sandbox::new(&logger).map_err(|e| { @@ -252,8 +255,8 @@ fn start_sandbox(logger: &Logger, config: &agentConfig) -> Result<()> { server.shutdown(); - if config.debug_console { - shell_handle.join().unwrap(); + if let Some(handle) = shell_handle { + handle.join().map_err(|e| format!("{:?}", e))?; } Ok(())