From 4a89970d22648667a6999f133e87c0f37a58cfb4 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Thu, 14 Jun 2018 16:13:37 +0800 Subject: [PATCH 1/2] Atomic create new files with permissions to owner in ethstore --- ethstore/src/accounts_dir/disk.rs | 34 +++++++++++++++---------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/ethstore/src/accounts_dir/disk.rs b/ethstore/src/accounts_dir/disk.rs index 79e8c0f4c3a..b1cadb38e0f 100644 --- a/ethstore/src/accounts_dir/disk.rs +++ b/ethstore/src/accounts_dir/disk.rs @@ -33,22 +33,25 @@ const IGNORED_FILES: &'static [&'static str] = &[ "vault.json", ]; -#[cfg(not(windows))] -fn restrict_permissions_to_owner(file_path: &Path) -> Result<(), i32> { - use std::ffi; + +#[cfg(unix)] +fn create_new_file_with_permissions_to_owner(file_path: &Path) -> io::Result { use libc; + use std::os::unix::fs::OpenOptionsExt; - let cstr = ffi::CString::new(&*file_path.to_string_lossy()) - .map_err(|_| -1)?; - match unsafe { libc::chmod(cstr.as_ptr(), libc::S_IWUSR | libc::S_IRUSR) } { - 0 => Ok(()), - x => Err(x), - } + fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(libc::S_IWUSR | libc::S_IRUSR) + .open(file_path) } -#[cfg(windows)] -fn restrict_permissions_to_owner(_file_path: &Path) -> Result<(), i32> { - Ok(()) +#[cfg(not(unix))] +fn create_new_file_with_permissions_to_owner(file_path: &Path) -> io::Result { + fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(file_path) } /// Root keys directory implementation @@ -173,17 +176,12 @@ impl DiskDirectory where T: KeyFileManager { { // save the file - let mut file = fs::File::create(&keyfile_path)?; + let mut file = create_new_file_with_permissions_to_owner(&keyfile_path)?; // write key content self.key_manager.write(original_account, &mut file).map_err(|e| Error::Custom(format!("{:?}", e)))?; file.flush()?; - - if let Err(_) = restrict_permissions_to_owner(keyfile_path.as_path()) { - return Err(Error::Io(io::Error::last_os_error())); - } - file.sync_all()?; } From 95bb4a885ebc0374cdb1d5a0deb638afa421aba2 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Thu, 14 Jun 2018 17:35:17 +0800 Subject: [PATCH 2/2] Allow replacing existing files We have two behaviors for `insert_with_filename` depending on whether `dedup` is true. Add `replace_file_with_permissions_to_owner` which use `OpenOptions::create(true)` instead of `create_new`. --- ethstore/src/accounts_dir/disk.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/ethstore/src/accounts_dir/disk.rs b/ethstore/src/accounts_dir/disk.rs index b1cadb38e0f..a5ce9c1a9c6 100644 --- a/ethstore/src/accounts_dir/disk.rs +++ b/ethstore/src/accounts_dir/disk.rs @@ -54,6 +54,24 @@ fn create_new_file_with_permissions_to_owner(file_path: &Path) -> io::Result io::Result { + use libc; + use std::os::unix::fs::PermissionsExt; + + let file = fs::File::create(file_path)?; + let mut permissions = file.metadata()?.permissions(); + permissions.set_mode(libc::S_IWUSR | libc::S_IRUSR); + file.set_permissions(permissions)?; + + Ok(file) +} + +#[cfg(not(unix))] +fn replace_file_with_permissions_to_owner(file_path: &Path) -> io::Result { + fs::File::create(file_path) +} + /// Root keys directory implementation pub type RootDiskDirectory = DiskDirectory; @@ -176,7 +194,11 @@ impl DiskDirectory where T: KeyFileManager { { // save the file - let mut file = create_new_file_with_permissions_to_owner(&keyfile_path)?; + let mut file = if dedup { + create_new_file_with_permissions_to_owner(&keyfile_path)? + } else { + replace_file_with_permissions_to_owner(&keyfile_path)? + }; // write key content self.key_manager.write(original_account, &mut file).map_err(|e| Error::Custom(format!("{:?}", e)))?;