From e2dc5174b4aa90de0f41c61534a9bdfcb068abdf Mon Sep 17 00:00:00 2001 From: Andrew Gunnerson Date: Thu, 13 Mar 2025 20:20:44 -0400 Subject: [PATCH] format/ota: Decouple OTA signature parsing from verification This way, we can fail hard for parsing errors, but not for verification errors in `avbroot ota verify`. Signed-off-by: Andrew Gunnerson --- avbroot/src/cli/ota.rs | 29 +++--- avbroot/src/format/ota.rs | 187 +++++++++++++++++++++++--------------- 2 files changed, 126 insertions(+), 90 deletions(-) diff --git a/avbroot/src/cli/ota.rs b/avbroot/src/cli/ota.rs index 9e38835..427a73b 100644 --- a/avbroot/src/cli/ota.rs +++ b/avbroot/src/cli/ota.rs @@ -1505,9 +1505,8 @@ pub fn extract_subcommand(cli: &ExtractCli, cancel_signal: &AtomicBool) -> Resul info!("Extracting embedded OTA certificate from zip signature"); let ota_sig = ota::parse_ota_sig(&mut raw_reader)?; - let embedded_cert = ota_sig.embedded_cert()?; - crypto::write_pem_cert_file(path, embedded_cert) + crypto::write_pem_cert_file(path, &ota_sig.cert) .with_context(|| format!("Failed to write OTA certificate: {path:?}"))?; } @@ -1706,28 +1705,27 @@ pub fn verify_subcommand(cli: &VerifyCli, cancel_signal: &AtomicBool) -> Result< info!("Verifying whole-file signature"); - let embedded_cert = match ota::verify_ota(&mut reader, cancel_signal) + let ota_sig = ota::parse_ota_sig(&mut reader).context("Failed to parse OTA signature")?; + + if let Err(e) = ota_sig + .verify_ota(&mut reader, cancel_signal) .context("Failed to verify OTA against embedded certificate") { - Ok(cert) => Some(cert), - Err(e) => { - fail_later!("{e:?}"); - None - } - }; + fail_later!("{e:?}"); + } let (metadata, ota_cert, header, properties) = ota::parse_zip_ota_info(&mut reader).context("Failed to parse OTA metadata")?; - if embedded_cert.as_ref() != Some(&ota_cert) { + if ota_cert != ota_sig.cert { fail_later!( - "CMS embedded certificate does not match {}", - ota::PATH_OTACERT + "{} does not match CMS embedded certificate", + ota::PATH_OTACERT, ); } else if let Some(p) = &cli.cert_ota { let verify_cert = crypto::read_pem_cert_file(p) .with_context(|| format!("Failed to load certificate: {p:?}"))?; - if embedded_cert != Some(verify_cert) { + if ota_sig.cert != verify_cert { fail_later!("OTA has a valid signature, but was not signed with: {p:?}"); } } else { @@ -1756,8 +1754,9 @@ pub fn verify_subcommand(cli: &VerifyCli, cancel_signal: &AtomicBool) -> Result< let section_reader = SectionReader::new(&mut reader, pf_payload.offset, pf_payload.size) .context("Failed to directly open payload section")?; - if let Err(e) = payload::verify_payload(section_reader, &ota_cert, &properties, cancel_signal) - .context("Failed to verify payload signatures and digests") + if let Err(e) = + payload::verify_payload(section_reader, &ota_sig.cert, &properties, cancel_signal) + .context("Failed to verify payload signatures and digests") { fail_later!("{e:?}"); } diff --git a/avbroot/src/format/ota.rs b/avbroot/src/format/ota.rs index 669b607..ba0af5b 100644 --- a/avbroot/src/format/ota.rs +++ b/avbroot/src/format/ota.rs @@ -10,7 +10,7 @@ use std::{ sync::atomic::AtomicBool, }; -use aws_lc_rs::digest::Context; +use aws_lc_rs::digest::{Algorithm, Context}; use clap::ValueEnum; use cms::signed_data::SignedData; use const_oid::{db::rfc5912, ObjectIdentifier}; @@ -566,14 +566,14 @@ pub fn verify_metadata( } #[derive(Clone, Debug)] -pub struct OtaSignature { +struct RawOtaSignature { /// Decoded CMS structure. - pub signed_data: SignedData, + signed_data: SignedData, /// Length of the file (from the beginning) that's covered by the signature. - pub hashed_size: u64, + hashed_size: u64, } -impl OtaSignature { +impl RawOtaSignature { pub fn embedded_cert(&self) -> Result<&Certificate> { let mut iter = crypto::iter_cms_certs(&self.signed_data); @@ -589,9 +589,110 @@ impl OtaSignature { } } +#[derive(Clone, Debug)] +pub struct OtaSignature { + pub cert: Certificate, + pub digest_algo: &'static Algorithm, + pub sig_algo: SignatureAlgorithm, + pub sig: Vec, + pub data_size: u64, +} + +impl TryFrom for OtaSignature { + type Error = Error; + + fn try_from(raw_ota_sig: RawOtaSignature) -> Result { + let cert = raw_ota_sig.embedded_cert()?; + + // Make sure this is a signature scheme we can handle. There's currently + // no Rust library to verify arbitrary CMS signatures for large files + // without fully reading them into memory. + let signers_len = raw_ota_sig.signed_data.signer_infos.0.len(); + if signers_len != 1 { + return Err(Error::NotOneCmsSignerInfo(signers_len)); + } + + let signer = raw_ota_sig.signed_data.signer_infos.0.get(0).unwrap(); + if signer.digest_alg.oid != rfc5912::ID_SHA_256 + && signer.digest_alg.oid != rfc5912::ID_SHA_1 + { + return Err(Error::UnsupportedDigestAlgorithm(signer.digest_alg.oid)); + } else if signer.signature_algorithm.oid != rfc5912::RSA_ENCRYPTION + && signer.signature_algorithm.oid != rfc5912::SHA_256_WITH_RSA_ENCRYPTION + { + return Err(Error::UnsupportedSignatureAlgorithm( + signer.signature_algorithm.oid, + )); + } + + // We support SHA1 for verification only. + let (digest_algo, sig_algo) = if signer.digest_alg.oid == rfc5912::ID_SHA_256 { + ( + &aws_lc_rs::digest::SHA256, + SignatureAlgorithm::Sha256WithRsa, + ) + } else { + ( + &aws_lc_rs::digest::SHA1_FOR_LEGACY_USE_ONLY, + SignatureAlgorithm::Sha1WithRsa, + ) + }; + + Ok(Self { + cert: cert.clone(), + digest_algo, + sig_algo, + sig: signer.signature.as_bytes().to_vec(), + data_size: raw_ota_sig.hashed_size, + }) + } +} + +impl OtaSignature { + /// Verify an OTA zip against its embedded certificate. This function makes + /// no assertion about whether the certificate is actually trusted. + /// + /// CMS signed attributes are intentionally not supported because AOSP + /// recovery does not support them either. It expects the CMS [`SignedData`] + /// structure to be used for nothing more than a raw signature transport + /// mechanism. + pub fn verify_ota( + &self, + mut reader: impl Read + Seek, + cancel_signal: &AtomicBool, + ) -> Result<()> { + let public_key = crypto::get_public_key(&self.cert).map_err(Error::OtaCertExtractPubKey)?; + + // Manually hash the parts of the file covered by the signature. + reader + .seek(SeekFrom::Start(0)) + .map_err(|e| Error::DataRead("raw_data", e))?; + + let mut hashing_reader = HashingReader::new(reader, Context::new(self.digest_algo)); + + stream::copy_n( + &mut hashing_reader, + io::sink(), + self.data_size, + cancel_signal, + ) + .map_err(|e| Error::DataRead("raw_data", e))?; + + let (_, context) = hashing_reader.finish(); + let digest = context.finish(); + + // Verify the signature against the public key. + public_key + .verify_sig(self.sig_algo, digest.as_ref(), &self.sig) + .map_err(Error::CmsVerify)?; + + Ok(()) + } +} + /// Parse the CMS signature from the OTA zip comment. This does not perform any /// parsing of zip data structures. -pub fn parse_ota_sig(mut reader: impl Read + Seek) -> Result { +fn parse_raw_ota_sig(mut reader: impl Read + Seek) -> Result { let file_size = reader .seek(SeekFrom::End(0)) .map_err(|e| Error::DataRead("file_size", e))?; @@ -642,80 +743,16 @@ pub fn parse_ota_sig(mut reader: impl Read + Seek) -> Result { // length field. let hashed_size = file_size - 2 - u64::from(comment_size); - Ok(OtaSignature { + Ok(RawOtaSignature { signed_data, hashed_size, }) } -/// Verify an OTA zip against its embedded certificates. This function makes no -/// assertion about whether the certificate is actually trusted. Returns the -/// embedded certificate. -/// -/// CMS signed attributes are intentionally not supported because AOSP recovery -/// does not support them either. It expects the CMS [`SignedData`] structure to -/// be used for nothing more than a raw signature transport mechanism. -pub fn verify_ota(mut reader: impl Read + Seek, cancel_signal: &AtomicBool) -> Result { - let ota_sig = parse_ota_sig(&mut reader)?; - let cert = ota_sig.embedded_cert()?; - let public_key = crypto::get_public_key(cert).map_err(Error::OtaCertExtractPubKey)?; - - // Make sure this is a signature scheme we can handle. There's currently no - // Rust library to verify arbitrary CMS signatures for large files without - // fully reading them into memory. - let signers_len = ota_sig.signed_data.signer_infos.0.len(); - if signers_len != 1 { - return Err(Error::NotOneCmsSignerInfo(signers_len)); - } - - let signer = ota_sig.signed_data.signer_infos.0.get(0).unwrap(); - if signer.digest_alg.oid != rfc5912::ID_SHA_256 && signer.digest_alg.oid != rfc5912::ID_SHA_1 { - return Err(Error::UnsupportedDigestAlgorithm(signer.digest_alg.oid)); - } else if signer.signature_algorithm.oid != rfc5912::RSA_ENCRYPTION - && signer.signature_algorithm.oid != rfc5912::SHA_256_WITH_RSA_ENCRYPTION - { - return Err(Error::UnsupportedSignatureAlgorithm( - signer.signature_algorithm.oid, - )); - } - - // Manually hash the parts of the file covered by the signature. - reader - .seek(SeekFrom::Start(0)) - .map_err(|e| Error::DataRead("raw_data", e))?; - - // We support SHA1 for verification only. - let (algorithm, algo) = if signer.digest_alg.oid == rfc5912::ID_SHA_256 { - ( - &aws_lc_rs::digest::SHA256, - SignatureAlgorithm::Sha256WithRsa, - ) - } else { - ( - &aws_lc_rs::digest::SHA1_FOR_LEGACY_USE_ONLY, - SignatureAlgorithm::Sha1WithRsa, - ) - }; - - let mut hashing_reader = HashingReader::new(reader, Context::new(algorithm)); - - stream::copy_n( - &mut hashing_reader, - io::sink(), - ota_sig.hashed_size, - cancel_signal, - ) - .map_err(|e| Error::DataRead("raw_data", e))?; - - let (_, context) = hashing_reader.finish(); - let digest = context.finish(); - - // Verify the signature against the public key. - public_key - .verify_sig(algo, digest.as_ref(), signer.signature.as_bytes()) - .map_err(Error::CmsVerify)?; - - Ok(cert.clone()) +/// Parse the signature information from the CMS signature embedded in the OTA +/// zip archive comment. +pub fn parse_ota_sig(reader: impl Read + Seek) -> Result { + parse_raw_ota_sig(reader)?.try_into() } /// Get and parse the protobuf-encoded OTA metadata, the PEM-encoded otacert,