Provide a way of detecting system library leakage

.cargo/xtask.toml

@@ -0,0 +1,41 @@
+# This config file is used by `cargo xtask verify-libraries`
+
+
+# These are libraries that we expect to show up in any executable produced
+# by the omicron repo.
+[libraries."libc.so.1"]
+[libraries."libcontract.so.1"]
+[libraries."libcrypto.so.3"]
+[libraries."libdevinfo.so.1"]
+[libraries."libdlpi.so.1"]
+[libraries."libdoor.so.1"]
+[libraries."libefi.so.1"]
+[libraries."libgcc_s.so.1"]
+[libraries."libipcc.so.1"]
+[libraries."libkstat.so.1"]
+[libraries."libm.so.2"]
+[libraries."libnsl.so.1"]
+[libraries."libnvpair.so.1"]
+[libraries."libpq.so.5"]
+[libraries."libpthread.so.1"]
+[libraries."libresolv.so.2"]
+[libraries."librt.so.1"]
+[libraries."libscf.so.1"]
+[libraries."libsocket.so.1"]
+[libraries."libssl.so.3"]
+[libraries."libumem.so.1"]
+[libraries."libxml2.so.2"]
+[libraries."libxmlsec1.so.1"]
+
+# libnvme is a global zone only library and therefore we must be sure that only
+# programs running in the gz require it. Additionally only sled-agent should be
+# managing a sled's hardware.
+[libraries."libnvme.so.1"]
+binary_allow_list = [
+  "installinator",
+  "omicron-dev",
+  "omicron-package",
+  "services-ledger-check-migrate",
+  "sled-agent",
+  "sled-agent-sim",
+]

.github/buildomat/build-and-test.sh

@@ -76,6 +76,19 @@ export RUSTC_BOOTSTRAP=1
 # We report build progress to stderr, and the "--timings=json" output goes to stdout.
 ptime -m cargo build -Z unstable-options --timings=json --workspace --tests --locked --verbose 1> "$OUTPUT_DIR/crate-build-timings.json"
 
+# If we are running on illumos we want to verify that we are not requiring
+# system libraries outside of specific binaries. If we encounter this situation
+# we bail.
+# NB: `cargo xtask verify-libraries` runs `cargo build --bins` to ensure it can
+# check the final executables.
+if [[ $target_os == "illumos" ]]; then
+    banner verify-libraries
+    # This has a separate timeout from `cargo nextest` since `timeout` expects
+    # to run an external command and therefore we cannot run bash functions or
+    # subshells.
+    ptime -m timeout 10m cargo xtask verify-libraries
+fi
+
 #
 # We apply our own timeout to ensure that we get a normal failure on timeout
 # rather than a buildomat timeout.  See oxidecomputer/buildomat#8.

README.adoc

