From d179bbfcac4d3dad0b93e2238457f87420200d33 Mon Sep 17 00:00:00 2001 From: Michael Dougherty Date: Wed, 6 Mar 2019 17:58:19 -0800 Subject: [PATCH 1/3] Allow dna/install_from_file to compare file hash with expected value --- conductor_api/src/conductor/admin.rs | 58 +++++++++++++++++++++++++- conductor_api/src/interface.rs | 1 + core_types/src/error/error.rs | 7 ++++ core_types/src/error/ribosome_error.rs | 1 + 4 files changed, 65 insertions(+), 2 deletions(-) diff --git a/conductor_api/src/conductor/admin.rs b/conductor_api/src/conductor/admin.rs index cee7edbab1..04f703943b 100644 --- a/conductor_api/src/conductor/admin.rs +++ b/conductor_api/src/conductor/admin.rs @@ -6,7 +6,9 @@ use crate::{ }, error::HolochainInstanceError, }; -use holochain_core_types::{cas::content::AddressableContent, error::HolochainError}; +use holochain_core_types::{ + cas::content::AddressableContent, error::HolochainError, hash::HashString, +}; use json_patch; use std::{fs, path::PathBuf, sync::Arc}; @@ -16,6 +18,7 @@ pub trait ConductorAdmin { path: PathBuf, id: String, copy: bool, + expected_hash: Option, properties: Option<&serde_json::Value>, ) -> Result<(), HolochainError>; fn uninstall_dna(&mut self, id: &String) -> Result<(), HolochainError>; @@ -65,6 +68,7 @@ impl ConductorAdmin for Conductor { path: PathBuf, id: String, copy: bool, + expected_hash: Option, properties: Option<&serde_json::Value>, ) -> Result<(), HolochainError> { let path_string = path @@ -79,6 +83,12 @@ impl ConductorAdmin for Conductor { )) })?; + if let Some(hash) = expected_hash { + if dna.address() != hash { + return Err(HolochainError::DnaHashMismatch(dna.address(), hash)); + } + } + if let Some(props) = properties { if !copy { return Err(HolochainError::ConfigError( @@ -673,6 +683,7 @@ pattern = '.*'"# new_dna_path.clone(), String::from("new-dna"), false, + None, None ), Ok(()), @@ -741,6 +752,7 @@ id = 'new-dna'"#, new_dna_path.clone(), String::from("new-dna"), true, + None, None ), Ok(()), @@ -779,6 +791,40 @@ id = 'new-dna'"#, assert!(output_dna_file.is_file()) } + #[test] + fn test_install_dna_with_expected_hash() { + let test_name = "test_install_dna_with_expected_hash"; + let mut conductor = create_test_conductor(test_name, 3000); + let mut new_dna_path = PathBuf::new(); + new_dna_path.push("new-dna.dna.json"); + let dna = Arc::get_mut(&mut conductor.dna_loader).unwrap()(&new_dna_path).unwrap(); + + assert_eq!( + conductor.install_dna_from_file( + new_dna_path.clone(), + String::from("new-dna"), + false, + Some(dna.address()), + None + ), + Ok(()), + ); + + assert_eq!( + conductor.install_dna_from_file( + new_dna_path.clone(), + String::from("new-dna"), + false, + Some("wrong-address".into()), + None + ), + Err(HolochainError::DnaHashMismatch( + dna.address(), + "wrong-address".into() + )), + ); + } + #[test] fn test_install_dna_from_file_with_properties() { let test_name = "test_install_dna_from_file_with_properties"; @@ -793,6 +839,7 @@ id = 'new-dna'"#, new_dna_path.clone(), String::from("new-dna-with-props"), false, + None, Some(&new_props) ), Err(HolochainError::ConfigError( @@ -805,6 +852,7 @@ id = 'new-dna'"#, new_dna_path.clone(), String::from("new-dna-with-props"), true, + None, Some(&new_props) ), Ok(()), @@ -854,7 +902,13 @@ id = 'new-dna'"#, let mut new_dna_path = PathBuf::new(); new_dna_path.push("new-dna.dna.json"); conductor - .install_dna_from_file(new_dna_path.clone(), String::from("new-dna"), false, None) + .install_dna_from_file( + new_dna_path.clone(), + String::from("new-dna"), + false, + None, + None, + ) .expect("Could not install DNA"); let add_result = conductor.add_instance( diff --git a/conductor_api/src/interface.rs b/conductor_api/src/interface.rs index 1b8c0ede5e..11efbcd4b8 100644 --- a/conductor_api/src/interface.rs +++ b/conductor_api/src/interface.rs @@ -397,6 +397,7 @@ impl ConductorApiBuilder { PathBuf::from(path), id.to_string(), copy, + None, properties ))?; Ok(json!({"success": true})) diff --git a/core_types/src/error/error.rs b/core_types/src/error/error.rs index d66bc68852..f1cbcb7710 100644 --- a/core_types/src/error/error.rs +++ b/core_types/src/error/error.rs @@ -4,6 +4,7 @@ use crate::{ json::*, }; use futures::channel::oneshot::Canceled as FutureCanceled; +use hash::HashString; use serde_json::Error as SerdeError; use std::{ error::Error, @@ -96,6 +97,7 @@ pub enum HolochainError { ConfigError(String), Timeout, InitializationFailed(String), + DnaHashMismatch(HashString, HashString), } pub type HcResult = Result; @@ -127,6 +129,11 @@ impl fmt::Display for HolochainError { ConfigError(err_msg) => write!(f, "{}", err_msg), Timeout => write!(f, "timeout"), InitializationFailed(err_msg) => write!(f, "{}", err_msg), + DnaHashMismatch(hash1, hash2) => write!( + f, + "DNA hash does not match expected hash!\n{} != {}", + hash1, hash2 + ), } } } diff --git a/core_types/src/error/ribosome_error.rs b/core_types/src/error/ribosome_error.rs index 97df5e43f9..989036a7e0 100644 --- a/core_types/src/error/ribosome_error.rs +++ b/core_types/src/error/ribosome_error.rs @@ -197,6 +197,7 @@ impl From for RibosomeErrorCode { HolochainError::ConfigError(_) => RibosomeErrorCode::Unspecified, HolochainError::Timeout => RibosomeErrorCode::Unspecified, HolochainError::InitializationFailed(_) => RibosomeErrorCode::Unspecified, + HolochainError::DnaHashMismatch(_, _) => RibosomeErrorCode::Unspecified, } } } From 20d424b21055f5cc4ff6a97906a07e3302a05350 Mon Sep 17 00:00:00 2001 From: Michael Dougherty Date: Wed, 6 Mar 2019 18:57:56 -0800 Subject: [PATCH 2/3] Expose expected_hash param to admin interface --- conductor_api/src/interface.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/conductor_api/src/interface.rs b/conductor_api/src/interface.rs index 11efbcd4b8..ca9e9fc108 100644 --- a/conductor_api/src/interface.rs +++ b/conductor_api/src/interface.rs @@ -392,12 +392,24 @@ impl ConductorApiBuilder { let id = Self::get_as_string("id", ¶ms_map)?; let path = Self::get_as_string("path", ¶ms_map)?; let copy = Self::get_as_bool("copy", ¶ms_map).unwrap_or(false); + let expected_hash = match params_map.get("expected_hash") { + Some(value) => Some( + value + .as_str() + .ok_or(jsonrpc_core::Error::invalid_params(format!( + "`{}` is not a valid json string", + &value + )))? + .into(), + ), + None => None, + }; let properties = params_map.get("properties"); conductor_call!(|c| c.install_dna_from_file( PathBuf::from(path), id.to_string(), copy, - None, + expected_hash, properties ))?; Ok(json!({"success": true})) From 3b1431c8ff81112683bd54f8af62d37713bb65bf Mon Sep 17 00:00:00 2001 From: Michael Dougherty Date: Thu, 7 Mar 2019 12:59:03 -0800 Subject: [PATCH 3/3] Update doc and changelog --- CHANGELOG.md | 1 + conductor_api/src/interface.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba049cf086..a1854bf167 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - get_as_type - Similar but for a single entry - link_entries_bidir - Same as link_entries but creates link in both directions - commit_and_link - Save a line and commit and link in a single function +- The `admin/dna/install_from_file` RPC method now takes an optional `expected_hash`, which performs an integrity check of the DNA file before installing it in the conductor [PR#1093](https://github.com/holochain/holochain-rust/pull/1093). ### Fixed - Validation of link entries gets retried now if base or target of the link were not yet accessible on the validating node. This fixes a bug where links have been invalid due to network timing issues [PR#1054](https://github.com/holochain/holochain-rust/pull/1054). diff --git a/conductor_api/src/interface.rs b/conductor_api/src/interface.rs index ca9e9fc108..70bda6a1d0 100644 --- a/conductor_api/src/interface.rs +++ b/conductor_api/src/interface.rs @@ -277,6 +277,7 @@ impl ConductorApiBuilder { /// Params: /// * `id`: [string] internal handle/name of the newly created DNA config /// * `path`: [string] local file path to DNA file + /// * `expected_hash`: [string] (optional) the hash of this DNA. If this does not match the actual hash, installation will fail. /// /// * `admin/dna/uninstall` /// Uninstalls a DNA from the conductor config. Recursively also removes (and stops)