Merge pull request #2906 from jodh-intel/trace-forwarder-drop-privs

forwarder: Drop privileges when using hybrid VSOCK

src/trace-forwarder/Cargo.lock

@@ -71,9 +71,9 @@ dependencies = [
 
 [[package]]
 name = "bitflags"
-version = "1.2.1"
+version = "1.3.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693"
+checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a"
 
 [[package]]
 name = "bumpalo"
@@ -335,6 +335,7 @@ dependencies = [
  "nix 0.21.0",
  "opentelemetry",
  "opentelemetry-jaeger",
+ "privdrop",
  "protobuf",
  "serde",
  "serde_json",
@@ -354,9 +355,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646"
 
 [[package]]
 name = "libc"
-version = "0.2.97"
+version = "0.2.105"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "12b8adadd720df158f4d70dfe7ccc6adb0472d7c55ca83445f6a5ab3e36f8fb6"
+checksum = "869d572136620d55835903746bcb5cdc54cb2851fd0aeec53220b4bb65ef3013"
 
 [[package]]
 name = "log"
@@ -433,6 +434,19 @@ dependencies = [
  "memoffset",
 ]
 
+[[package]]
+name = "nix"
+version = "0.23.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f305c2c2e4c39a82f7bf0bf65fb557f9070ce06781d4f2454295cc34b1c43188"
+dependencies = [
+ "bitflags",
+ "cc",
+ "cfg-if 1.0.0",
+ "libc",
+ "memoffset",
+]
+
 [[package]]
 name = "num-integer"
 version = "0.1.43"
@@ -546,6 +560,16 @@ version = "0.2.8"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "237a5ed80e274dbc66f86bd59c1e25edc039660be53194b5fe0a482e0f2612ea"
 
+[[package]]
+name = "privdrop"
+version = "0.5.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "4c02cf257b10e4b807bccadb19630d5dea7e0369c3c5e84673ee8e58dc8da6a5"
+dependencies = [
+ "libc",
+ "nix 0.23.0",
+]
+
 [[package]]
 name = "proc-macro-hack"
 version = "0.5.19"

src/trace-forwarder/Cargo.toml

@@ -28,7 +28,12 @@ tracing = "0.1.26"
 tracing-subscriber = "0.2.18"
 
 logging = { path = "../../pkg/logging" }
-slog = "2.5.2"
+
+# Note: The 'max_*' features allow changing the log level at runtime
+# (by stopping the compiler from removing log calls).
+slog = { version = "2.5.2", features = ["dynamic-keys", "max_level_trace", "release_max_level_trace"] }
+
+privdrop = "0.5.1"
 
 [dev-dependencies]
 tempfile = "3.1.0"

src/trace-forwarder/README.md

@@ -71,16 +71,16 @@ represents the real sandbox ID or name.
 #### Configured hypervisor is Cloud Hypervisor
 
 ```bash
-$ socket_path=$(sudo kata-runtime env --json | jq '.Hypervisor.SocketPath')
+$ socket_path_template=$(sudo kata-runtime env --json | jq '.Hypervisor.SocketPath')
-$ echo "$socket_path"
+$ echo "$socket_path_template"
 "/run/vc/vm/{ID}/clh.sock"
 ```
 
 #### Configured hypervisor is Firecracker
 
 ```bash
-$ socket_path=$(sudo kata-runtime env --json | jq '.Hypervisor.SocketPath')
+$ socket_path_template=$(sudo kata-runtime env --json | jq '.Hypervisor.SocketPath')
-$ echo "$socket_path"
+$ echo "$socket_path_template"
 "/run/vc/firecracker/{ID}/root/kata.hvsock"
 ```
 
@@ -118,14 +118,15 @@ You will need to change the `sandbox_id` variable below to match the name of
 the container (sandbox) you plan to create _after_ starting the trace
 forwarder.
 
-The `socket_path` variable was set in the
-[Cloud Hypervisor and Firecracker](#cloud-hypervisor-and-firecracker) section.
-
 ```bash
 $ sandbox_id="foo"
+$ socket_path=$(echo "$socket_path_template" | sed "s/{ID}/${sandbox_id}/g")
 $ sudo mkdir -p $(dirname "$socket_path")
 ```
 
+> **Note:** The `socket_path_template` variable was set in the
+> [Cloud Hypervisor and Firecracker](#cloud-hypervisor-and-firecracker) section.
+
 #### Run the forwarder specifying socket path
 
 ```bash
@@ -140,7 +141,8 @@ You can now proceed as normal to create the "foo" Kata container.
 > directory, and since that directory is owned by the `root` user, the trace
 > forwarder must also be run as `root`. This requirement is unique to
 > hypervisors that use hybrid VSOCK: QEMU does not require special privileges
-> to run the trace forwarder.
+> to run the trace forwarder. To reduce the impact of this, once the forwarder
+> is running it drops privileges to run as user `nobody`.
 
 ## Full details
 

src/trace-forwarder/src/main.rs

@@ -16,45 +16,6 @@ const DEFAULT_TRACE_NAME: &str = "kata-agent";
 
 const ABOUT_TEXT: &str = "Kata Containers Trace Forwarder";
 
-const DESCRIPTION_TEXT: &str = r#"
-DESCRIPTION:
-    Kata Containers component that runs on the host and forwards
-    trace data from the container to a trace collector on the host.
-
-    This tool requires agent tracing to be enabled in the Kata
-    configuration file. It uses VSOCK to listen for trace data originating
-    from the Kata agent running inside the Kata Container.
-
-    The variety of VSOCK used depends on the configuration hypervisor:
-
-    |------------------------|--------------------|----------------|
-    | Hypervisor             | Type of VSOCK      | Run as user    |
-    |------------------------|--------------------|----------------|
-    | Cloud Hypervisor (CLH) | Firecracker Hybrid | privileged     |
-    |------------------------|--------------------|----------------|
-    | QEMU                   | Standard           | non-privileged |
-    |------------------------|--------------------|----------------|
-    | Firecracker (FC)       | Firecracker Hybrid | privileged     |
-    |------------------------|--------------------|----------------|
-
-        Key:
-
-        - Firecracker Hybrid VSOCK: See the Firecracker
-          VSOCK documentation.
-        - Standard VSOCK: see vsock(7).
-
-    The way this tool is run depends on the configured hypervisor.
-    See EXAMPLES for further information.
-
-    Note that Hybrid VSOCK requries root privileges. Due to the way the
-    hybrid protocol works, the specified "master socket" itself is not used: to
-    communicate with the agent, this tool must generate a socket path using
-    the specified socket path as a prefix. Since the master socket will be
-    created in a root-owned directory when the Kata Containers VM (sandbox) is
-    created, this tool must be run as root to allow it to create the second
-    agent-specific socket.
-    "#;
-
 const DEFAULT_LOG_LEVEL: slog::Level = slog::Level::Info;
 
 // VSOCK port this program listens to for trace data, sent by the agent.
@@ -84,6 +45,50 @@ fn announce(logger: &Logger, version: &str, dump_only: bool) {
     "dump-only" => dump_only);
 }
 
+fn make_description_text() -> String {
+    format!(
+        r#" DESCRIPTION:
+    Kata Containers component that runs on the host and forwards
+    trace data from the container to a trace collector on the host.
+
+    This tool requires agent tracing to be enabled in the Kata
+    configuration file. It uses VSOCK to listen for trace data originating
+    from the Kata agent running inside the Kata Container.
+
+    The variety of VSOCK used depends on the configuration hypervisor:
+
+    |------------------------|--------------------|----------------|
+    | Hypervisor             | Type of VSOCK      | Run as user    |
+    |------------------------|--------------------|----------------|
+    | Cloud Hypervisor (CLH) | Firecracker Hybrid | privileged     |
+    |------------------------|--------------------|----------------|
+    | QEMU                   | Standard           | non-privileged |
+    |------------------------|--------------------|----------------|
+    | Firecracker (FC)       | Firecracker Hybrid | privileged     |
+    |------------------------|--------------------|----------------|
+
+        Key:
+
+        - Firecracker Hybrid VSOCK: See the Firecracker
+          VSOCK documentation.
+        - Standard VSOCK: see vsock(7).
+
+    The way this tool is run depends on the configured hypervisor.
+    See EXAMPLES for further information.
+
+    Note that Hybrid VSOCK requries root privileges initially. Due to the way the
+    hybrid protocol works, the specified "master socket" itself is not used: to
+    communicate with the agent, this tool must generate a socket path using
+    the specified socket path as a prefix. Since the master socket will be
+    created in a root-owned directory when the Kata Containers VM (sandbox) is
+    created, this tool must be run as root to allow it to create the second
+    agent-specific socket. However, once the forwarder has started running, it
+    drops privileges and will continue running as user {user:?}.
+    "#,
+        user = server::NON_PRIV_USER
+    )
+}
+
 fn make_examples_text(program_name: &str) -> String {
     format!(
         r#"EXAMPLES:
@@ -135,7 +140,7 @@ fn real_main() -> Result<()> {
         .version(version)
         .version_short("v")
         .about(ABOUT_TEXT)
-        .long_about(DESCRIPTION_TEXT)
+        .long_about(&*make_description_text())
         .after_help(&*make_examples_text(name))
         .arg(
             Arg::with_name("dump-only")

src/trace-forwarder/src/server.rs

@@ -6,6 +6,7 @@
 use crate::handler;
 use anyhow::{anyhow, Result};
 use opentelemetry::sdk::export::trace::SpanExporter;
+use privdrop::PrivDrop;
 use slog::{debug, o, Logger};
 use std::os::unix::io::AsRawFd;
 use std::os::unix::net::UnixListener;
@@ -13,6 +14,12 @@ use vsock::{SockAddr, VsockListener};
 
 use crate::tracer;
 
+// Username that is assumed to exist, used when dropping root privileges
+// when running with Hybrid VSOCK.
+pub const NON_PRIV_USER: &str = "nobody";
+
+const ROOT_DIR: &str = "/";
+
 #[derive(Debug, Clone, PartialEq)]
 pub enum VsockType {
     Standard { port: u32, cid: u32 },
@@ -79,21 +86,44 @@ impl VsockTraceServer {
     }
 }
 
+fn drop_privs(logger: &Logger) -> Result<()> {
+    debug!(logger, "Dropping privileges"; "new-user" => NON_PRIV_USER);
+
+    nix::unistd::chdir(ROOT_DIR)
+        .map_err(|e| anyhow!("Unable to chdir to {:?}: {:?}", ROOT_DIR, e))?;
+
+    PrivDrop::default()
+        .user(NON_PRIV_USER)
+        .apply()
+        .map_err(|e| anyhow!("Failed to drop privileges to user {}: {}", NON_PRIV_USER, e))?;
+
+    Ok(())
+}
+
 fn start_hybrid_vsock(
     logger: Logger,
     exporter: &mut dyn SpanExporter,
     socket_path: &str,
     dump_only: bool,
 ) -> Result<()> {
+    let logger =
+        logger.new(o!("vsock-type" => "hybrid", "vsock-socket-path" => socket_path.to_string()));
+
+    let effective = nix::unistd::Uid::effective();
+
+    if !effective.is_root() {
+        return Err(anyhow!("You need to be root"));
+    }
+
     // Remove the socket if it already exists
     let _ = std::fs::remove_file(socket_path);
 
-    let listener =
+    let listener = UnixListener::bind(socket_path)?;
-        UnixListener::bind(socket_path).map_err(|e| anyhow!("You need to be root: {:?}", e))?;
 
-    debug!(logger, "Waiting for connections";
+    // Having bound to the socket, drop privileges
-        "vsock-type" => "hybrid",
+    drop_privs(&logger)?;
-        "vsock-socket-path" => socket_path);
+
+    debug!(logger, "Waiting for connections");
 
     for conn in listener.incoming() {
         let conn = conn?;