Skip to content

Commit

Permalink
Verify checksum after download, with retry
Browse files Browse the repository at this point in the history
The self.verify_checksum(...) call's return value wasn't checked in the
package download call. Even if we do it there we should rather move it
into the retry loop and make it explicit whether we expect certain
checksums or not.
Check the checksum after the download, and also retry when it
mismatches.
  • Loading branch information
pothos committed Jan 5, 2024
1 parent f88b0aa commit e27e533
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 11 deletions.
2 changes: 1 addition & 1 deletion examples/download_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ fn main() -> Result<(), Box<dyn Error>> {

let tempdir = tempfile::tempdir()?;
let path = tempdir.path().join("tmpfile");
let res = download_and_hash(&client, url, &path, false)?;
let res = download_and_hash(&client, url, &path, None, None, false)?;
tempdir.close()?;

println!("hash: {}", res.hash_sha256);
Expand Down
2 changes: 1 addition & 1 deletion examples/full_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fn main() -> Result<(), Box<dyn Error>> {

let tempdir = tempfile::tempdir()?;
let path = tempdir.path().join("tmpfile");
let res = ue_rs::download_and_hash(&client, url.clone(), &path, false).context(format!("download_and_hash({url:?}) failed"))?;
let res = ue_rs::download_and_hash(&client, url.clone(), &path, Some(expected_sha256.clone()), None, false).context(format!("download_and_hash({url:?}) failed"))?;
tempdir.close()?;

println!("\texpected sha256: {}", expected_sha256);
Expand Down
13 changes: 10 additions & 3 deletions src/bin/download_sysext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,14 @@ impl<'a> Package<'a> {
info!("downloading {}...", self.url);

let path = into_dir.join(&*self.name);
let res = match ue_rs::download_and_hash(client, self.url.clone(), &path, print_progress) {
match ue_rs::download_and_hash(
client,
self.url.clone(),
&path,
self.hash_sha256.clone(),
self.hash_sha1.clone(),
print_progress,
) {
Ok(ok) => ok,
Err(err) => {
error!("Downloading failed with error {}", err);
Expand All @@ -118,7 +125,7 @@ impl<'a> Package<'a> {
}
};

self.verify_checksum(res.hash_sha256, res.hash_sha1);
self.status = PackageStatus::Unverified;
Ok(())
}

Expand Down Expand Up @@ -249,7 +256,7 @@ where
U: reqwest::IntoUrl + From<U> + std::clone::Clone + std::fmt::Debug,
Url: From<U>,
{
let r = ue_rs::download_and_hash(client, input_url.clone(), path, print_progress).context(format!("unable to download data(url {:?})", input_url))?;
let r = ue_rs::download_and_hash(client, input_url.clone(), path, None, None, print_progress).context(format!("unable to download data(url {:?})", input_url))?;

Ok(Package {
name: Cow::Borrowed(path.file_name().unwrap_or(OsStr::new("fakepackage")).to_str().unwrap_or("fakepackage")),
Expand Down
52 changes: 46 additions & 6 deletions src/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use anyhow::{Context, Result, bail};
use std::io::{BufReader, Read};
use std::fs::File;
use std::path::Path;
use log::info;
use log::{info, debug};
use url::Url;

use reqwest::StatusCode;
Expand Down Expand Up @@ -59,7 +59,14 @@ pub fn hash_on_disk<T: omaha::HashAlgo>(path: &Path, maxlen: Option<usize>) -> R
Ok(omaha::Hash::from_bytes(Box::new(hasher).finalize()))
}

fn do_download_and_hash<U>(client: &Client, url: U, path: &Path, print_progress: bool) -> Result<DownloadResult>
fn do_download_and_hash<U>(
client: &Client,
url: U,
path: &Path,
expected_sha256: Option<omaha::Hash<omaha::Sha256>>,
expected_sha1: Option<omaha::Hash<omaha::Sha1>>,
print_progress: bool,
) -> Result<DownloadResult>
where
U: reqwest::IntoUrl + Clone,
Url: From<U>,
Expand Down Expand Up @@ -95,20 +102,53 @@ where
let mut file = File::create(path).context(format!("failed to create path ({:?})", path.display()))?;
res.copy_to(&mut file)?;

let calculated_sha256 = hash_on_disk::<omaha::Sha256>(path, None)?;
let calculated_sha1 = hash_on_disk::<omaha::Sha1>(path, None)?;

debug!(" expected sha256: {:?}", expected_sha256);
debug!(" calculated sha256: {}", calculated_sha256);
debug!(" sha256 match? {}", expected_sha256 == Some(calculated_sha256.clone()));
debug!(" expected sha1: {:?}", expected_sha1);
debug!(" calculated sha1: {}", calculated_sha1);
debug!(" sha1 match? {}", expected_sha1 == Some(calculated_sha1.clone()));

if expected_sha256.is_some() && expected_sha256 != Some(calculated_sha256.clone()) {
bail!("Checksum mismatch for sha256");
}
if expected_sha1.is_some() && expected_sha1 != Some(calculated_sha1.clone()) {
bail!("Checksum mismatch for sha1");
}

Ok(DownloadResult {
hash_sha256: hash_on_disk::<omaha::Sha256>(path, None)?,
hash_sha1: hash_on_disk::<omaha::Sha1>(path, None)?,
hash_sha256: calculated_sha256,
hash_sha1: calculated_sha1,
data: file,
})
}

pub fn download_and_hash<U>(client: &Client, url: U, path: &Path, print_progress: bool) -> Result<DownloadResult>
pub fn download_and_hash<U>(
client: &Client,
url: U,
path: &Path,
expected_sha256: Option<omaha::Hash<omaha::Sha256>>,
expected_sha1: Option<omaha::Hash<omaha::Sha1>>,
print_progress: bool,
) -> Result<DownloadResult>
where
U: reqwest::IntoUrl + Clone,
Url: From<U>,
{
crate::retry_loop(
|| do_download_and_hash(client, url.clone(), path, print_progress),
|| {
do_download_and_hash(
client,
url.clone(),
path,
expected_sha256.clone(),
expected_sha1.clone(),
print_progress,
)
},
MAX_DOWNLOAD_RETRY,
)
}

0 comments on commit e27e533

Please sign in to comment.