diff --git a/Cargo.lock b/Cargo.lock index f18d2350bc..73aa6c2898 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -523,6 +523,8 @@ dependencies = [ "serde_json", "thiserror", "tokio", + "winapi", + "windows-acl", "zeroize", ] @@ -1047,6 +1049,16 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d0870c84016d4b481be5c9f323c24f65e31e901ae618f0e80f4308fb00de1d2d" +[[package]] +name = "field-offset" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "38e2275cc4e4fc009b0669731a1e5ab7ebf11f469eaede2bab9309a5b4d6057f" +dependencies = [ + "memoffset 0.9.0", + "rustc_version", +] + [[package]] name = "fixed-hash" version = "0.8.0" @@ -3761,6 +3773,12 @@ version = "0.25.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "14247bb57be4f377dfb94c72830b8ce8fc6beac03cf4bf7b9732eadd414123fc" +[[package]] +name = "widestring" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c168940144dd21fd8046987c16a46a33d5fc84eec29ef9dcddc2ac9e31526b7c" + [[package]] name = "winapi" version = "0.3.9" @@ -3805,6 +3823,18 @@ dependencies = [ "windows_x86_64_msvc 0.36.1", ] +[[package]] +name = "windows-acl" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "177b1723986bcb4c606058e77f6e8614b51c7f9ad2face6f6fd63dd5c8b3cec3" +dependencies = [ + "field-offset", + "libc", + "widestring", + "winapi", +] + [[package]] name = "windows-sys" version = "0.45.0" diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 42f8e1d971..6c0a28cc01 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Account::switch` command to allow changing accounts quickly; - UX improvements (Ctrl+l, TAB completion/suggestions and more) during interactive account management; - `WalletCommand::SetPow` command; +- Check for existing stronghold on `restore`; ### Changed @@ -38,6 +39,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `print_wallet_help` changed to `WalletCli::print_help`; - `print_account_help` changed to `AccountCli::print_help`; - `AccountCommand::Addresses` now prints an overview that includes NTs, NFTs, Aliases and Foundries; +- Restrict permissions of mnemonic file on Windows; ## 1.0.1 - 2023-MM-DD diff --git a/cli/Cargo.toml b/cli/Cargo.toml index be4f265c62..fd783982d0 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -44,3 +44,7 @@ serde_json = { version = "1.0.107", default-features = false } thiserror = { version = "1.0.48", default-features = false } tokio = { version = "1.32.0", default-features = false, features = ["fs"] } zeroize = { version = "1.6.0", default-features = false } + +[target.'cfg(target_os = "windows")'.dependencies] +winapi = { version = "0.3.9", default-features = false } +windows-acl = { version = "0.3.0", default-features = false } diff --git a/cli/src/command/wallet.rs b/cli/src/command/wallet.rs index 605baee51e..7d4c26f5a5 100644 --- a/cli/src/command/wallet.rs +++ b/cli/src/command/wallet.rs @@ -247,14 +247,22 @@ pub async fn node_info_command(storage_path: &Path) -> Result { pub async fn restore_command(storage_path: &Path, snapshot_path: &Path, backup_path: &Path) -> Result { check_file_exists(backup_path).await?; - let password = get_password("Stronghold password", false)?; - let secret_manager = SecretManager::Stronghold( - StrongholdSecretManager::builder() - .password(password.clone()) - .build(snapshot_path)?, - ); - let wallet = Wallet::builder() - .with_secret_manager(secret_manager) + let mut builder = Wallet::builder(); + if check_file_exists(snapshot_path).await.is_ok() { + println!( + "Detected a stronghold file at {}. Enter password to unlock:", + snapshot_path.to_str().unwrap() + ); + let password = get_password("Stronghold password", false)?; + let secret_manager = SecretManager::Stronghold( + StrongholdSecretManager::builder() + .password(password.clone()) + .build(snapshot_path)?, + ); + builder = builder.with_secret_manager(secret_manager); + } + + let wallet = builder // Will be overwritten by the backup's value. .with_client_options(ClientOptions::new().with_node(DEFAULT_NODE_URL)?) .with_storage_path(storage_path.to_str().expect("invalid unicode")) @@ -263,6 +271,7 @@ pub async fn restore_command(storage_path: &Path, snapshot_path: &Path, backup_p .finish() .await?; + let password = get_password("Stronghold backup password", false)?; wallet.restore_backup(backup_path.into(), password, None, None).await?; println_log_info!( diff --git a/cli/src/helper.rs b/cli/src/helper.rs index e0a031c28e..41669185c0 100644 --- a/cli/src/helper.rs +++ b/cli/src/helper.rs @@ -211,6 +211,83 @@ async fn write_mnemonic_to_file(path: &str, mnemonic: &str) -> Result<(), Error> let mut file = open_options.open(path).await?; file.write_all(format!("{mnemonic}\n").as_bytes()).await?; + #[cfg(windows)] + restrict_file_permissions(path)?; + + Ok(()) +} + +// Slightly modified from https://github.com/sile/sloggers/blob/master/src/permissions.rs +#[cfg(windows)] +pub fn restrict_file_permissions>(path: P) -> std::io::Result<()> { + use std::io; + + use winapi::um::winnt::{FILE_GENERIC_READ, FILE_GENERIC_WRITE, PSID, STANDARD_RIGHTS_ALL}; + use windows_acl::{ + acl::{AceType, ACL}, + helper::sid_to_string, + }; + + /// This is the security identifier in Windows for the owner of a file. See: + /// - https://docs.microsoft.com/en-us/troubleshoot/windows-server/identity/security-identifiers-in-windows#well-known-sids-all-versions-of-windows + const OWNER_SID_STR: &str = "S-1-3-4"; + /// We don't need any of the `AceFlags` listed here: + /// - https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-ace_header + const OWNER_ACL_ENTRY_FLAGS: u8 = 0; + /// Generic Rights: + /// - https://docs.microsoft.com/en-us/windows/win32/fileio/file-security-and-access-rights + /// Individual Read/Write/Execute Permissions (referenced in generic rights link): + /// - https://docs.microsoft.com/en-us/windows/win32/wmisdk/file-and-directory-access-rights-constants + /// STANDARD_RIGHTS_ALL + /// - https://docs.microsoft.com/en-us/windows/win32/secauthz/access-mask + const OWNER_ACL_ENTRY_MASK: u32 = FILE_GENERIC_READ | FILE_GENERIC_WRITE | STANDARD_RIGHTS_ALL; + + let path_str = path + .as_ref() + .to_str() + .ok_or_else(|| io::Error::new(io::ErrorKind::Other, "Unable to open file path.".to_string()))?; + + let mut acl = ACL::from_file_path(path_str, false) + .map_err(|e| io::Error::new(io::ErrorKind::Other, format!("Unable to retrieve ACL: {:?}", e)))?; + + let owner_sid = windows_acl::helper::string_to_sid(OWNER_SID_STR) + .map_err(|e| io::Error::new(io::ErrorKind::Other, format!("Unable to convert SID: {:?}", e)))?; + + let entries = acl.all().map_err(|e| { + io::Error::new( + io::ErrorKind::Other, + format!("Unable to enumerate ACL entries: {:?}", e), + ) + })?; + + // Add single entry for file owner. + acl.add_entry( + owner_sid.as_ptr() as PSID, + AceType::AccessAllow, + OWNER_ACL_ENTRY_FLAGS, + OWNER_ACL_ENTRY_MASK, + ) + .map_err(|e| { + io::Error::new( + io::ErrorKind::Other, + format!("Failed to add ACL entry for SID {} error={}", OWNER_SID_STR, e), + ) + })?; + // Remove all AccessAllow entries from the file that aren't the owner_sid. + for entry in &entries { + if let Some(ref entry_sid) = entry.sid { + let entry_sid_str = sid_to_string(entry_sid.as_ptr() as PSID).unwrap_or_else(|_| "BadFormat".to_string()); + if entry_sid_str != OWNER_SID_STR { + acl.remove(entry_sid.as_ptr() as PSID, Some(AceType::AccessAllow), None) + .map_err(|_| { + io::Error::new( + io::ErrorKind::Other, + format!("Failed to remove ACL entry for SID {}", entry_sid_str), + ) + })?; + } + } + } Ok(()) } diff --git a/sdk/src/client/stronghold/mod.rs b/sdk/src/client/stronghold/mod.rs index 7cd0552567..f91b41a3b7 100644 --- a/sdk/src/client/stronghold/mod.rs +++ b/sdk/src/client/stronghold/mod.rs @@ -182,6 +182,16 @@ impl StrongholdAdapterBuilder { /// [`password()`]: Self::password() /// [`timeout()`]: Self::timeout() pub fn build>(self, snapshot_path: P) -> Result { + if snapshot_path.as_ref().is_dir() { + // TODO: Add Error in 2.0 as its breaking. + // Issue #1197 + return Err(std::io::Error::new( + std::io::ErrorKind::Other, + format!("Path is not a file: {:?}", snapshot_path.as_ref().to_path_buf()), + ) + .into()); + } + // In any case, Stronghold - as a necessary component - needs to be present at this point. let stronghold = self.stronghold.unwrap_or_default(); @@ -484,6 +494,16 @@ impl StrongholdAdapter { /// /// [`unload_stronghold_snapshot()`]: Self::unload_stronghold_snapshot() pub async fn write_stronghold_snapshot(&self, snapshot_path: Option<&Path>) -> Result<(), Error> { + if let Some(p) = snapshot_path { + if p.is_dir() { + // TODO: Add Error in 2.0 as its breaking. + // Issue #1197 + return Err( + std::io::Error::new(std::io::ErrorKind::Other, format!("Path is not a file: {:?}", p)).into(), + ); + } + } + // The key needs to be supplied first. let locked_key_provider = self.key_provider.lock().await; let key_provider = if let Some(key_provider) = &*locked_key_provider {