Skip to content
This repository was archived by the owner on Nov 6, 2020. It is now read-only.

Pyethereum keystore support #9710

Merged
merged 24 commits into from
Jan 3, 2019
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9c860d6
support for keystore format produced by pyethereum lib + some debug msgs
tworec Aug 8, 2018
5e7e9ab
made salt unbound also for Scrypt
tworec Aug 9, 2018
cff6f23
made address optional and skip if its none
tworec Aug 9, 2018
012dd5b
ignore values of any type, not just i32
tworec Aug 9, 2018
09d5a66
WIP: local deps
tworec Aug 29, 2018
b86fcb0
cleanup
tworec Aug 30, 2018
e9f2d11
enable optional address + more cleanup
tworec Aug 31, 2018
b74936b
yet another cleanup
tworec Aug 31, 2018
216fcd3
enable keystore wo address import (using passwd)
tworec Oct 5, 2018
1ba0de3
cleanup
tworec Oct 5, 2018
0f31116
doc enchancement
tworec Oct 5, 2018
93f782b
parity/account fix
tworec Oct 9, 2018
fc34ade
redesign after review
tworec Nov 27, 2018
00a9fb7
Merge branch 'master' into pyethereum-keystore-support
tworec Nov 27, 2018
4133b73
fix indentation
tworec Nov 27, 2018
43c12d6
ignore just version in json under crypto
tworec Nov 27, 2018
1919bd5
remove unnecessary borrowing within match str
niklasad1 Dec 3, 2018
1c6cdbe
remove another unnecessary borrowing
niklasad1 Dec 3, 2018
69b4958
default derived instead of implementing
tworec Dec 3, 2018
0282536
log dependency removed
tworec Dec 3, 2018
b42f277
[ethstore] warn restored + env_logger initialized in CLI
tworec Dec 19, 2018
8736654
Merge branch 'master' into pyethereum-keystore-support
tworec Dec 28, 2018
53d4d84
applied suggestion from @tomusdrw
tworec Jan 2, 2019
97e0405
Merge branch 'master' into pyethereum-keystore-support
tworec Jan 3, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion ethkey/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ fn main() {
Ok(ok) => println!("{}", ok),
Err(Error::Docopt(ref e)) => e.exit(),
Err(err) => {
println!("{}", err);
error!("{}", err);
process::exit(1);
}
}
Expand Down
2 changes: 2 additions & 0 deletions ethstore/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ version = "0.1.0"
authors = ["Parity Technologies <[email protected]>"]

[dependencies]
log = "0.4"
env_logger = "0.5"
docopt = "0.8"
num_cpus = "1.6"
rustc-hex = "1.0"
Expand Down
15 changes: 10 additions & 5 deletions ethstore/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

