From 1534bbb7cbae1e437def94485901f4a9f2f905c5 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 9 Feb 2017 18:47:22 +0300 Subject: [PATCH] Fix key.meta.vault for root dir keys && read vault.meta without vault key (#4482) * fix vault for root && read vault meta without key * support for old vaults (wthout meta field) --- ethstore/src/dir/disk.rs | 11 ++++- ethstore/src/dir/mod.rs | 2 + ethstore/src/dir/vault.rs | 52 ++++++++++------------ ethstore/src/ethstore.rs | 39 +++++++++++++++- ethstore/src/json/vault_file.rs | 27 ++++++++++- rpc/src/v1/tests/mocked/parity_accounts.rs | 18 ++++++++ 6 files changed, 118 insertions(+), 31 deletions(-) diff --git a/ethstore/src/dir/disk.rs b/ethstore/src/dir/disk.rs index 9162f8508..4a4637850 100755 --- a/ethstore/src/dir/disk.rs +++ b/ethstore/src/dir/disk.rs @@ -234,6 +234,10 @@ impl VaultKeyDirectoryProvider for DiskDirectory where T: KeyFileManager { }) .collect()) } + + fn vault_meta(&self, name: &str) -> Result { + VaultDiskDirectory::meta_at(&self.path, name) + } } impl KeyFileManager for DiskKeyFileManager { @@ -242,7 +246,12 @@ impl KeyFileManager for DiskKeyFileManager { Ok(SafeAccount::from_file(key_file, filename)) } - fn write(&self, account: SafeAccount, writer: &mut T) -> Result<(), Error> where T: io::Write { + fn write(&self, mut account: SafeAccount, writer: &mut T) -> Result<(), Error> where T: io::Write { + // when account is moved back to root directory from vault + // => remove vault field from meta + account.meta = json::remove_vault_name_from_json_meta(&account.meta) + .map_err(|err| Error::Custom(format!("{:?}", err)))?; + let key_file: json::KeyFile = account.into(); key_file.write(writer).map_err(|e| Error::Custom(format!("{:?}", e))) } diff --git a/ethstore/src/dir/mod.rs b/ethstore/src/dir/mod.rs index 79890650b..6e4326968 100755 --- a/ethstore/src/dir/mod.rs +++ b/ethstore/src/dir/mod.rs @@ -72,6 +72,8 @@ pub trait VaultKeyDirectoryProvider { fn open(&self, name: &str, key: VaultKey) -> Result, Error>; /// List all vaults fn list_vaults(&self) -> Result, Error>; + /// Get vault meta + fn vault_meta(&self, name: &str) -> Result; } /// Vault directory diff --git a/ethstore/src/dir/vault.rs b/ethstore/src/dir/vault.rs index 8699a9e49..2e777360a 100755 --- a/ethstore/src/dir/vault.rs +++ b/ethstore/src/dir/vault.rs @@ -67,11 +67,23 @@ impl VaultDiskDirectory { } // check that passed key matches vault file - let meta = read_vault_file(&vault_dir_path, &key)?; + let meta = read_vault_file(&vault_dir_path, Some(&key))?; Ok(DiskDirectory::new(vault_dir_path, VaultKeyFileManager::new(name, key, &meta))) } + /// Read vault meta without actually opening the vault + pub fn meta_at

(root: P, name: &str) -> Result where P: AsRef { + // check that vault directory exists + let vault_dir_path = make_vault_dir_path(root, name, true)?; + if !vault_dir_path.is_dir() { + return Err(Error::VaultNotFound); + } + + // check that passed key matches vault file + read_vault_file(&vault_dir_path, None) + } + fn create_temp_vault(&self, key: VaultKey) -> Result { let original_path = self.path().expect("self is instance of DiskDirectory; DiskDirectory always returns path; qed"); let mut path: PathBuf = original_path.clone(); @@ -241,7 +253,7 @@ fn create_vault_file

(vault_dir_path: P, key: &VaultKey, meta: &str) -> Result } /// When vault is opened => we must check that password matches && read metadata -fn read_vault_file

(vault_dir_path: P, key: &VaultKey) -> Result where P: AsRef { +fn read_vault_file

(vault_dir_path: P, key: Option<&VaultKey>) -> Result where P: AsRef { let mut vault_file_path: PathBuf = vault_dir_path.as_ref().into(); vault_file_path.push(VAULT_FILE_NAME); @@ -250,10 +262,12 @@ fn read_vault_file

(vault_dir_path: P, key: &VaultKey) -> Result Result { + // vault meta contains password hint + // => allow reading meta even if vault is not yet opened self.vaults.lock() .get(name) + .and_then(|v| Some(v.meta())) .ok_or(Error::VaultNotFound) - .and_then(|v| Ok(v.meta())) + .or_else(|_| { + let vault_provider = self.dir.as_vault_provider().ok_or(Error::VaultsAreNotSupported)?; + vault_provider.vault_meta(name) + }) + } fn set_vault_meta(&self, name: &str, meta: &str) -> Result<(), Error> { @@ -861,4 +868,34 @@ mod tests { assert!(opened_vaults.iter().any(|v| &*v == name1)); assert!(opened_vaults.iter().any(|v| &*v == name3)); } + + #[test] + fn should_manage_vaults_meta() { + // given + let mut dir = RootDiskDirectoryGuard::new(); + let store = EthStore::open(dir.key_dir.take().unwrap()).unwrap(); + let name1 = "vault1"; let password1 = "password1"; + + // when + store.create_vault(name1, password1).unwrap(); + + // then + assert_eq!(store.get_vault_meta(name1).unwrap(), "{}".to_owned()); + assert!(store.set_vault_meta(name1, "Hello, world!!!").is_ok()); + assert_eq!(store.get_vault_meta(name1).unwrap(), "Hello, world!!!".to_owned()); + + // and when + store.close_vault(name1).unwrap(); + store.open_vault(name1, password1).unwrap(); + + // then + assert_eq!(store.get_vault_meta(name1).unwrap(), "Hello, world!!!".to_owned()); + + // and when + store.close_vault(name1).unwrap(); + + // then + assert_eq!(store.get_vault_meta(name1).unwrap(), "Hello, world!!!".to_owned()); + assert!(store.get_vault_meta("vault2").is_err()); + } } diff --git a/ethstore/src/json/vault_file.rs b/ethstore/src/json/vault_file.rs index eb7440a85..c2d86cd0c 100755 --- a/ethstore/src/json/vault_file.rs +++ b/ethstore/src/json/vault_file.rs @@ -81,7 +81,7 @@ impl Visitor for VaultFileVisitor { loop { match visitor.visit_key()? { Some(VaultFileField::Crypto) => { crypto = Some(visitor.visit_value()?); }, - Some(VaultFileField::Meta) => { meta = Some(visitor.visit_value()?); } + Some(VaultFileField::Meta) => { meta = visitor.visit_value().ok(); }, // meta is optional None => { break; }, } } @@ -141,4 +141,29 @@ mod test { assert_eq!(file, deserialized); } + + #[test] + fn to_and_from_json_no_meta() { + let file = VaultFile { + crypto: Crypto { + cipher: Cipher::Aes128Ctr(Aes128Ctr { + iv: "0155e3690be19fbfbecabcd440aa284b".into(), + }), + ciphertext: "4d6938a1f49b7782".into(), + kdf: Kdf::Pbkdf2(Pbkdf2 { + c: 1024, + dklen: 32, + prf: Prf::HmacSha256, + salt: "b6a9338a7ccd39288a86dba73bfecd9101b4f3db9c9830e7c76afdbd4f6872e5".into(), + }), + mac: "16381463ea11c6eb2239a9f339c2e780516d29d234ce30ac5f166f9080b5a262".into(), + }, + meta: None, + }; + + let serialized = serde_json::to_string(&file).unwrap(); + let deserialized = serde_json::from_str(&serialized).unwrap(); + + assert_eq!(file, deserialized); + } } diff --git a/rpc/src/v1/tests/mocked/parity_accounts.rs b/rpc/src/v1/tests/mocked/parity_accounts.rs index ad0ead9c0..33671b161 100644 --- a/rpc/src/v1/tests/mocked/parity_accounts.rs +++ b/rpc/src/v1/tests/mocked/parity_accounts.rs @@ -314,6 +314,14 @@ fn rpc_parity_vault_adds_vault_field_to_acount_meta() { let response = format!(r#"{{"jsonrpc":"2.0","result":{{"0x{}":{{"meta":"{{\"vault\":\"vault1\"}}","name":"","uuid":"{}"}}}},"id":1}}"#, address1.hex(), uuid1); assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned())); + + // and then + assert!(tester.accounts.change_vault(address1, "").is_ok()); + + let request = r#"{"jsonrpc": "2.0", "method": "parity_allAccountsInfo", "params":[], "id": 1}"#; + let response = format!(r#"{{"jsonrpc":"2.0","result":{{"0x{}":{{"meta":"{{}}","name":"","uuid":"{}"}}}},"id":1}}"#, address1.hex(), uuid1); + + assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned())); } #[test] @@ -358,6 +366,14 @@ fn rpc_parity_get_set_vault_meta() { let tester = setup_with_vaults_support(temp_path.as_str()); assert!(tester.accounts.create_vault("vault1", "password1").is_ok()); + + // when no meta set + let request = r#"{"jsonrpc": "2.0", "method": "parity_getVaultMeta", "params":["vault1"], "id": 1}"#; + let response = r#"{"jsonrpc":"2.0","result":"{}","id":1}"#; + + assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned())); + + // when meta set assert!(tester.accounts.set_vault_meta("vault1", "vault1_meta").is_ok()); let request = r#"{"jsonrpc": "2.0", "method": "parity_getVaultMeta", "params":["vault1"], "id": 1}"#; @@ -365,11 +381,13 @@ fn rpc_parity_get_set_vault_meta() { assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned())); + // change meta let request = r#"{"jsonrpc": "2.0", "method": "parity_setVaultMeta", "params":["vault1", "updated_vault1_meta"], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":true,"id":1}"#; assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned())); + // query changed meta let request = r#"{"jsonrpc": "2.0", "method": "parity_getVaultMeta", "params":["vault1"], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":"updated_vault1_meta","id":1}"#;