@@ -106,6 +106,10 @@ When you're happy with things and want to make sure you haven't missed something
 cargo nextest run
 ```
 
+=== Adding a new system library dependency
+
+We check that certain system library dependencies are not leaked outside of their intended binaries via `cargo xtask verify-libraries` in CI. If you are adding a new dependency on a illumos/helios library it is recommended that you update xref:.cargo/xtask.toml[] with an allow list of where you expect the dependency to show up. For example some libraries such as `libnvme.so.1` are only available in the global zone and therefore will not be present in any other zone. This check is here to help us catch any leakage before we go to deploy on a rack. You can inspect a compiled binary in the target directory for what it requires by using `elfedit` - for example `elfedit -r -e 'dyn:tag NEEDED' /path/to/omicron/target/debug/sled-agent`.
+
 === Rust packages in Omicron
 
 NOTE: The term "package" is overloaded: most programming languages and operating systems have their own definitions of a package.  On top of that, Omicron bundles up components into our own kind of "package" that gets delivered via the install and update systems.  These are described in the `package-manifest.toml` file in the root of the repo.  In this section, we're just concerned with Rust packages.

dev-tools/xtask/Cargo.toml

@@ -10,3 +10,7 @@ camino.workspace = true
 cargo_toml = "0.19"
 cargo_metadata = "0.18"
 clap.workspace = true
+serde.workspace = true
+toml.workspace = true
+fs-err.workspace = true
+swrite.workspace = true

dev-tools/xtask/src/illumos.rs

@@ -0,0 +1,142 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+use anyhow::{bail, Context, Result};
+use camino::Utf8Path;
+use cargo_metadata::Message;
+use fs_err as fs;
+use serde::Deserialize;
+use std::{
+    collections::{BTreeMap, BTreeSet},
+    io::BufReader,
+    process::{Command, Stdio},
+};
+use swrite::{swriteln, SWrite};
+
+use crate::load_workspace;
+
+#[derive(Deserialize, Debug)]
+struct LibraryConfig {
+    binary_allow_list: Option<BTreeSet<String>>,
+}
+
+#[derive(Deserialize, Debug)]
+struct XtaskConfig {
+    libraries: BTreeMap<String, LibraryConfig>,
+}
+
+#[derive(Debug)]
+enum LibraryError {
+    Unexpected(String),
+    NotAllowed(String),
+}
+
+/// Verify that the binary at the provided path complies with the rules laid out
+/// in the xtask.toml config file. Errors are pushed to a hashmap so that we can
+/// display to a user the entire list of issues in one go.
+fn verify_executable(
+    config: &XtaskConfig,
+    path: &Utf8Path,
+    errors: &mut BTreeMap<String, Vec<LibraryError>>,
+) -> Result<()> {
+    let binary = path.file_name().context("basename of executable")?;
+
+    let command = Command::new("elfedit")
+        .args(["-o", "simple", "-r", "-e", "dyn:tag NEEDED"])
+        .arg(path)
+        .output()
+        .context("exec elfedit")?;
+
+    if !command.status.success() {
+        bail!("Failed to execute elfedit successfully {}", command.status);
+    }
+
+    let stdout = String::from_utf8(command.stdout)?;
+    // `elfedit -o simple -r -e "dyn:tag NEEDED" /file/path` will return
+    // a new line seperated list of required libraries so we walk over
+    // them looking for a match in our configuration file. If we find
+    // the library we make sure the binary is allowed to pull it in via
+    // the whitelist.
+    for library in stdout.lines() {
+        let library_config = match config.libraries.get(library.trim()) {
+            Some(config) => config,
+            None => {
+                errors
+                    .entry(binary.to_string())
+                    .or_default()
+                    .push(LibraryError::Unexpected(library.to_string()));
+
+                continue;
+            }
+        };
+
+        if let Some(allowed) = &library_config.binary_allow_list {
+            if !allowed.contains(binary) {
+                errors
+                    .entry(binary.to_string())
+                    .or_default()
+                    .push(LibraryError::NotAllowed(library.to_string()));
+            }
+        }
+    }
+
+    Ok(())
+}
+pub fn cmd_verify_libraries() -> Result<()> {
+    let metadata = load_workspace()?;
+    let mut config_path = metadata.workspace_root;
+    config_path.push(".cargo/xtask.toml");
+    let config = read_xtask_toml(&config_path)?;
+
+    let cargo = std::env::var("CARGO").unwrap_or_else(|_| "cargo".to_string());
+    let mut command = Command::new(cargo)
+        .args(["build", "--bins", "--message-format=json-render-diagnostics"])
+        .stdout(Stdio::piped())
+        .spawn()
+        .context("failed to spawn cargo build")?;
+
+    let reader = BufReader::new(command.stdout.take().context("take stdout")?);
+
+    let mut errors = Default::default();
+    for message in cargo_metadata::Message::parse_stream(reader) {
+        if let Message::CompilerArtifact(artifact) = message? {
+            // We are only interested in artifacts that are binaries
+            if let Some(executable) = artifact.executable {
+                verify_executable(&config, &executable, &mut errors)?;
+            }
+        }
+    }
+
+    let status = command.wait()?;
+    if !status.success() {
+        bail!("Failed to execute cargo build successfully {}", status);
+    }
+
+    if !errors.is_empty() {
+        let mut msg = String::new();
+        errors.iter().for_each(|(binary, errors)| {
+            swriteln!(msg, "{binary}");
+            errors.iter().for_each(|error| match error {
+                LibraryError::Unexpected(lib) => {
+                    swriteln!(msg, "\tUNEXPECTED dependency on {lib}");
+                }
+                LibraryError::NotAllowed(lib) => {
+                    swriteln!(msg, "\tNEEDS {lib} but is not allowed");
+                }
+            });
+        });
+
+        bail!(
+            "Found library issues with the following:\n{msg}\n\n\
+        If depending on a new library was intended please add it to xtask.toml"
+        );
+    }
+
+    Ok(())
+}
+
+fn read_xtask_toml(path: &Utf8Path) -> Result<XtaskConfig> {
+    let config_str = fs::read_to_string(path)?;
+    toml::from_str(&config_str).with_context(|| format!("parse {:?}", path))
+}

dev-tools/xtask/src/main.rs

@@ -11,8 +11,14 @@ use camino::Utf8Path;
 use cargo_metadata::Metadata;
 use cargo_toml::{Dependency, Manifest};
 use clap::{Parser, Subcommand};
+use fs_err as fs;
 use std::{collections::BTreeMap, process::Command};
 
+#[cfg(target_os = "illumos")]
+mod illumos;
+#[cfg(target_os = "illumos")]
+use illumos::cmd_verify_libraries;
+
 #[derive(Parser)]
 #[command(name = "cargo xtask", about = "Workspace-related developer tools")]
 struct Args {
@@ -27,6 +33,9 @@ enum Cmds {
     CheckWorkspaceDeps,
     /// Run configured clippy checks
     Clippy(ClippyArgs),
+    /// Verify we are not leaking library bindings outside of intended
+    /// crates
+    VerifyLibraries,
 }
 
 #[derive(Parser)]
@@ -41,6 +50,7 @@ fn main() -> Result<()> {
     match args.cmd {
         Cmds::Clippy(args) => cmd_clippy(args),
         Cmds::CheckWorkspaceDeps => cmd_check_workspace_deps(),
+        Cmds::VerifyLibraries => cmd_verify_libraries(),
     }
 }
 
@@ -213,9 +223,13 @@ fn cmd_check_workspace_deps() -> Result<()> {
     Ok(())
 }
 
+#[cfg(not(target_os = "illumos"))]
+fn cmd_verify_libraries() -> Result<()> {
+    unimplemented!("Library verification is only available on illumos!")
+}
+
 fn read_cargo_toml(path: &Utf8Path) -> Result<Manifest> {
-    let bytes =
-        std::fs::read(path).with_context(|| format!("read {:?}", path))?;
+    let bytes = fs::read(path)?;
     Manifest::from_slice(&bytes).with_context(|| format!("parse {:?}", path))
 }