From 04dece2f09feb48d45636d563fea2d78aa613fdd Mon Sep 17 00:00:00 2001 From: Andrew Pan Date: Wed, 29 Nov 2023 10:22:54 -0600 Subject: [PATCH] tuf: remove chaff in RepositoryHelper Signed-off-by: Andrew Pan --- src/tuf/repository_helper.rs | 379 ++++++++++++++++------------------- 1 file changed, 173 insertions(+), 206 deletions(-) diff --git a/src/tuf/repository_helper.rs b/src/tuf/repository_helper.rs index e821c230e4..a581619638 100644 --- a/src/tuf/repository_helper.rs +++ b/src/tuf/repository_helper.rs @@ -13,7 +13,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -use rustls_pki_types::CertificateDer; use sha2::{Digest, Sha256}; use std::fs; use std::io::Read; @@ -21,13 +20,14 @@ use std::path::{Path, PathBuf}; use tough::{RepositoryLoader, TargetName}; use url::Url; -use super::super::errors::{Result, SigstoreError}; -use super::trustroot::{CertificateAuthority, TimeRange, TransparencyLogInstance, TrustedRoot}; +use super::{ + super::errors::{Result, SigstoreError}, + constants::{SIGSTORE_FULCIO_CERT_TARGET_REGEX, SIGSTORE_REKOR_PUB_KEY_TARGET}, +}; pub(crate) struct RepositoryHelper { repository: tough::Repository, checkout_dir: Option, - trusted_root: Option, } impl RepositoryHelper { @@ -40,7 +40,7 @@ impl RepositoryHelper { where R: Read, { - let repository = RepositoryLoader::new(SIGSTORE_ROOT, metadata_base, target_base) + let repository = RepositoryLoader::new(root, metadata_base, target_base) .expiration_enforcement(tough::ExpirationEnforcement::Safe) .load() .map_err(Box::new)?; @@ -48,130 +48,68 @@ impl RepositoryHelper { Ok(Self { repository, checkout_dir: checkout_dir.map(|s| s.to_owned()), - trusted_root: None, }) } - pub(crate) fn from_repo(repo: tough::Repository, checkout_dir: Option<&Path>) -> Self { - Self { - repository: repo, - checkout_dir: checkout_dir.map(|s| s.to_owned()), - trusted_root: None, - } - } - - fn trusted_root(&self) -> Result<&TrustedRoot> { - if let Some(result) = self.trusted_root { - return Ok(&result); - } - - let trusted_root_target = TargetName::new("trusted_root.json").map_err(Box::new)?; - let local_path = self - .checkout_dir - .as_ref() - .map(|d| d.join(trusted_root_target.raw())); - - let data = fetch_target_or_reuse_local_cache( - &self.repository, - &trusted_root_target, - local_path.as_ref(), - )?; - - let result = serde_json::from_slice(&data[..])?; - Ok(self.trusted_root.insert(result)) - } - - #[inline] - fn tlog_keys(&self, tlogs: &Vec) -> Vec<&[u8]> { - let mut result = Vec::new(); - - for key in tlogs { - // We won't accept expired keys for transparency logs. - if !is_timerange_valid(key.public_key.valid_for, false) { - continue; - } - - if let Some(raw) = key.public_key.raw_bytes { - result.push(&raw[..]); - } - } - - result - } - - #[inline] - fn ca_keys(&self, cas: &Vec, allow_expired: bool) -> Vec<&[u8]> { - let mut certs = Vec::new(); - - for ca in cas { - if !is_timerange_valid(Some(ca.valid_for), allow_expired) { - continue; - } - - let certs_in_ca = ca.cert_chain.certificates; - certs.extend(certs_in_ca.iter().map(|cert| &cert.raw_bytes[..])); - } - - return certs; - } - /// 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. - pub(crate) fn fulcio_certs(&self) -> Result> { - let root = self.trusted_root()?; - - // Allow expired certificates: they may have been active when the - // certificate was used to sign. - let certs = self.ca_keys(&root.certificate_authorities, true); - let certs: Vec<_> = certs.iter().map(|v| CertificateDer::from(*v)).collect(); - - if certs.is_empty() { - Err(SigstoreError::TufMetadataError( - "Fulcio certificates not found", - )) - } else { - Ok(certs) + pub(crate) fn fulcio_certs(&self) -> Result> { + let fulcio_target_names = self.fulcio_cert_target_names(); + let mut certs = vec![]; + + for fulcio_target_name in &fulcio_target_names { + let local_fulcio_path = self + .checkout_dir + .as_ref() + .map(|d| Path::new(d).join(fulcio_target_name.raw())); + + let cert_data = fetch_target_or_reuse_local_cache( + &self.repository, + fulcio_target_name, + local_fulcio_path.as_ref(), + )?; + certs.push(crate::registry::Certificate { + data: cert_data, + encoding: crate::registry::CertificateEncoding::Pem, + }); } + Ok(certs) } - /// Fetch Rekor public keys from the given TUF repository or reuse + fn fulcio_cert_target_names(&self) -> Vec { + self.repository + .targets() + .signed + .targets_iter() + .filter_map(|(target_name, _target)| { + if SIGSTORE_FULCIO_CERT_TARGET_REGEX.is_match(target_name.raw()) { + Some(target_name.clone()) + } else { + None + } + }) + .collect() + } + + /// Fetch Rekor public key from the given TUF repository or reuse /// the local cache if it's not outdated. /// /// The contents of the local cache are updated when they are outdated. - pub(crate) fn rekor_keys(&self) -> Result> { - let root = self.trusted_root()?; - let keys = self.tlog_keys(&root.tlogs); + pub(crate) fn rekor_pub_key(&self) -> Result> { + let rekor_target_name = TargetName::new(SIGSTORE_REKOR_PUB_KEY_TARGET).map_err(Box::new)?; - if keys.len() != 1 { - Err(SigstoreError::TufMetadataError( - "Did not find exactly 1 active Rekor key", - )) - } else { - Ok(keys) - } - } -} + let local_rekor_path = self + .checkout_dir + .as_ref() + .map(|d| Path::new(d).join(SIGSTORE_REKOR_PUB_KEY_TARGET)); -/// Given a `range`, checks that the the current time is not before `start`. If -/// `allow_expired` is `false`, also checks that the current time is not after -/// `end`. -fn is_timerange_valid(range: Option, allow_expired: bool) -> bool { - let time = chrono::Utc::now(); - - match range { - // If there was no validity period specified, the key is always valid. - None => true, - // Active: if the current time is before the starting period, we are not yet valid. - Some(range) if time < range.start => false, - // If we want Expired keys, then the key is valid at this point. - _ if allow_expired => true, - // Otherwise, check that we are in range if the range has an end. - Some(range) => match range.end { - None => true, - Some(end) => time <= end, - }, + fetch_target_or_reuse_local_cache( + &self.repository, + &rekor_target_name, + local_rekor_path.as_ref(), + ) } } @@ -254,11 +192,20 @@ fn is_local_file_outdated( // local data is not outdated Ok((false, Some(data))) } else { - Ok(keys) + Ok((true, None)) } + } else { + Ok((true, None)) } } +/// Gets the goods from a read and makes a Vec +fn read_to_end(mut reader: R) -> Result> { + let mut v = Vec::new(); + reader.read_to_end(&mut v)?; + Ok(v) +} + #[cfg(test)] mod tests { use super::super::constants::*; @@ -305,92 +252,61 @@ mod tests { )) })?; // It's fine to ignore timestamp.json expiration inside of test env - let repo = RepositoryLoader::new(SIGSTORE_ROOT, metadata_base_url, target_base_url) - .expiration_enforcement(tough::ExpirationEnforcement::Unsafe) - .load() - .map_err(Box::new)?; + let repo = + RepositoryLoader::new(SIGSTORE_ROOT.as_bytes(), metadata_base_url, target_base_url) + .expiration_enforcement(tough::ExpirationEnforcement::Unsafe) + .load() + .map_err(Box::new)?; Ok(repo) } - fn find_target(name: &str) -> Result { - let path = test_data().join("repository").join("targets"); - - for entry in fs::read_dir(path)? { - let path = entry?.path(); - if path.is_dir() { - continue; - } - - // Heuristic: Filter for consistent snapshot targets. SHA256 hashes in hexadecimal - // comprise of 64 characters, so our filename must be at least that long. The TUF repo - // shouldn't ever contain paths with invalid Unicode (knock on wood), so we're doing - // the lossy OsStr conversion here. - let filename = path.file_name().unwrap().to_str().unwrap(); - if filename.len() < 64 { - continue; - } - - // Heuristic: see if the filename is in consistent snapshot format (.). - // NB: The consistent snapshot prefix should be ASCII, so indexing the string as - // bytes is safe enough. - if filename.as_bytes()[64] != b'.' { - continue; - } - - // At this point, we're probably dealing with a consistent snapshot. - // Check if the name matches. - if filename.ends_with(name) { - return Ok(path); - } - } - - Err(SigstoreError::UnexpectedError( - "Couldn't find a matching target".to_string(), - )) - } - - fn check_against_disk(helper: &RepositoryHelper) { - let mut actual: Vec<&[u8]> = helper - .fulcio_certs() - .expect("fulcio certs could not be read") - .iter() - .map(|c| c.as_ref()) - .collect(); - let expected = ["fulcio.crt.pem", "fulcio_v1.crt.pem"].iter().map(|t| { - let path = find_target(t)?; - Ok(fs::read(path)?) - }); - let mut expected = expected - .collect::>>>() - .expect("could not find targets"); - actual.sort(); - expected.sort(); - - assert_eq!(actual, expected, "The fulcio cert is not what was expected"); - - let actual = helper.rekor_keys().expect("rekor key cannot be read"); - let expected = fs::read(find_target("rekor.pub").expect("could not find targets")) - .expect("cannot read rekor key from test data"); - let expected = pem::parse(expected).unwrap(); - assert_eq!(expected.tag(), "PUBLIC KEY"); - - assert_eq!( - actual, - &[expected.contents()], - "The rekor key is not what was expected" - ); - } - #[test] fn get_files_without_using_local_cache() { let repository = local_tuf_repo().expect("Local TUF repo should not fail"); let helper = RepositoryHelper { repository, checkout_dir: None, - trusted_root: None, }; - check_against_disk(&helper); + let mut actual = helper.fulcio_certs().expect("fulcio certs cannot be read"); + actual.sort(); + let mut expected: Vec = + ["fulcio.crt.pem", "fulcio_v1.crt.pem"] + .iter() + .map(|filename| { + let data = fs::read( + test_data() + .join("repository") + .join("targets") + .join(filename), + ) + .unwrap_or_else(|_| panic!("cannot read {} from test data", filename)); + crate::registry::Certificate { + data, + encoding: crate::registry::CertificateEncoding::Pem, + } + }) + .collect(); + expected.sort(); + + assert_eq!( + actual, expected, + "The fulcio cert read from the TUF repository is not what was expected" + ); + + let actual = helper.rekor_pub_key().expect("rekor key cannot be read"); + let expected = fs::read( + test_data() + .join("repository") + .join("targets") + .join("rekor.pub"), + ) + .expect("cannot read rekor key from test data"); + + assert_eq!( + actual, expected, + "The rekor key read from the TUF repository is not what was expected" + ); } #[test] @@ -401,10 +317,42 @@ mod tests { let helper = RepositoryHelper { repository, checkout_dir: Some(cache_dir.path().to_path_buf()), - trusted_root: None, }; - check_against_disk(&helper); + let mut actual = helper.fulcio_certs().expect("fulcio certs cannot be read"); + actual.sort(); + let mut expected: Vec = + ["fulcio.crt.pem", "fulcio_v1.crt.pem"] + .iter() + .map(|filename| { + let data = fs::read( + test_data() + .join("repository") + .join("targets") + .join(filename), + ) + .unwrap_or_else(|_| panic!("cannot read {} from test data", filename)); + crate::registry::Certificate { + data, + encoding: crate::registry::CertificateEncoding::Pem, + } + }) + .collect(); + expected.sort(); + + assert_eq!( + actual, expected, + "The fulcio cert read from the cache dir is not what was expected" + ); + + let expected = helper.rekor_pub_key().expect("rekor key cannot be read"); + let actual = fs::read(cache_dir.path().join("rekor.pub")) + .expect("cannot read rekor key from cache dir"); + + assert_eq!( + actual, expected, + "The rekor key read from the cache dir is not what was expected" + ); } #[test] @@ -417,8 +365,8 @@ mod tests { .expect("Cannot write file to cache dir"); } fs::write( - cache_dir.path().join("trusted_root.json"), - b"fake trusted root", + cache_dir.path().join(SIGSTORE_REKOR_PUB_KEY_TARGET), + b"fake rekor", ) .expect("Cannot write file to cache dir"); @@ -426,22 +374,41 @@ mod tests { let helper = RepositoryHelper { repository, checkout_dir: Some(cache_dir.path().to_path_buf()), - trusted_root: None, }; - check_against_disk(&helper); - } + let mut actual = helper.fulcio_certs().expect("fulcio certs cannot be read"); + actual.sort(); + let mut expected: Vec = + ["fulcio.crt.pem", "fulcio_v1.crt.pem"] + .iter() + .map(|filename| { + let data = fs::read( + test_data() + .join("repository") + .join("targets") + .join(filename), + ) + .unwrap_or_else(|_| panic!("cannot read {} from test data", filename)); + crate::registry::Certificate { + data, + encoding: crate::registry::CertificateEncoding::Pem, + } + }) + .collect(); + expected.sort(); - #[test] - fn deser_trusted_root() { - let metadata_base_path = test_data().join("repository"); - let targets_base_path = metadata_base_path.join("targets"); + assert_eq!( + actual, expected, + "The fulcio cert read from the TUF repository is not what was expected" + ); - let repository = local_tuf_repo().expect("Local TUF repo should not fail"); - let helper = RepositoryHelper::from_repo(repository, None); + let expected = helper.rekor_pub_key().expect("rekor key cannot be read"); + let actual = fs::read(cache_dir.path().join("rekor.pub")) + .expect("cannot read rekor key from cache dir"); - helper - .trusted_root() - .expect("Trusted Root should deserialize"); + assert_eq!( + actual, expected, + "The rekor key read from the cache dir is not what was expected" + ); } }