extern crate dir;
extern crate docopt;
#[macro_use]
extern crate log;
extern crate env_logger;
extern crate ethstore;
extern crate num_cpus;
extern crate panic_hook;
Expand Down Expand Up @@ -45,7 +48,7 @@ Usage:
ethstore insert <secret> <password> [--dir DIR] [--vault VAULT] [--vault-pwd VAULTPWD]
ethstore change-pwd <address> <old-pwd> <new-pwd> [--dir DIR] [--vault VAULT] [--vault-pwd VAULTPWD]
ethstore list [--dir DIR] [--vault VAULT] [--vault-pwd VAULTPWD]
ethstore import [--src DIR] [--dir DIR]
ethstore import [<password>] [--src DIR] [--dir DIR]
ethstore import-wallet <path> <password> [--dir DIR] [--vault VAULT] [--vault-pwd VAULTPWD]
ethstore find-wallet-pass <path> <password>
ethstore remove <address> <password> [--dir DIR] [--vault VAULT] [--vault-pwd VAULTPWD]
Expand Down Expand Up @@ -146,12 +149,13 @@ impl fmt::Display for Error {

fn main() {
panic_hook::set_abort();
env_logger::try_init().expect("Logger initialized only once.");

match execute(env::args()) {
Ok(result) => println!("{}", result),
Err(Error::Docopt(ref e)) => e.exit(),
Err(err) => {
println!("{}", err);
error!("{}", err);
process::exit(1);
}
}
Expand Down Expand Up @@ -202,9 +206,9 @@ fn format_vaults(vaults: &[String]) -> String {
}

fn load_password(path: &str) -> Result<Password, Error> {
let mut file = fs::File::open(path).map_err(|e| ethstore::Error::Custom(format!("Error opening password file {}: {}", path, e)))?;
let mut file = fs::File::open(path).map_err(|e| ethstore::Error::Custom(format!("Error opening password file '{}': {}", path, e)))?;
let mut password = String::new();
file.read_to_string(&mut password).map_err(|e| ethstore::Error::Custom(format!("Error reading password file {}: {}", path, e)))?;
file.read_to_string(&mut password).map_err(|e| ethstore::Error::Custom(format!("Error reading password file '{}': {}", path, e)))?;
// drop EOF
let _ = password.pop();
Ok(password.into())
Expand Down Expand Up @@ -241,7 +245,8 @@ fn execute<S, I>(command: I) -> Result<String, Error> where I: IntoIterator<Item
} else if args.cmd_import {
let src = key_dir(&args.flag_src)?;
let dst = key_dir(&args.flag_dir)?;
let accounts = import_accounts(&*src, &*dst)?;
let password = load_password(&args.arg_password).ok();
let accounts = import_accounts(&*src, &*dst, &password)?;
Ok(format_accounts(&accounts))
} else if args.cmd_import_wallet {
let wallet = PresaleWallet::open(&args.arg_path)?;
Expand Down
5 changes: 3 additions & 2 deletions ethstore/src/account/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ impl Crypto {

// two parts of derived key
// DK = [ DK[0..15] DK[16..31] ] = [derived_left_bits, derived_right_bits]
let (derived_left_bits, derived_right_bits) = crypto::derive_key_iterations(password.as_bytes(), &salt, iterations);
let (derived_left_bits, derived_right_bits) =
crypto::derive_key_iterations(password.as_bytes(), &salt, iterations);

// preallocated (on-stack in case of `Secret`) buffer to hold cipher
// length = length(plain) as we are using CTR-approach
Expand All @@ -104,7 +105,7 @@ impl Crypto {
ciphertext: ciphertext.into_vec(),
kdf: Kdf::Pbkdf2(Pbkdf2 {
dklen: crypto::KEY_LENGTH as u32,
salt: salt,
salt: salt.to_vec(),
c: iterations,
prf: Prf::HmacSha256,
}),
Expand Down
4 changes: 2 additions & 2 deletions ethstore/src/account/kdf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub struct Pbkdf2 {
pub c: u32,
pub dklen: u32,
pub prf: Prf,
pub salt: [u8; 32],
pub salt: Vec<u8>,
}

#[derive(Debug, PartialEq, Clone)]
Expand All @@ -35,7 +35,7 @@ pub struct Scrypt {
pub p: u32,
pub n: u32,
pub r: u32,
pub salt: [u8; 32],
pub salt: Vec<u8>,
}

#[derive(Debug, PartialEq, Clone)]
Expand Down
33 changes: 22 additions & 11 deletions ethstore/src/account/safe_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl Into<json::KeyFile> for SafeAccount {
json::KeyFile {
id: From::from(self.id),
version: self.version.into(),
address: self.address.into(),
address: Some(self.address.into()),
crypto: self.crypto.into(),
name: Some(self.name.into()),
meta: Some(self.meta.into()),
Expand Down Expand Up @@ -76,17 +76,28 @@ impl SafeAccount {

/// Create a new `SafeAccount` from the given `json`; if it was read from a
/// file, the `filename` should be `Some` name. If it is as yet anonymous, then it
/// can be left `None`.
pub fn from_file(json: json::KeyFile, filename: Option<String>) -> Self {
SafeAccount {
/// can be left `None`. `password` is used for reading keystore files w/o address.
pub fn from_file(json: json::KeyFile, filename: Option<String>, password: &Option<Password>) -> Result<Self, Error> {
let crypto = Crypto::from(json.crypto);
let address = match json.address {
Some(address) => address.into(),
None => match password {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if the json file contains address and we provided a password? imo, this should result in an Error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I would silently ignore this. As for now all errors reading kestores are silently ignored, which is confusing me too, but to be consistent I propose to ignore it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sustain to ignore this case. There can be many key files in src dir, and just some of them need passwd.. rest should be ignored, even if rest is all.

Some(password) =>
KeyPair::from_secret(crypto.secret(&password).map_err(|_| Error::InvalidPassword)?)?.address(),
None =>
return Err(Error::Custom("This keystore does not contain address. You need to provide password to import it".to_owned()))
}
};

Ok(SafeAccount {
id: json.id.into(),
version: json.version.into(),
address: json.address.into(),
crypto: json.crypto.into(),
filename: filename,
address,
crypto,
filename,
name: json.name.unwrap_or(String::new()),
meta: json.meta.unwrap_or("{}".to_owned()),
}
})
}

/// Create a new `SafeAccount` from the given vault `json`; if it was read from a
Expand All @@ -97,14 +108,14 @@ impl SafeAccount {
let meta_plain = meta_crypto.decrypt(password)?;
let meta_plain = json::VaultKeyMeta::load(&meta_plain).map_err(|e| Error::Custom(format!("{:?}", e)))?;

Ok(SafeAccount::from_file(json::KeyFile {
SafeAccount::from_file(json::KeyFile {
id: json.id,
version: json.version,
crypto: json.crypto,
address: meta_plain.address,
address: Some(meta_plain.address),
name: meta_plain.name,
meta: meta_plain.meta,
}, filename))
}, filename, &None)
}

/// Create a new `VaultKeyFile` from the given `self`
Expand Down
27 changes: 18 additions & 9 deletions ethstore/src/accounts_dir/disk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use {json, SafeAccount, Error};
use json::Uuid;
use super::{KeyDirectory, VaultKeyDirectory, VaultKeyDirectoryProvider, VaultKey};
use super::vault::{VAULT_FILE_NAME, VaultDiskDirectory};
use ethkey::Password;

const IGNORED_FILES: &'static [&'static str] = &[
"thumbs.db",
Expand Down Expand Up @@ -104,8 +105,16 @@ pub type RootDiskDirectory = DiskDirectory<DiskKeyFileManager>;

/// Disk directory key file manager
pub trait KeyFileManager: Send + Sync {
/// Read `SafeAccount` from given key file stream with password
#[allow(unused_variables)]
fn read_with_password<T>(&self, filename: Option<String>, reader: T, password: &Option<Password>)
-> Result<SafeAccount, Error> where T: io::Read {
unimplemented!()
}
/// Read `SafeAccount` from given key file stream
fn read<T>(&self, filename: Option<String>, reader: T) -> Result<SafeAccount, Error> where T: io::Read;
fn read<T>(&self, filename: Option<String>, reader: T) -> Result<SafeAccount, Error> where T: io::Read {
self.read_with_password(filename, reader, &None)
}
/// Write `SafeAccount` to given key file stream
fn write<T>(&self, account: SafeAccount, writer: &mut T) -> Result<(), Error> where T: io::Write;
}
Expand Down Expand Up @@ -179,7 +188,7 @@ impl<T> DiskDirectory<T> where T: KeyFileManager {
}

/// all accounts found in keys directory
fn files_content(&self) -> Result<HashMap<PathBuf, SafeAccount>, Error> {
fn files_content(&self, password: &Option<Password>) -> Result<HashMap<PathBuf, SafeAccount>, Error> {
// it's not done using one iterator cause
// there is an issue with rustc and it takes tooo much time to compile
let paths = self.files()?;
Expand All @@ -189,9 +198,9 @@ impl<T> DiskDirectory<T> where T: KeyFileManager {
let filename = Some(path.file_name().and_then(|n| n.to_str()).expect("Keys have valid UTF8 names only.").to_owned());
fs::File::open(path.clone())
.map_err(Into::into)
.and_then(|file| self.key_manager.read(filename, file))
.and_then(|file| self.key_manager.read_with_password(filename, file, password))
.map_err(|err| {
warn!("Invalid key file: {:?} ({})", path, err);
error!("Invalid key file: {:?} ({})", path, err);
err
})
.map(|account| (path, account))
Expand Down Expand Up @@ -241,8 +250,8 @@ impl<T> DiskDirectory<T> where T: KeyFileManager {
}

impl<T> KeyDirectory for DiskDirectory<T> where T: KeyFileManager {
fn load(&self) -> Result<Vec<SafeAccount>, Error> {
let accounts = self.files_content()?
fn load_with_password(&self, password: &Option<Password>) -> Result<Vec<SafeAccount>, Error> {
let accounts = self.files_content(password)?
.into_iter()
.map(|(_, account)| account)
.collect();
Expand All @@ -263,7 +272,7 @@ impl<T> KeyDirectory for DiskDirectory<T> where T: KeyFileManager {
fn remove(&self, account: &SafeAccount) -> Result<(), Error> {
// enumerate all entries in keystore
// and find entry with given address
let to_remove = self.files_content()?
let to_remove = self.files_content(&None)?
.into_iter()
.find(|&(_, ref acc)| acc.id == account.id && acc.address == account.address);

Expand Down Expand Up @@ -317,9 +326,9 @@ impl<T> VaultKeyDirectoryProvider for DiskDirectory<T> where T: KeyFileManager {
}

impl KeyFileManager for DiskKeyFileManager {
fn read<T>(&self, filename: Option<String>, reader: T) -> Result<SafeAccount, Error> where T: io::Read {
fn read_with_password<T>(&self, filename: Option<String>, reader: T, password: &Option<Password>) -> Result<SafeAccount, Error> where T: io::Read {
let key_file = json::KeyFile::load(reader).map_err(|e| Error::Custom(format!("{:?}", e)))?;
Ok(SafeAccount::from_file(key_file, filename))
SafeAccount::from_file(key_file, filename, password)
}

fn write<T>(&self, mut account: SafeAccount, writer: &mut T) -> Result<(), Error> where T: io::Write {
Expand Down
9 changes: 8 additions & 1 deletion ethstore/src/accounts_dir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,15 @@ pub struct VaultKey {

/// Keys directory
pub trait KeyDirectory: Send + Sync {
/// Read keys from directory with password
#[allow(unused_variables)]
fn load_with_password(&self, password: &Option<Password>) -> Result<Vec<SafeAccount>, Error> {
unimplemented!()
}
/// Read keys from directory
fn load(&self) -> Result<Vec<SafeAccount>, Error>;
fn load(&self) -> Result<Vec<SafeAccount>, Error> {
self.load_with_password(&None)
}
/// Insert new key to directory
fn insert(&self, account: SafeAccount) -> Result<SafeAccount, Error>;
/// Update key in the directory
Expand Down
2 changes: 1 addition & 1 deletion ethstore/src/ethstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ impl SecretStore for EthStore {

fn import_wallet(&self, vault: SecretVaultRef, json: &[u8], password: &Password, gen_id: bool) -> Result<StoreAccountRef, Error> {
let json_keyfile = json::KeyFile::load(json).map_err(|_| Error::InvalidKeyFile("Invalid JSON format".to_owned()))?;
let mut safe_account = SafeAccount::from_file(json_keyfile, None);
let mut safe_account = SafeAccount::from_file(json_keyfile, None, &None)?;

if gen_id {
safe_account.id = Random::random();
Expand Down
10 changes: 6 additions & 4 deletions ethstore/src/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::collections::HashSet;
use std::path::Path;
use std::fs;

use ethkey::Address;
use ethkey::{Address, Password};
use accounts_dir::{KeyDirectory, RootDiskDirectory, DiskKeyFileManager, KeyFileManager};
use dir;
use Error;
Expand All @@ -40,9 +40,11 @@ pub fn import_account(path: &Path, dst: &KeyDirectory) -> Result<Address, Error>
}

/// Import all accounts from one directory to the other.
pub fn import_accounts(src: &KeyDirectory, dst: &KeyDirectory) -> Result<Vec<Address>, Error> {
let accounts = src.load()?;
let existing_accounts = dst.load()?.into_iter().map(|a| a.address).collect::<HashSet<_>>();
pub fn import_accounts(src: &KeyDirectory, dst: &KeyDirectory, password: &Option<Password>) -> Result<Vec<Address>, Error> {
let accounts = src.load_with_password(password)?;
let existing_accounts = dst.load()?.into_iter()
.map(|a| a.address)
.collect::<HashSet<_>>();

accounts.into_iter()
.filter(|a| !existing_accounts.contains(&a.address))
Expand Down
2 changes: 1 addition & 1 deletion ethstore/src/json/bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer};
use serde::de::Error;
use rustc_hex::{ToHex, FromHex, FromHexError};

#[derive(Debug, PartialEq)]
#[derive(Debug, PartialEq, Clone)]
pub struct Bytes(Vec<u8>);

impl ops::Deref for Bytes {
Expand Down
4 changes: 3 additions & 1 deletion ethstore/src/json/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ enum CryptoField {
Kdf,
KdfParams,
Mac,
Ignored,
}

impl<'a> Deserialize<'a> for CryptoField {
Expand Down Expand Up @@ -81,7 +82,7 @@ impl<'a> Visitor<'a> for CryptoFieldVisitor {
"kdf" => Ok(CryptoField::Kdf),
"kdfparams" => Ok(CryptoField::KdfParams),
"mac" => Ok(CryptoField::Mac),
_ => Err(Error::custom(format!("Unknown field: '{}'", value))),
_ => Ok(CryptoField::Ignored),
}
}
}
Expand Down Expand Up @@ -122,6 +123,7 @@ impl<'a> Visitor<'a> for CryptoVisitor {
Some(CryptoField::Kdf) => { kdf = Some(visitor.next_value()?); }
Some(CryptoField::KdfParams) => { kdfparams = Some(visitor.next_value()?); }
Some(CryptoField::Mac) => { mac = Some(visitor.next_value()?); }
Some(CryptoField::Ignored) => { visitor.next_value().unwrap_or(()) }
None => { break; }
}
}
Expand Down
6 changes: 3 additions & 3 deletions ethstore/src/json/kdf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
use std::fmt;
use serde::{Serialize, Serializer, Deserialize, Deserializer};
use serde::de::{Visitor, Error as SerdeError};
use super::{Error, H256};
use super::{Error, Bytes};

#[derive(Debug, PartialEq)]
pub enum KdfSer {
Expand Down Expand Up @@ -111,7 +111,7 @@ pub struct Pbkdf2 {
pub c: u32,
pub dklen: u32,
pub prf: Prf,
pub salt: H256,
pub salt: Bytes,
}

#[derive(Debug, PartialEq, Serialize, Deserialize)]
Expand All @@ -120,7 +120,7 @@ pub struct Scrypt {
pub p: u32,
pub n: u32,
pub r: u32,
pub salt: H256,
pub salt: Bytes,
}

#[derive(Debug, PartialEq)]
Expand Down
7 changes: 1 addition & 6 deletions ethstore/src/json/key_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub struct KeyFile {
pub id: Uuid,
pub version: Version,
pub crypto: Crypto,
pub address: H160,
pub address: Option<H160>,
pub name: Option<String>,
pub meta: Option<String>,
}
Expand Down Expand Up @@ -158,11 +158,6 @@ impl<'a> Visitor<'a> for KeyFileVisitor {
None => return Err(V::Error::missing_field("crypto")),
};

let address = match address {
Some(address) => address,
None => return Err(V::Error::missing_field("address")),
};

let result = KeyFile {
id: id,
version: version,
Expand Down
Loading