From 4e4740c8b36200d2f3a60b75bc228d533ddcc14a 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 | 4 +-- update-format-crau/src/delta_update.rs | 37 +++++++++++++++----------- 3 files changed, 25 insertions(+), 18 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..99e8180 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) => data, _ => return Err("unable to parse signature data".into()), }; @@ -78,7 +78,7 @@ fn main() -> Result<(), Box> { // Store signature into a file. let mut sigfile = fs::File::create(sigpath.clone())?; - let _ = sigfile.write_all(sigdata); + let _ = sigfile.write_all(sigdata.as_slice()); println!("Wrote signature data into file {:?}", sigpath); diff --git a/update-format-crau/src/delta_update.rs b/update-format-crau/src/delta_update.rs index 6be6e44..352b359 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,36 @@ 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)) + for sig in sigmessage.signatures { + match verify_sig_pubkey(digest, &sig, pubkeyfile) { + Ok(sbox) => { + return Ok(sbox.to_vec()); + } + _ => { + info!("failed to verify signature, jumping to the next slot"); + continue + } + }; + } + + 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 +186,8 @@ 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, + _ => bail!("empty signature data, nothing to verify"), }; debug!("digest: {:?}", digest); @@ -189,8 +198,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 +206,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()) }