From 7246d9806d359c4adc43d68793d5bc5b38e23e65 Mon Sep 17 00:00:00 2001 From: Flavio Castelli Date: Tue, 9 Apr 2024 18:50:29 +0200 Subject: [PATCH 1/3] fix: allow `ManualTrustRoot` to hold multiple Rekor keys The `sigstore::trust::sigstore::SigstoreTrustRoot` can hold multiple Rekor keys. The possibility to have multiple keys is also reflected by the [`sigstore::trust::TrustRoot.rekor_keys`](https://docs.rs/sigstore/0.9.0/sigstore/trust/trait.TrustRoot.html#tymethod.rekor_keys) trait. This commit allows the `ManualTrustRoot` object to hold multiple Rekor keys. Signed-off-by: Flavio Castelli --- examples/cosign/verify/main.rs | 6 ++---- src/lib.rs | 2 +- src/trust/mod.rs | 6 +++--- src/trust/sigstore/mod.rs | 13 +++++++++++-- 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/examples/cosign/verify/main.rs b/examples/cosign/verify/main.rs index 24980d1b04..8978c827dc 100644 --- a/examples/cosign/verify/main.rs +++ b/examples/cosign/verify/main.rs @@ -237,10 +237,8 @@ async fn fulcio_and_rekor_data(cli: &Cli) -> anyhow::Result { pub fulcio_certs: Option>>, - pub rekor_key: Option>, + pub rekor_keys: Option>>, } #[cfg(not(target_arch = "wasm32"))] @@ -47,8 +47,8 @@ impl TrustRoot for ManualTrustRoot<'_> { } async fn rekor_keys(&self) -> crate::errors::Result> { - Ok(match &self.rekor_key { - Some(key) => vec![&key[..]], + Ok(match &self.rekor_keys { + Some(keys) => keys.iter().map(|k| k.as_slice()).collect(), None => Vec::new(), }) } diff --git a/src/trust/sigstore/mod.rs b/src/trust/sigstore/mod.rs index b843b9b783..509a57279c 100644 --- a/src/trust/sigstore/mod.rs +++ b/src/trust/sigstore/mod.rs @@ -312,15 +312,24 @@ fn is_local_file_outdated( #[cfg(test)] mod tests { - use crate::trust::sigstore::SigstoreTrustRoot; + use super::*; #[tokio::test] async fn prefetch() { - let _repo = SigstoreTrustRoot::new(None) + let repo = SigstoreTrustRoot::new(None) .await .expect("initialize SigstoreRepository") .prefetch() .await .expect("prefetch"); + + let fulcio_certs = repo + .fulcio_certs() + .await + .expect("cannot fetch Fulcio certs"); + assert!(!fulcio_certs.is_empty()); + + let rekor_keys = repo.rekor_keys().await.expect("cannot fetch Rekor keys"); + assert!(!rekor_keys.is_empty()); } } From f871cb8f1309a486ebd8a66ce9b93babb50dac51 Mon Sep 17 00:00:00 2001 From: Flavio Castelli Date: Tue, 9 Apr 2024 18:45:14 +0200 Subject: [PATCH 2/3] feat: allow to create static instances of some objects Allow to create `static` instances of the `cosign::Client`, `sigstore::trust::ManualTrustRoot`. The ability to create `static` instances of these objects can simplify the downstream consumers of this library. The approach taken to create these `static` instance is the same used by [`rustls_pki_types::CertificateDer::into_owned`](https://docs.rs/rustls-pki-types/1.4.1/rustls_pki_types/struct.CertificateDer.html#method.into_owned) Signed-off-by: Flavio Castelli --- Cargo.toml | 1 + src/cosign/client.rs | 35 +++++++++++++++++++----------- src/crypto/certificate_pool.rs | 14 ++++++++++++ src/mock_client.rs | 20 +++++++++-------- src/registry/mod.rs | 7 ++++-- src/registry/oci_caching_client.rs | 1 + src/registry/oci_client.rs | 1 + 7 files changed, 55 insertions(+), 24 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 007981d74b..834be7126b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -77,6 +77,7 @@ cfg-if = "1.0.0" chrono = { version = "0.4.27", default-features = false, features = ["serde"] } const-oid = "0.9.1" digest = { version = "0.10.3", default-features = false } +dyn-clone = "1.0" ecdsa = { version = "0.16.7", features = ["pkcs8", "digest", "der", "signing"] } ed25519 = { version = "2.2.1", features = ["alloc"] } ed25519-dalek = { version = "2.0.0-rc.2", features = ["pkcs8", "rand_core"] } diff --git a/src/cosign/client.rs b/src/cosign/client.rs index af70bf457b..3b28a55b81 100644 --- a/src/cosign/client.rs +++ b/src/cosign/client.rs @@ -12,24 +12,21 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. - -use std::collections::HashMap; -use std::ops::Add; - use async_trait::async_trait; use oci_distribution::manifest::OCI_IMAGE_MEDIA_TYPE; -use tracing::warn; +use std::{collections::HashMap, ops::Add}; +use tracing::{debug, warn}; -use super::constants::{SIGSTORE_OCI_MEDIA_TYPE, SIGSTORE_SIGNATURE_ANNOTATION}; -use super::{CosignCapabilities, SignatureLayer}; -use crate::cosign::signature_layers::build_signature_layers; -use crate::crypto::CosignVerificationKey; -use crate::registry::{Auth, OciReference, PushResponse}; use crate::{ - crypto::certificate_pool::CertificatePool, + cosign::{ + constants::{SIGSTORE_OCI_MEDIA_TYPE, SIGSTORE_SIGNATURE_ANNOTATION}, + signature_layers::build_signature_layers, + CosignCapabilities, SignatureLayer, + }, + crypto::{certificate_pool::CertificatePool, CosignVerificationKey}, errors::{Result, SigstoreError}, + registry::{Auth, OciReference, PushResponse}, }; -use tracing::debug; /// Used to generate an empty [OCI Configuration](https://github.com/opencontainers/image-spec/blob/v1.0.0/config.md). pub const CONFIG_DATA: &str = "{}"; @@ -43,6 +40,17 @@ pub struct Client<'a> { pub(crate) fulcio_cert_pool: Option>, } +impl Client<'_> { + /// Yield a `'static` lifetime of the `Client` + pub fn to_owned(&self) -> Client<'static> { + Client { + registry_client: self.registry_client.to_owned(), + rekor_pub_key: self.rekor_pub_key.as_ref().map(|k| k.to_owned()), + fulcio_cert_pool: self.fulcio_cert_pool.as_ref().map(|cp| cp.to_owned()), + } + } +} + #[cfg_attr(not(target_arch = "wasm32"), async_trait)] #[cfg_attr(target_arch = "wasm32", async_trait(?Send))] impl CosignCapabilities for Client<'_> { @@ -176,6 +184,7 @@ mod tests { use crate::cosign::tests::{get_fulcio_cert_pool, REKOR_PUB_KEY}; use crate::crypto::SigningScheme; use crate::mock_client::test::MockOciClient; + use std::sync::Arc; fn build_test_client(mock_client: MockOciClient) -> Client<'static> { let rekor_pub_key = @@ -196,7 +205,7 @@ mod tests { String::from("sha256:f3cfc9d0dbf931d3db4685ec659b7ac68e2a578219da4aae65427886e649b06b"); let expected_image = "docker.io/library/busybox:sha256-f3cfc9d0dbf931d3db4685ec659b7ac68e2a578219da4aae65427886e649b06b.sig".parse().unwrap(); let mock_client = MockOciClient { - fetch_manifest_digest_response: Some(Ok(image_digest.clone())), + fetch_manifest_digest_response: Some(Arc::new(Ok(image_digest.clone()))), pull_response: None, pull_manifest_response: None, push_response: None, diff --git a/src/crypto/certificate_pool.rs b/src/crypto/certificate_pool.rs index 731a68c6c3..e651c7ce5f 100644 --- a/src/crypto/certificate_pool.rs +++ b/src/crypto/certificate_pool.rs @@ -28,6 +28,20 @@ pub(crate) struct CertificatePool<'a> { intermediates: Vec>, } +impl CertificatePool<'_> { + /// Yield a `'static` lifetime of the `CertificatePool` + pub(crate) fn to_owned(&self) -> CertificatePool<'static> { + CertificatePool { + trusted_roots: self.trusted_roots.iter().map(|ta| ta.to_owned()).collect(), + intermediates: self + .intermediates + .iter() + .map(|c| c.as_ref().to_owned().into()) + .collect(), + } + } +} + impl<'a> CertificatePool<'a> { /// Builds a `CertificatePool` instance using the provided list of [`Certificate`]. pub(crate) fn from_certificates( diff --git a/src/mock_client.rs b/src/mock_client.rs index 25c00d0943..adf76bb312 100644 --- a/src/mock_client.rs +++ b/src/mock_client.rs @@ -24,13 +24,15 @@ pub(crate) mod test { secrets::RegistryAuth, Reference, }; + use std::sync::Arc; - #[derive(Default)] + #[derive(Default, Clone)] pub struct MockOciClient { - pub fetch_manifest_digest_response: Option>, - pub pull_response: Option>, - pub pull_manifest_response: Option>, - pub push_response: Option>, + // Note: all the `Result` objects have to be wrapped inside of an `Arc` to be able to clone them + pub fetch_manifest_digest_response: Option>>, + pub pull_response: Option>>, + pub pull_manifest_response: Option>>, + pub push_response: Option>>, } impl crate::registry::ClientCapabilitiesDeps for MockOciClient {} @@ -51,7 +53,7 @@ pub(crate) mod test { error: String::from("No fetch_manifest_digest_response provided!"), })?; - match mock_response { + match mock_response.as_ref() { Ok(r) => Ok(r.clone()), Err(e) => Err(SigstoreError::RegistryFetchManifestError { image: image.whole(), @@ -74,7 +76,7 @@ pub(crate) mod test { error: String::from("No pull_response provided!"), })?; - match mock_response { + match mock_response.as_ref() { Ok(r) => Ok(r.clone()), Err(e) => Err(SigstoreError::RegistryPullError { image: image.whole(), @@ -95,7 +97,7 @@ pub(crate) mod test { } })?; - match mock_response { + match mock_response.as_ref() { Ok(r) => Ok(r.clone()), Err(e) => Err(SigstoreError::RegistryPullError { image: image.whole(), @@ -120,7 +122,7 @@ pub(crate) mod test { error: String::from("No push_response provided!"), })?; - match mock_response { + match mock_response.as_ref() { Ok(r) => Ok(PushResponse { config_url: r.config_url.clone(), manifest_url: r.manifest_url.clone(), diff --git a/src/registry/mod.rs b/src/registry/mod.rs index 1e946f4432..efbab8a2b7 100644 --- a/src/registry/mod.rs +++ b/src/registry/mod.rs @@ -34,6 +34,7 @@ pub(crate) use oci_caching_client::*; use crate::errors::Result; use async_trait::async_trait; +use dyn_clone::DynClone; /// Workaround to ensure the `Send + Sync` supertraits are /// required by ClientCapabilities only when the target @@ -43,7 +44,7 @@ use async_trait::async_trait; /// to define ClientCapabilities twice (one with `#[cfg(target_arch = "wasm32")]`, /// the other with `#[cfg(not(target_arch = "wasm32"))]` #[cfg(not(target_arch = "wasm32"))] -pub(crate) trait ClientCapabilitiesDeps: Send + Sync {} +pub(crate) trait ClientCapabilitiesDeps: Send + Sync + DynClone {} /// Workaround to ensure the `Send + Sync` supertraits are /// required by ClientCapabilities only when the target @@ -53,7 +54,7 @@ pub(crate) trait ClientCapabilitiesDeps: Send + Sync {} /// to define ClientCapabilities twice (one with `#[cfg(target_arch = "wasm32")]`, /// the other with `#[cfg(not(target_arch = "wasm32"))]` #[cfg(target_arch = "wasm32")] -pub(crate) trait ClientCapabilitiesDeps {} +pub(crate) trait ClientCapabilitiesDeps: DynClone {} #[cfg_attr(not(target_arch = "wasm32"), async_trait)] #[cfg_attr(target_arch = "wasm32", async_trait(?Send))] @@ -87,3 +88,5 @@ pub(crate) trait ClientCapabilities: ClientCapabilitiesDeps { manifest: Option, ) -> Result; } + +dyn_clone::clone_trait_object!(ClientCapabilities); diff --git a/src/registry/oci_caching_client.rs b/src/registry/oci_caching_client.rs index 035bd3ad93..11ec71fe28 100644 --- a/src/registry/oci_caching_client.rs +++ b/src/registry/oci_caching_client.rs @@ -29,6 +29,7 @@ use tracing::{debug, error}; /// /// For testing purposes, use instead the client inside of the /// `mock_client` module. +#[derive(Clone)] pub(crate) struct OciCachingClient { pub registry_client: oci_distribution::Client, } diff --git a/src/registry/oci_client.rs b/src/registry/oci_client.rs index c6159ee572..17593b2cbf 100644 --- a/src/registry/oci_client.rs +++ b/src/registry/oci_client.rs @@ -23,6 +23,7 @@ use async_trait::async_trait; /// /// For testing purposes, use instead the client inside of the /// `mock_client` module. +#[derive(Clone)] pub(crate) struct OciClient { pub registry_client: oci_distribution::Client, } From c6302b0a32806a37968003363b186a50277a65fc Mon Sep 17 00:00:00 2001 From: Flavio Castelli Date: Wed, 10 Apr 2024 16:40:44 +0200 Subject: [PATCH 3/3] refactor: remove async from `sigstore::trust::TrustRoot` trait Make the `sigstore::trust::TrustRoot` methods not async. Fixes https://github.com/sigstore/sigstore-rs/issues/346 Signed-off-by: Flavio Castelli --- examples/cosign/verify/main.rs | 7 ++-- src/cosign/client_builder.rs | 9 ++--- src/lib.rs | 1 - src/trust/mod.rs | 12 +++---- src/trust/sigstore/mod.rs | 63 ++++++++++------------------------ 5 files changed, 29 insertions(+), 63 deletions(-) diff --git a/examples/cosign/verify/main.rs b/examples/cosign/verify/main.rs index 8978c827dc..15fc8abd72 100644 --- a/examples/cosign/verify/main.rs +++ b/examples/cosign/verify/main.rs @@ -130,7 +130,7 @@ async fn run_app( let mut client_builder = sigstore::cosign::ClientBuilder::default().with_oci_client_config(oci_client_config); - client_builder = client_builder.with_trust_repository(frd).await?; + client_builder = client_builder.with_trust_repository(frd)?; let cert_chain: Option> = match cli.cert_chain.as_ref() { None => None, @@ -184,7 +184,7 @@ async fn run_app( } if let Some(path_to_cert) = cli.cert.as_ref() { let cert = fs::read(path_to_cert).map_err(|e| anyhow!("Cannot read cert: {:?}", e))?; - let require_rekor_bundle = if !frd.rekor_keys().await?.is_empty() { + let require_rekor_bundle = if !frd.rekor_keys()?.is_empty() { true } else { warn!("certificate based verification is weaker when Rekor integration is disabled"); @@ -229,8 +229,7 @@ async fn fulcio_and_rekor_data(cli: &Cli) -> anyhow::Result = - SigstoreTrustRoot::new(None).await?.prefetch().await; + let repo: sigstore::errors::Result = SigstoreTrustRoot::new(None).await; return Ok(Box::new(repo?)); }; diff --git a/src/cosign/client_builder.rs b/src/cosign/client_builder.rs index d37bca7cf9..2e7d494082 100644 --- a/src/cosign/client_builder.rs +++ b/src/cosign/client_builder.rs @@ -72,15 +72,12 @@ impl<'a> ClientBuilder<'a> { /// /// Enables Fulcio and Rekor integration with the given trust repository. /// See [crate::sigstore::TrustRoot] for more details on trust repositories. - pub async fn with_trust_repository( - mut self, - repo: &'a R, - ) -> Result { - let rekor_keys = repo.rekor_keys().await?; + pub fn with_trust_repository(mut self, repo: &'a R) -> Result { + let rekor_keys = repo.rekor_keys()?; if !rekor_keys.is_empty() { self.rekor_pub_key = Some(rekor_keys[0]); } - self.fulcio_certs = repo.fulcio_certs().await?; + self.fulcio_certs = repo.fulcio_certs()?; Ok(self) } diff --git a/src/lib.rs b/src/lib.rs index 279b782275..c407fa0b96 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -100,7 +100,6 @@ //! //! let mut client = sigstore::cosign::ClientBuilder::default() //! .with_trust_repository(&repo) -//! .await //! .expect("Cannot construct cosign client from given materials") //! .build() //! .expect("Unexpected failure while building Client"); diff --git a/src/trust/mod.rs b/src/trust/mod.rs index 9064844645..fa67f7ec72 100644 --- a/src/trust/mod.rs +++ b/src/trust/mod.rs @@ -13,18 +13,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -use async_trait::async_trait; use webpki::types::CertificateDer; #[cfg(feature = "sigstore-trust-root")] pub mod sigstore; /// A `TrustRoot` owns all key material necessary for establishing a root of trust. -#[cfg_attr(not(target_arch = "wasm32"), async_trait)] -#[cfg_attr(target_arch = "wasm32", async_trait(?Send))] pub trait TrustRoot: Send + Sync { - async fn fulcio_certs(&self) -> crate::errors::Result>; - async fn rekor_keys(&self) -> crate::errors::Result>; + fn fulcio_certs(&self) -> crate::errors::Result>; + fn rekor_keys(&self) -> crate::errors::Result>; } /// A `ManualTrustRoot` is a [TrustRoot] with out-of-band trust materials. @@ -36,17 +33,16 @@ pub struct ManualTrustRoot<'a> { } #[cfg(not(target_arch = "wasm32"))] -#[async_trait] impl TrustRoot for ManualTrustRoot<'_> { #[cfg(not(target_arch = "wasm32"))] - async fn fulcio_certs(&self) -> crate::errors::Result> { + fn fulcio_certs(&self) -> crate::errors::Result> { Ok(match &self.fulcio_certs { Some(certs) => certs.clone(), None => Vec::new(), }) } - async fn rekor_keys(&self) -> crate::errors::Result> { + fn rekor_keys(&self) -> crate::errors::Result> { Ok(match &self.rekor_keys { Some(keys) => keys.iter().map(|k| k.as_slice()).collect(), None => Vec::new(), diff --git a/src/trust/sigstore/mod.rs b/src/trust/sigstore/mod.rs index 509a57279c..7deac4fd8b 100644 --- a/src/trust/sigstore/mod.rs +++ b/src/trust/sigstore/mod.rs @@ -23,20 +23,18 @@ //! //! # Example //! -//! The `SigstoreRootTrust` instance can be created via the [`SigstoreTrustRoot::prefetch`] +//! The `SigstoreRootTrust` instance can be created via the [`SigstoreTrustRoot::new`] //! method. //! /// ```rust /// # use sigstore::trust::sigstore::SigstoreTrustRoot; -/// # use sigstore::errors::Result; /// # #[tokio::main] /// # async fn main() -> std::result::Result<(), anyhow::Error> { -/// let repo: Result = SigstoreTrustRoot::new(None).await?.prefetch().await; +/// let repo: SigstoreTrustRoot = SigstoreTrustRoot::new(None).await?; /// // Now, get Fulcio and Rekor trust roots with the returned `SigstoreRootTrust` /// # Ok(()) /// # } /// ``` -use async_trait::async_trait; use futures::StreamExt; use sha2::{Digest, Sha256}; use std::{ @@ -77,11 +75,14 @@ impl SigstoreTrustRoot { .await .map_err(Box::new)?; - Ok(Self { + let tr = Self { repository, checkout_dir: checkout_dir.map(ToOwned::to_owned), trusted_root: OnceCell::default(), - }) + }; + tr.trusted_root().await?; + + Ok(tr) } async fn trusted_root(&self) -> Result<&TrustedRoot> { @@ -111,27 +112,6 @@ impl SigstoreTrustRoot { .await } - /// Prefetches trust materials. - /// - /// [TrustRoot::fulcio_certs()] and [TrustRoot::rekor_keys()] on [SigstoreTrustRoot] lazily - /// fetches the requested data, which is problematic for async callers. Those callers should - /// use this method to fetch the trust root ahead of time. - /// - /// ```rust - /// # use sigstore::trust::sigstore::SigstoreTrustRoot; - /// # use sigstore::errors::Result; - /// # #[tokio::main] - /// # async fn main() -> std::result::Result<(), anyhow::Error> { - /// let repo: Result = SigstoreTrustRoot::new(None).await?.prefetch().await; - /// // Now, get Fulcio and Rekor trust roots with the returned `SigstoreRootTrust` - /// # Ok(()) - /// # } - /// ``` - pub async fn prefetch(self) -> Result { - let _ = self.trusted_root().await?; - Ok(self) - } - #[inline] fn tlog_keys(tlogs: &[TransparencyLogInstance]) -> impl Iterator { tlogs @@ -153,16 +133,15 @@ impl SigstoreTrustRoot { } } -#[cfg(not(target_arch = "wasm32"))] -#[async_trait] impl crate::trust::TrustRoot for SigstoreTrustRoot { /// Fetch Fulcio certificates from the given TUF repository or reuse /// the local cache if its contents are not outdated. /// /// The contents of the local cache are updated when they are outdated. - #[cfg(not(target_arch = "wasm32"))] - async fn fulcio_certs(&self) -> Result> { - let root = self.trusted_root().await?; + fn fulcio_certs(&self) -> Result> { + let root = self.trusted_root.get().ok_or_else(|| { + SigstoreError::TufMetadataError("Trusted root not initialized".into()) + })?; // Allow expired certificates: they may have been active when the // certificate was used to sign. @@ -182,8 +161,10 @@ impl crate::trust::TrustRoot for SigstoreTrustRoot { /// the local cache if it's not outdated. /// /// The contents of the local cache are updated when they are outdated. - async fn rekor_keys(&self) -> Result> { - let root = self.trusted_root().await?; + fn rekor_keys(&self) -> Result> { + let root = self.trusted_root.get().ok_or_else(|| { + SigstoreError::TufMetadataError("Trusted root not initialized".into()) + })?; let keys: Vec<_> = Self::tlog_keys(&root.tlogs).collect(); if keys.len() != 1 { @@ -315,21 +296,15 @@ mod tests { use super::*; #[tokio::test] - async fn prefetch() { + async fn initialize() { let repo = SigstoreTrustRoot::new(None) .await - .expect("initialize SigstoreRepository") - .prefetch() - .await - .expect("prefetch"); + .expect("initialize SigstoreRepository"); - let fulcio_certs = repo - .fulcio_certs() - .await - .expect("cannot fetch Fulcio certs"); + let fulcio_certs = repo.fulcio_certs().expect("cannot fetch Fulcio certs"); assert!(!fulcio_certs.is_empty()); - let rekor_keys = repo.rekor_keys().await.expect("cannot fetch Rekor keys"); + let rekor_keys = repo.rekor_keys().expect("cannot fetch Rekor keys"); assert!(!rekor_keys.is_empty()); } }