From 9ca3a690df94b8c5c6700ae52658bf2ca3d3fccb Mon Sep 17 00:00:00 2001 From: Andrew Gunnerson Date: Mon, 18 Sep 2023 15:00:43 -0400 Subject: [PATCH] Verify FEC data in HashtreeDescriptor::verify() This also fixes the wrong error being reported when the hash tree does not match and adds additional checks to ensure that there are no gaps between the image data, hash tree, and FEC data. Issue: #145 Signed-off-by: Andrew Gunnerson --- avbroot/src/cli/avb.rs | 3 + avbroot/src/format/avb.rs | 132 ++++++++++++++++++++++++++------------ 2 files changed, 94 insertions(+), 41 deletions(-) diff --git a/avbroot/src/cli/avb.rs b/avbroot/src/cli/avb.rs index 5f48684..2d5f268 100644 --- a/avbroot/src/cli/avb.rs +++ b/avbroot/src/cli/avb.rs @@ -209,6 +209,9 @@ struct DumpCli { } /// Verify vbmeta signatures. +/// +/// For hash tree descriptors, the FEC (forward error correction) data will also +/// be verified. #[derive(Debug, Parser)] struct VerifyCli { /// Path to input image. diff --git a/avbroot/src/format/avb.rs b/avbroot/src/format/avb.rs index 7a9805d..479c822 100644 --- a/avbroot/src/format/avb.rs +++ b/avbroot/src/format/avb.rs @@ -20,7 +20,10 @@ use sha2::{Digest, Sha256, Sha512}; use thiserror::Error; use crate::{ - format::padding, + format::{ + fec::{self, Fec}, + padding, + }, stream::{ self, CountingReader, FromReader, ReadDiscardExt, ReadSeek, ReadStringExt, ToWriter, WriteStringExt, WriteZerosExt, @@ -54,8 +57,6 @@ pub enum Error { PaddingTooLong, #[error("{0:?} field padding contains non-zero bytes")] PaddingNotZero(&'static str), - #[error("{0:?} field + {1:?} field is out of bounds")] - OutOfBounds(&'static str, &'static str), #[error("{0:?} field size does not equal size of contained items")] IncorrectCombinedSize(&'static str), #[error("Invalid VBMeta header magic: {0:?}")] @@ -68,20 +69,31 @@ pub enum Error { UnsupportedAlgorithm(AlgorithmType), #[error("Hashing algorithm not supported: {0:?}")] UnsupportedHashAlgorithm(String), - #[error("Incorrect key size ({0} bytes) for algorithm {1:?} ({2} bytes)")] - IncorrectKeySize(usize, AlgorithmType, usize), - #[error("Expected root digest {0}, but have {1}")] - InvalidRootDigest(String, String), - #[error("Expected hash tree {0}, but have {1}")] - InvalidHashtree(String, String), + #[error("Incorrect key size ({key_size} bytes) for algorithm {algo:?} ({} bytes)", algo.public_key_len())] + IncorrectKeySize { + key_size: usize, + algo: AlgorithmType, + }, + #[error("Expected root digest {expected}, but have {actual}")] + InvalidRootDigest { expected: String, actual: String }, + #[error("Expected hash tree {expected}, but have {actual}")] + InvalidHashtree { expected: String, actual: String }, + #[error("Hash tree does not immediately follow image data")] + HashtreeGap, + #[error("FEC data does not immediately follow hash tree")] + FecDataGap, + #[error("FEC requires data block size ({data}) and hash block size ({hash}) to match")] + MismatchedFecBlockSizes { data: u32, hash: u32 }, #[error("Failed to RSA sign digest")] - RsaSignError(rsa::Error), + RsaSign(rsa::Error), #[error("Failed to RSA verify signature")] - RsaVerifyError(rsa::Error), + RsaVerify(rsa::Error), #[error("{0} byte image size is too small to fit header or footer")] ImageSizeTooSmall(u64), + #[error("FEC error")] + Fec(#[from] fec::Error), #[error("I/O error")] - IoError(#[from] io::Error), + Io(#[from] io::Error), } type Result = std::result::Result; @@ -172,11 +184,11 @@ impl AlgorithmType { Self::None | Self::Unknown(_) => vec![], Self::Sha256Rsa2048 | Self::Sha256Rsa4096 | Self::Sha256Rsa8192 => { let scheme = Pkcs1v15Sign::new::(); - key.sign(scheme, digest).map_err(Error::RsaSignError)? + key.sign(scheme, digest).map_err(Error::RsaSign)? } Self::Sha512Rsa2048 | Self::Sha512Rsa4096 | Self::Sha512Rsa8192 => { let scheme = Pkcs1v15Sign::new::(); - key.sign(scheme, digest).map_err(Error::RsaSignError)? + key.sign(scheme, digest).map_err(Error::RsaSign)? } }; @@ -189,12 +201,12 @@ impl AlgorithmType { Self::Sha256Rsa2048 | Self::Sha256Rsa4096 | Self::Sha256Rsa8192 => { let scheme = Pkcs1v15Sign::new::(); key.verify(scheme, digest, signature) - .map_err(Error::RsaVerifyError)?; + .map_err(Error::RsaVerify)?; } Self::Sha512Rsa2048 | Self::Sha512Rsa4096 | Self::Sha512Rsa8192 => { let scheme = Pkcs1v15Sign::new::(); key.verify(scheme, digest, signature) - .map_err(Error::RsaVerifyError)?; + .map_err(Error::RsaVerify)?; } } @@ -500,10 +512,14 @@ impl HashtreeDescriptor { )?; if self.root_digest != actual_root_digest { - return Err(Error::InvalidRootDigest( - hex::encode(&self.root_digest), - hex::encode(actual_root_digest), - )); + return Err(Error::InvalidRootDigest { + expected: hex::encode(&self.root_digest), + actual: hex::encode(actual_root_digest), + }); + } + + if self.tree_offset != self.image_size { + return Err(Error::HashtreeGap); } let mut reader = open_input()?; @@ -517,10 +533,44 @@ impl HashtreeDescriptor { let expected = ring::digest::digest(algorithm, &hash_tree); let actual = ring::digest::digest(algorithm, &actual_hash_tree); - return Err(Error::InvalidRootDigest( - hex::encode(expected), - hex::encode(actual), - )); + return Err(Error::InvalidHashtree { + expected: hex::encode(expected), + actual: hex::encode(actual), + }); + } + + // The FEC data section is optional. + if self.fec_size != 0 { + if self.fec_offset != self.tree_offset + self.tree_size { + return Err(Error::FecDataGap); + } else if self.data_block_size != self.hash_block_size { + return Err(Error::MismatchedFecBlockSizes { + data: self.data_block_size, + hash: self.hash_block_size, + }); + } + + let fec_size = self + .fec_size + .to_usize() + .ok_or_else(|| Error::IntegerTooLarge("fec_size"))?; + let parity = self + .fec_num_roots + .to_u8() + .ok_or_else(|| Error::IntegerTooLarge("fec_num_roots"))?; + + // The FEC covers the hash tree as well. + let fec = Fec::new( + self.tree_offset + self.tree_size, + self.data_block_size, + parity, + )?; + + let mut fec_data = vec![0u8; fec_size]; + // Already seeked to FEC. + reader.read_exact(&mut fec_data)?; + + fec.verify(open_input, &fec_data, cancel_signal)?; } Ok(()) @@ -696,10 +746,10 @@ impl HashDescriptor { let digest = context.finish(); if self.root_digest != digest.as_ref() { - return Err(Error::InvalidRootDigest( - hex::encode(&self.root_digest), - hex::encode(digest), - )); + return Err(Error::InvalidRootDigest { + expected: hex::encode(&self.root_digest), + actual: hex::encode(digest), + }); } Ok(()) @@ -1205,11 +1255,10 @@ impl Header { } if key_raw.len() != self.algorithm_type.public_key_len() { - return Err(Error::IncorrectKeySize( - key_raw.len(), - self.algorithm_type, - self.algorithm_type.public_key_len(), - )); + return Err(Error::IncorrectKeySize { + key_size: key_raw.len(), + algo: self.algorithm_type, + }); } // The public key and the sizes of the hash and signature are included @@ -1376,7 +1425,7 @@ impl FromReader for Header { .checked_add(hash_size) .map_or(false, |s| s > auth_block.len()) { - return Err(Error::OutOfBounds("hash_offset", "hash_size")); + return Err(Error::IntegerTooLarge("hash_offset + hash_size")); } let hash = &auth_block[hash_offset..hash_offset + hash_size]; @@ -1384,7 +1433,7 @@ impl FromReader for Header { .checked_add(signature_size) .map_or(false, |s| s > auth_block.len()) { - return Err(Error::OutOfBounds("signature_offset", "signature_size")); + return Err(Error::IntegerTooLarge("signature_offset + signature_size")); } let signature = &auth_block[signature_offset..signature_offset + signature_size]; @@ -1394,7 +1443,9 @@ impl FromReader for Header { .checked_add(public_key_size) .map_or(false, |s| s > aux_block.len()) { - return Err(Error::OutOfBounds("public_key_offset", "public_key_size")); + return Err(Error::IntegerTooLarge( + "public_key_offset + public_key_size", + )); } let public_key = &aux_block[public_key_offset..public_key_offset + public_key_size]; @@ -1402,9 +1453,8 @@ impl FromReader for Header { .checked_add(public_key_metadata_size) .map_or(false, |s| s > aux_block.len()) { - return Err(Error::OutOfBounds( - "public_key_metadata_offset", - "public_key_metadata_size", + return Err(Error::IntegerTooLarge( + "public_key_metadata_offset + public_key_metadata_size", )); } let public_key_metadata = &aux_block @@ -1577,7 +1627,7 @@ pub fn decode_public_key(data: &[u8]) -> Result { let modulus = BigUint::from_bytes_be(&modulus_raw); let public_key = - RsaPublicKey::new(modulus, BigUint::from(65537u32)).map_err(Error::RsaVerifyError)?; + RsaPublicKey::new(modulus, BigUint::from(65537u32)).map_err(Error::RsaVerify)?; Ok(public_key) } @@ -1592,7 +1642,7 @@ pub fn load_image(mut reader: impl Read + Seek) -> Result<(Header, Option