From 1af0583b06362d29bbccd2661670718f13e835da Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Thu, 27 Aug 2026 18:46:51 -0400 Subject: [PATCH 1/9] feat(signing): sign the boot FIT from the key registry via signing.fit_key The FIT signing key came in through AVOCADO_FIT_KEY_DIR / AVOCADO_FIT_UNSIGNED, so a signed boot image was not reproducible from avocado.yaml. Move it into config and the signing-key registry: - `runtimes..signing.fit_key` names an RSA PEM key in the registry (by name or key id); the runtime build materializes it as FIT.key/FIT.crt in a private temp dir mounted read-only at /tmp/fit-keys, where the FIT assembly already looks. `signing.fit_unsigned: true` is the explicit opt-out; both set is an error. The env variables are no longer read (a hint is printed). - The registry gains RSA PEM entries (rsa2048/rsa4096): `signing-keys import --key --cert` stores an existing pair, `signing-keys create --algorithm rsa2048` generates one with the host's openssl. Key id is the SHA-256 of the certificate DER. PKCS#11 and ed25519 entries are refused for FIT signing with a message saying why. --- CHANGELOG.md | 7 ++ docs/signing-keys.md | 27 ++++++ src/commands/runtime/build.rs | 60 +++++++++---- src/commands/signing_keys/create.rs | 57 +++++++++++- src/commands/signing_keys/import.rs | 78 ++++++++++++++++ src/commands/signing_keys/mod.rs | 2 + src/main.rs | 32 ++++++- src/utils/config.rs | 51 +++++++++++ src/utils/signing_keys.rs | 135 ++++++++++++++++++++++++++++ tests/pkcs11_integration_test.rs | 2 + 10 files changed, 431 insertions(+), 20 deletions(-) create mode 100644 src/commands/signing_keys/import.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index a200c2bb..9d9915ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- **`runtimes..signing.fit_key` — boot-FIT signing from the key registry.** + Names an RSA PEM key (`avocado signing-keys import --key --cert`, or + `signing-keys create --algorithm rsa2048`) that the runtime build + materializes as `FIT.key`/`FIT.crt` for `mkimage`, so a signed boot image is + reproducible from `avocado.yaml` alone. `signing.fit_unsigned: true` is the + explicit opt-out. Replaces the interim `AVOCADO_FIT_KEY_DIR` / + `AVOCADO_FIT_UNSIGNED` environment variables, which are no longer read. - **`runtimes..var.encrypt: true` — encrypted `/var`.** Opts a runtime into a LUKS2 `/var` sealed to the target's hardware key store (OP-TEE fTPM on Jetson). The cli adds `cryptsetup-var` to the initramfs and diff --git a/docs/signing-keys.md b/docs/signing-keys.md index 76b5dcfa..dbcbd8cc 100644 --- a/docs/signing-keys.md +++ b/docs/signing-keys.md @@ -9,6 +9,33 @@ The avocado CLI supports managing signing keys for runtime image signing through ## Global Key Management +### Boot-FIT signing keys (RSA) + +FIT-booting machines (i.MX) verify the boot image with an RSA key embedded in +U-Boot. The runtime build signs the FIT it assembles with a key from this same +registry: + +```bash +# Generate (needs openssl on the host) ... +avocado signing-keys create product-fit --algorithm rsa2048 +# ... or import an existing PEM key and certificate +avocado signing-keys import product-fit --key FIT.key --cert FIT.crt +``` + +```yaml +runtimes: + prod: + signing: + fit_key: product-fit # or: fit_unsigned: true +``` + +The build copies the key pair into a private temp dir mounted read-only into +the SDK as `FIT.key`/`FIT.crt` (the template's `key-name-hint`). With +`rootfs.image.verity: true` one of the two settings is required, since the +root hash rides in the FIT. RSA keys are file-based only: `mkimage` cannot use +a PKCS#11 URI. + + ### Creating Keys ```bash diff --git a/src/commands/runtime/build.rs b/src/commands/runtime/build.rs index 9b968d3d..5922f98f 100644 --- a/src/commands/runtime/build.rs +++ b/src/commands/runtime/build.rs @@ -722,31 +722,55 @@ impl RuntimeBuildCommand { }; // FIT signing key for the boot image assembled below (FIT machines - // only). AVOCADO_FIT_KEY_DIR names a host directory holding - // FIT.key/FIT.crt - the key-name-hint the feed's FIT template uses - - // and is bind-mounted read-only at /tmp/fit-keys. Optional: without - // it the FIT is assembled unsigned, which only boots on a U-Boot with - // no embedded key. Env rather than config until signing_keys can - // materialize PEM for external tools. - if std::env::var("AVOCADO_FIT_UNSIGNED").as_deref() == Ok("1") { + // only), from `runtimes..signing.fit_key`: an RSA PEM key in the + // signing-key registry, materialized as the FIT.key/FIT.crt pair the + // feed's FIT template names (key-name-hint "FIT") in a private temp dir + // that is bind-mounted read-only at /tmp/fit-keys. `signing.fit_unsigned` + // is the explicit opt-out; without either the FIT is left as the feed + // built it (or the build fails, when verity needs the FIT rebuilt). + for stale in ["AVOCADO_FIT_KEY_DIR", "AVOCADO_FIT_UNSIGNED"] { + if std::env::var_os(stale).is_some() { + print_info( + &format!( + "{stale} is no longer read; set runtimes.{}.signing.fit_key (or fit_unsigned: true) in avocado.yaml instead.", + self.runtime_name + ), + OutputLevel::Normal, + ); + } + } + let (fit_key_name, fit_unsigned) = config.get_runtime_fit_signing(&self.runtime_name); + if fit_key_name.is_some() && fit_unsigned { + return Err(anyhow::anyhow!( + "runtime '{}' sets both signing.fit_key and signing.fit_unsigned; pick one.", + self.runtime_name + )); + } + if fit_unsigned { env_vars.insert("AVOCADO_FIT_UNSIGNED".to_string(), "1".to_string()); } - let fit_keydir_host_path: Option = match std::env::var("AVOCADO_FIT_KEY_DIR") { - Ok(dir) => { - if !std::path::Path::new(&dir).join("FIT.key").is_file() { - return Err(anyhow::anyhow!( - "AVOCADO_FIT_KEY_DIR points to '{}' but it has no FIT.key.", - dir - )); - } + let fit_keys_tempdir: Option = match fit_key_name { + Some(ref name) => { + let (key, cert) = crate::utils::signing_keys::pem_key_files(name)?; + let dir = tempfile::Builder::new() + .prefix("avocado-fit-keys-") + .tempdir() + .context("Failed to create a temp dir for the FIT signing key")?; + std::fs::copy(&key, dir.path().join("FIT.key")) + .with_context(|| format!("Failed to stage {}", key.display()))?; + std::fs::copy(&cert, dir.path().join("FIT.crt")) + .with_context(|| format!("Failed to stage {}", cert.display()))?; env_vars.insert( "AVOCADO_FIT_KEY_DIR".to_string(), "/tmp/fit-keys".to_string(), ); Some(dir) } - Err(_) => None, + None => None, }; + let fit_keydir_host_path: Option = fit_keys_tempdir + .as_ref() + .map(|d| d.path().to_string_lossy().into_owned()); // The in-container sign_amf shell helper checks // $AVOCADO_AMF_KOS. Only kos runtimes set it; everyone else @@ -3321,10 +3345,10 @@ FIT_ITS="$OUTPUT_DIR/fit-image.its" if [ -f "$FIT_ITS" ] && [ -f "$OUTPUT_DIR/linux.bin" ] && [ -n "${AVOCADO_INITRAMFS_IMAGE:-}" ]; then if [ -z "${AVOCADO_FIT_KEY_DIR:-}" ] && [ "${AVOCADO_FIT_UNSIGNED:-0}" != "1" ]; then if [ -n "${AVOCADO_ROOTFS_ROOTHASH:-}" ]; then - echo "ERROR: rootfs.image.verity is on, which needs the boot FIT rebuilt with the root hash, but no FIT signing key is configured. Set AVOCADO_FIT_KEY_DIR to a directory holding FIT.key/FIT.crt, or AVOCADO_FIT_UNSIGNED=1 if this machine's U-Boot enforces no key." >&2 + echo "ERROR: rootfs.image.verity is on, which needs the boot FIT rebuilt with the root hash, but no FIT signing key is configured. Set runtimes..signing.fit_key to an RSA key in the signing-key registry, or signing.fit_unsigned: true if this machine's U-Boot enforces no key." >&2 exit 1 fi - echo "WARNING: boot FIT not rebuilt (no AVOCADO_FIT_KEY_DIR, AVOCADO_FIT_UNSIGNED not set): the feed's fitImage, with the feed's initramfs, will be used." >&2 + echo "WARNING: boot FIT not rebuilt (no signing.fit_key, signing.fit_unsigned not set): the feed's fitImage, with the feed's initramfs, will be used." >&2 else echo "Assembling boot FIT from $FIT_ITS..." FIT_WORK_ITS="$OUTPUT_DIR/fit-image.project.its" diff --git a/src/commands/signing_keys/create.rs b/src/commands/signing_keys/create.rs index 50863315..f42b8f0f 100644 --- a/src/commands/signing_keys/create.rs +++ b/src/commands/signing_keys/create.rs @@ -24,9 +24,14 @@ pub struct SigningKeysCreateCommand { pub generate: bool, /// Authentication method for PKCS#11 device pub auth: String, + /// Key algorithm: ed25519 (default, the cli's own signer) or rsa2048/rsa4096 + /// (a PEM key + self-signed certificate for boot-FIT signing, generated + /// with the host's `openssl`). + pub algorithm: String, } impl SigningKeysCreateCommand { + #[allow(clippy::too_many_arguments)] pub fn new( name: Option, uri: Option, @@ -35,6 +40,7 @@ impl SigningKeysCreateCommand { key_label: Option, generate: bool, auth: String, + algorithm: String, ) -> Self { Self { name, @@ -44,9 +50,42 @@ impl SigningKeysCreateCommand { key_label, generate, auth, + algorithm, } } + /// RSA: `openssl req -x509 -newkey rsa:N` into a temp dir, then the same + /// storage path as `import`. Returns (keyid, uri). + fn create_rsa_pem(&self) -> Result<(String, String)> { + use crate::utils::signing_keys::{keyid_for_pem_cert, save_pem_keypair}; + let bits = self.algorithm.trim_start_matches("rsa"); + let subject = format!("/CN={}", self.name.as_deref().unwrap_or("avocado-fit")); + let dir = tempfile::Builder::new() + .prefix("avocado-rsa-key-") + .tempdir()?; + let key = dir.path().join("key.pem"); + let cert = dir.path().join("cert.pem"); + let status = std::process::Command::new("openssl") + .args([ + "req", "-batch", "-new", "-x509", "-sha256", "-nodes", "-days", "3650", + "-newkey", &format!("rsa:{bits}"), "-subj", &subject, + ]) + .arg("-keyout").arg(&key) + .arg("-out").arg(&cert) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::inherit()) + .status() + .map_err(|e| anyhow::anyhow!("could not run openssl ({e}); install it, or generate the key elsewhere and use `avocado signing-keys import`"))?; + if !status.success() { + anyhow::bail!("openssl failed to generate the {} key", self.algorithm); + } + let key_pem = std::fs::read(&key)?; + let cert_pem = std::fs::read_to_string(&cert)?; + let keyid = keyid_for_pem_cert(&cert_pem)?; + let base = save_pem_keypair(&keyid, &key_pem, cert_pem.as_bytes())?; + Ok((keyid, path_to_file_uri(&base))) + } + pub fn execute(&self) -> Result<()> { use crate::utils::pkcs11_devices::{ build_pkcs11_uri, find_existing_key, generate_keypair as generate_pkcs11_keypair, @@ -56,7 +95,23 @@ impl SigningKeysCreateCommand { let mut registry = KeysRegistry::load()?; - let (keyid, uri, algorithm, key_type) = if let Some(device_type_str) = &self.pkcs11_device { + let (keyid, uri, algorithm, key_type) = if crate::utils::signing_keys::is_pem_algorithm( + &self.algorithm, + ) { + if self.pkcs11_device.is_some() || self.uri.is_some() { + anyhow::bail!( + "--algorithm {} is file-based; it cannot be combined with PKCS#11 options", + self.algorithm + ); + } + let (keyid, uri) = self.create_rsa_pem()?; + (keyid, uri, self.algorithm.clone(), "file".to_string()) + } else if self.algorithm != "ed25519" { + anyhow::bail!( + "--algorithm {}: expected ed25519, rsa2048 or rsa4096", + self.algorithm + ); + } else if let Some(device_type_str) = &self.pkcs11_device { // PKCS#11 hardware device flow let device_type = DeviceType::from_str(device_type_str)?; let auth_method = Pkcs11AuthMethod::from_str(&self.auth)?; diff --git a/src/commands/signing_keys/import.rs b/src/commands/signing_keys/import.rs new file mode 100644 index 00000000..dab1d35d --- /dev/null +++ b/src/commands/signing_keys/import.rs @@ -0,0 +1,78 @@ +//! Import an existing PEM key + certificate (RSA, for boot-FIT signing). + +use anyhow::{Context, Result}; +use chrono::Utc; +use std::path::PathBuf; + +use crate::utils::signing_keys::{ + is_pem_algorithm, keyid_for_pem_cert, path_to_file_uri, save_pem_keypair, KeyEntry, + KeysRegistry, +}; + +/// `avocado signing-keys import --key FIT.key --cert FIT.crt` +/// +/// Registers an RSA private key / X.509 certificate pair under the registry so +/// a runtime can name it in `signing.fit_key`. The key id is the SHA-256 of the +/// certificate's DER. +pub struct SigningKeysImportCommand { + pub name: String, + pub key: PathBuf, + pub cert: PathBuf, + pub algorithm: String, +} + +impl SigningKeysImportCommand { + pub fn new(name: String, key: PathBuf, cert: PathBuf, algorithm: String) -> Self { + Self { + name, + key, + cert, + algorithm, + } + } + + pub fn execute(&self) -> Result<()> { + if !is_pem_algorithm(&self.algorithm) { + anyhow::bail!( + "--algorithm {}: import handles RSA PEM keys only (rsa2048, rsa4096)", + self.algorithm + ); + } + let key_pem = std::fs::read(&self.key) + .with_context(|| format!("Failed to read {}", self.key.display()))?; + let cert_pem = std::fs::read_to_string(&self.cert) + .with_context(|| format!("Failed to read {}", self.cert.display()))?; + let keyid = keyid_for_pem_cert(&cert_pem)?; + + let mut registry = KeysRegistry::load()?; + if registry.get_key(&self.name).is_some() { + anyhow::bail!("A key with name '{}' already exists", self.name); + } + let base_path = save_pem_keypair(&keyid, &key_pem, cert_pem.as_bytes())?; + registry.add_key( + self.name.clone(), + KeyEntry { + keyid: keyid.clone(), + algorithm: self.algorithm.clone(), + created_at: Utc::now(), + uri: path_to_file_uri(&base_path), + }, + )?; + registry.save()?; + + println!("Imported signing key:"); + println!(" Name: {}", self.name); + println!(" Key ID: {keyid}"); + println!(" Algorithm: {}", self.algorithm); + println!( + " Files: {}.key / {}.crt", + base_path.display(), + base_path.display() + ); + println!( + "Use it with: runtimes..signing.fit_key: {}", + self.name + ); + Ok(()) + } +} diff --git a/src/commands/signing_keys/mod.rs b/src/commands/signing_keys/mod.rs index 149c514c..0fdc5e9c 100644 --- a/src/commands/signing_keys/mod.rs +++ b/src/commands/signing_keys/mod.rs @@ -4,9 +4,11 @@ //! stored in the global avocado configuration. pub mod create; +pub mod import; pub mod list; pub mod remove; pub use create::SigningKeysCreateCommand; +pub use import::SigningKeysImportCommand; pub use list::SigningKeysListCommand; pub use remove::SigningKeysRemoveCommand; diff --git a/src/main.rs b/src/main.rs index 21f995ac..43314a67 100644 --- a/src/main.rs +++ b/src/main.rs @@ -75,7 +75,8 @@ use commands::sdk::{ }; use commands::sign::SignCommand; use commands::signing_keys::{ - SigningKeysCreateCommand, SigningKeysListCommand, SigningKeysRemoveCommand, + SigningKeysCreateCommand, SigningKeysImportCommand, SigningKeysListCommand, + SigningKeysRemoveCommand, }; use commands::unlock::UnlockCommand; use commands::update::UpdateCommand; @@ -1467,6 +1468,24 @@ enum SigningKeysCommands { /// Authentication method for PKCS#11 device (none, prompt, env) #[arg(long, default_value = "prompt", value_name = "METHOD")] auth: String, + /// Key algorithm: ed25519 (default), or rsa2048 / rsa4096 for boot-FIT + /// signing (a PEM key + self-signed certificate, generated with openssl) + #[arg(long, default_value = "ed25519", value_name = "ALGORITHM")] + algorithm: String, + }, + /// Import an existing RSA PEM key and certificate (for boot-FIT signing) + Import { + /// Name for the key, referenced from `runtimes..signing.fit_key` + name: String, + /// PEM private key file + #[arg(long, value_name = "FILE")] + key: std::path::PathBuf, + /// PEM X.509 certificate for that key + #[arg(long, value_name = "FILE")] + cert: std::path::PathBuf, + /// rsa2048 or rsa4096 + #[arg(long, default_value = "rsa2048", value_name = "ALGORITHM")] + algorithm: String, }, /// List all registered signing keys List, @@ -2485,6 +2504,7 @@ async fn main() -> Result<()> { key_label, generate, auth, + algorithm, } => { let cmd = SigningKeysCreateCommand::new( name, @@ -2494,10 +2514,20 @@ async fn main() -> Result<()> { key_label, generate, auth, + algorithm, ); cmd.execute()?; Ok(()) } + SigningKeysCommands::Import { + name, + key, + cert, + algorithm, + } => { + SigningKeysImportCommand::new(name, key, cert, algorithm).execute()?; + Ok(()) + } SigningKeysCommands::List => { let cmd = SigningKeysListCommand::new(); cmd.execute()?; diff --git a/src/utils/config.rs b/src/utils/config.rs index 861b66a2..c1ee175d 100644 --- a/src/utils/config.rs +++ b/src/utils/config.rs @@ -576,6 +576,18 @@ pub struct SigningConfig { /// this key for targets/snapshot/timestamp roles. Overrides connect.server_key. #[serde(default)] pub server_key: Option, + /// Key that signs this runtime's boot FIT (FIT-booting machines only): a + /// registry name or key id of an RSA PEM key (`avocado signing-keys import` + /// / `create --algorithm rsa2048`). The build materializes it as the + /// `FIT.key`/`FIT.crt` pair `mkimage -k` expects. Required when + /// `rootfs.image.verity` is on, since the root hash rides in the FIT. + #[serde(default)] + pub fit_key: Option, + /// Build this runtime's boot FIT unsigned. Only boots on a U-Boot that + /// enforces no key; mutually exclusive with `fit_key`. Explicit so an + /// unsigned boot image is never the accidental result of a missing key. + #[serde(default)] + pub fit_unsigned: Option, } fn default_checksum_algorithm() -> String { @@ -4176,6 +4188,22 @@ impl Config { .filter(|key| !key.trim().is_empty()) } + /// The runtime's boot-FIT signing choice: `(fit_key, fit_unsigned)`. + /// Both set is a configuration error, reported by the caller. + pub fn get_runtime_fit_signing(&self, runtime_name: &str) -> (Option, bool) { + let signing = self + .runtimes + .as_ref() + .and_then(|r| r.get(runtime_name)) + .and_then(|r| r.signing.as_ref()); + ( + signing + .and_then(|s| s.fit_key.clone()) + .filter(|k| !k.trim().is_empty()), + signing.and_then(|s| s.fit_unsigned).unwrap_or(false), + ) + } + /// Get the declared content key name for a runtime (for signing delegated-targets only). /// /// Returns Some(key_name) if the runtime has a content_key configured, @@ -5985,6 +6013,29 @@ pub fn find_active_compile_sections( #[cfg(test)] mod tests { + #[test] + fn fit_signing_reads_key_and_explicit_unsigned_per_runtime() { + let yaml = r#" +default_target: imx8mp-evk +runtimes: + prod: + signing: + fit_key: product-fit + lab: + signing: + fit_unsigned: true + plain: {} +"#; + let config: Config = serde_yaml::from_str(yaml).unwrap(); + assert_eq!( + config.get_runtime_fit_signing("prod"), + (Some("product-fit".to_string()), false) + ); + assert_eq!(config.get_runtime_fit_signing("lab"), (None, true)); + assert_eq!(config.get_runtime_fit_signing("plain"), (None, false)); + assert_eq!(config.get_runtime_fit_signing("missing"), (None, false)); + } + #[test] fn image_verity_is_strict_bool() { use super::get_ext_image_verity; diff --git a/src/utils/signing_keys.rs b/src/utils/signing_keys.rs index c8198fbf..a7e1f6e7 100644 --- a/src/utils/signing_keys.rs +++ b/src/utils/signing_keys.rs @@ -631,3 +631,138 @@ mod tests { assert_eq!(keyid.len(), 64, "keyid should be 64 hex characters"); } } + +// --------------------------------------------------------------------------- +// PEM keys for external signers (U-Boot mkimage FIT signing) +// --------------------------------------------------------------------------- + +/// Algorithms whose material is a PEM RSA private key + X.509 certificate, +/// consumed by external tools (`mkimage -k ` expects `.key` and +/// `.crt`) rather than by the cli's own ed25519 signer. +pub fn is_pem_algorithm(algorithm: &str) -> bool { + matches!(algorithm, "rsa2048" | "rsa4096") +} + +/// Key ID for a certificate: SHA-256 of its DER (the PEM body decoded), so the +/// same certificate imported twice gets the same id regardless of line +/// wrapping or trailing whitespace. +pub fn keyid_for_pem_cert(cert_pem: &str) -> Result { + let body: String = cert_pem + .lines() + .filter(|l| !l.starts_with("-----")) + .map(str::trim) + .collect(); + let der = BASE64_STANDARD + .decode(body.as_bytes()) + .context("certificate is not a PEM-encoded X.509 certificate")?; + if der.is_empty() { + anyhow::bail!("certificate PEM body is empty"); + } + let mut hasher = Sha256::new(); + hasher.update(&der); + Ok(hex::encode(&hasher.finalize())) +} + +/// Store a PEM private key and certificate under the registry as +/// `.key` (0600) and `.crt`. Returns the base path the registry +/// URI points at. +pub fn save_pem_keypair(keyid: &str, key_pem: &[u8], cert_pem: &[u8]) -> Result { + if !key_pem.starts_with(b"-----BEGIN") { + anyhow::bail!("private key is not PEM (expected -----BEGIN ... PRIVATE KEY-----)"); + } + let keys_dir = get_signing_keys_dir()?; + fs::create_dir_all(&keys_dir).with_context(|| { + format!( + "Failed to create signing keys directory: {}", + keys_dir.display() + ) + })?; + let base_path = get_key_file_path(keyid)?; + let key_path = base_path.with_extension("key"); + let cert_path = base_path.with_extension("crt"); + fs::write(&key_path, key_pem) + .with_context(|| format!("Failed to write private key: {}", key_path.display()))?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(&key_path, std::fs::Permissions::from_mode(0o600)) + .with_context(|| format!("Failed to set permissions on {}", key_path.display()))?; + } + fs::write(&cert_path, cert_pem) + .with_context(|| format!("Failed to write certificate: {}", cert_path.display()))?; + Ok(base_path) +} + +/// The PEM files behind a registry entry, for handing to an external signer. +/// +/// Resolves `name` as a registry name or a key id. Refuses anything that is +/// not a file-backed PEM key: the ed25519 seeds are for the cli's own signer, +/// and a PKCS#11 URI cannot be handed to `mkimage` as a directory. +pub fn pem_key_files(name: &str) -> Result<(PathBuf, PathBuf)> { + let entries = get_key_entries(std::slice::from_ref(&name.to_string()))?; + let (registry_name, entry) = entries + .into_iter() + .next() + .ok_or_else(|| anyhow::anyhow!("signing key '{name}' is not in the registry"))?; + if !is_pem_algorithm(&entry.algorithm) { + anyhow::bail!( + "signing key '{registry_name}' is {} - FIT signing needs an RSA PEM key \ + (rsa2048/rsa4096). Import one with `avocado signing-keys import`.", + entry.algorithm + ); + } + if !is_file_uri(&entry.uri) { + anyhow::bail!( + "signing key '{registry_name}' is {} - FIT signing needs a file-backed PEM key; \ + mkimage cannot use a PKCS#11 URI", + entry.uri + ); + } + let base = PathBuf::from(entry.uri.trim_start_matches("file://")); + let key = base.with_extension("key"); + let cert = base.with_extension("crt"); + for f in [&key, &cert] { + if !f.is_file() { + anyhow::bail!( + "signing key '{registry_name}' is registered but {} is missing", + f.display() + ); + } + } + Ok((key, cert)) +} + +#[cfg(test)] +mod pem_tests { + use super::*; + + const CERT: &str = "-----BEGIN CERTIFICATE-----\nAAECAwQFBgc=\n-----END CERTIFICATE-----\n"; + + #[test] + fn a_cert_keyid_is_the_sha256_of_its_der() { + let id = keyid_for_pem_cert(CERT).unwrap(); + let mut h = Sha256::new(); + h.update([0u8, 1, 2, 3, 4, 5, 6, 7]); + assert_eq!(id, hex::encode(&h.finalize())); + // Same certificate, different wrapping and whitespace: same id. + let rewrapped = + "-----BEGIN CERTIFICATE-----\nAAEC\n AwQF\nBgc= \n-----END CERTIFICATE-----"; + assert_eq!(keyid_for_pem_cert(rewrapped).unwrap(), id); + } + + #[test] + fn a_non_certificate_is_refused() { + assert!(keyid_for_pem_cert("hello").is_err()); + assert!( + keyid_for_pem_cert("-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----").is_err() + ); + } + + #[test] + fn only_rsa_is_a_pem_algorithm() { + assert!(is_pem_algorithm("rsa2048")); + assert!(is_pem_algorithm("rsa4096")); + assert!(!is_pem_algorithm("ed25519")); + assert!(!is_pem_algorithm("ecdsa-p256")); + } +} diff --git a/tests/pkcs11_integration_test.rs b/tests/pkcs11_integration_test.rs index 1dc3ee05..20a7edc1 100644 --- a/tests/pkcs11_integration_test.rs +++ b/tests/pkcs11_integration_test.rs @@ -512,6 +512,7 @@ fn test_tpm_key_registration_and_removal() { Some("test-key-label".to_string()), false, // don't generate, reference existing "none".to_string(), + "ed25519".to_string(), ); // First, generate the key in TPM directly @@ -574,6 +575,7 @@ fn test_tpm_key_registration_and_removal() { Some("test-key-label-2".to_string()), false, "none".to_string(), + "ed25519".to_string(), ); let (_public_key_bytes2, keyid2, _algo_str2) = From c12c294cd77d003f4ca4f691993c13cf11657630 Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Thu, 27 Aug 2026 19:08:53 -0400 Subject: [PATCH 2/9] feat(signing): put the FIT key in the bootloader, and make sure the FIT is actually signed - signing.fit_key_in_bootloader (default true with fit_key): after assembling the FIT, run the feed's imx-boot-tools/rekey-imx-boot.sh so U-Boot enforces the project key, then prove it with fit_check_sign against the keyed control DTB. A feed without the tooling fails the build rather than shipping a bootloader that ignores the key. - A feed built without verified-boot ships a FIT template whose configuration nodes carry no signature-* subnode, and `mkimage -r` then signs nothing without complaint - the previous "(signed)" was not. Inject a signature-1 node (algo from the registry key, key-name-hint FIT, sign-images from the configuration's own image properties) when the template lacks them, and fail the build unless `mkimage -l` shows a signature afterwards. --- CHANGELOG.md | 4 ++ docs/signing-keys.md | 10 +++++ src/commands/runtime/build.rs | 75 ++++++++++++++++++++++++++++++++++- src/utils/config.rs | 20 ++++++++++ src/utils/signing_keys.rs | 4 +- 5 files changed, 110 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d9915ba..8cfe1402 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 reproducible from `avocado.yaml` alone. `signing.fit_unsigned: true` is the explicit opt-out. Replaces the interim `AVOCADO_FIT_KEY_DIR` / `AVOCADO_FIT_UNSIGNED` environment variables, which are no longer read. + With `fit_key` set the build also re-packs the feed's bootloader so U-Boot + enforces that key (`signing.fit_key_in_bootloader`, default true; i.MX8M via + the feed's `imx-boot-tools/rekey-imx-boot.sh`), so provisioning writes a + bootloader closed to the project key from the first flash. - **`runtimes..var.encrypt: true` — encrypted `/var`.** Opts a runtime into a LUKS2 `/var` sealed to the target's hardware key store (OP-TEE fTPM on Jetson). The cli adds `cryptsetup-var` to the initramfs and diff --git a/docs/signing-keys.md b/docs/signing-keys.md index dbcbd8cc..6c5cefbc 100644 --- a/docs/signing-keys.md +++ b/docs/signing-keys.md @@ -35,6 +35,16 @@ the SDK as `FIT.key`/`FIT.crt` (the template's `key-name-hint`). With root hash rides in the FIT. RSA keys are file-based only: `mkimage` cannot use a PKCS#11 URI. +With `fit_key` set the build also rebuilds the feed's bootloader so U-Boot +*requires* that key (`signing.fit_key_in_bootloader`, default `true`): on +i.MX8M the public key is injected into the U-Boot control DTB and `imx-boot` +is re-packed from the feed's own inputs with the procedure the feed ships +(`imx-boot-tools/rekey-imx-boot.sh`). The re-packed images take the feed's +file names, so `avocado provision` flashes a bootloader that is closed to the +project key from the first boot. Set it to `false` to keep the distro +bootloader (a feed built with `verified-boot` enforces the distro key +instead). + ### Creating Keys diff --git a/src/commands/runtime/build.rs b/src/commands/runtime/build.rs index 5922f98f..03e675a3 100644 --- a/src/commands/runtime/build.rs +++ b/src/commands/runtime/build.rs @@ -751,7 +751,13 @@ impl RuntimeBuildCommand { } let fit_keys_tempdir: Option = match fit_key_name { Some(ref name) => { - let (key, cert) = crate::utils::signing_keys::pem_key_files(name)?; + let (key, cert, algorithm) = crate::utils::signing_keys::pem_key_files(name)?; + // mkimage's algo string for the FIT signature nodes and for + // fdt_add_pubkey: ",". + env_vars.insert( + "AVOCADO_FIT_ALGO".to_string(), + format!("sha256,{algorithm}"), + ); let dir = tempfile::Builder::new() .prefix("avocado-fit-keys-") .tempdir() @@ -771,6 +777,11 @@ impl RuntimeBuildCommand { let fit_keydir_host_path: Option = fit_keys_tempdir .as_ref() .map(|d| d.path().to_string_lossy().into_owned()); + if fit_keys_tempdir.is_some() + && config.get_runtime_fit_key_in_bootloader(&self.runtime_name) + { + env_vars.insert("AVOCADO_FIT_KEY_IN_BOOTLOADER".to_string(), "1".to_string()); + } // The in-container sign_amf shell helper checks // $AVOCADO_AMF_KOS. Only kos runtimes set it; everyone else @@ -3367,6 +3378,35 @@ if [ -f "$FIT_ITS" ] && [ -f "$OUTPUT_DIR/linux.bin" ] && [ -n "${AVOCADO_INITRA FIT_SIGN_ARGS="" if [ -n "${AVOCADO_FIT_KEY_DIR:-}" ]; then FIT_SIGN_ARGS="-k $AVOCADO_FIT_KEY_DIR -r" + # A feed built without verified-boot ships a template whose + # configuration nodes carry no signature-* subnode, and mkimage -r + # then signs nothing without complaint. Give every configuration + # one (algo, key-name-hint "FIT", sign-images = the image + # properties that configuration actually names) unless the + # template already has them. The result is checked below. + if ! grep -qE '^[[:space:]]*signature-[0-9]+ \{' "$FIT_WORK_ITS"; then + awk -v algo="${AVOCADO_FIT_ALGO:-sha256,rsa2048}" ' + /^[[:space:]]*conf-[^[:space:]]+ \{$/ { inconf=1; imgs=""; depth=0 } + inconf && /^[[:space:]]*(kernel|fdt|ramdisk|loadables) = / { + p=$1; imgs = imgs (imgs==""?"":", ") "\"" p "\"" + } + inconf && /\{[[:space:]]*$/ { depth++ } + inconf && /^[[:space:]]*\};/ { + depth-- + if (depth==0) { + print "\t\t\tsignature-1 {" + print "\t\t\t\talgo = \"" algo "\";" + print "\t\t\t\tkey-name-hint = \"FIT\";" + print "\t\t\t\tsign-images = " imgs ";" + print "\t\t\t};" + inconf=0 + } + } + { print } + ' "$FIT_WORK_ITS" > "$FIT_WORK_ITS.signed" && mv "$FIT_WORK_ITS.signed" "$FIT_WORK_ITS" + grep -qE '^[[:space:]]*signature-1 \{' "$FIT_WORK_ITS" \ + || { echo "ERROR: could not add signature nodes to the FIT configurations in $FIT_ITS" >&2; exit 1; } + fi else # Explicitly unsigned: strip the signature nodes so mkimage does not look for a key. sed -i -E '/^[[:space:]]*signature-[0-9]+ \{/,/^[[:space:]]*\};/d' "$FIT_WORK_ITS" @@ -3376,7 +3416,36 @@ if [ -f "$FIT_ITS" ] && [ -f "$OUTPUT_DIR/linux.bin" ] && [ -n "${AVOCADO_INITRA || { echo "ERROR: mkimage failed to assemble the boot FIT" >&2; exit 1; } [ -s "$OUTPUT_DIR/fitImage" ] || { echo "ERROR: boot FIT was not produced" >&2; exit 1; } mkimage -l "$OUTPUT_DIR/fitImage" | grep -E 'Default Configuration|Sign algo' | head -2 | sed 's/^/ /' + if [ -n "${AVOCADO_FIT_KEY_DIR:-}" ]; then + # Prove the signature exists rather than trust mkimage's silence. + mkimage -l "$OUTPUT_DIR/fitImage" | grep -q 'Sign algo' \ + || { echo "ERROR: boot FIT was built but carries no configuration signature" >&2; exit 1; } + fi echo "Built boot FIT: $OUTPUT_DIR/fitImage${AVOCADO_FIT_KEY_DIR:+ (signed)}${AVOCADO_ROOTFS_ROOTHASH:+ (rootfs root hash embedded)}" + # Make the bootloader enforce that key. The feed ships the procedure and + # its inputs (imx-boot-tools/rekey-imx-boot.sh + rekey.env, i.MX8M); the + # re-packed images take the feed's file names, so stone's imx_boot* image + # keys resolve to them ahead of the SDK's copies. A feed without the + # tooling is an error, not a silent distro bootloader: a project that + # asked for its key in the bootloader must not ship one that ignores it. + if [ "${AVOCADO_FIT_KEY_IN_BOOTLOADER:-0}" = "1" ]; then + REKEY="$OUTPUT_DIR/imx-boot-tools/rekey-imx-boot.sh" + if [ ! -x "$REKEY" ]; then + echo "ERROR: signing.fit_key_in_bootloader is on but this feed ships no imx-boot-tools/rekey-imx-boot.sh for $TARGET_ARCH. Set signing.fit_key_in_bootloader: false to keep the distro bootloader." >&2 + exit 1 + fi + echo "Rebuilding the bootloader to enforce the FIT key..." + "$REKEY" "$OUTPUT_DIR/imx-boot-tools" "$AVOCADO_FIT_KEY_DIR" "$OUTPUT_DIR" "${AVOCADO_FIT_ALGO:-sha256,rsa2048}" \ + || { echo "ERROR: bootloader re-key failed" >&2; exit 1; } + # The keyed control DTB is what U-Boot will verify with; run that + # verification here so a mismatch fails the build, not the boot. + KEYED_DTB=$(ls "$OUTPUT_DIR"/u-boot-*.dtb.keyed 2>/dev/null | head -1) + if [ -n "$KEYED_DTB" ] && command -v fit_check_sign >/dev/null 2>&1; then + fit_check_sign -f "$OUTPUT_DIR/fitImage" -k "$KEYED_DTB" >/dev/null 2>&1 \ + || { echo "ERROR: the re-keyed bootloader does not verify this runtime's boot FIT" >&2; exit 1; } + echo "Bootloader re-keyed: fitImage verifies against $(basename "$KEYED_DTB")" + fi + fi fi fi "# @@ -3401,6 +3470,10 @@ mod tests { "rootfs root hash must be embedded in the FIT" ); assert!(s.contains("-k $AVOCADO_FIT_KEY_DIR -r"), "signing path"); + assert!( + s.contains("rekey-imx-boot.sh") && s.contains("AVOCADO_FIT_KEY_IN_BOOTLOADER"), + "bootloader re-key runs only when asked, and fails closed without the feed tooling" + ); assert!( s.contains("signature-[0-9]+"), "unsigned path must strip signature nodes" diff --git a/src/utils/config.rs b/src/utils/config.rs index c1ee175d..dd16a96d 100644 --- a/src/utils/config.rs +++ b/src/utils/config.rs @@ -588,6 +588,15 @@ pub struct SigningConfig { /// unsigned boot image is never the accidental result of a missing key. #[serde(default)] pub fit_unsigned: Option, + /// Also rebuild the feed's bootloader so U-Boot enforces `fit_key` + /// (i.MX8M: the public key goes into the control DTB and imx-boot is + /// re-packed from the feed's own inputs, using the tooling the feed ships + /// in `imx-boot-tools/`). Default true whenever `fit_key` is set: a signed + /// FIT that the bootloader does not require is only half the feature. + /// Set false to keep the distro bootloader (e.g. a feed built with + /// `verified-boot`, where the distro key is the one enforced). + #[serde(default)] + pub fit_key_in_bootloader: Option, } fn default_checksum_algorithm() -> String { @@ -4204,6 +4213,17 @@ impl Config { ) } + /// Whether the bootloader should be rebuilt to enforce `fit_key` + /// (`signing.fit_key_in_bootloader`, default true). + pub fn get_runtime_fit_key_in_bootloader(&self, runtime_name: &str) -> bool { + self.runtimes + .as_ref() + .and_then(|r| r.get(runtime_name)) + .and_then(|r| r.signing.as_ref()) + .and_then(|s| s.fit_key_in_bootloader) + .unwrap_or(true) + } + /// Get the declared content key name for a runtime (for signing delegated-targets only). /// /// Returns Some(key_name) if the runtime has a content_key configured, diff --git a/src/utils/signing_keys.rs b/src/utils/signing_keys.rs index a7e1f6e7..778a5332 100644 --- a/src/utils/signing_keys.rs +++ b/src/utils/signing_keys.rs @@ -698,7 +698,7 @@ pub fn save_pem_keypair(keyid: &str, key_pem: &[u8], cert_pem: &[u8]) -> Result< /// Resolves `name` as a registry name or a key id. Refuses anything that is /// not a file-backed PEM key: the ed25519 seeds are for the cli's own signer, /// and a PKCS#11 URI cannot be handed to `mkimage` as a directory. -pub fn pem_key_files(name: &str) -> Result<(PathBuf, PathBuf)> { +pub fn pem_key_files(name: &str) -> Result<(PathBuf, PathBuf, String)> { let entries = get_key_entries(std::slice::from_ref(&name.to_string()))?; let (registry_name, entry) = entries .into_iter() @@ -729,7 +729,7 @@ pub fn pem_key_files(name: &str) -> Result<(PathBuf, PathBuf)> { ); } } - Ok((key, cert)) + Ok((key, cert, entry.algorithm)) } #[cfg(test)] From b733a61f562d77e19bc0563f344680f91f8fc364 Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Thu, 27 Aug 2026 19:45:27 -0400 Subject: [PATCH 3/9] fix(var.encrypt): give the var partition room for the LUKS2 header at first boot The var partition is sized from the image (stone --partition-size) and only grown to the disk later, on the device. A runtime that encrypts /var on first boot therefore found a partition 3 MiB larger than the filesystem filling it, and cryptsetup-var's in-place reencrypt (which needs 32 MiB in front of the data) could neither find the room nor shrink a 285 MiB btrfs to make it - observed on a freshly provisioned imx8mp-evk. Add 64 MiB to the partition when the runtime opts into var.encrypt; plaintext runtimes keep the exact image size. --- src/commands/runtime/build.rs | 57 ++++++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/src/commands/runtime/build.rs b/src/commands/runtime/build.rs index 03e675a3..3605f104 100644 --- a/src/commands/runtime/build.rs +++ b/src/commands/runtime/build.rs @@ -2687,6 +2687,14 @@ kill $_PROGRESS_PID 2>/dev/null; wait $_PROGRESS_PID 2>/dev/null || true {post_creation_section} FINAL_SIZE=$(stat -c%s "$VAR_IMAGE" 2>/dev/null || echo 0) FINAL_MB=$(( FINAL_SIZE / 1048576 )) +# The var partition is sized from the image (stone --partition-size) and only +# grown to the disk later, by whatever runs on the device. A runtime that +# encrypts /var on first boot needs room the image does not have: the LUKS2 +# header goes in front of the data (cryptsetup reencrypt --reduce-device-size +# 32M), so the partition must be larger than the filesystem that fills it - +# 64 MiB covers the header and the shrink granularity btrfs needs. Plaintext +# runtimes keep the exact image size, so their layout is unchanged. +VAR_PART_BYTES=$(( FINAL_SIZE + {var_part_headroom} )) echo "" echo "Built var image: ${{FINAL_MB}}MB" @@ -2740,7 +2748,7 @@ stone bundle \ -m "$STONE_MANIFEST" \ $STONE_INCLUDE_FLAGS \ $STONE_OVERLAY_FLAG \ - --partition-size "var=$FINAL_SIZE" \ + --partition-size "var=$VAR_PART_BYTES" \ -o "$STONE_AOS_OUTPUT" \ --build-dir "$STONE_BUILD_DIR" @@ -2843,6 +2851,11 @@ sign_amf "$AVOCADO_MANIFEST_PATH" update_authority_section = update_authority_section, docker_section = docker_section, device_tree_overlay_section = device_tree_overlay_section, + var_part_headroom = if var_encrypt(target_arch) { + 64 * 1024 * 1024 + } else { + 0 + }, ); Ok(script) @@ -4146,6 +4159,48 @@ runtimes: assert!(!script.contains("$INITRAMFS_SYSROOT/etc/avocado/var-encrypt")); } + #[test] + fn var_encrypt_gives_the_var_partition_headroom_for_the_luks_header() { + let temp_dir = TempDir::new().unwrap(); + let on = r#" +connect: + org: test + +runtimes: + test-runtime: + target: "x86_64" + var: + encrypt: true +"#; + let build = |content: &str| { + let config_path = create_test_config_file(&temp_dir, content); + let parsed: serde_yaml::Value = serde_yaml::from_str(content).unwrap(); + let cmd = RuntimeBuildCommand::new( + "test-runtime".to_string(), + config_path, + false, + Some("x86_64".to_string()), + None, + None, + ); + let config = Config::load(&cmd.config_path).unwrap(); + cmd.create_build_script(&config, &parsed, "x86_64", &[]) + .unwrap() + }; + let script = build(on); + assert!( + script.contains("VAR_PART_BYTES=$(( FINAL_SIZE + 67108864 ))"), + "encrypted /var gets 64 MiB of headroom for the LUKS2 header" + ); + assert!(script.contains("--partition-size \"var=$VAR_PART_BYTES\"")); + + let script = build(&on.replace("encrypt: true", "encrypt: false")); + assert!( + script.contains("VAR_PART_BYTES=$(( FINAL_SIZE + 0 ))"), + "a plaintext runtime keeps the exact image size" + ); + } + /// `encrypt:` under a `target-:` override is honored like every other /// `var:` key; the package union reads the same merged source, so the /// marker never ships without cryptsetup-var behind it. From 7fe710ce176d3ac31d9459bc015d89881eaa7e13 Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Thu, 27 Aug 2026 19:49:51 -0400 Subject: [PATCH 4/9] fix(var): always leave 64 MiB for a LUKS2 header in the var partition Not only when the runtime encrypts: a device flashed plaintext must be able to turn var.encrypt on over an update later, and without the dev extension its partition is never grown, so the headroom has to be there from the first flash. var is the last partition and the image is sparse-flashed, so the only cost is 64 MiB of zeros; the layout no longer depends on a runtime flag. --- src/commands/runtime/build.rs | 67 ++++++++++++++--------------------- 1 file changed, 26 insertions(+), 41 deletions(-) diff --git a/src/commands/runtime/build.rs b/src/commands/runtime/build.rs index 3605f104..a6e1501e 100644 --- a/src/commands/runtime/build.rs +++ b/src/commands/runtime/build.rs @@ -2688,13 +2688,14 @@ kill $_PROGRESS_PID 2>/dev/null; wait $_PROGRESS_PID 2>/dev/null || true FINAL_SIZE=$(stat -c%s "$VAR_IMAGE" 2>/dev/null || echo 0) FINAL_MB=$(( FINAL_SIZE / 1048576 )) # The var partition is sized from the image (stone --partition-size) and only -# grown to the disk later, by whatever runs on the device. A runtime that -# encrypts /var on first boot needs room the image does not have: the LUKS2 -# header goes in front of the data (cryptsetup reencrypt --reduce-device-size -# 32M), so the partition must be larger than the filesystem that fills it - -# 64 MiB covers the header and the shrink granularity btrfs needs. Plaintext -# runtimes keep the exact image size, so their layout is unchanged. -VAR_PART_BYTES=$(( FINAL_SIZE + {var_part_headroom} )) +# grown to the disk later, by whatever runs on the device. Always leave room +# for a LUKS2 header in front of the data (cryptsetup reencrypt +# --reduce-device-size 32M; 64 MiB covers it and the shrink granularity btrfs +# needs), whether or not this runtime encrypts: a device flashed plaintext must +# still be able to turn var.encrypt on over an update later, and its partition +# may never have been grown. var is the last partition, so nothing else moves; +# the cost is 64 MiB of a sparse-flashed image. +VAR_PART_BYTES=$(( FINAL_SIZE + 67108864 )) echo "" echo "Built var image: ${{FINAL_MB}}MB" @@ -2851,11 +2852,6 @@ sign_amf "$AVOCADO_MANIFEST_PATH" update_authority_section = update_authority_section, docker_section = docker_section, device_tree_overlay_section = device_tree_overlay_section, - var_part_headroom = if var_encrypt(target_arch) { - 64 * 1024 * 1024 - } else { - 0 - }, ); Ok(script) @@ -4160,45 +4156,34 @@ runtimes: } #[test] - fn var_encrypt_gives_the_var_partition_headroom_for_the_luks_header() { + fn the_var_partition_always_leaves_room_for_a_luks_header() { let temp_dir = TempDir::new().unwrap(); - let on = r#" + let content = r#" connect: org: test runtimes: test-runtime: target: "x86_64" - var: - encrypt: true "#; - let build = |content: &str| { - let config_path = create_test_config_file(&temp_dir, content); - let parsed: serde_yaml::Value = serde_yaml::from_str(content).unwrap(); - let cmd = RuntimeBuildCommand::new( - "test-runtime".to_string(), - config_path, - false, - Some("x86_64".to_string()), - None, - None, - ); - let config = Config::load(&cmd.config_path).unwrap(); - cmd.create_build_script(&config, &parsed, "x86_64", &[]) - .unwrap() - }; - let script = build(on); - assert!( - script.contains("VAR_PART_BYTES=$(( FINAL_SIZE + 67108864 ))"), - "encrypted /var gets 64 MiB of headroom for the LUKS2 header" + let config_path = create_test_config_file(&temp_dir, content); + let parsed: serde_yaml::Value = serde_yaml::from_str(content).unwrap(); + let cmd = RuntimeBuildCommand::new( + "test-runtime".to_string(), + config_path, + false, + Some("x86_64".to_string()), + None, + None, ); + let config = Config::load(&cmd.config_path).unwrap(); + let script = cmd + .create_build_script(&config, &parsed, "x86_64", &[]) + .unwrap(); + // Even a plaintext runtime: the device must be able to turn var.encrypt + // on later, and its partition may never have been grown by then. + assert!(script.contains("VAR_PART_BYTES=$(( FINAL_SIZE + 67108864 ))")); assert!(script.contains("--partition-size \"var=$VAR_PART_BYTES\"")); - - let script = build(&on.replace("encrypt: true", "encrypt: false")); - assert!( - script.contains("VAR_PART_BYTES=$(( FINAL_SIZE + 0 ))"), - "a plaintext runtime keeps the exact image size" - ); } /// `encrypt:` under a `target-:` override is honored like every other From fb15bd51b4d7a896e84b5959392c64de8283b45c Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Thu, 27 Aug 2026 20:57:20 -0400 Subject: [PATCH 5/9] fix(var): let stone own the LUKS header headroom stone now adds it to every override-sized partition, for the runtime bundle and for provisioning alike; carrying a second copy here would double it. --- src/commands/runtime/build.rs | 44 ++++------------------------------- 1 file changed, 4 insertions(+), 40 deletions(-) diff --git a/src/commands/runtime/build.rs b/src/commands/runtime/build.rs index a6e1501e..222fc205 100644 --- a/src/commands/runtime/build.rs +++ b/src/commands/runtime/build.rs @@ -2687,15 +2687,10 @@ kill $_PROGRESS_PID 2>/dev/null; wait $_PROGRESS_PID 2>/dev/null || true {post_creation_section} FINAL_SIZE=$(stat -c%s "$VAR_IMAGE" 2>/dev/null || echo 0) FINAL_MB=$(( FINAL_SIZE / 1048576 )) -# The var partition is sized from the image (stone --partition-size) and only -# grown to the disk later, by whatever runs on the device. Always leave room -# for a LUKS2 header in front of the data (cryptsetup reencrypt -# --reduce-device-size 32M; 64 MiB covers it and the shrink granularity btrfs -# needs), whether or not this runtime encrypts: a device flashed plaintext must -# still be able to turn var.encrypt on over an update later, and its partition -# may never have been grown. var is the last partition, so nothing else moves; -# the cost is 64 MiB of a sparse-flashed image. -VAR_PART_BYTES=$(( FINAL_SIZE + 67108864 )) +# The var partition is sized from the image: stone adds the headroom a later +# in-place LUKS2 conversion needs (see stone's resolve_partition_size_bytes), +# so the same policy applies here and to `avocado provision`. +VAR_PART_BYTES=$FINAL_SIZE echo "" echo "Built var image: ${{FINAL_MB}}MB" @@ -4155,37 +4150,6 @@ runtimes: assert!(!script.contains("$INITRAMFS_SYSROOT/etc/avocado/var-encrypt")); } - #[test] - fn the_var_partition_always_leaves_room_for_a_luks_header() { - let temp_dir = TempDir::new().unwrap(); - let content = r#" -connect: - org: test - -runtimes: - test-runtime: - target: "x86_64" -"#; - let config_path = create_test_config_file(&temp_dir, content); - let parsed: serde_yaml::Value = serde_yaml::from_str(content).unwrap(); - let cmd = RuntimeBuildCommand::new( - "test-runtime".to_string(), - config_path, - false, - Some("x86_64".to_string()), - None, - None, - ); - let config = Config::load(&cmd.config_path).unwrap(); - let script = cmd - .create_build_script(&config, &parsed, "x86_64", &[]) - .unwrap(); - // Even a plaintext runtime: the device must be able to turn var.encrypt - // on later, and its partition may never have been grown by then. - assert!(script.contains("VAR_PART_BYTES=$(( FINAL_SIZE + 67108864 ))")); - assert!(script.contains("--partition-size \"var=$VAR_PART_BYTES\"")); - } - /// `encrypt:` under a `target-:` override is honored like every other /// `var:` key; the package union reads the same merged source, so the /// marker never ships without cryptsetup-var behind it. From c25362a8b89a8b6615c4ddc8a61bd21e3f46881a Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Thu, 27 Aug 2026 21:09:19 -0400 Subject: [PATCH 6/9] fix(var.encrypt): build the var image with room to shrink for the LUKS2 header The first boot converts /var to LUKS2 in place, which needs 32 MiB in front of the data that cryptsetup-var obtains by shrinking the filesystem. mkfs.btrfs -r packs its chunks to the content, so the flashed image had nothing to shrink into and the conversion failed on a freshly provisioned board. When the runtime declares var.encrypt, rebuild the image at its tight size plus 64 MiB so the room is inside the filesystem that asked for it. Partition sizing is untouched everywhere (still exactly the image size), and a plaintext runtime's image is unchanged. --- src/commands/runtime/build.rs | 66 ++++++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 5 deletions(-) diff --git a/src/commands/runtime/build.rs b/src/commands/runtime/build.rs index 222fc205..db08b375 100644 --- a/src/commands/runtime/build.rs +++ b/src/commands/runtime/build.rs @@ -2682,15 +2682,24 @@ mkfs.btrfs -r "$VAR_DIR" \ {mkfs_flags} \ {global_compress_flag} -f "$VAR_IMAGE" +# var.encrypt: the first boot converts this filesystem to LUKS2 in place, which +# needs 32 MiB in front of the data (cryptsetup reencrypt --reduce-device-size) +# that cryptsetup-var obtains by shrinking the filesystem. `mkfs.btrfs -r` +# packs its chunks to the content, so a tight image has nothing to shrink into; +# rebuild it at the tight size plus 64 MiB so the room is inside the filesystem +# the runtime declared it needs. The partition stays exactly the image size. +if [ "{var_luks_room}" = "1" ]; then + VAR_TIGHT_SIZE=$(stat -c%s "$VAR_IMAGE") + mkfs.btrfs -r "$VAR_DIR" \ +{mkfs_flags} \ +{global_compress_flag} -b $(( VAR_TIGHT_SIZE + 67108864 )) -f "$VAR_IMAGE" +fi + kill $_PROGRESS_PID 2>/dev/null; wait $_PROGRESS_PID 2>/dev/null || true {post_creation_section} FINAL_SIZE=$(stat -c%s "$VAR_IMAGE" 2>/dev/null || echo 0) FINAL_MB=$(( FINAL_SIZE / 1048576 )) -# The var partition is sized from the image: stone adds the headroom a later -# in-place LUKS2 conversion needs (see stone's resolve_partition_size_bytes), -# so the same policy applies here and to `avocado provision`. -VAR_PART_BYTES=$FINAL_SIZE echo "" echo "Built var image: ${{FINAL_MB}}MB" @@ -2744,7 +2753,7 @@ stone bundle \ -m "$STONE_MANIFEST" \ $STONE_INCLUDE_FLAGS \ $STONE_OVERLAY_FLAG \ - --partition-size "var=$VAR_PART_BYTES" \ + --partition-size "var=$FINAL_SIZE" \ -o "$STONE_AOS_OUTPUT" \ --build-dir "$STONE_BUILD_DIR" @@ -2847,6 +2856,7 @@ sign_amf "$AVOCADO_MANIFEST_PATH" update_authority_section = update_authority_section, docker_section = docker_section, device_tree_overlay_section = device_tree_overlay_section, + var_luks_room = if var_encrypt(target_arch) { "1" } else { "0" }, ); Ok(script) @@ -4150,6 +4160,52 @@ runtimes: assert!(!script.contains("$INITRAMFS_SYSROOT/etc/avocado/var-encrypt")); } + #[test] + fn var_encrypt_builds_the_var_image_with_room_to_shrink_for_the_luks_header() { + let temp_dir = TempDir::new().unwrap(); + let on = r#" +connect: + org: test + +runtimes: + test-runtime: + target: "x86_64" + var: + encrypt: true +"#; + let build = |content: &str| { + let config_path = create_test_config_file(&temp_dir, content); + let parsed: serde_yaml::Value = serde_yaml::from_str(content).unwrap(); + let cmd = RuntimeBuildCommand::new( + "test-runtime".to_string(), + config_path, + false, + Some("x86_64".to_string()), + None, + None, + ); + let config = Config::load(&cmd.config_path).unwrap(); + cmd.create_build_script(&config, &parsed, "x86_64", &[]) + .unwrap() + }; + let script = build(on); + assert!( + script.contains("if [ \"1\" = \"1\" ]; then"), + "second mkfs pass is armed" + ); + assert!(script.contains("-b $(( VAR_TIGHT_SIZE + 67108864 ))")); + assert!( + script.contains("--partition-size \"var=$FINAL_SIZE\""), + "partition stays the image size" + ); + + let script = build(&on.replace("encrypt: true", "encrypt: false")); + assert!( + script.contains("if [ \"0\" = \"1\" ]; then"), + "plaintext runtime keeps the tight image" + ); + } + /// `encrypt:` under a `target-:` override is honored like every other /// `var:` key; the package union reads the same merged source, so the /// marker never ships without cryptsetup-var behind it. From b97730bb65d311287c390ee5e13047b13619d202 Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Fri, 28 Aug 2026 07:31:55 -0400 Subject: [PATCH 7/9] fix(signing): refuse a non-certificate PEM for the key id, create the key file 0600 keyid_for_pem_cert now requires the CERTIFICATE label: a private key or any other PEM block also base64-decodes, and a key id derived from the wrong file only surfaces later as a FIT that does not verify. The private key file is opened with mode 0600 from the start instead of tightened after the write. --- src/utils/signing_keys.rs | 40 +++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/src/utils/signing_keys.rs b/src/utils/signing_keys.rs index 778a5332..519c68d8 100644 --- a/src/utils/signing_keys.rs +++ b/src/utils/signing_keys.rs @@ -647,11 +647,18 @@ pub fn is_pem_algorithm(algorithm: &str) -> bool { /// same certificate imported twice gets the same id regardless of line /// wrapping or trailing whitespace. pub fn keyid_for_pem_cert(cert_pem: &str) -> Result { - let body: String = cert_pem - .lines() - .filter(|l| !l.starts_with("-----")) - .map(str::trim) - .collect(); + // Insist on the CERTIFICATE label: a private key or any other PEM block + // would also base64-decode, and a key id silently derived from the wrong + // file only surfaces later as a FIT that does not verify. + let mut lines = cert_pem.lines().map(str::trim).filter(|l| !l.is_empty()); + match lines.next() { + Some("-----BEGIN CERTIFICATE-----") => {} + Some(other) => anyhow::bail!( + "expected a PEM X.509 certificate (-----BEGIN CERTIFICATE-----), found {other:?}" + ), + None => anyhow::bail!("certificate file is empty"), + } + let body: String = lines.take_while(|l| !l.starts_with("-----END")).collect(); let der = BASE64_STANDARD .decode(body.as_bytes()) .context("certificate is not a PEM-encoded X.509 certificate")?; @@ -680,11 +687,26 @@ pub fn save_pem_keypair(keyid: &str, key_pem: &[u8], cert_pem: &[u8]) -> Result< let base_path = get_key_file_path(keyid)?; let key_path = base_path.with_extension("key"); let cert_path = base_path.with_extension("crt"); - fs::write(&key_path, key_pem) + // Create the key file 0600 from the start rather than tightening after the + // write: with a permissive umask the bytes would otherwise be readable in + // the window between the two. + let mut opts = fs::OpenOptions::new(); + opts.write(true).create(true).truncate(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + opts.mode(0o600); + } + opts.open(&key_path) + .and_then(|mut f| { + use std::io::Write; + f.write_all(key_pem) + }) .with_context(|| format!("Failed to write private key: {}", key_path.display()))?; #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; + // An existing file keeps its old mode through OpenOptions; make sure. fs::set_permissions(&key_path, std::fs::Permissions::from_mode(0o600)) .with_context(|| format!("Failed to set permissions on {}", key_path.display()))?; } @@ -756,6 +778,12 @@ mod pem_tests { assert!( keyid_for_pem_cert("-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----").is_err() ); + // A PEM block that is not a certificate - the private key handed in by + // mistake - is refused by its label, not accepted because it decodes. + assert!(keyid_for_pem_cert( + "-----BEGIN PRIVATE KEY-----\nAAECAwQFBgc=\n-----END PRIVATE KEY-----\n" + ) + .is_err()); } #[test] From 9973f88e5d1bbcfec8d210f1a2d8606279d6afc9 Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Fri, 28 Aug 2026 07:52:54 -0400 Subject: [PATCH 8/9] ci: re-trigger CodeQL default-setup analysis The check still shows alert 65 (dismissed as a false positive: the logged value is the public certificate's hash); the dynamic CodeQL workflow cannot be re-run from the API, only a new analysis clears it. From 4c61a94b533ed90a07898f0ee9a7814f1468a268 Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Fri, 28 Aug 2026 08:41:19 -0400 Subject: [PATCH 9/9] ci: re-trigger CodeQL analysis after dismissing alert 66 Same false positive as alert 65 on the create --algorithm rsa* path: the logged value is the public certificate's hash. The default-setup CodeQL workflow only re-evaluates on a new commit.