diff --git a/docs/design/diagrams/build-sequence.svg b/docs/design/diagrams/build-sequence.svg index 62edabb8c..2453b1833 100644 --- a/docs/design/diagrams/build-sequence.svg +++ b/docs/design/diagrams/build-sequence.svg @@ -1 +1 @@ -Twoliter ContainerSDKTwoliter HostTwoliter HostTwoliter ContainerTwoliter ContainerCargoCargoBuildsysBuildsysSDK ContainerSDK Container1twoliter build variant2cargo make variant3cargo build variantcargo build kit(s)cargo build package(s)buildsys build-package4rpm build package.spec5package.rpm6buildsys build-kit7rpm build kit.spec8kit.rpm9kit.rpm10buildsys aggregate-kits11aggregated kits docker image12buildsys build-variant13rpm2img14bottlerocket.img15bottlerocket.img \ No newline at end of file +Twoliter ContainerSDKTwoliter HostTwoliter ContainerCargoBuildsysSDK ContainerTwoliter HostTwoliter HostTwoliter ContainerTwoliter ContainerCargoCargoBuildsysBuildsysSDK ContainerSDK Container1twoliter build variant2cargo make variant3cargo build variantcargo build kit(s)cargo build package(s)buildsys build-package4rpm build package.spec5package.rpm6buildsys build-kit7rpm build kit.spec8kit.rpm9kit.rpm10buildsys aggregate-kits11aggregated kits docker image12buildsys build-variant13rpm2img14bottlerocket.img15bottlerocket.img \ No newline at end of file diff --git a/twoliter/src/cmd/build.rs b/twoliter/src/cmd/build.rs index bab236b26..afcda82ad 100644 --- a/twoliter/src/cmd/build.rs +++ b/twoliter/src/cmd/build.rs @@ -6,6 +6,7 @@ use crate::project::{self, Locked}; use crate::tools::install_tools; use anyhow::{Context, Result}; use clap::Parser; +use oci_cli_wrapper::ImageTool; use std::path::PathBuf; use tempfile::TempDir; @@ -67,7 +68,10 @@ impl BuildKit { optional_envs.push(("BUILDSYS_LOOKASIDE_CACHE", lookaside_cache)) } - CargoMake::new(&project.sdk_image().project_image_uri().to_string())? + let image_tool = ImageTool::from_builtin_krane(); + let sdk_uri = project.sdk_image_uri(&image_tool).await?; + + CargoMake::new(&sdk_uri)? .env("TWOLITER_TOOLS_DIR", toolsdir.display().to_string()) .env("BUILDSYS_ARCH", &self.arch) .env("BUILDSYS_KIT", &self.kit) @@ -142,7 +146,10 @@ impl BuildVariant { )) } - CargoMake::new(&project.sdk_image().project_image_uri().to_string())? + let image_tool = ImageTool::from_builtin_krane(); + let sdk_uri = project.sdk_image_uri(&image_tool).await?; + + CargoMake::new(&sdk_uri)? .env("TWOLITER_TOOLS_DIR", toolsdir.display().to_string()) .env("BUILDSYS_ARCH", &self.arch) .env("BUILDSYS_VARIANT", &self.variant) diff --git a/twoliter/src/cmd/build_clean.rs b/twoliter/src/cmd/build_clean.rs index 4707b62bf..c4f1ce92f 100644 --- a/twoliter/src/cmd/build_clean.rs +++ b/twoliter/src/cmd/build_clean.rs @@ -20,7 +20,10 @@ impl BuildClean { tools::install_tools(&toolsdir).await?; let makefile_path = toolsdir.join("Makefile.toml"); - CargoMake::new(&project.sdk_image().project_image_uri().to_string())? + // `clean` does not run anything inside the SDK, so skip the registry lookup. + let sdk_uri = project.sdk_image().project_image_uri().to_string(); + + CargoMake::new(&sdk_uri)? .env("TWOLITER_TOOLS_DIR", toolsdir.display().to_string()) .makefile(makefile_path) .project_dir(project.project_dir()) diff --git a/twoliter/src/cmd/make.rs b/twoliter/src/cmd/make.rs index 58689acd0..b424496c0 100644 --- a/twoliter/src/cmd/make.rs +++ b/twoliter/src/cmd/make.rs @@ -4,6 +4,7 @@ use crate::project::{self, Locked, SDKLocked, Unlocked}; use crate::tools::install_tools; use anyhow::Result; use clap::Parser; +use oci_cli_wrapper::ImageTool; use std::path::PathBuf; // Most subcommands do not require kits and thus do not need to resolve and verify them against the @@ -79,19 +80,22 @@ impl Make { target_allows_kit_verification_skip && project_has_explicit_sdk_dep } - /// Returns the locked SDK image for the project. + /// Returns the digest-pinned SDK image URI for the project. /// /// Fetches kits if needed. async fn lock_and_fetch(&self, project: &project::Project) -> Result { - Ok(if self.can_skip_kit_verification(project) { - project.load_lock::().await?.sdk_image() + let image_tool = ImageTool::from_builtin_krane(); + if self.can_skip_kit_verification(project) { + project + .load_lock::() + .await? + .sdk_image_uri(&image_tool) + .await } else { let project = project.load_lock::().await?; project.fetch(self.arch.as_str()).await?; - project.sdk_image() + project.sdk_image_uri(&image_tool).await } - .project_image_uri() - .to_string()) } } @@ -221,7 +225,8 @@ mod test { .await .unwrap(); let project = project.load_lock::().await.unwrap(); - let sdk_source = project.sdk_image().project_image_uri().to_string(); + let image_tool = ImageTool::from_builtin_krane(); + let sdk_source = project.sdk_image_uri(&image_tool).await.unwrap(); if delete_verifier_tags { // Clean up tags so that the build fails diff --git a/twoliter/src/cmd/publish_kit.rs b/twoliter/src/cmd/publish_kit.rs index a009f5557..5ef0ffc91 100644 --- a/twoliter/src/cmd/publish_kit.rs +++ b/twoliter/src/cmd/publish_kit.rs @@ -3,6 +3,7 @@ use crate::project::{self, Locked}; use crate::tools::install_tools; use anyhow::Result; use clap::Parser; +use oci_cli_wrapper::ImageTool; use std::path::PathBuf; /// Group all publish commands @@ -48,7 +49,10 @@ impl PublishKit { Some(kit_repo) => kit_repo, None => &self.kit_name, }; - CargoMake::new(project.sdk_image().project_image_uri().to_string().as_str())? + let image_tool = ImageTool::from_builtin_krane(); + let sdk_uri = project.sdk_image_uri(&image_tool).await?; + + CargoMake::new(&sdk_uri)? .env("TWOLITER_TOOLS_DIR", toolsdir.display().to_string()) .env("BUILDSYS_KIT", &self.kit_name) .env("BUILDSYS_VERSION_IMAGE", project.release_version()) diff --git a/twoliter/src/project/lock/image.rs b/twoliter/src/project/lock/image.rs index 86becf8c7..e2054d7a1 100644 --- a/twoliter/src/project/lock/image.rs +++ b/twoliter/src/project/lock/image.rs @@ -2,8 +2,9 @@ use super::archive::OCIArchive; use super::views::ManifestListView; use crate::common::fs::create_dir_all; use crate::compatibility::SUPPORTED_KIT_METADATA_VERSION; +use crate::docker::ImageUri; use crate::project::{Image, ProjectImage, ValidIdentifier, VendedArtifact}; -use anyhow::{bail, Context, Result}; +use anyhow::{bail, ensure, Context, Result}; use base64::Engine; use futures::{pin_mut, stream, StreamExt, TryStreamExt}; use log::trace; @@ -192,6 +193,8 @@ impl Debug for EncodedKitMetadata { pub struct ImageResolver { image: ProjectImage, skip_metadata_retrieval: bool, + /// Memoized manifest list, fetched at most once per resolver. + manifest_cache: tokio::sync::OnceCell<(ManifestListView, Vec)>, } impl ImageResolver { @@ -199,6 +202,7 @@ impl ImageResolver { Ok(Self { image: image.clone(), skip_metadata_retrieval: false, + manifest_cache: tokio::sync::OnceCell::new(), }) } @@ -210,21 +214,22 @@ impl ImageResolver { self } + /// Encodes a manifest's SHA-256 as the base64 form stored in `Twoliter.lock`. + fn manifest_digest_b64(manifest_bytes: &[u8]) -> String { + let raw = sha2::Sha256::digest(manifest_bytes); + base64::engine::general_purpose::STANDARD.encode(raw) + } + + /// Calculates the digest of the locked image by hashing the cached manifest bytes. #[instrument( level = "trace", fields(image = %self.image, uri = %self.image.project_image_uri()) )] - /// Calculate the digest of the locked image async fn calculate_digest(&self, image_tool: &ImageTool) -> Result { let image_uri = self.image.project_image_uri(); - let image_uri_str = image_uri.to_string(); - let manifest_bytes = image_tool.get_manifest(image_uri_str.as_str()).await?; - let digest = sha2::Sha256::digest(manifest_bytes.as_slice()); - let digest = base64::engine::general_purpose::STANDARD.encode(digest); - debug!( - "Calculated digest for locked image '{}': '{}'", - image_uri, digest, - ); + let (_list, manifest_bytes) = self.get_manifest_with_bytes(image_tool).await?; + let digest = Self::manifest_digest_b64(manifest_bytes.as_slice()); + debug!("Calculated digest for locked image '{image_uri}': '{digest}'"); Ok(digest) } @@ -233,11 +238,35 @@ impl ImageResolver { fields(image = %self.image, uri = %self.image.project_image_uri()) )] async fn get_manifest(&self, image_tool: &ImageTool) -> Result { - let uri = self.image.project_image_uri().to_string(); - debug!(image=%self.image, uri, "Fetching image manifest."); - let manifest_bytes = image_tool.get_manifest(uri.as_str()).await?; - serde_json::from_slice(manifest_bytes.as_slice()) - .context("failed to deserialize manifest list") + let (list, _bytes) = self.get_manifest_with_bytes(image_tool).await?; + Ok(list.clone()) + } + + /// Fetches the manifest list once per resolver and returns cached bytes + parse. + /// + /// Callers that need to verify the bytes against a lockfile-recorded digest (or + /// re-parse without a second HTTP round-trip) reuse this. The result is memoized in + /// `self.manifest_cache`, so the SDK manifest is fetched at most once per command + /// even though `resolve`, `calculate_digest`, `resolve_arch_digest`, and `extract` + /// each need it. + #[instrument( + level = "trace", + fields(image = %self.image, uri = %self.image.project_image_uri()) + )] + async fn get_manifest_with_bytes( + &self, + image_tool: &ImageTool, + ) -> Result<&(ManifestListView, Vec)> { + self.manifest_cache + .get_or_try_init(|| async { + let uri = self.image.project_image_uri().to_string(); + debug!(image=%self.image, uri, "Fetching image manifest."); + let manifest_bytes = image_tool.get_manifest(uri.as_str()).await?; + let list: ManifestListView = serde_json::from_slice(manifest_bytes.as_slice()) + .context("failed to deserialize manifest list")?; + anyhow::Ok((list, manifest_bytes)) + }) + .await } #[instrument( @@ -305,11 +334,73 @@ impl ImageResolver { Ok((locked_image, Some(metadata))) } + /// Returns the per-arch image manifest digest (`sha256:`) after verifying the + /// fetched manifest-list bytes against `expected_lock_digest` from `Twoliter.lock`. + #[instrument( + level = "trace", + fields(uri = %self.image.project_image_uri(), arch) + )] + pub(crate) async fn resolve_arch_digest( + &self, + image_tool: &ImageTool, + arch: &str, + expected_lock_digest: &str, + ) -> Result { + let uri = self.image.project_image_uri(); + let (manifest_list, manifest_bytes) = self.get_manifest_with_bytes(image_tool).await?; + + let computed = Self::manifest_digest_b64(manifest_bytes.as_slice()); + if computed != expected_lock_digest { + error!( + %uri, + expected = %expected_lock_digest, + actual = %computed, + "Manifest list digest does not match Twoliter.lock" + ); + bail!( + "manifest list digest for {uri} does not match Twoliter.lock \ + (expected '{expected_lock_digest}', got '{computed}'); \ + the registry served different bytes than were authorized in the lockfile — \ + refusing to build a pinned URI from unauthorized content" + ); + } + + let docker_arch = DockerArchitecture::try_from(arch)?; + let manifest = manifest_list + .manifests + .iter() + .find(|m| { + m.platform + .as_ref() + .map(|p| p.architecture == docker_arch) + .unwrap_or(false) + }) + .cloned() + .with_context(|| { + format!("could not find image for architecture '{docker_arch}' at {uri}") + })?; + + validate_oci_digest(&manifest.digest).with_context(|| { + format!( + "manifest for arch '{docker_arch}' at {uri} has malformed digest '{}'", + manifest.digest + ) + })?; + + Ok(manifest.digest) + } + #[instrument( level = "trace", fields(uri = %self.image.project_image_uri(), path = %path.as_ref().display()) )] - pub(crate) async fn extract

(&self, image_tool: &ImageTool, path: P, arch: &str) -> Result<()> + pub(crate) async fn extract

( + &self, + image_tool: &ImageTool, + path: P, + arch: &str, + expected_lock_digest: &str, + ) -> Result<()> where P: AsRef, { @@ -327,24 +418,16 @@ impl ImageResolver { create_dir_all(&target_path).await?; create_dir_all(&cache_path).await?; - // First get the manifest for the specific requested architecture let uri = self.image.project_image_uri(); - let manifest_list = self.get_manifest(image_tool).await?; - let docker_arch = DockerArchitecture::try_from(arch)?; - let manifest = manifest_list - .manifests - .iter() - .find(|x| x.platform.as_ref().unwrap().architecture == docker_arch) - .cloned() - .context(format!( - "could not find image for architecture '{docker_arch}' at {uri}" - ))?; + let arch_digest = self + .resolve_arch_digest(image_tool, arch, expected_lock_digest) + .await?; let registry = uri.registry.context("failed to resolve image registry")?; let oci_archive = OCIArchive::new( registry.as_str(), uri.repo.as_str(), - manifest.digest.as_str(), + arch_digest.as_str(), &cache_path, )?; @@ -359,6 +442,53 @@ impl ImageResolver { } } +/// Builds a `registry/repo@sha256:` reference pinned to the per-arch image digest, +/// after verifying the manifest list against `expected_lock_digest` from `Twoliter.lock`. +pub(crate) async fn build_pinned_uri( + image: &ProjectImage, + image_tool: &ImageTool, + arch: &str, + expected_lock_digest: &str, +) -> Result { + let uri = image.project_image_uri(); + let base = uri_without_tag(&uri)?; + let arch_digest = ImageResolver::from_image(image)? + .resolve_arch_digest(image_tool, arch, expected_lock_digest) + .await?; + Ok(format!("{base}@{arch_digest}")) +} + +fn uri_without_tag(uri: &ImageUri) -> Result { + let registry = uri.registry.as_ref().with_context(|| { + format!( + "cannot build a digest-pinned reference for '{}': no registry recorded — \ + refusing to fall back to Docker Hub", + uri.repo + ) + })?; + Ok(format!("{}/{}", registry, uri.repo)) +} + +/// Enforces canonical OCI digest form: `sha256:` + 64 lowercase-hex characters. +pub(crate) fn validate_oci_digest(digest: &str) -> Result<()> { + const PREFIX: &str = "sha256:"; + const HEX_LEN: usize = 64; + + let hex = digest + .strip_prefix(PREFIX) + .with_context(|| format!("digest must start with '{PREFIX}', got '{digest}'"))?; + ensure!( + hex.len() == HEX_LEN, + "digest hex portion must be {HEX_LEN} characters, got {} ('{digest}')", + hex.len() + ); + ensure!( + hex.bytes().all(|b| matches!(b, b'0'..=b'9' | b'a'..=b'f')), + "digest hex portion must be lowercase hexadecimal ('{digest}')" + ); + Ok(()) +} + #[cfg(test)] mod test { use super::*; @@ -448,4 +578,118 @@ mod test { "bar".to_string() ); } + + fn hex64(byte: u8) -> String { + std::iter::repeat_n(char::from(byte), 64).collect() + } + + #[test] + fn validate_oci_digest_accepts_canonical_form() { + validate_oci_digest(&format!("sha256:{}", hex64(b'a'))).expect("all-a hex"); + validate_oci_digest(&format!("sha256:{}", hex64(b'0'))).expect("all-0 hex"); + validate_oci_digest(&format!( + "sha256:{}", + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + )) + .expect("mixed hex"); + } + + #[test] + fn validate_oci_digest_rejects_empty() { + let err = validate_oci_digest("").unwrap_err().to_string(); + assert!(err.contains("must start with 'sha256:'"), "got: {err}"); + } + + #[test] + fn validate_oci_digest_rejects_missing_prefix() { + let err = validate_oci_digest(&hex64(b'a')).unwrap_err().to_string(); + assert!(err.contains("must start with 'sha256:'"), "got: {err}"); + } + + #[test] + fn validate_oci_digest_rejects_wrong_algorithm() { + let err = validate_oci_digest(&format!("sha512:{}", hex64(b'a'))) + .unwrap_err() + .to_string(); + assert!(err.contains("must start with 'sha256:'"), "got: {err}"); + } + + #[test] + fn validate_oci_digest_rejects_short_hex() { + let err = validate_oci_digest("sha256:abc").unwrap_err().to_string(); + assert!(err.contains("64 characters"), "got: {err}"); + } + + #[test] + fn validate_oci_digest_rejects_long_hex() { + let err = validate_oci_digest(&format!("sha256:{}a", hex64(b'a'))) + .unwrap_err() + .to_string(); + assert!(err.contains("64 characters"), "got: {err}"); + } + + #[test] + fn validate_oci_digest_rejects_uppercase_hex() { + let err = validate_oci_digest(&format!("sha256:{}", hex64(b'A'))) + .unwrap_err() + .to_string(); + assert!(err.contains("lowercase"), "got: {err}"); + } + + #[test] + fn validate_oci_digest_rejects_non_hex() { + let err = validate_oci_digest(&format!("sha256:{}", hex64(b'z'))) + .unwrap_err() + .to_string(); + assert!(err.contains("lowercase"), "got: {err}"); + } + + #[test] + fn validate_oci_digest_rejects_shell_metacharacters() { + // The critical property: a digest that would inject arguments into a `docker` or + // `krane` command line if concatenated unquoted must not slip through. + for injection in [ + "sha256:aaa' ; docker run --privileged evil #", + "sha256:aaa aaa", + "sha256:$(pwn)aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "sha256:\naaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + ] { + assert!( + validate_oci_digest(injection).is_err(), + "digest with injection payload should be rejected: {injection:?}" + ); + } + } + + #[test] + fn uri_without_tag_requires_registry() { + let with_registry = ImageUri { + registry: Some("example.com".to_string()), + repo: "org/repo".to_string(), + tag: "v1.0.0".to_string(), + }; + assert_eq!( + uri_without_tag(&with_registry).unwrap(), + "example.com/org/repo" + ); + + let without_registry = ImageUri { + registry: None, + repo: "org/repo".to_string(), + tag: "v1.0.0".to_string(), + }; + let err = uri_without_tag(&without_registry).unwrap_err().to_string(); + assert!(err.contains("no registry recorded"), "got: {err}"); + assert!(err.contains("Docker Hub"), "got: {err}"); + } + + #[test] + fn manifest_digest_b64_matches_known_vector() { + // sha256("") = e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + // base64(sha256("")) = 47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU= + assert_eq!( + ImageResolver::manifest_digest_b64(b""), + "47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=" + ); + } } diff --git a/twoliter/src/project/lock/mod.rs b/twoliter/src/project/lock/mod.rs index 01b83051f..fd70e3ad4 100644 --- a/twoliter/src/project/lock/mod.rs +++ b/twoliter/src/project/lock/mod.rs @@ -12,6 +12,7 @@ mod verification; /// Implements view models of common OCI manifest and configuration types mod views; +pub(crate) use self::image::{build_pinned_uri, LockedImage}; pub(crate) use self::verification::VerificationTagger; use crate::common::fs::{create_dir_all, read, write}; @@ -20,7 +21,7 @@ use crate::project::{Project, ValidIdentifier}; use crate::schema_version::SchemaVersion; use anyhow::{bail, ensure, Context, Result}; use futures::{stream, StreamExt, TryStreamExt}; -use image::{ImageResolver, LockedImage}; +use image::ImageResolver; use oci_cli_wrapper::ImageTool; use olpc_cjson::CanonicalFormatter as CanonicalJsonFormatter; use semver::Version; @@ -264,7 +265,7 @@ impl Lock { let image = project.as_project_image(kit)?; let resolver = ImageResolver::from_image(&image)?; resolver - .extract(&image_tool, &project.external_kits_dir(), arch) + .extract(&image_tool, &project.external_kits_dir(), arch, &kit.digest) .await?; Ok(()) }) diff --git a/twoliter/src/project/lock/views.rs b/twoliter/src/project/lock/views.rs index e1622698d..91267440a 100644 --- a/twoliter/src/project/lock/views.rs +++ b/twoliter/src/project/lock/views.rs @@ -3,7 +3,7 @@ use serde::de::Error; use serde::{Deserialize, Deserializer}; use std::fmt::{Display, Formatter}; -#[derive(Deserialize, Debug)] +#[derive(Deserialize, Debug, Clone)] pub(crate) struct ManifestListView { pub manifests: Vec, } diff --git a/twoliter/src/project/mod.rs b/twoliter/src/project/mod.rs index 8ae0cf27b..af62a3a81 100644 --- a/twoliter/src/project/mod.rs +++ b/twoliter/src/project/mod.rs @@ -6,7 +6,7 @@ pub(crate) use self::vendor::ArtifactVendor; pub(crate) use lock::VerificationTagger; use path_absolutize::Absolutize; -use self::lock::{Lock, LockedSDK, Override}; +use self::lock::{build_pinned_uri, Lock, LockedSDK, Override}; use self::migrate::parser::UnvalidatedProject; use crate::common::fs::{self, read_to_string}; use crate::compatibility::LATEST_TWOLITER_PROJECT_SCHEMA_VERSION; @@ -279,12 +279,38 @@ impl Project { } } +/// Returns the SDK image URI pinned to the host-arch image manifest digest. +/// +/// The SDK runs on the host, so we always pin to the host arch, not `--arch`. +async fn sdk_image_uri_for( + sdk_image: &ProjectImage, + image_tool: &oci_cli_wrapper::ImageTool, + expected_lock_digest: &str, +) -> Result { + build_pinned_uri( + sdk_image, + image_tool, + std::env::consts::ARCH, + expected_lock_digest, + ) + .await +} + impl Project { pub(crate) fn sdk_image(&self) -> ProjectImage { let SDKLocked(lock) = &self.lock; self.as_project_image(&lock.0) .expect("Could not find SDK vendor despite lock resolution succeeding?") } + + /// See [`sdk_image_uri_for`]. + pub(crate) async fn sdk_image_uri( + &self, + image_tool: &oci_cli_wrapper::ImageTool, + ) -> Result { + let SDKLocked(locked) = &self.lock; + sdk_image_uri_for(&self.sdk_image(), image_tool, &locked.0.digest).await + } } impl Project { @@ -309,6 +335,15 @@ impl Project { self.as_project_image(&lock.sdk) .expect("Could not find SDK vendor despite lock resolution succeeding?") } + + /// See [`sdk_image_uri_for`]. + pub(crate) async fn sdk_image_uri( + &self, + image_tool: &oci_cli_wrapper::ImageTool, + ) -> Result { + let Locked(lock) = &self.lock; + sdk_image_uri_for(&self.sdk_image(), image_tool, &lock.sdk.digest).await + } } #[derive(Debug, Clone, Eq, PartialEq, Hash)]