Skip to content

Commit

Permalink
change unwrap to ok_or or unwrap_or
Browse files Browse the repository at this point in the history
Replace a plain `unwrap()` with `ok_or` or `unwrap_or` to prevent
from binaries or libs from panicking at runtime.
  • Loading branch information
dongsupark committed Nov 30, 2023
1 parent ee3918a commit ac37636
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 25 deletions.
35 changes: 25 additions & 10 deletions omaha/src/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,11 @@ impl<'__input: 'a, 'a> hard_xml::XmlRead<'__input> for Manifest<'a> {
}
}

if let Token::ElementEnd { end: ElementEnd::Empty, .. }
= reader.next().unwrap()?
if let Ok(Token::ElementEnd { end: ElementEnd::Empty, .. })
= reader.next().ok_or(XmlError::MissingField {
name: "Manifest".to_owned(),
field: "version".to_owned(),
})?
{
return Ok(Manifest {
version: __self_version
Expand All @@ -170,8 +173,11 @@ impl<'__input: 'a, 'a> hard_xml::XmlRead<'__input> for Manifest<'a> {

while (reader.find_attribute()?).is_some() {}

if let Token::ElementEnd { end: ElementEnd::Empty, .. }
= reader.next().unwrap()?
if let Ok(Token::ElementEnd { end: ElementEnd::Empty, .. })
= reader.next().ok_or(XmlError::MissingField {
name: "Manifest".to_owned(),
field: "version".to_owned(),
})?
{
continue;
}
Expand All @@ -197,8 +203,11 @@ impl<'__input: 'a, 'a> hard_xml::XmlRead<'__input> for Manifest<'a> {

while (reader.find_attribute()?).is_some() {}

if let Token::ElementEnd { end: ElementEnd::Empty, .. }
= reader.next().unwrap()?
if let Ok(Token::ElementEnd { end: ElementEnd::Empty, .. })
= reader.next().ok_or(XmlError::MissingField {
name: "Manifest".to_owned(),
field: "version".to_owned(),
})?
{
continue;
}
Expand Down Expand Up @@ -264,8 +273,11 @@ impl<'__input: 'a, 'a> hard_xml::XmlRead<'__input> for UpdateCheck<'a> {
}
}

if let Token::ElementEnd { end: ElementEnd::Empty, .. }
= reader.next().unwrap()?
if let Ok(Token::ElementEnd { end: ElementEnd::Empty, .. })
= reader.next().ok_or(XmlError::MissingField {
name: "UpdateCheck".to_owned(),
field: "manifest".to_owned(),
})?
{
return Ok(UpdateCheck {
status: __self_status
Expand All @@ -288,8 +300,11 @@ impl<'__input: 'a, 'a> hard_xml::XmlRead<'__input> for UpdateCheck<'a> {
reader.read_till_element_start("urls")?;

while (reader.find_attribute()?).is_some() {}
if let Token::ElementEnd { end: ElementEnd::Empty, .. }
= reader.next().unwrap()?
if let Ok(Token::ElementEnd { end: ElementEnd::Empty, .. })
= reader.next().ok_or(XmlError::MissingField {
name: "UpdateCheck".to_owned(),
field: "manifest".to_owned(),
})?
{
continue;
}
Expand Down
2 changes: 1 addition & 1 deletion src/bin/download_sysext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl<'a> Package<'a> {
let datablobspath = tmpdir.join("ue_data_blobs");

// Get length of header and data, including header and manifest.
let header_data_length = delta_update::get_header_data_length(&header, &delta_archive_manifest);
let header_data_length = delta_update::get_header_data_length(&header, &delta_archive_manifest).context("failed to get header data length")?;
let hdhash = self.hash_on_disk(from_path, Some(header_data_length)).context(format!("failed to hash_on_disk path ({:?}) failed", from_path.display()))?;
let hdhashvec: Vec<u8> = hdhash.clone().into();

Expand Down
3 changes: 2 additions & 1 deletion test/crau_verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::fs;

use update_format_crau::delta_update;

use anyhow::Context;
use argh::FromArgs;

const PUBKEY_FILE: &str = "../src/testdata/public_key_test_pkcs8.pem";
Expand Down Expand Up @@ -43,7 +44,7 @@ fn main() -> Result<(), Box<dyn Error>> {
let headerdatapath = tmpdir.join("ue_header_data");

// Get length of header and data, including header and manifest.
let header_data_length = delta_update::get_header_data_length(&header, &delta_archive_manifest);
let header_data_length = delta_update::get_header_data_length(&header, &delta_archive_manifest).context("failed to get header data length")?;
let hdhash = ue_rs::hash_on_disk_sha256(headerdatapath.as_path(), Some(header_data_length))?;
let hdhashvec: Vec<u8> = hdhash.clone().into();

Expand Down
16 changes: 8 additions & 8 deletions update-format-crau/src/delta_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::fs::File;
use std::path::Path;
use log::{debug, info};
use bzip2::read::BzDecoder;
use anyhow::{Context, Result, bail};
use anyhow::{Context, Result, anyhow, bail};

use protobuf::Message;

Expand Down Expand Up @@ -91,25 +91,25 @@ pub fn get_signatures_bytes<'a>(f: &'a mut BufReader<File>, header: &'a DeltaUpd
_ => None,
};

Ok(signatures_bytes.unwrap())
signatures_bytes.ok_or(anyhow!("failed to get signature bytes slice"))
}

// Return data length, including header and manifest.
pub fn get_header_data_length(header: &DeltaUpdateFileHeader, manifest: &proto::DeltaArchiveManifest) -> usize {
pub fn get_header_data_length(header: &DeltaUpdateFileHeader, manifest: &proto::DeltaArchiveManifest) -> Result<usize> {
// Read from the beginning of the stream, which means the whole buffer including
// delta update header as well as manifest. That is because data that must be verified
// with signatures start from the beginning.
//
// Payload data structure:
// | header | manifest | data blobs | signatures |

header.translate_offset(manifest.signatures_offset.unwrap()) as usize
Ok(header.translate_offset(manifest.signatures_offset.ok_or(anyhow!(""))?) as usize)
}

// Take a buffer reader, delta file header, manifest as input.
// Return path to data blobs, without header, manifest, or signatures.
pub fn get_data_blobs<'a>(f: &'a mut BufReader<File>, header: &'a DeltaUpdateFileHeader, manifest: &proto::DeltaArchiveManifest, tmpfile: &Path) -> Result<File> {
let tmpdir = tmpfile.parent().unwrap();
let tmpdir = tmpfile.parent().ok_or(anyhow!("unable to get parent directory"))?;
fs::create_dir_all(tmpdir).context(format!("failed to create directory {:?}", tmpdir))?;
let mut outfile = File::create(tmpfile).context(format!("failed to create file {:?}", tmpfile))?;

Expand All @@ -118,8 +118,8 @@ pub fn get_data_blobs<'a>(f: &'a mut BufReader<File>, header: &'a DeltaUpdateFil
// get_header_data_length.
// Iterate each partition_operations to get data offset and data length.
for pop in &manifest.partition_operations {
let data_offset = pop.data_offset.unwrap();
let data_length = pop.data_length.unwrap();
let data_offset = pop.data_offset.ok_or(anyhow!("unable to get data offset"))?;
let data_length = pop.data_length.ok_or(anyhow!("unable to get data length"))?;

let mut partdata = vec![0u8; data_length as usize];

Expand All @@ -128,7 +128,7 @@ pub fn get_data_blobs<'a>(f: &'a mut BufReader<File>, header: &'a DeltaUpdateFil
f.read_exact(&mut partdata).context(format!("failed to read data with length {:?}", data_length))?;

// In case of bzip2-compressed chunks, extract.
if pop.type_.unwrap() == proto::install_operation::Type::REPLACE_BZ.into() {
if pop.type_.ok_or(anyhow!("unable to get type_ from partition operations"))? == proto::install_operation::Type::REPLACE_BZ.into() {
let mut bzdecoder = BzDecoder::new(&partdata[..]);
let mut partdata_unpacked = Vec::new();
bzdecoder.read_to_end(&mut partdata_unpacked).context(format!("failed to unpack bzip2ed data at offset {:?}", translated_offset))?;
Expand Down
20 changes: 15 additions & 5 deletions update-format-crau/src/verify_sig.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use anyhow::{Context, Result, bail};
use anyhow::{Context, Result, anyhow, bail};
use rsa::{RsaPrivateKey, RsaPublicKey};
use rsa::pkcs1::{DecodeRsaPrivateKey, DecodeRsaPublicKey};
use rsa::pkcs8::{DecodePrivateKey, DecodePublicKey};
Expand Down Expand Up @@ -40,7 +40,12 @@ pub fn verify_rsa_pkcs_buf(databuf: &[u8], signature: &[u8], public_key: RsaPubl

let verifying_key = pkcs1v15::VerifyingKey::<Sha256>::new(public_key);

verifying_key.verify(databuf, &pkcs1v15::Signature::try_from(signature).unwrap()).context(format!("failed to verify signature ({:?})", signature))
verifying_key
.verify(
databuf,
&pkcs1v15::Signature::try_from(signature).context(anyhow!("unable to convert signature into pkcs1v15::Signature"))?,
)
.context(format!("failed to verify signature ({:?})", signature))
}

// Takes a data buffer, signature and a public key, to verify the data
Expand All @@ -52,11 +57,16 @@ pub fn verify_rsa_pkcs_buf(databuf: &[u8], signature: &[u8], public_key: RsaPubl
pub fn verify_rsa_pkcs_prehash(digestbuf: &[u8], signature: &[u8], public_key: RsaPublicKey) -> Result<()> {
let verifying_key = pkcs1v15::VerifyingKey::<Sha256>::new(public_key);

verifying_key.verify_prehash(digestbuf, &pkcs1v15::Signature::try_from(signature).unwrap()).context(format!("failed to verify_prehash signature ({:?})", signature))
verifying_key
.verify_prehash(
digestbuf,
&pkcs1v15::Signature::try_from(signature).context(anyhow!("unable to convert signature into pkcs1v15::Signature"))?,
)
.context(format!("failed to verify_prehash signature ({:?})", signature))
}

pub fn get_private_key_pkcs_pem(private_key_path: &str, key_type: KeyType) -> Result<RsaPrivateKey> {
let private_key_buf = fs::read_to_string(private_key_path).unwrap();
let private_key_buf = fs::read_to_string(private_key_path).context(format!("failed to read private key from path {:?}", private_key_path))?;
let out_key = match key_type {
KeyType::KeyTypePkcs1 => RsaPrivateKey::from_pkcs1_pem(private_key_buf.as_str()).or_else(|error| {
bail!("failed to parse PKCS1 PEM message: {:?}", error);
Expand All @@ -73,7 +83,7 @@ pub fn get_private_key_pkcs_pem(private_key_path: &str, key_type: KeyType) -> Re
}

pub fn get_public_key_pkcs_pem(public_key_path: &str, key_type: KeyType) -> Result<RsaPublicKey> {
let public_key_buf = fs::read_to_string(public_key_path).unwrap();
let public_key_buf = fs::read_to_string(public_key_path).context(format!("failed to read public key from path {:?}", public_key_path))?;
let out_key = match key_type {
KeyType::KeyTypePkcs1 => RsaPublicKey::from_pkcs1_pem(public_key_buf.as_str()).or_else(|error| {
bail!("failed to parse PKCS1 PEM message: {:?}", error);
Expand Down

0 comments on commit ac37636

Please sign in to comment.