diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml index 75b01606932..5c46cee66c1 100644 --- a/.github/workflows/test-suite.yml +++ b/.github/workflows/test-suite.yml @@ -44,6 +44,20 @@ jobs: run: sudo npm install -g ganache-cli - name: Run tests in release run: make test-release + release-tests-windows: + name: release-tests-windows + runs-on: windows-latest + needs: cargo-fmt + steps: + - uses: actions/checkout@v1 + - name: Get latest version of stable Rust + run: rustup update stable + - name: Install ganache-cli + run: npm install -g ganache-cli + - name: Install make + run: choco install -y make + - name: Run tests in release + run: make test-release debug-tests-ubuntu: name: debug-tests-ubuntu runs-on: ubuntu-latest diff --git a/Cargo.lock b/Cargo.lock index a0ced6cf841..f2a0f1adcc4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -19,6 +19,7 @@ dependencies = [ "eth2_ssz_derive", "eth2_wallet", "eth2_wallet_manager", + "filesystem", "futures 0.3.14", "hex", "libc", @@ -44,6 +45,7 @@ dependencies = [ "directory", "eth2_keystore", "eth2_wallet", + "filesystem", "rand 0.7.3", "regex", "rpassword", @@ -986,7 +988,7 @@ version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cc38c385bfd7e444464011bb24820f40dd1c76bcdfa1b78611cb7c2e5cafab75" dependencies = [ - "rustc_version", + "rustc_version 0.2.3", ] [[package]] @@ -2242,6 +2244,24 @@ dependencies = [ "subtle 2.4.0", ] +[[package]] +name = "field-offset" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf539fba70056b50f40a22e0da30639518a12ee18c35807858a63b158cb6dde7" +dependencies = [ + "memoffset", + "rustc_version 0.3.3", +] + +[[package]] +name = "filesystem" +version = "0.1.0" +dependencies = [ + "winapi 0.3.9", + "windows-acl", +] + [[package]] name = "fixed-hash" version = "0.6.1" @@ -4480,6 +4500,15 @@ version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d4fd5641d01c8f18a23da7b6fe29298ff4b55afcccdf78973b24cf3175fee32e" +[[package]] +name = "pest" +version = "2.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "10f4872ae94d7b90ae48754df22fd42ad52ce740b8f370b03da4835417403e53" +dependencies = [ + "ucd-trie", +] + [[package]] name = "petgraph" version = "0.5.1" @@ -4734,7 +4763,7 @@ dependencies = [ "byteorder", "libc", "nom", - "rustc_version", + "rustc_version 0.2.3", ] [[package]] @@ -5189,6 +5218,8 @@ dependencies = [ "tempfile", "tokio 1.5.0", "types", + "winapi 0.3.9", + "windows-acl", ] [[package]] @@ -5336,7 +5367,16 @@ version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "138e3e0acb6c9fb258b19b67cb8abd63c00679d2851805ea151465464fe9030a" dependencies = [ - "semver", + "semver 0.9.0", +] + +[[package]] +name = "rustc_version" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f0dfe2087c51c460008730de8b57e6a320782fbfb312e1f4d520e6c6fae155ee" +dependencies = [ + "semver 0.11.0", ] [[package]] @@ -5503,7 +5543,16 @@ version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d7eb9ef2c18661902cc47e535f9bc51b78acd254da71d375c2f6720d9a40403" dependencies = [ - "semver-parser", + "semver-parser 0.7.0", +] + +[[package]] +name = "semver" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f301af10236f6df4160f7c3f04eec6dbc70ace82d23326abad5edee88801c6b6" +dependencies = [ + "semver-parser 0.10.2", ] [[package]] @@ -5512,6 +5561,15 @@ version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" +[[package]] +name = "semver-parser" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "00b0bef5b7f9e0df16536d3961cfb6e84331c065b4066afb39768d0e319411f7" +dependencies = [ + "pest", +] + [[package]] name = "sensitive_url" version = "0.1.0" @@ -5761,6 +5819,7 @@ dependencies = [ "byteorder", "eth2_ssz", "eth2_ssz_derive", + "filesystem", "flate2", "lazy_static", "lighthouse_metrics", @@ -5803,6 +5862,7 @@ dependencies = [ name = "slashing_protection" version = "0.1.0" dependencies = [ + "filesystem", "parking_lot", "r2d2", "r2d2_sqlite", @@ -5956,7 +6016,7 @@ dependencies = [ "rand 0.7.3", "rand_core 0.5.1", "ring", - "rustc_version", + "rustc_version 0.2.3", "sha2 0.9.3", "subtle 2.4.0", "x25519-dalek", @@ -6063,7 +6123,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d022496b16281348b52d0e30ae99e01a73d737b2f45d38fed4edf79f9325a1d5" dependencies = [ "discard", - "rustc_version", + "rustc_version 0.2.3", "stdweb-derive", "stdweb-internal-macros", "stdweb-internal-runtime", @@ -6903,6 +6963,12 @@ dependencies = [ "tree_hash_derive", ] +[[package]] +name = "ucd-trie" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56dee185309b50d1f11bfedef0fe6d036842e3fb77413abef29f8f8d1c5d4c1c" + [[package]] name = "uint" version = "0.8.5" @@ -7058,6 +7124,7 @@ dependencies = [ "eth2_ssz_derive", "exit-future", "fallback", + "filesystem", "futures 0.3.14", "hex", "hyper 0.14.7", @@ -7102,6 +7169,7 @@ dependencies = [ "deposit_contract", "derivative", "eth2_keystore", + "filesystem", "hex", "lockfile", "rand 0.7.3", @@ -7420,6 +7488,12 @@ dependencies = [ "libc", ] +[[package]] +name = "widestring" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c168940144dd21fd8046987c16a46a33d5fc84eec29ef9dcddc2ac9e31526b7c" + [[package]] name = "winapi" version = "0.2.8" @@ -7463,6 +7537,18 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[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 0.3.9", +] + [[package]] name = "winreg" version = "0.7.0" diff --git a/account_manager/Cargo.toml b/account_manager/Cargo.toml index 5cd5d03988b..4986151c038 100644 --- a/account_manager/Cargo.toml +++ b/account_manager/Cargo.toml @@ -34,6 +34,7 @@ slashing_protection = { path = "../validator_client/slashing_protection" } eth2 = {path = "../common/eth2"} safe_arith = {path = "../consensus/safe_arith"} slot_clock = { path = "../common/slot_clock" } +filesystem = { path = "../common/filesystem" } sensitive_url = { path = "../common/sensitive_url" } [dev-dependencies] diff --git a/account_manager/src/validator/create.rs b/account_manager/src/validator/create.rs index 7960c55baee..5099c2de5e2 100644 --- a/account_manager/src/validator/create.rs +++ b/account_manager/src/validator/create.rs @@ -105,6 +105,8 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { ) .arg( Arg::with_name(STDIN_INPUTS_FLAG) + .takes_value(false) + .hidden(cfg!(windows)) .long(STDIN_INPUTS_FLAG) .help("If present, read all user inputs from stdin instead of tty."), ) @@ -118,7 +120,8 @@ pub fn cli_run( let spec = env.core_context().eth2_config.spec; let name: Option = clap_utils::parse_optional(matches, WALLET_NAME_FLAG)?; - let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG); + let stdin_inputs = cfg!(windows) || matches.is_present(STDIN_INPUTS_FLAG); + let wallet_base_dir = if matches.value_of("datadir").is_some() { let path: PathBuf = clap_utils::parse_required(matches, "datadir")?; path.join(DEFAULT_WALLET_DIR) diff --git a/account_manager/src/validator/exit.rs b/account_manager/src/validator/exit.rs index 0cf066610a2..6c15615af46 100644 --- a/account_manager/src/validator/exit.rs +++ b/account_manager/src/validator/exit.rs @@ -61,6 +61,8 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { ) .arg( Arg::with_name(STDIN_INPUTS_FLAG) + .takes_value(false) + .hidden(cfg!(windows)) .long(STDIN_INPUTS_FLAG) .help("If present, read all user inputs from stdin instead of tty."), ) @@ -70,7 +72,8 @@ pub fn cli_run(matches: &ArgMatches, env: Environment) -> Result< let keystore_path: PathBuf = clap_utils::parse_required(matches, KEYSTORE_FLAG)?; let password_file_path: Option = clap_utils::parse_optional(matches, PASSWORD_FILE_FLAG)?; - let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG); + + let stdin_inputs = cfg!(windows) || matches.is_present(STDIN_INPUTS_FLAG); let no_wait = matches.is_present(NO_WAIT); let spec = env.eth2_config().spec.clone(); diff --git a/account_manager/src/validator/import.rs b/account_manager/src/validator/import.rs index 7ce73076c9f..96da154031d 100644 --- a/account_manager/src/validator/import.rs +++ b/account_manager/src/validator/import.rs @@ -57,6 +57,8 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { ) .arg( Arg::with_name(STDIN_INPUTS_FLAG) + .takes_value(false) + .hidden(cfg!(windows)) .long(STDIN_INPUTS_FLAG) .help("If present, read all user inputs from stdin instead of tty."), ) @@ -83,7 +85,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), String> { let keystore: Option = clap_utils::parse_optional(matches, KEYSTORE_FLAG)?; let keystores_dir: Option = clap_utils::parse_optional(matches, DIR_FLAG)?; - let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG); + let stdin_inputs = cfg!(windows) || matches.is_present(STDIN_INPUTS_FLAG); let reuse_password = matches.is_present(REUSE_PASSWORD_FLAG); let keystore_password_path: Option = clap_utils::parse_optional(matches, PASSWORD_FLAG)?; diff --git a/account_manager/src/validator/recover.rs b/account_manager/src/validator/recover.rs index 27d8aa71d4f..d9b05e7756e 100644 --- a/account_manager/src/validator/recover.rs +++ b/account_manager/src/validator/recover.rs @@ -71,6 +71,8 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { ) .arg( Arg::with_name(STDIN_INPUTS_FLAG) + .takes_value(false) + .hidden(cfg!(windows)) .long(STDIN_INPUTS_FLAG) .help("If present, read all user inputs from stdin instead of tty."), ) @@ -86,7 +88,7 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin let first_index: u32 = clap_utils::parse_required(matches, FIRST_INDEX_FLAG)?; let count: u32 = clap_utils::parse_required(matches, COUNT_FLAG)?; let mnemonic_path: Option = clap_utils::parse_optional(matches, MNEMONIC_FLAG)?; - let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG); + let stdin_inputs = cfg!(windows) || matches.is_present(STDIN_INPUTS_FLAG); eprintln!("secrets-dir path: {:?}", secrets_dir); diff --git a/account_manager/src/wallet/create.rs b/account_manager/src/wallet/create.rs index 3965443cdf7..1390f35033c 100644 --- a/account_manager/src/wallet/create.rs +++ b/account_manager/src/wallet/create.rs @@ -9,11 +9,9 @@ use eth2_wallet::{ PlainText, }; use eth2_wallet_manager::{LockedWallet, WalletManager, WalletType}; +use filesystem::create_with_600_perms; use std::ffi::OsStr; use std::fs; -use std::fs::File; -use std::io::prelude::*; -use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; pub const CMD: &str = "create"; @@ -83,6 +81,8 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { ) .arg( Arg::with_name(STDIN_INPUTS_FLAG) + .takes_value(false) + .hidden(cfg!(windows)) .long(STDIN_INPUTS_FLAG) .help("If present, read all user inputs from stdin instead of tty."), ) @@ -153,8 +153,7 @@ pub fn create_wallet_from_mnemonic( let name: Option = clap_utils::parse_optional(matches, NAME_FLAG)?; let wallet_password_path: Option = clap_utils::parse_optional(matches, PASSWORD_FLAG)?; let type_field: String = clap_utils::parse_required(matches, TYPE_FLAG)?; - let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG); - + let stdin_inputs = cfg!(windows) || matches.is_present(STDIN_INPUTS_FLAG); let wallet_type = match type_field.as_ref() { HD_TYPE => WalletType::Hd, unknown => return Err(format!("--{} {} is not supported", TYPE_FLAG, unknown)), @@ -237,26 +236,3 @@ pub fn read_new_wallet_password_from_cli( }, } } - -/// Creates a file with `600 (-rw-------)` permissions. -pub fn create_with_600_perms>(path: P, bytes: &[u8]) -> Result<(), String> { - let path = path.as_ref(); - - let mut file = - File::create(&path).map_err(|e| format!("Unable to create {:?}: {}", path, e))?; - - let mut perm = file - .metadata() - .map_err(|e| format!("Unable to get {:?} metadata: {}", path, e))? - .permissions(); - - perm.set_mode(0o600); - - file.set_permissions(perm) - .map_err(|e| format!("Unable to set {:?} permissions: {}", path, e))?; - - file.write_all(bytes) - .map_err(|e| format!("Unable to write to {:?}: {}", path, e))?; - - Ok(()) -} diff --git a/account_manager/src/wallet/recover.rs b/account_manager/src/wallet/recover.rs index 0ac30fe27ab..ec4cbb2ada1 100644 --- a/account_manager/src/wallet/recover.rs +++ b/account_manager/src/wallet/recover.rs @@ -54,6 +54,8 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { ) .arg( Arg::with_name(STDIN_INPUTS_FLAG) + .takes_value(false) + .hidden(cfg!(windows)) .long(STDIN_INPUTS_FLAG) .help("If present, read all user inputs from stdin instead of tty."), ) @@ -61,7 +63,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { pub fn cli_run(matches: &ArgMatches, wallet_base_dir: PathBuf) -> Result<(), String> { let mnemonic_path: Option = clap_utils::parse_optional(matches, MNEMONIC_FLAG)?; - let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG); + let stdin_inputs = cfg!(windows) || matches.is_present(STDIN_INPUTS_FLAG); eprintln!(); eprintln!("WARNING: KEY RECOVERY CAN LEAD TO DUPLICATING VALIDATORS KEYS, WHICH CAN LEAD TO SLASHING."); diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 1bd12b6e308..d5378d627bc 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -805,7 +805,7 @@ fn verify_block_for_gossip_slashing_detection() { let slasher_dir = tempdir().unwrap(); let slasher = Arc::new( Slasher::open( - SlasherConfig::new(slasher_dir.path().into()), + SlasherConfig::new(slasher_dir.path().into()).for_testing(), harness.logger().clone(), ) .unwrap(), @@ -824,4 +824,8 @@ fn verify_block_for_gossip_slashing_detection() { slasher.process_queued(Epoch::new(0)).unwrap(); let proposer_slashings = slasher.get_proposer_slashings(); assert_eq!(proposer_slashings.len(), 1); + // windows won't delete the temporary directory if you don't do this.. + drop(harness); + drop(slasher); + slasher_dir.close().unwrap(); } diff --git a/book/src/installation-source.md b/book/src/installation-source.md index 286379390be..864e647ad7e 100644 --- a/book/src/installation-source.md +++ b/book/src/installation-source.md @@ -1,9 +1,10 @@ # Installation: Build from Source -Lighthouse builds on Linux, macOS, and Windows (via [WSL][] only). +Lighthouse builds on Linux, macOS, and Windows (native Windows support in +BETA, we also support Windows via [WSL][]). -Compilation should be easy. In fact, if you already have Rust installed all you -need is: +Compilation should be easy. In fact, if you already have Rust and the build +dependencies installed, all you need is: - `git clone https://github.com/sigp/lighthouse.git` - `cd lighthouse` @@ -26,35 +27,43 @@ directory will be the location you cloned Lighthouse to during the installation - `git checkout ${VERSION}` - `make` + ## Detailed Instructions -1. Install Rust and Cargo with [rustup](https://rustup.rs/). - - Use the `stable` toolchain (it's the default). - - Check the [Troubleshooting](#troubleshooting) section for additional - dependencies (e.g., `cmake`). +1. Install the build dependencies for your platform + - Check the [Dependencies](#dependencies) section for additional + information. 1. Clone the Lighthouse repository. - Run `$ git clone https://github.com/sigp/lighthouse.git` - Change into the newly created directory with `$ cd lighthouse` 1. Build Lighthouse with `$ make`. -1. Installation was successful if `$ lighthouse --help` displays the - command-line documentation. +1. Installation was successful if `$ lighthouse --help` displays the command-line documentation. > First time compilation may take several minutes. If you experience any > failures, please reach out on [discord](https://discord.gg/cyAszAh) or > [create an issue](https://github.com/sigp/lighthouse/issues/new). -## Windows Support -Compiling or running Lighthouse natively on Windows is not currently supported. However, -Lighthouse can run successfully under the [Windows Subsystem for Linux (WSL)][WSL]. If using -Ubuntu under WSL, you can should install the Ubuntu dependencies listed in the [Dependencies -(Ubuntu)](#dependencies-ubuntu) section. +## Dependencies -[WSL]: https://docs.microsoft.com/en-us/windows/wsl/about +#### Installing Rust -## Troubleshooting +The best way to install Rust (regardless of platform) is usually with [rustup](https://rustup.rs/) +- Use the `stable` toolchain (it's the default). + +#### Windows Support + +These instructions are for compiling or running Lighthouse natively on Windows, which is currently in +BETA testing. Lighthouse can also run successfully under the [Windows Subsystem for Linux (WSL)][WSL]. +If using Ubuntu under WSL, you should follow the instructions for Ubuntu listed in the +[Dependencies (Ubuntu)](#ubuntu) section. -### Dependencies +[WSL]: https://docs.microsoft.com/en-us/windows/wsl/about + +1. Install [Git](https://git-scm.com/book/en/v2/Getting-Started-Installing-Git) +1. Install [Chocolatey](https://chocolatey.org/install) Package Manager for Windows + - Install `make` via `choco install make` + - Install `cmake` via `choco install cmake --installargs 'ADD_CMAKE_TO_PATH=System'` #### Ubuntu @@ -71,6 +80,9 @@ You will need `cmake`. You can install via homebrew: brew install cmake + +## Troubleshooting + ### Command is not found Lighthouse will be installed to `CARGO_HOME` or `$HOME/.cargo`. This directory diff --git a/book/src/slasher.md b/book/src/slasher.md index 27f03dcac33..d4d70567c5f 100644 --- a/book/src/slasher.md +++ b/book/src/slasher.md @@ -12,6 +12,7 @@ of the immaturity of the slasher UX and the extra resources required. * Quad-core CPU * 16 GB RAM * 256 GB solid state storage (in addition to space for the beacon node DB) +* ⚠️ **If you are running natively on Windows**: LMDB will pre-allocate the entire 256 GB for the slasher database ## How to Run diff --git a/common/account_utils/Cargo.toml b/common/account_utils/Cargo.toml index 1b7580ded44..91a31e39534 100644 --- a/common/account_utils/Cargo.toml +++ b/common/account_utils/Cargo.toml @@ -10,6 +10,7 @@ edition = "2018" rand = "0.7.3" eth2_wallet = { path = "../../crypto/eth2_wallet" } eth2_keystore = { path = "../../crypto/eth2_keystore" } +filesystem = { path = "../filesystem" } zeroize = { version = "1.1.1", features = ["zeroize_derive"] } serde = "1.0.116" serde_derive = "1.0.116" diff --git a/common/account_utils/src/lib.rs b/common/account_utils/src/lib.rs index 3832801486c..0ac8c8151c6 100644 --- a/common/account_utils/src/lib.rs +++ b/common/account_utils/src/lib.rs @@ -6,12 +6,12 @@ use eth2_wallet::{ bip39::{Language, Mnemonic, MnemonicType}, Wallet, }; +use filesystem::{create_with_600_perms, Error as FsError}; use rand::{distributions::Alphanumeric, Rng}; use serde_derive::{Deserialize, Serialize}; use std::fs::{self, File}; use std::io; use std::io::prelude::*; -use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; use zeroize::Zeroize; @@ -59,23 +59,6 @@ pub fn read_password>(path: P) -> Result { fs::read(path).map(strip_off_newlines).map(Into::into) } -/// Creates a file with `600 (-rw-------)` permissions. -pub fn create_with_600_perms>(path: P, bytes: &[u8]) -> Result<(), io::Error> { - let path = path.as_ref(); - - let mut file = File::create(&path)?; - - let mut perm = file.metadata()?.permissions(); - - perm.set_mode(0o600); - - file.set_permissions(perm)?; - - file.write_all(bytes)?; - - Ok(()) -} - /// Write a file atomically by using a temporary file as an intermediate. /// /// Care is taken to preserve the permissions of the file at `file_path` being written. @@ -86,18 +69,18 @@ pub fn write_file_via_temporary( file_path: &Path, temp_path: &Path, bytes: &[u8], -) -> Result<(), io::Error> { +) -> Result<(), FsError> { // If the file already exists, preserve its permissions by copying it. // Otherwise, create a new file with restricted permissions. if file_path.exists() { - fs::copy(&file_path, &temp_path)?; - fs::write(&temp_path, &bytes)?; + fs::copy(&file_path, &temp_path).map_err(FsError::UnableToCopyFile)?; + fs::write(&temp_path, &bytes).map_err(FsError::UnableToWriteFile)?; } else { create_with_600_perms(&temp_path, &bytes)?; } // With the temporary file created, perform an atomic rename. - fs::rename(&temp_path, &file_path)?; + fs::rename(&temp_path, &file_path).map_err(FsError::UnableToRenameFile)?; Ok(()) } diff --git a/common/account_utils/src/validator_definitions.rs b/common/account_utils/src/validator_definitions.rs index 2deb95dd347..47aaff2155e 100644 --- a/common/account_utils/src/validator_definitions.rs +++ b/common/account_utils/src/validator_definitions.rs @@ -36,7 +36,7 @@ pub enum Error { /// The config file could not be serialized as YAML. UnableToEncodeFile(serde_yaml::Error), /// The config file or temp file could not be written to the filesystem. - UnableToWriteFile(io::Error), + UnableToWriteFile(filesystem::Error), /// The public key from the keystore is invalid. InvalidKeystorePubkey, /// The keystore was unable to be opened. diff --git a/common/filesystem/Cargo.toml b/common/filesystem/Cargo.toml new file mode 100644 index 00000000000..f263f680cee --- /dev/null +++ b/common/filesystem/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "filesystem" +version = "0.1.0" +authors = ["Mark Mackey "] +edition = "2018" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] + +[target.'cfg(windows)'.dependencies] +winapi = "~0.3.5" +windows-acl = "~0.3.0" + diff --git a/common/filesystem/src/lib.rs b/common/filesystem/src/lib.rs new file mode 100644 index 00000000000..127fcd55b99 --- /dev/null +++ b/common/filesystem/src/lib.rs @@ -0,0 +1,144 @@ +use std::fs::File; +use std::io; +use std::io::Write; +use std::path::Path; +#[cfg(windows)] +use winapi::um::winnt::{FILE_GENERIC_READ, FILE_GENERIC_WRITE, STANDARD_RIGHTS_ALL}; + +/// 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 +#[cfg(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 +#[cfg(windows)] +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 +#[cfg(windows)] +const OWNER_ACL_ENTRY_MASK: u32 = FILE_GENERIC_READ | FILE_GENERIC_WRITE | STANDARD_RIGHTS_ALL; + +#[derive(Debug)] +pub enum Error { + /// The file could not be created + UnableToCreateFile(io::Error), + /// The file could not be copied + UnableToCopyFile(io::Error), + /// The file could not be opened + UnableToOpenFile(io::Error), + /// The file could not be renamed + UnableToRenameFile(io::Error), + /// Failed to set permissions + UnableToSetPermissions(io::Error), + /// Failed to retrieve file metadata + UnableToRetrieveMetadata(io::Error), + /// Failed to write bytes to file + UnableToWriteFile(io::Error), + /// Failed to obtain file path + UnableToObtainFilePath, + /// Failed to convert string to SID + UnableToConvertSID(u32), + /// Failed to retrieve ACL for file + UnableToRetrieveACL(u32), + /// Failed to enumerate ACL entries + UnableToEnumerateACLEntries(u32), + /// Failed to add new ACL entry + UnableToAddACLEntry(String), + /// Failed to remove ACL entry + UnableToRemoveACLEntry(String), +} + +/// Creates a file with `600 (-rw-------)` permissions. +pub fn create_with_600_perms>(path: P, bytes: &[u8]) -> Result<(), Error> { + let path = path.as_ref(); + let mut file = File::create(&path).map_err(Error::UnableToCreateFile)?; + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perm = file + .metadata() + .map_err(Error::UnableToRetrieveMetadata)? + .permissions(); + perm.set_mode(0o600); + file.set_permissions(perm) + .map_err(Error::UnableToSetPermissions)?; + } + + file.write_all(bytes).map_err(Error::UnableToWriteFile)?; + #[cfg(windows)] + { + restrict_file_permissions(path)?; + } + + Ok(()) +} + +pub fn restrict_file_permissions>(path: P) -> Result<(), Error> { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let file = File::open(path.as_ref()).map_err(Error::UnableToOpenFile)?; + let mut perm = file + .metadata() + .map_err(Error::UnableToRetrieveMetadata)? + .permissions(); + perm.set_mode(0o600); + file.set_permissions(perm) + .map_err(Error::UnableToSetPermissions)?; + } + + #[cfg(windows)] + { + use winapi::um::winnt::PSID; + use windows_acl::acl::{AceType, ACL}; + use windows_acl::helper::sid_to_string; + + let path_str = path + .as_ref() + .to_str() + .ok_or(Error::UnableToObtainFilePath)?; + let mut acl = ACL::from_file_path(&path_str, false).map_err(Error::UnableToRetrieveACL)?; + + let owner_sid = + windows_acl::helper::string_to_sid(OWNER_SID_STR).map_err(Error::UnableToConvertSID)?; + + let entries = acl.all().map_err(Error::UnableToEnumerateACLEntries)?; + + // 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(|code| { + Error::UnableToAddACLEntry(format!( + "Failed to add ACL entry for SID {} error={}", + OWNER_SID_STR, code + )) + })?; + // 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(|_| { + Error::UnableToRemoveACLEntry(format!( + "Failed to remove ACL entry for SID {}", + entry_sid_str + )) + })?; + } + } + } + } + + Ok(()) +} diff --git a/common/lockfile/src/lib.rs b/common/lockfile/src/lib.rs index 28e5b90a0e1..82e28256f74 100644 --- a/common/lockfile/src/lib.rs +++ b/common/lockfile/src/lib.rs @@ -81,12 +81,19 @@ mod test { fn new_lock() { let temp = tempdir().unwrap(); let path = temp.path().join("lockfile"); - let _lock = Lockfile::new(path.clone()).unwrap(); - assert!(matches!( - Lockfile::new(path).unwrap_err(), - LockfileError::FileLocked(..) - )); + if cfg!(windows) { + assert!(matches!( + Lockfile::new(path).unwrap_err(), + // windows returns an IoError because the lockfile is already open :/ + LockfileError::IoError(..), + )); + } else { + assert!(matches!( + Lockfile::new(path).unwrap_err(), + LockfileError::FileLocked(..) + )); + } } #[test] diff --git a/common/remote_signer_consumer/tests/post.rs b/common/remote_signer_consumer/tests/post.rs index 1667ee2c621..543605fb5be 100644 --- a/common/remote_signer_consumer/tests/post.rs +++ b/common/remote_signer_consumer/tests/post.rs @@ -18,11 +18,25 @@ mod post { match signature.unwrap_err() { Error::Reqwest(e) => { let error_msg = e.to_string(); - assert!(error_msg.contains("error sending request for url")); - assert!(error_msg.contains(PUBLIC_KEY_1)); - assert!(error_msg.contains("error trying to connect")); - assert!(error_msg.contains("tcp connect error")); - assert!(error_msg.contains("Connection refused")); + let pubkey_string = PUBLIC_KEY_1.to_string(); + let msgs = vec![ + "error sending request for url", + &pubkey_string, + "error trying to connect", + "tcp connect error", + match cfg!(windows) { + true => "No connection could be made because the target machine actively refused it", + false => "Connection refused", + } + ]; + for msg in msgs.iter() { + assert!( + error_msg.contains(msg), + "{:?} should contain {:?}", + error_msg, + msg + ); + } } e => panic!("{:?}", e), } @@ -31,15 +45,15 @@ mod post { #[test] fn server_error() { let (test_signer, tmp_dir) = set_up_api_test_signer_to_sign_message(); - set_permissions(tmp_dir.path(), 0o40311); - set_permissions(&tmp_dir.path().join(PUBLIC_KEY_1), 0o40311); + restrict_permissions(tmp_dir.path()); + restrict_permissions(&tmp_dir.path().join(PUBLIC_KEY_1)); let test_client = set_up_test_consumer(&test_signer.address); let test_input = get_input_data_block(0xc137); let signature = do_sign_request(&test_client, test_input); - set_permissions(tmp_dir.path(), 0o40755); - set_permissions(&tmp_dir.path().join(PUBLIC_KEY_1), 0o40755); + unrestrict_permissions(tmp_dir.path()); + unrestrict_permissions(&tmp_dir.path().join(PUBLIC_KEY_1)); match signature.unwrap_err() { Error::ServerMessage(message) => assert_eq!(message, "Storage error: PermissionDenied"), @@ -145,7 +159,6 @@ mod post { let testcase = |u: &str, msgs: Vec<&str>| { let r = run_testcase(u).unwrap_err(); - for msg in msgs.iter() { assert!(r.contains(msg), "{:?} should contain {:?}", r, msg); } @@ -159,7 +172,10 @@ mod post { &format!("/sign/{}", PUBLIC_KEY_1), "hyper::Error(Connect, ConnectError", "dns error", - "failed to lookup address information", + match cfg!(windows) { + true => "No such host is known.", + false => "failed to lookup address information", + }, ], ); diff --git a/common/validator_dir/Cargo.toml b/common/validator_dir/Cargo.toml index f993197c6a9..eb6a4e9e432 100644 --- a/common/validator_dir/Cargo.toml +++ b/common/validator_dir/Cargo.toml @@ -12,6 +12,7 @@ insecure_keys = [] [dependencies] bls = { path = "../../crypto/bls" } eth2_keystore = { path = "../../crypto/eth2_keystore" } +filesystem = { path = "../filesystem" } types = { path = "../../consensus/types" } rand = "0.7.3" deposit_contract = { path = "../deposit_contract" } diff --git a/common/validator_dir/src/builder.rs b/common/validator_dir/src/builder.rs index 70cb13ae99b..d284e2d6732 100644 --- a/common/validator_dir/src/builder.rs +++ b/common/validator_dir/src/builder.rs @@ -2,10 +2,10 @@ use crate::{Error as DirError, ValidatorDir}; use bls::get_withdrawal_credentials; use deposit_contract::{encode_eth1_tx_data, Error as DepositError}; use eth2_keystore::{Error as KeystoreError, Keystore, KeystoreBuilder, PlainText}; +use filesystem::create_with_600_perms; use rand::{distributions::Alphanumeric, Rng}; -use std::fs::{create_dir_all, File, OpenOptions}; +use std::fs::{create_dir_all, OpenOptions}; use std::io::{self, Write}; -use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; use types::{ChainSpec, DepositData, Hash256, Keypair, Signature}; @@ -33,7 +33,7 @@ pub enum Error { KeystoreAlreadyExists(PathBuf), UnableToSaveKeystore(io::Error), PasswordAlreadyExists(PathBuf), - UnableToSavePassword(io::Error), + UnableToSavePassword(filesystem::Error), KeystoreError(KeystoreError), UnableToOpenDir(DirError), UninitializedVotingKeystore, @@ -283,19 +283,7 @@ pub fn write_password_to_file>(path: P, bytes: &[u8]) -> Result<( return Err(Error::PasswordAlreadyExists(path.into())); } - let mut file = File::create(&path).map_err(Error::UnableToSavePassword)?; - - let mut perm = file - .metadata() - .map_err(Error::UnableToSavePassword)? - .permissions(); - - perm.set_mode(0o600); - - file.set_permissions(perm) - .map_err(Error::UnableToSavePassword)?; - - file.write_all(bytes).map_err(Error::UnableToSavePassword)?; + create_with_600_perms(path, bytes).map_err(Error::UnableToSavePassword)?; Ok(()) } diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index ef1edb77475..d21a4845cd7 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -681,6 +681,7 @@ fn compact_db_flag() { fn slasher_flag() { CommandLineTest::new() .flag("slasher", None) + .flag("slasher-max-db-size", Some("16")) .run() .with_config_and_dir(|config, dir| { if let Some(slasher_config) = &config.slasher { @@ -699,6 +700,7 @@ fn slasher_dir_flag() { CommandLineTest::new() .flag("slasher", None) .flag("slasher-dir", dir.path().as_os_str().to_str()) + .flag("slasher-max-db-size", Some("16")) .run() .with_config(|config| { if let Some(slasher_config) = &config.slasher { @@ -712,6 +714,7 @@ fn slasher_dir_flag() { fn slasher_update_period_flag() { CommandLineTest::new() .flag("slasher", None) + .flag("slasher-max-db-size", Some("16")) .flag("slasher-update-period", Some("100")) .run() .with_config(|config| { @@ -726,6 +729,7 @@ fn slasher_update_period_flag() { fn slasher_history_length_flag() { CommandLineTest::new() .flag("slasher", None) + .flag("slasher-max-db-size", Some("16")) .flag("slasher-history-length", Some("2048")) .run() .with_config(|config| { @@ -755,6 +759,7 @@ fn slasher_chunk_size_flag() { CommandLineTest::new() .flag("slasher", None) .flag("slasher-chunk-size", Some("32")) + .flag("slasher-max-db-size", Some("16")) .run() .with_config(|config| { let slasher_config = config @@ -768,6 +773,7 @@ fn slasher_chunk_size_flag() { fn slasher_validator_chunk_size_flag() { CommandLineTest::new() .flag("slasher", None) + .flag("slasher-max-db-size", Some("16")) .flag("slasher-validator-chunk-size", Some("512")) .run() .with_config(|config| { @@ -783,6 +789,7 @@ fn slasher_broadcast_flag() { CommandLineTest::new() .flag("slasher", None) .flag("slasher-broadcast", None) + .flag("slasher-max-db-size", Some("16")) .run() .with_config(|config| { let slasher_config = config diff --git a/remote_signer/backend/src/lib.rs b/remote_signer/backend/src/lib.rs index 9631f00b1db..4c3c2464e18 100644 --- a/remote_signer/backend/src/lib.rs +++ b/remote_signer/backend/src/lib.rs @@ -163,7 +163,14 @@ pub mod backend_new { #[test] fn given_path_is_not_a_dir() { - let matches = set_matches(vec!["this_test", "--storage-raw-dir", "/dev/null"]); + let matches = set_matches(vec![ + "this_test", + "--storage-raw-dir", + match cfg!(windows) { + true => "C:\\Windows\\system.ini", + false => "/dev/null", + }, + ]); assert_backend_new_error(&matches, "Storage Raw Dir: Path is not a directory."); } @@ -171,7 +178,7 @@ pub mod backend_new { #[test] fn given_inaccessible() { let tmp_dir = tempdir().unwrap(); - set_permissions(tmp_dir.path(), 0o40311); + restrict_permissions(tmp_dir.path()); let matches = set_matches(vec![ "this_test", @@ -184,7 +191,7 @@ pub mod backend_new { // A `d-wx--x--x` directory is innaccesible but not unwrittable. // By switching back to `drwxr-xr-x` we can get rid of the // temporal directory once we leave this scope. - set_permissions(tmp_dir.path(), 0o40755); + unrestrict_permissions(tmp_dir.path()); match result { Ok(_) => panic!("This invocation to Backend::new() should return error"), @@ -263,16 +270,16 @@ pub mod backend_raw_dir_sign_message { fn storage_error() { let (backend, tmp_dir) = new_backend_for_signing(); - set_permissions(tmp_dir.path(), 0o40311); - set_permissions(&tmp_dir.path().join(PUBLIC_KEY_1), 0o40311); + restrict_permissions(tmp_dir.path()); + restrict_permissions(&tmp_dir.path().join(PUBLIC_KEY_1)); let result = backend.sign_message( PUBLIC_KEY_1, Hash256::from_slice(&hex::decode(SIGNING_ROOT).unwrap()), ); - set_permissions(tmp_dir.path(), 0o40755); - set_permissions(&tmp_dir.path().join(PUBLIC_KEY_1), 0o40755); + unrestrict_permissions(tmp_dir.path()); + unrestrict_permissions(&tmp_dir.path().join(PUBLIC_KEY_1)); assert_eq!( result.unwrap_err().to_string(), diff --git a/remote_signer/backend/src/storage_raw_dir.rs b/remote_signer/backend/src/storage_raw_dir.rs index 4990ca574ef..8ae5dfd6766 100644 --- a/remote_signer/backend/src/storage_raw_dir.rs +++ b/remote_signer/backend/src/storage_raw_dir.rs @@ -83,12 +83,12 @@ mod get_keys { add_key_files(&tmp_dir); // All good and fancy, let's make the dir innacessible now. - set_permissions(tmp_dir.path(), 0o40311); + restrict_permissions(tmp_dir.path()); let result = storage.get_keys(); // Give permissions back, we want the tempdir to be deleted. - set_permissions(tmp_dir.path(), 0o40755); + unrestrict_permissions(tmp_dir.path()); assert_eq!( result.unwrap_err().to_string(), @@ -141,13 +141,13 @@ mod get_secret_key { let (storage, tmp_dir) = new_storage_with_tmp_dir(); add_key_files(&tmp_dir); - set_permissions(tmp_dir.path(), 0o40311); - set_permissions(&tmp_dir.path().join(PUBLIC_KEY_1), 0o40311); + restrict_permissions(tmp_dir.path()); + restrict_permissions(&tmp_dir.path().join(PUBLIC_KEY_1)); let result = storage.get_secret_key(PUBLIC_KEY_1); - set_permissions(tmp_dir.path(), 0o40755); - set_permissions(&tmp_dir.path().join(PUBLIC_KEY_1), 0o40755); + unrestrict_permissions(tmp_dir.path()); + unrestrict_permissions(&tmp_dir.path().join(PUBLIC_KEY_1)); assert_eq!( result.unwrap_err().to_string(), diff --git a/remote_signer/tests/get_keys.rs b/remote_signer/tests/get_keys.rs index 88d981a11b6..887a16a80d3 100644 --- a/remote_signer/tests/get_keys.rs +++ b/remote_signer/tests/get_keys.rs @@ -68,14 +68,14 @@ mod get_keys { add_non_key_files(&tmp_dir); // Somebody tripped over a wire. - set_permissions(tmp_dir.path(), 0o40311); + restrict_permissions(tmp_dir.path()); let url = format!("{}/keys", test_signer.address); let resp = http_get(&url); // Be able to delete the tempdir afterward, regardless of this test result. - set_permissions(tmp_dir.path(), 0o40755); + unrestrict_permissions(tmp_dir.path()); assert_error(resp, 500, "Storage error: PermissionDenied"); diff --git a/remote_signer/tests/sign.rs b/remote_signer/tests/sign.rs index d019c446149..ee124c80db3 100644 --- a/remote_signer/tests/sign.rs +++ b/remote_signer/tests/sign.rs @@ -20,14 +20,14 @@ mod sign { fn storage_error() { let (test_signer, tmp_dir) = set_up_api_test_signer_to_sign_message(); let test_block_body = get_test_block_body(0xc137); - set_permissions(tmp_dir.path(), 0o40311); - set_permissions(&tmp_dir.path().join(PUBLIC_KEY_1), 0o40311); + restrict_permissions(tmp_dir.path()); + restrict_permissions(&tmp_dir.path().join(PUBLIC_KEY_1)); let url = format!("{}/sign/{}", test_signer.address, PUBLIC_KEY_1); let response = http_post_custom_body(&url, &test_block_body); - set_permissions(tmp_dir.path(), 0o40755); - set_permissions(&tmp_dir.path().join(PUBLIC_KEY_1), 0o40755); + unrestrict_permissions(tmp_dir.path()); + unrestrict_permissions(&tmp_dir.path().join(PUBLIC_KEY_1)); assert_sign_error(response, 500, "Storage error: PermissionDenied"); diff --git a/slasher/Cargo.toml b/slasher/Cargo.toml index aa1f85006ee..3e6461e783d 100644 --- a/slasher/Cargo.toml +++ b/slasher/Cargo.toml @@ -12,6 +12,7 @@ eth2_ssz_derive = { path = "../consensus/ssz_derive" } flate2 = { version = "1.0.14", features = ["zlib"], default-features = false } lazy_static = "1.4.0" lighthouse_metrics = { path = "../common/lighthouse_metrics" } +filesystem = { path = "../common/filesystem" } lmdb = "0.8" lmdb-sys = "0.8" parking_lot = "0.11.0" diff --git a/slasher/src/config.rs b/slasher/src/config.rs index 0f44d8d4890..5a701048bdb 100644 --- a/slasher/src/config.rs +++ b/slasher/src/config.rs @@ -10,6 +10,13 @@ pub const DEFAULT_UPDATE_PERIOD: u64 = 12; pub const DEFAULT_MAX_DB_SIZE: usize = 256 * 1024; // 256 GiB pub const DEFAULT_BROADCAST: bool = false; +/// Database size to use for tests. +/// +/// Mostly a workaround for Windows due to a bug in LMDB, see: +/// +/// https://github.com/sigp/lighthouse/issues/2342 +pub const TESTING_MAX_DB_SIZE: usize = 16; // MiB + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Config { pub database_path: PathBuf, @@ -38,6 +45,12 @@ impl Config { } } + /// Use a smaller max DB size for testing. + pub fn for_testing(mut self) -> Self { + self.max_db_size_mbs = TESTING_MAX_DB_SIZE; + self + } + pub fn validate(&self) -> Result<(), Error> { if self.chunk_size == 0 || self.validator_chunk_size == 0 diff --git a/slasher/src/database.rs b/slasher/src/database.rs index 8f1c97aa4ee..7576d18483d 100644 --- a/slasher/src/database.rs +++ b/slasher/src/database.rs @@ -195,6 +195,15 @@ impl SlasherDB { let proposers_db = env.create_db(Some(PROPOSERS_DB), Self::db_flags())?; let metadata_db = env.create_db(Some(METADATA_DB), Self::db_flags())?; + #[cfg(windows)] + { + use filesystem::restrict_file_permissions; + let data = config.database_path.join("data.mdb"); + let lock = config.database_path.join("lock.mdb"); + restrict_file_permissions(data).map_err(Error::DatabasePermissionsError)?; + restrict_file_permissions(lock).map_err(Error::DatabasePermissionsError)?; + } + let db = Self { env, indexed_attestation_db, diff --git a/slasher/src/error.rs b/slasher/src/error.rs index 97b3f8cd05f..d40a54f7144 100644 --- a/slasher/src/error.rs +++ b/slasher/src/error.rs @@ -6,6 +6,7 @@ use types::{Epoch, Hash256}; pub enum Error { DatabaseError(lmdb::Error), DatabaseIOError(io::Error), + DatabasePermissionsError(filesystem::Error), SszDecodeError(ssz::DecodeError), BincodeError(bincode::Error), ArithError(safe_arith::ArithError), diff --git a/slasher/tests/attester_slashings.rs b/slasher/tests/attester_slashings.rs index 3b541e7a8fe..138d5526cbd 100644 --- a/slasher/tests/attester_slashings.rs +++ b/slasher/tests/attester_slashings.rs @@ -170,7 +170,7 @@ fn slasher_test( should_process_after: impl Fn(usize) -> bool, ) { let tempdir = tempdir().unwrap(); - let config = Config::new(tempdir.path().into()); + let config = Config::new(tempdir.path().into()).for_testing(); let slasher = Slasher::open(config, logger()).unwrap(); let current_epoch = Epoch::new(current_epoch); @@ -189,6 +189,8 @@ fn slasher_test( // Pruning should not error. slasher.prune_database(current_epoch).unwrap(); + // windows won't delete the temporary directory if you don't do this.. + drop(slasher); } fn parallel_slasher_test( @@ -197,7 +199,7 @@ fn parallel_slasher_test( current_epoch: u64, ) { let tempdir = tempdir().unwrap(); - let config = Config::new(tempdir.path().into()); + let config = Config::new(tempdir.path().into()).for_testing(); let slasher = Slasher::open(config, logger()).unwrap(); let current_epoch = Epoch::new(current_epoch); @@ -212,4 +214,6 @@ fn parallel_slasher_test( let slashings = slasher.get_attester_slashings(); let slashed_validators = slashed_validators_from_slashings(&slashings); assert_eq!(slashed_validators, expected_slashed_validators); + // windows won't delete the temporary directory if you don't do this.. + drop(slasher); } diff --git a/slasher/tests/proposer_slashings.rs b/slasher/tests/proposer_slashings.rs index f584bacfba0..ec46f6db852 100644 --- a/slasher/tests/proposer_slashings.rs +++ b/slasher/tests/proposer_slashings.rs @@ -8,7 +8,7 @@ use types::{Epoch, EthSpec}; #[test] fn empty_pruning() { let tempdir = tempdir().unwrap(); - let config = Config::new(tempdir.path().into()); + let config = Config::new(tempdir.path().into()).for_testing(); let slasher = Slasher::::open(config, logger()).unwrap(); slasher.prune_database(Epoch::new(0)).unwrap(); } @@ -18,7 +18,7 @@ fn block_pruning() { let slots_per_epoch = E::slots_per_epoch(); let tempdir = tempdir().unwrap(); - let mut config = Config::new(tempdir.path().into()); + let mut config = Config::new(tempdir.path().into()).for_testing(); config.chunk_size = 2; config.history_length = 2; diff --git a/slasher/tests/random.rs b/slasher/tests/random.rs index 47fce46f4fb..641a52b7efe 100644 --- a/slasher/tests/random.rs +++ b/slasher/tests/random.rs @@ -40,7 +40,7 @@ fn random_test(seed: u64, test_config: TestConfig) { let tempdir = tempdir().unwrap(); - let mut config = Config::new(tempdir.path().into()); + let mut config = Config::new(tempdir.path().into()).for_testing(); config.validator_chunk_size = 1 << rng.gen_range(1, 4); let chunk_size_exponent = rng.gen_range(1, 4); diff --git a/slasher/tests/wrap_around.rs b/slasher/tests/wrap_around.rs index 1480704e6bf..79ec91fbe6b 100644 --- a/slasher/tests/wrap_around.rs +++ b/slasher/tests/wrap_around.rs @@ -8,7 +8,7 @@ use types::Epoch; #[test] fn attestation_pruning_empty_wrap_around() { let tempdir = tempdir().unwrap(); - let mut config = Config::new(tempdir.path().into()); + let mut config = Config::new(tempdir.path().into()).for_testing(); config.validator_chunk_size = 1; config.chunk_size = 16; config.history_length = 16; @@ -42,7 +42,7 @@ fn attestation_pruning_empty_wrap_around() { #[test] fn pruning_with_map_full() { let tempdir = tempdir().unwrap(); - let mut config = Config::new(tempdir.path().into()); + let mut config = Config::new(tempdir.path().into()).for_testing(); config.validator_chunk_size = 1; config.chunk_size = 16; config.history_length = 1024; diff --git a/testing/eth1_test_rig/src/ganache.rs b/testing/eth1_test_rig/src/ganache.rs index a890f7c7133..720c6825355 100644 --- a/testing/eth1_test_rig/src/ganache.rs +++ b/testing/eth1_test_rig/src/ganache.rs @@ -74,8 +74,11 @@ impl GanacheInstance { /// RPC connections. pub fn new(network_id: u64, chain_id: u64) -> Result { let port = unused_port()?; - - let child = Command::new("ganache-cli") + let binary = match cfg!(windows) { + true => "ganache-cli.cmd", + false => "ganache-cli", + }; + let child = Command::new(binary) .stdout(Stdio::piped()) .arg("--defaultBalanceEther") .arg("1000000000") @@ -83,6 +86,8 @@ impl GanacheInstance { .arg("1000000000") .arg("--accounts") .arg("10") + .arg("--keepAliveTimeout") + .arg("0") .arg("--port") .arg(format!("{}", port)) .arg("--mnemonic") @@ -94,9 +99,9 @@ impl GanacheInstance { .spawn() .map_err(|e| { format!( - "Failed to start ganache-cli. \ - Is it ganache-cli installed and available on $PATH? Error: {:?}", - e + "Failed to start {}. \ + Is it installed and available on $PATH? Error: {:?}", + binary, e ) })?; @@ -105,21 +110,26 @@ impl GanacheInstance { pub fn fork(&self) -> Result { let port = unused_port()?; - - let child = Command::new("ganache-cli") + let binary = match cfg!(windows) { + true => "ganache-cli.cmd", + false => "ganache-cli", + }; + let child = Command::new(binary) .stdout(Stdio::piped()) .arg("--fork") .arg(self.endpoint()) .arg("--port") .arg(format!("{}", port)) + .arg("--keepAliveTimeout") + .arg("0") .arg("--chainId") .arg(format!("{}", self.chain_id)) .spawn() .map_err(|e| { format!( - "Failed to start ganache-cli. \ - Is it ganache-cli installed and available on $PATH? Error: {:?}", - e + "Failed to start {}. \ + Is it installed and available on $PATH? Error: {:?}", + binary, e ) })?; @@ -202,6 +212,23 @@ pub fn unused_port() -> Result { impl Drop for GanacheInstance { fn drop(&mut self) { - let _ = self.child.kill(); + if cfg!(windows) { + // Calling child.kill() in Windows will only kill the process + // that spawned ganache, leaving the actual ganache process + // intact. You have to kill the whole process tree. What's more, + // if you don't spawn ganache with --keepAliveTimeout=0, Windows + // will STILL keep the server running even after you've ended + // the process tree and it's disappeared from the task manager. + // Unbelievable... + Command::new("taskkill") + .arg("/pid") + .arg(self.child.id().to_string()) + .arg("/T") + .arg("/F") + .output() + .expect("failed to execute taskkill"); + } else { + let _ = self.child.kill(); + } } } diff --git a/testing/remote_signer_test/Cargo.toml b/testing/remote_signer_test/Cargo.toml index 1daf8c2c582..e6a75c66947 100644 --- a/testing/remote_signer_test/Cargo.toml +++ b/testing/remote_signer_test/Cargo.toml @@ -18,3 +18,8 @@ tempfile = "3.1.0" tokio = { version = "1.1.0", features = ["time"] } types = { path = "../../consensus/types" } sensitive_url = { path = "../../common/sensitive_url" } + +[target.'cfg(windows)'.dependencies] +winapi = "~0.3.5" +windows-acl = "~0.3.0" + diff --git a/testing/remote_signer_test/src/utils.rs b/testing/remote_signer_test/src/utils.rs index 858f4d60244..e1e04f11003 100644 --- a/testing/remote_signer_test/src/utils.rs +++ b/testing/remote_signer_test/src/utils.rs @@ -5,11 +5,9 @@ pub use local_signer_test_data::*; pub use mock::*; use remote_signer_client::Client; pub use remote_signer_test_data::*; -use std::fs; use std::fs::{create_dir, File}; use std::io::Write; use std::net::IpAddr::{V4, V6}; -use std::os::unix::fs::PermissionsExt; use std::path::Path; use tempfile::TempDir; use types::{ @@ -18,6 +16,32 @@ use types::{ Hash256, IndexedAttestation, ProposerSlashing, PublicKeyBytes, Signature, SignatureBytes, SignedBeaconBlockHeader, SignedVoluntaryExit, Slot, Unsigned, VariableList, VoluntaryExit, }; +#[cfg(windows)] +use winapi::um::winnt::{ + FILE_GENERIC_READ, FILE_GENERIC_WRITE, FILE_READ_ATTRIBUTES, FILE_READ_EA, READ_CONTROL, + STANDARD_RIGHTS_ALL, SYNCHRONIZE, WRITE_DAC, +}; + +/// 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 +#[cfg(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 +#[cfg(windows)] +const OWNER_ACL_ENTRY_FLAGS: u8 = 0; +/// See here for explanation: +/// - https://docs.microsoft.com/en-us/windows/win32/wmisdk/file-and-directory-access-rights-constants +#[cfg(windows)] +const OWNER_ACL_ENTRY_RESTRICT_MASK: u32 = + FILE_READ_ATTRIBUTES | FILE_READ_EA | READ_CONTROL | WRITE_DAC | SYNCHRONIZE; +/// Generic Rights: +/// - https://docs.microsoft.com/en-us/windows/win32/fileio/file-security-and-access-rights +/// STANDARD_RIGHTS_ALL +/// - https://docs.microsoft.com/en-us/windows/win32/secauthz/access-mask +#[cfg(windows)] +const OWNER_ACL_ENTRY_UNRESTRICT_MASK: u32 = + FILE_GENERIC_READ | FILE_GENERIC_WRITE | STANDARD_RIGHTS_ALL; pub fn get_address(client: &Client) -> String { let listening_address = client.get_listening_address(); @@ -29,11 +53,78 @@ pub fn get_address(client: &Client) -> String { format!("http://{}:{}", ip, listening_address.port()) } -pub fn set_permissions(path: &Path, perm_octal: u32) { - let metadata = fs::metadata(path).unwrap(); - let mut permissions = metadata.permissions(); - permissions.set_mode(perm_octal); - fs::set_permissions(path, permissions).unwrap(); +pub fn restrict_permissions(path: &Path) { + #[cfg(unix)] + { + use std::fs; + use std::os::unix::fs::PermissionsExt; + + let metadata = fs::metadata(path).unwrap(); + let mut permissions = metadata.permissions(); + permissions.set_mode(0o0311); // set to '*-wx--x--x' + fs::set_permissions(path, permissions).unwrap(); + } + + #[cfg(windows)] + { + use winapi::um::winnt::PSID; + use windows_acl::acl::{AceType, ACL}; + + let path_str = path.to_str().unwrap(); + let mut acl = ACL::from_file_path(&path_str, false).unwrap(); + + let owner_sid = windows_acl::helper::string_to_sid(OWNER_SID_STR).unwrap(); + let entries = acl.all().unwrap(); + // remove all AccessAllow entries + for entry in &entries { + if let Some(ref entry_sid) = entry.sid { + acl.remove(entry_sid.as_ptr() as PSID, Some(AceType::AccessAllow), None) + .unwrap(); + } + } + // add single entry for minimal access to file owner + // allowing them only to read attributes of the file + // and read/modify permissions + acl.add_entry( + owner_sid.as_ptr() as PSID, + AceType::AccessAllow, + OWNER_ACL_ENTRY_FLAGS, + OWNER_ACL_ENTRY_RESTRICT_MASK, + ) + .unwrap(); + } +} + +pub fn unrestrict_permissions(path: &Path) { + #[cfg(unix)] + { + use std::fs; + use std::os::unix::fs::PermissionsExt; + + let metadata = fs::metadata(path).unwrap(); + let mut permissions = metadata.permissions(); + permissions.set_mode(0o0755); // set to '*rwxr-xr-x' + fs::set_permissions(path, permissions).unwrap(); + } + + #[cfg(windows)] + { + use winapi::um::winnt::PSID; + use windows_acl::acl::{AceType, ACL}; + + let path_str = path.to_str().unwrap(); + let mut acl = ACL::from_file_path(&path_str, false).unwrap(); + + let owner_sid = windows_acl::helper::string_to_sid(OWNER_SID_STR).unwrap(); + // add single entry for file owner + acl.add_entry( + owner_sid.as_ptr() as PSID, + AceType::AccessAllow, + OWNER_ACL_ENTRY_FLAGS, + OWNER_ACL_ENTRY_UNRESTRICT_MASK, + ) + .unwrap(); + } } pub fn add_key_files(tmp_dir: &TempDir) { diff --git a/validator_client/Cargo.toml b/validator_client/Cargo.toml index 2ac165b1666..7fd3cb3b139 100644 --- a/validator_client/Cargo.toml +++ b/validator_client/Cargo.toml @@ -39,6 +39,7 @@ logging = { path = "../common/logging" } environment = { path = "../lighthouse/environment" } parking_lot = "0.11.0" exit-future = "0.2.0" +filesystem = { path = "../common/filesystem" } libc = "0.2.79" eth2_ssz_derive = "0.1.0" hex = "0.4.2" diff --git a/validator_client/slashing_protection/Cargo.toml b/validator_client/slashing_protection/Cargo.toml index a1abb855631..655d58b1f27 100644 --- a/validator_client/slashing_protection/Cargo.toml +++ b/validator_client/slashing_protection/Cargo.toml @@ -16,6 +16,7 @@ serde = "1.0.116" serde_derive = "1.0.116" serde_json = "1.0.58" serde_utils = { path = "../../consensus/serde_utils" } +filesystem = { path = "../../common/filesystem" } [dev-dependencies] rayon = "1.4.1" diff --git a/validator_client/slashing_protection/Makefile b/validator_client/slashing_protection/Makefile index 79960ec5aa1..ae1f2e3d08a 100644 --- a/validator_client/slashing_protection/Makefile +++ b/validator_client/slashing_protection/Makefile @@ -4,8 +4,21 @@ OUTPUT_DIR := interchange-tests TARBALL := $(OUTPUT_DIR)-$(TESTS_TAG).tar.gz ARCHIVE_URL := https://github.com/eth2-clients/slashing-protection-interchange-tests/tarball/$(TESTS_TAG) +ifeq ($(OS),Windows_NT) +ifeq (, $(shell where rm)) + rmfile = if exist $(1) (del /F /Q $(1)) + rmdir = if exist $(1) (rmdir /Q /S $(1)) +else + rmfile = rm -f $(1) + rmdir = rm -rf $(1) +endif +else + rmfile = rm -f $(1) + rmdir = rm -rf $(1) +endif + $(OUTPUT_DIR): $(TARBALL) - rm -rf $@ + $(call rmdir,$@) mkdir $@ tar --strip-components=1 -xzf $^ -C $@ @@ -13,13 +26,13 @@ $(TARBALL): curl --fail -L -o $@ $(ARCHIVE_URL) clean-test-files: - rm -rf $(OUTPUT_DIR) + $(call rmdir,$(OUTPUT_DIR)) clean-archives: - rm -f $(TARBALL) + $(call rmfile,$(TARBALL)) generate: - rm -rf $(GENERATE_DIR) + $(call rmdir,$(GENERATE_DIR)) cargo run --release --bin test_generator -- $(GENERATE_DIR) clean: clean-test-files clean-archives diff --git a/validator_client/slashing_protection/src/lib.rs b/validator_client/slashing_protection/src/lib.rs index 8f6bdb50e9e..ed93b346a7a 100644 --- a/validator_client/slashing_protection/src/lib.rs +++ b/validator_client/slashing_protection/src/lib.rs @@ -30,6 +30,7 @@ pub enum NotSafe { UnregisteredValidator(PublicKeyBytes), InvalidBlock(InvalidBlock), InvalidAttestation(InvalidAttestation), + PermissionsError, IOError(ErrorKind), SQLError(String), SQLPoolError(String), diff --git a/validator_client/slashing_protection/src/slashing_database.rs b/validator_client/slashing_protection/src/slashing_database.rs index 79bcec7a93d..23de7d30004 100644 --- a/validator_client/slashing_protection/src/slashing_database.rs +++ b/validator_client/slashing_protection/src/slashing_database.rs @@ -5,9 +5,10 @@ use crate::interchange::{ use crate::signed_attestation::InvalidAttestation; use crate::signed_block::InvalidBlock; use crate::{hash256_from_row, NotSafe, Safe, SignedAttestation, SignedBlock, SigningRoot}; +use filesystem::restrict_file_permissions; use r2d2_sqlite::SqliteConnectionManager; use rusqlite::{params, OptionalExtension, Transaction, TransactionBehavior}; -use std::fs::{File, OpenOptions}; +use std::fs::OpenOptions; use std::path::Path; use std::time::Duration; use types::{AttestationData, BeaconBlockHeader, Epoch, Hash256, PublicKeyBytes, SignedRoot, Slot}; @@ -46,13 +47,13 @@ impl SlashingDatabase { /// /// Error if a database (or any file) already exists at `path`. pub fn create(path: &Path) -> Result { - let file = OpenOptions::new() + let _file = OpenOptions::new() .write(true) .read(true) .create_new(true) .open(path)?; - Self::set_db_file_permissions(&file)?; + restrict_file_permissions(path).map_err(|_| NotSafe::PermissionsError)?; let conn_pool = Self::open_conn_pool(path)?; let conn = conn_pool.get()?; @@ -121,21 +122,6 @@ impl SlashingDatabase { Ok(()) } - /// Set the database file to readable and writable only by its owner (0600). - #[cfg(unix)] - fn set_db_file_permissions(file: &File) -> Result<(), NotSafe> { - use std::os::unix::fs::PermissionsExt; - - let mut perm = file.metadata()?.permissions(); - perm.set_mode(0o600); - file.set_permissions(perm)?; - Ok(()) - } - - // TODO: add support for Windows ACLs - #[cfg(windows)] - fn set_db_file_permissions(file: &File) -> Result<(), NotSafe> {} - /// Creates an empty transaction and drops it. Used to test whether the database is locked. pub fn test_transaction(&self) -> Result<(), NotSafe> { let mut conn = self.conn_pool.get()?; diff --git a/validator_client/src/key_cache.rs b/validator_client/src/key_cache.rs index 66d69622300..5947621023a 100644 --- a/validator_client/src/key_cache.rs +++ b/validator_client/src/key_cache.rs @@ -146,7 +146,7 @@ impl KeyCache { let bytes = serde_json::to_vec(self).map_err(Error::UnableToEncodeFile)?; write_file_via_temporary(&cache_path, &temp_path, &bytes) - .map_err(Error::UnableToWriteFile)?; + .map_err(Error::UnableToCreateFile)?; self.state = State::DecryptedAndSaved; Ok(true) @@ -245,6 +245,7 @@ pub enum Error { UnableToEncodeFile(serde_json::Error), /// The cache file or its temporary could not be written to the filesystem. UnableToWriteFile(io::Error), + UnableToCreateFile(filesystem::Error), /// Couldn't decrypt the cache file UnableToDecrypt(KeystoreError), UnableToEncrypt(KeystoreError),