From 3c89abf65c1ca5d8520acab2b6cbf962833114ec Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Fri, 24 Nov 2023 10:44:34 +0100 Subject: [PATCH] Change return types to Result in signature verification To deal with signature verification errors in a detailed way, let's change return type of parse_signature_data functions back to Result. Make it iterate over all signature slots to find the first valid signature. If nothing is found, return error. --- src/bin/download_sysext.rs | 2 +- test/crau_verify.rs | 2 +- update-format-crau/src/delta_update.rs | 46 +++++++++++++++++--------- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/bin/download_sysext.rs b/src/bin/download_sysext.rs index d4a8c70..94300e4 100644 --- a/src/bin/download_sysext.rs +++ b/src/bin/download_sysext.rs @@ -217,7 +217,7 @@ impl<'a> Package<'a> { // Parse signature data from sig blobs, data blobs, public key, and verify. match delta_update::parse_signature_data(&sigbytes, hdhashvec.as_slice(), pubkey_path) { - Some(_) => (), + Ok(_) => (), _ => { self.status = PackageStatus::BadSignature; bail!( diff --git a/test/crau_verify.rs b/test/crau_verify.rs index e2b9fee..634f259 100644 --- a/test/crau_verify.rs +++ b/test/crau_verify.rs @@ -70,7 +70,7 @@ fn main() -> Result<(), Box> { // Parse signature data from the signature containing data, version, special fields. let sigdata = match delta_update::parse_signature_data(&sigbytes, hdhashvec.as_slice(), PUBKEY_FILE) { - Some(data) => Box::leak(data), + Ok(data) => Box::leak(data), _ => return Err("unable to parse signature data".into()), }; diff --git a/update-format-crau/src/delta_update.rs b/update-format-crau/src/delta_update.rs index 6be6e44..8c437aa 100644 --- a/update-format-crau/src/delta_update.rs +++ b/update-format-crau/src/delta_update.rs @@ -2,7 +2,7 @@ use std::io::{BufReader, Read, Seek, SeekFrom, Write}; use std::fs; use std::fs::File; use std::path::Path; -use log::{error, debug}; +use log::{debug, info}; use bzip2::read::BzDecoder; use anyhow::{Context, Result, bail}; @@ -147,27 +147,42 @@ pub fn get_data_blobs<'a>(f: &'a mut BufReader, header: &'a DeltaUpdateFil // parse_signature_data takes bytes slices for signature and digest of data blobs, // and path to public key, to parse and verify the signature. // Return only actual signature data, without version and special fields. -pub fn parse_signature_data(sigbytes: &[u8], digest: &[u8], pubkeyfile: &str) -> Option> { +pub fn parse_signature_data(sigbytes: &[u8], digest: &[u8], pubkeyfile: &str) -> Result> { // Signatures has a container of the fields, i.e. version, data, and // special fields. let sigmessage = match proto::Signatures::parse_from_bytes(sigbytes) { Ok(data) => data, - _ => return None, + _ => bail!("failed to parse signature messages"), }; // sigmessages.signatures[] has a single element in case of dev update payloads, // while it could have multiple elements in case of production update payloads. // For now we assume only dev update payloads are supported. // Return the first valid signature, iterate into the next slot if invalid. - sigmessage.signatures.iter() - .find_map(|sig| - verify_sig_pubkey(digest, sig, pubkeyfile) - .map(Vec::into_boxed_slice)) + let mut sigres: Option> = None; + + for sig in sigmessage.signatures { + match verify_sig_pubkey(digest, &sig, pubkeyfile) { + Ok(sbox) => { + sigres = Some(sbox.to_vec()); + break + } + _ => { + info!("failed to verify signature, jumping to the next slot"); + continue + } + }; + } + + match sigres { + Some(s) => Ok(s.into()), + None => bail!("failed to find a valid signature in any slot"), + } } // verify_sig_pubkey verifies signature with the given digest and the public key. // Return the verified signature data. -pub fn verify_sig_pubkey(digest: &[u8], sig: &Signature, pubkeyfile: &str) -> Option> { +pub fn verify_sig_pubkey(digest: &[u8], sig: &Signature, pubkeyfile: &str) -> Result> { // The signature version is actually a numeration of the present signatures, // with the index starting at 2 if only one signature is present. // The Flatcar dev payload has only one signature but @@ -177,8 +192,11 @@ pub fn verify_sig_pubkey(digest: &[u8], sig: &Signature, pubkeyfile: &str) -> Op // for a signature version, as the number could differ in some cases. debug!("supported signature version: {:?}", sig.version()); let sigvec = match &sig.data { - Some(sigdata) => Some(sigdata), - _ => None, + Some(sigdata) => sigdata, + _ => { + info!("empty signature data, nothing to verify"); + return Ok(sig.data.clone().unwrap().into_boxed_slice()); + } }; debug!("digest: {:?}", digest); @@ -189,8 +207,7 @@ pub fn verify_sig_pubkey(digest: &[u8], sig: &Signature, pubkeyfile: &str) -> Op let pkcspem_pubkey = match get_public_key_pkcs_pem(pubkeyfile, KeyTypePkcs8) { Ok(key) => key, Err(err) => { - error!("failed to get PKCS8 PEM public key ({:?}) with error {:?}", pubkeyfile, err); - return None; + bail!("failed to get PKCS8 PEM public key ({:?}) with error {:?}", pubkeyfile, err); } }; @@ -198,10 +215,9 @@ pub fn verify_sig_pubkey(digest: &[u8], sig: &Signature, pubkeyfile: &str) -> Op match res_verify { Ok(res_verify) => res_verify, Err(err) => { - error!("verify_rsa_pkcs signature ({:?}) failed with error {:?}", sig, err); - return None; + bail!("verify_rsa_pkcs signature ({:?}) failed with error {:?}", sig, err); } }; - sigvec.cloned() + Ok(sigvec.clone().into_boxed_slice()) }