diff --git a/.cargo/xtask.toml b/.cargo/xtask.toml new file mode 100644 index 00000000000..0055cf723c6 --- /dev/null +++ b/.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", +] diff --git a/.github/buildomat/build-and-test.sh b/.github/buildomat/build-and-test.sh index 30abd02f90a..7d0aadcb56f 100755 --- a/.github/buildomat/build-and-test.sh +++ b/.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. diff --git a/Cargo.lock b/Cargo.lock index 9ca0599fb3f..daade608394 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11041,6 +11041,10 @@ dependencies = [ "cargo_metadata", "cargo_toml", "clap 4.5.1", + "fs-err", + "serde", + "swrite", + "toml 0.8.10", ] [[package]] diff --git a/README.adoc b/README.adoc index 93d1fa4fb8a..04bc33b2afd 100644 --- a/README.adoc +++ b/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. diff --git a/dev-tools/xtask/Cargo.toml b/dev-tools/xtask/Cargo.toml index 0429fcae791..73bfe0b37a5 100644 --- a/dev-tools/xtask/Cargo.toml +++ b/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 diff --git a/dev-tools/xtask/src/illumos.rs b/dev-tools/xtask/src/illumos.rs new file mode 100644 index 00000000000..a2daab2c9e2 --- /dev/null +++ b/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>, +} + +#[derive(Deserialize, Debug)] +struct XtaskConfig { + libraries: BTreeMap, +} + +#[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>, +) -> 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 { + let config_str = fs::read_to_string(path)?; + toml::from_str(&config_str).with_context(|| format!("parse {:?}", path)) +} diff --git a/dev-tools/xtask/src/main.rs b/dev-tools/xtask/src/main.rs index 42e6c10d64e..c682fc247e0 100644 --- a/dev-tools/xtask/src/main.rs +++ b/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 { - 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)) }