From ba55e140aed2799a41b3a501c707cc33e0f54c06 Mon Sep 17 00:00:00 2001 From: ethDreamer Date: Wed, 19 May 2021 23:05:16 +0000 Subject: [PATCH] Enable Compatibility with Windows (#2333) ## Issue Addressed Windows incompatibility. ## Proposed Changes On windows, lighthouse needs to default to STDIN as tty doesn't exist. Also Windows uses ACLs for file permissions. So to mirror chmod 600, we will remove every entry in a file's ACL and add only a single SID that is an alias for the file owner. Beyond that, there were several changes made to different unit tests because windows has slightly different error messages as well as frustrating nuances around killing a process :/ ## Additional Info Tested on my Windows VM and it appears to work, also compiled & tested on Linux with these changes. Permissions look correct on both platforms now. Just waiting for my validator to activate on Prater so I can test running full validator client on windows. Co-authored-by: ethDreamer <37123614+ethDreamer@users.noreply.github.com> Co-authored-by: Michael Sproul --- .github/workflows/test-suite.yml | 14 ++ Cargo.lock | 98 +++++++++++- account_manager/Cargo.toml | 1 + account_manager/src/validator/create.rs | 5 +- account_manager/src/validator/exit.rs | 5 +- account_manager/src/validator/import.rs | 4 +- account_manager/src/validator/recover.rs | 4 +- account_manager/src/wallet/create.rs | 32 +--- account_manager/src/wallet/recover.rs | 4 +- .../beacon_chain/tests/block_verification.rs | 6 +- book/src/installation-source.md | 46 +++--- book/src/slasher.md | 1 + common/account_utils/Cargo.toml | 1 + common/account_utils/src/lib.rs | 27 +--- .../src/validator_definitions.rs | 2 +- common/filesystem/Cargo.toml | 14 ++ common/filesystem/src/lib.rs | 144 ++++++++++++++++++ common/lockfile/src/lib.rs | 17 ++- common/remote_signer_consumer/tests/post.rs | 38 +++-- common/validator_dir/Cargo.toml | 1 + common/validator_dir/src/builder.rs | 20 +-- lighthouse/tests/beacon_node.rs | 7 + remote_signer/backend/src/lib.rs | 21 ++- remote_signer/backend/src/storage_raw_dir.rs | 12 +- remote_signer/tests/get_keys.rs | 4 +- remote_signer/tests/sign.rs | 8 +- slasher/Cargo.toml | 1 + slasher/src/config.rs | 13 ++ slasher/src/database.rs | 9 ++ slasher/src/error.rs | 1 + slasher/tests/attester_slashings.rs | 8 +- slasher/tests/proposer_slashings.rs | 4 +- slasher/tests/random.rs | 2 +- slasher/tests/wrap_around.rs | 4 +- testing/eth1_test_rig/src/ganache.rs | 49 ++++-- testing/remote_signer_test/Cargo.toml | 5 + testing/remote_signer_test/src/utils.rs | 105 ++++++++++++- validator_client/Cargo.toml | 1 + .../slashing_protection/Cargo.toml | 1 + validator_client/slashing_protection/Makefile | 21 ++- .../slashing_protection/src/lib.rs | 1 + .../src/slashing_database.rs | 22 +-- validator_client/src/key_cache.rs | 3 +- 43 files changed, 607 insertions(+), 179 deletions(-) create mode 100644 common/filesystem/Cargo.toml create mode 100644 common/filesystem/src/lib.rs 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),