From e869fc3da5595c5d0788252c2e4523a1cd0e386d Mon Sep 17 00:00:00 2001 From: Eric Seidel Date: Fri, 24 Mar 2023 15:53:40 -0700 Subject: [PATCH] feat: Teach rust side to validate hashes when installing (#171) --- updater/library/Cargo.toml | 6 ++-- updater/library/src/cache.rs | 11 ------ updater/library/src/network.rs | 3 +- updater/library/src/updater.rs | 62 ++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 14 deletions(-) diff --git a/updater/library/Cargo.toml b/updater/library/Cargo.toml index f193d3a1..a3d00b23 100644 --- a/updater/library/Cargo.toml +++ b/updater/library/Cargo.toml @@ -27,8 +27,6 @@ log = "0.4.14" once_cell = "1.17.1" # For reading shorebird.yaml serde_yaml = "0.9.19" -# For validating hashes of downloaded patch files. -sha2 = "0.10.6" # For inflating compressed patch files. bipatch = "1.0.0" # comde is a wrapper around several compression libraries. @@ -36,6 +34,10 @@ bipatch = "1.0.0" comde = {version = "0.2.3", default-features = false, features = ["zstandard"]} # Pipe is a simple in-memory pipe implementation, there might be a std way too? pipe = "0.4.0" +# For computing hashes of patch files for validation. +sha2 = "0.10.6" +# For decoding the hex-encoded hashes in Patch network responses. +hex = "0.4.3" [target.'cfg(target_os = "android")'.dependencies] # For logging to Android logcat. diff --git a/updater/library/src/cache.rs b/updater/library/src/cache.rs index 6ebe3522..e1ac6580 100644 --- a/updater/library/src/cache.rs +++ b/updater/library/src/cache.rs @@ -70,17 +70,6 @@ impl UpdaterState { } } -// fn compute_hash(path: &Path) -> anyhow::Result { -// use sha2::{Digest, Sha256}; -// use std::{fs, io}; - -// let mut file = fs::File::open(&path)?; -// let mut hasher = Sha256::new(); -// let n = io::copy(&mut file, &mut hasher)?; -// let hash = hasher.finalize(); -// Ok(format!("{:x}", hash)) -// } - impl UpdaterState { pub fn is_known_good_patch(&self, patch: &PatchInfo) -> bool { self.successful_patches.iter().any(|v| v == &patch.number) diff --git a/updater/library/src/network.rs b/updater/library/src/network.rs index e82b218e..e1fb00eb 100644 --- a/updater/library/src/network.rs +++ b/updater/library/src/network.rs @@ -19,7 +19,8 @@ pub struct Patch { /// The patch number. Starts at 1 for each new release and increases /// monotonically. pub number: usize, - /// The hash of the final uncompressed patch file. + /// The hex-encoded sha256 hash of the final uncompressed patch file. + /// Legacy: originally "#" before we implemented hash checks (remove). pub hash: String, /// The URL to download the patch file from. pub download_url: String, diff --git a/updater/library/src/updater.rs b/updater/library/src/updater.rs index bc4c16f6..1b09e7f3 100644 --- a/updater/library/src/updater.rs +++ b/updater/library/src/updater.rs @@ -97,6 +97,39 @@ pub fn check_for_update() -> bool { return with_config(check_for_update_internal); } +fn check_hash(path: &Path, expected_string: &str) -> anyhow::Result { + let result = hex::decode(expected_string); + // Remove this legacy behavior. + if result.is_err() { + warn!("Failed to decode hash from server, allowing: {expected_string}"); + return Ok(true); + } + let expected = result.unwrap(); + + use sha2::{Digest, Sha256}; + use std::{fs, io}; + // Based on guidance from: + // https://github.com/RustCrypto/hashes#hashing-readable-objects + + let mut file = fs::File::open(&path)?; + let mut hasher = Sha256::new(); + io::copy(&mut file, &mut hasher)?; + // Check that the length from copy is the same as the file size? + let hash = hasher.finalize(); + let hash_matches = hash.as_slice() == expected; + if !hash_matches { + warn!( + "Hash mismatch: {:?}, expected: {}, got: {:?}", + path, + expected_string, + hex::encode(hash) + ); + } else { + info!("Hash match: {:?}", path); + } + return Ok(hash_matches); +} + fn update_internal(config: &ResolvedConfig) -> anyhow::Result { // Load the state from disk. let mut state = UpdaterState::load_or_new_on_error(&config.cache_dir, &config.release_version); @@ -121,6 +154,10 @@ fn update_internal(config: &ResolvedConfig) -> anyhow::Result { } // Check the hash before moving into place. + let hash_ok = check_hash(&download_path, &patch.hash)?; + if !hash_ok { + return Err(UpdateError::InvalidState("Hash mismatch".to_string()).into()); + } // Move/state update should be "atomic". // Consider supporting allowing the system to download for us (e.g. iOS). @@ -129,6 +166,7 @@ fn update_internal(config: &ResolvedConfig) -> anyhow::Result { number: patch.number, }; state.install_patch(patch_info)?; + info!("Patch {} successfully installed.", patch.number); // Set the state to "restart required". return Ok(UpdateStatus::UpdateInstalled); @@ -317,4 +355,28 @@ mod tests { // ask for current patch (should get none). assert!(crate::active_patch().is_none()); } + + #[test] + fn hash_matches() { + let tmp_dir = TempDir::new("example").unwrap(); + + let input_path = tmp_dir.path().join("input"); + std::fs::write(&input_path, "hello world").unwrap(); + + let expected = "b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9"; + assert!(super::check_hash(&input_path, expected).unwrap()); + + // modify hash to not match + let expected = "a94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9"; + assert_eq!(super::check_hash(&input_path, expected).unwrap(), false); + + // invalid hashes should not match either + // Except for now they do (legacy behavior). + let expected = "foo"; + assert_eq!(super::check_hash(&input_path, expected).unwrap(), true); + + // Remove this case when legacy clients are gone. + let expected = "#"; + assert!(super::check_hash(&input_path, expected).unwrap()); + } }