From a36fceca7097259e1cbe025f79e3a60385e1fc1e Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Tue, 1 Mar 2022 09:24:12 +0000 Subject: [PATCH 01/21] Make a zcashd-wallet-tool executable. Signed-off-by: Daira Hopwood --- .gitignore | 1 + Cargo.lock | 299 ++++++++++++++++++-- Cargo.toml | 11 + doc/release-notes.md | 22 +- src/Makefile.am | 21 +- src/rust/src/wallet_tool.rs | 524 ++++++++++++++++++++++++++++++++++++ src/wallet/rpcwallet.cpp | 6 +- zcutil/clean.sh | 1 + 8 files changed, 846 insertions(+), 39 deletions(-) create mode 100644 src/rust/src/wallet_tool.rs diff --git a/.gitignore b/.gitignore index 601e53519..3402293c6 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ *.exe src/bitcoin src/zcashd +src/zcashd-wallet-tool src/zcash-cli src/zcash-gtest src/zcash-tx diff --git a/Cargo.lock b/Cargo.lock index 93b4f694e..46eb0fb5e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,21 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "addr2line" +version = "0.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9ecd88a8c8378ca913a680cd98f0f13ac67383d35993f86c90a70e3f137816b" +dependencies = [ + "gimli", +] + +[[package]] +name = "adler" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f26201604c87b1e01bd3d98f8d5d9a8fcbb815e8cedb41ffccbeb4bf593a35fe" + [[package]] name = "aead" version = "0.4.3" @@ -17,7 +32,7 @@ version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9e8b47f52ea9bae42228d07ec09eb676433d7c4ed1ebdf0f1d1c29ed446f1ab8" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "cipher", "cpufeatures", "opaque-debug", @@ -85,6 +100,21 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" +[[package]] +name = "backtrace" +version = "0.3.64" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e121dee8023ce33ab248d9ce1493df03c3b38a659b240096fcbd7048ff9c31f" +dependencies = [ + "addr2line", + "cc", + "cfg-if 1.0.0", + "libc", + "miniz_oxide", + "object", + "rustc-demangle", +] + [[package]] name = "base64ct" version = "1.0.1" @@ -136,7 +166,7 @@ checksum = "d0830ae4cc96b0617cc912970c2b17e89456fecbf55e8eed53a956f37ab50c41" dependencies = [ "hmac", "pbkdf2", - "rand", + "rand 0.8.5", "sha2", "unicode-normalization", "zeroize", @@ -284,6 +314,12 @@ version = "1.0.73" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2fff2a6927b3bb87f9595d67196a70493f627687a71d87a0d692242c33f58c11" +[[package]] +name = "cfg-if" +version = "0.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" + [[package]] name = "cfg-if" version = "1.0.0" @@ -296,7 +332,7 @@ version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "01b72a433d0cf2aef113ba70f62634c56fddb0f244e6377185c56a7cadbd8f91" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "cipher", "cpufeatures", "zeroize", @@ -324,6 +360,19 @@ dependencies = [ "generic-array", ] +[[package]] +name = "clearscreen" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a7ed49b0e894fe6264a58496c7ec4e9d3c46f66b59efae527cd5bee429d0a418" +dependencies = [ + "nix", + "terminfo", + "thiserror", + "which", + "winapi", +] + [[package]] name = "constant_time_eq" version = "0.1.5" @@ -345,7 +394,7 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e54ea8bc3fb1ee042f5aace6e3c6e025d3874866da222930f70ce62aceba0bfa" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "crossbeam-utils", ] @@ -355,7 +404,7 @@ version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6455c0ca19f0d2fbf751b908d5c55c1f5cbc65e03c4225427254b46890bdde1e" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "crossbeam-epoch", "crossbeam-utils", ] @@ -366,7 +415,7 @@ version = "0.9.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c00d6d2ea26e8b151d99093005cb442fb9a37aeaca582a03ec70946f49ab5ed9" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "crossbeam-utils", "lazy_static", "memoffset", @@ -379,7 +428,7 @@ version = "0.8.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b5e5bed1f1c269533fa816a0a5492b3545209a205ca1a54842be180eb63a16a6" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "lazy_static", ] @@ -428,7 +477,7 @@ version = "4.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e77a43b28d0668df09411cb0bc9a8c2adc40f9a048afe863e05fd43251e8e39c" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "num_cpus", ] @@ -460,6 +509,16 @@ dependencies = [ "dirs-sys", ] +[[package]] +name = "dirs" +version = "2.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "13aea89a5c93364a98e9b37b2fa237effbb694d5cfe01c5b70941f7eb087d5e3" +dependencies = [ + "cfg-if 0.1.10", + "dirs-sys", +] + [[package]] name = "dirs-sys" version = "0.3.6" @@ -601,7 +660,7 @@ version = "0.1.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8fc3cb4d91f53b50155bdcfd23f6a4c39ae1969c2ae85982b135750cccaf5fce" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "libc", "wasi 0.9.0+wasi-snapshot-preview1", ] @@ -612,11 +671,17 @@ version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d39cd93900197114fa1fcb7ae84ca742095eed9442088988ae74fa744e930e77" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "libc", "wasi 0.10.2+wasi-snapshot-preview1", ] +[[package]] +name = "gimli" +version = "0.26.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "78cc372d058dcf6d5ecd98510e7fbc9e5aec4d21de70f65fea8fecebcd881bd4" + [[package]] name = "group" version = "0.11.0" @@ -629,6 +694,26 @@ dependencies = [ "subtle", ] +[[package]] +name = "gumdrop" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46571f5d540478cf70d2a42dd0d6d8e9f4b9cc7531544b93311e657b86568a0b" +dependencies = [ + "gumdrop_derive", +] + +[[package]] +name = "gumdrop_derive" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "915ef07c710d84733522461de2a734d4d62a3fd39a4d4f404c2f385ef8618d05" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "halo2" version = "0.1.0-beta.1" @@ -639,7 +724,7 @@ dependencies = [ "ff", "group", "pasta_curves", - "rand", + "rand 0.8.5", "rayon", ] @@ -770,7 +855,7 @@ version = "0.1.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7a5bbe824c507c5da5956355e86a746d82e0e1464f65d862cc5e71da70e94b2c" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", ] [[package]] @@ -836,13 +921,16 @@ checksum = "33a33a362ce288760ec6a508b94caaec573ae7d3bbbd91b87aa0bad4456839db" name = "librustzcash" version = "0.2.0" dependencies = [ + "backtrace", "bellman", "blake2b_simd 1.0.0", "blake2s_simd 1.0.0", "bls12_381", "byteorder", + "clearscreen", "ed25519-zebra", "group", + "gumdrop", "hyper", "incrementalmerkletree", "ipnet", @@ -853,10 +941,12 @@ dependencies = [ "metrics-exporter-prometheus", "nonempty", "orchard", + "rand 0.8.5", "rand_core 0.6.3", "secp256k1", "subtle", "thiserror", + "time", "tokio", "tracing", "tracing-appender", @@ -885,7 +975,7 @@ version = "0.4.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "51b9bbe6c47d51fc3e1a9b945965946b4c44142ab8792c50835a980d362c2710" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", ] [[package]] @@ -992,6 +1082,16 @@ dependencies = [ "sketches-ddsketch", ] +[[package]] +name = "miniz_oxide" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a92518e98c078586bc6c934028adcca4c92a53d6a958196de835170a01d84e4b" +dependencies = [ + "adler", + "autocfg", +] + [[package]] name = "mio" version = "0.8.0" @@ -1023,6 +1123,29 @@ dependencies = [ "smallvec", ] +[[package]] +name = "nix" +version = "0.22.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e4916f159ed8e5de0082076562152a76b7a1f64a01fd9d1e0fea002c37624faf" +dependencies = [ + "bitflags", + "cc", + "cfg-if 1.0.0", + "libc", + "memoffset", +] + +[[package]] +name = "nom" +version = "5.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ffb4262d26ed83a1c0a33a38fe2bb15797329c85770da05e6b828ddb782627af" +dependencies = [ + "memchr", + "version_check", +] + [[package]] name = "nonempty" version = "0.7.0" @@ -1087,6 +1210,15 @@ dependencies = [ "libc", ] +[[package]] +name = "object" +version = "0.27.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "67ac1d3f9a1d3616fd9a60c8d74296f22406a238b6a72f5cc1e6f314df4ffbf9" +dependencies = [ + "memchr", +] + [[package]] name = "once_cell" version = "1.9.0" @@ -1119,7 +1251,7 @@ dependencies = [ "memuse", "nonempty", "pasta_curves", - "rand", + "rand 0.8.5", "reddsa", "serde", "subtle", @@ -1161,7 +1293,7 @@ version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d76e8e1493bcac0d2766c42737f34458f1c8c50c0d23bcb24ea953affb273216" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "instant", "libc", "redox_syscall", @@ -1190,7 +1322,7 @@ dependencies = [ "ff", "group", "lazy_static", - "rand", + "rand 0.8.5", "static_assertions", "subtle", ] @@ -1205,6 +1337,44 @@ dependencies = [ "password-hash", ] +[[package]] +name = "phf" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3dfb61232e34fcb633f43d12c58f83c1df82962dcdfa565a4e866ffc17dafe12" +dependencies = [ + "phf_shared", +] + +[[package]] +name = "phf_codegen" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cbffee61585b0411840d3ece935cce9cb6321f01c45477d30066498cd5e1a815" +dependencies = [ + "phf_generator", + "phf_shared", +] + +[[package]] +name = "phf_generator" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "17367f0cc86f2d25802b2c26ee58a7b23faeccf78a396094c13dced0d0182526" +dependencies = [ + "phf_shared", + "rand 0.7.3", +] + +[[package]] +name = "phf_shared" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c00cf8b9eafe68dde5e9eaa2cef8ee84a9336a47d566ec55ca16589633b65af7" +dependencies = [ + "siphasher", +] + [[package]] name = "pin-project" version = "1.0.10" @@ -1304,6 +1474,20 @@ dependencies = [ "nibble_vec", ] +[[package]] +name = "rand" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a6b1679d49b24bbfe0c803429aa1874472f50d9b363131f0e89fc356b544d03" +dependencies = [ + "getrandom 0.1.16", + "libc", + "rand_chacha 0.2.2", + "rand_core 0.5.1", + "rand_hc", + "rand_pcg", +] + [[package]] name = "rand" version = "0.8.5" @@ -1311,10 +1495,20 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" dependencies = [ "libc", - "rand_chacha", + "rand_chacha 0.3.1", "rand_core 0.6.3", ] +[[package]] +name = "rand_chacha" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f4c8ed856279c9737206bf725bf36935d8666ead7aa69b52be55af369d193402" +dependencies = [ + "ppv-lite86", + "rand_core 0.5.1", +] + [[package]] name = "rand_chacha" version = "0.3.1" @@ -1343,6 +1537,24 @@ dependencies = [ "getrandom 0.2.5", ] +[[package]] +name = "rand_hc" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ca3129af7b92a17112d59ad498c6f81eaf463253766b90396d39ea7a39d6613c" +dependencies = [ + "rand_core 0.5.1", +] + +[[package]] +name = "rand_pcg" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "16abd0c1b639e9eb4d7c50c0b8100b0d0f849be2349829c740fe8e6eb4816429" +dependencies = [ + "rand_core 0.5.1", +] + [[package]] name = "raw-cpuid" version = "10.2.0" @@ -1464,6 +1676,12 @@ dependencies = [ "digest 0.10.3", ] +[[package]] +name = "rustc-demangle" +version = "0.1.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7ef03e0a2b150c7a90d01faf6254c9c48a41e95fb2a8c2ac1c6f0d2b9aefc342" + [[package]] name = "scopeguard" version = "1.1.0" @@ -1515,7 +1733,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4d58a1e1bf39749807d89cf2d98ac2dfa0ff1cb3faa38fbb64dd88ac8013d800" dependencies = [ "block-buffer 0.9.0", - "cfg-if", + "cfg-if 1.0.0", "cpufeatures", "digest 0.9.0", "opaque-debug", @@ -1530,6 +1748,12 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "siphasher" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a86232ab60fa71287d7f2ddae4a7073f6b7aac33631c3015abb556f08c6d0a3e" + [[package]] name = "sketches-ddsketch" version = "0.1.2" @@ -1548,7 +1772,7 @@ version = "0.3.19" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "122e570113d28d773067fab24266b66753f6ea915758651696b6e35e49f88d6e" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "libc", "winapi", ] @@ -1610,6 +1834,19 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" +[[package]] +name = "terminfo" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "76971977e6121664ec1b960d1313aacfa75642adc93b9d4d53b247bd4cb1747e" +dependencies = [ + "dirs", + "fnv", + "nom", + "phf", + "phf_codegen", +] + [[package]] name = "thiserror" version = "1.0.30" @@ -1648,8 +1885,15 @@ dependencies = [ "itoa 1.0.1", "libc", "num_threads", + "time-macros", ] +[[package]] +name = "time-macros" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "25eb0ca3468fc0acc11828786797f6ef9aa1555e4a211a60d64cc8e4d1be47d6" + [[package]] name = "tinyvec" version = "1.5.1" @@ -1702,7 +1946,7 @@ version = "0.1.31" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f6c650a8ef0cd2dd93736f033d21cbd1224c5a967aa0c258d00fcf7dafef9b9f" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "pin-project-lite", "tracing-attributes", "tracing-core", @@ -1840,7 +2084,7 @@ version = "0.2.79" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "25f1af7423d8588a3d840681122e72e6a24ddbcb3f0ec385cac0d12d24256c06" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "wasm-bindgen-macro", ] @@ -1898,6 +2142,17 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "which" +version = "4.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a5a7e487e921cf220206864a94a89b6c6905bfc19f1057fa26a4cb360e5c1d2" +dependencies = [ + "either", + "lazy_static", + "libc", +] + [[package]] name = "winapi" version = "0.3.9" @@ -1996,7 +2251,7 @@ dependencies = [ "memuse", "nonempty", "orchard", - "rand", + "rand 0.8.5", "rand_core 0.6.3", "ripemd", "secp256k1", diff --git a/Cargo.toml b/Cargo.toml index e163acca3..e544a2d4a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,10 @@ name = "rustzcash" path = "src/rust/src/rustzcash.rs" crate-type = ["staticlib"] +[[bin]] +name = "zcashd-wallet-tool" +path = "src/rust/src/wallet_tool.rs" + [dependencies] bellman = "0.11" blake2b_simd = "1" @@ -60,6 +64,13 @@ metrics-exporter-prometheus = "0.6" thiserror = "1" tokio = { version = "1.0", features = ["rt", "net", "time", "macros"] } +# Wallet tool +backtrace = "0.3" +clearscreen = "1.0" +gumdrop = "0.8" +rand = "0.8" +time = { version = "0.3", features = ["formatting", "macros"] } + [dependencies.tracing-subscriber] version = "0.3" default-features = false diff --git a/doc/release-notes.md b/doc/release-notes.md index 796a44d7c..f70d71263 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -19,17 +19,17 @@ Zcashd release. Following the upgrade to 4.5.2, Zcashd will require that the user confirm that they have backed up their new emergency recovery phrase, which may be obtained from the output of the `z_exportwallet` RPC call. This confirmation can be -performed manually using the `zcashd-wallet-tool` utility that is supplied with -this release. The wallet will not allow the generation of new addresses until -this confirmation has been performed. It is recommended that after this -upgrade, that funds tied to preexisting addresses be migrated to newly -generated addresses so that all wallet funds are recoverable using the -emergency recovery phrase going forward. If you choose not to migrate funds in -this fashion, you will continue to need to securely back up the entire -`wallet.dat` file to ensure that you do not lose access to existing funds; -EXISTING FUNDS WILL NOT BE RECOVERABLE USING THE EMERGENCY RECOVERY PHRASE -UNLESS THEY HAVE BEEN MOVED TO A NEWLY GENERATED ADDRESS FOLLOWING THE 4.5.2 -UPGRADE. +performed manually using the `zcashd-wallet-tool` utility that is supplied +with this release (built or installed in the same directory as `zcashd`). +The wallet will not allow the generation of new addresses until this +confirmation has been performed. It is recommended that after this upgrade, +that funds tied to preexisting addresses be migrated to newly generated +addresses so that all wallet funds are recoverable using the emergency +recovery phrase going forward. If you choose not to migrate funds in this +fashion, you will continue to need to securely back up the entire `wallet.dat` +file to ensure that you do not lose access to existing funds; EXISTING FUNDS +WILL NOT BE RECOVERABLE USING THE EMERGENCY RECOVERY PHRASE UNLESS THEY HAVE +BEEN MOVED TO A NEWLY GENERATED ADDRESS FOLLOWING THE 4.5.2 UPGRADE. New RPC Methods --------------- diff --git a/src/Makefile.am b/src/Makefile.am index 388c3f1ff..e3ce37c62 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -29,6 +29,9 @@ LIBSECP256K1=secp256k1/libsecp256k1.la LIBUNIVALUE=univalue/libunivalue.la LIBZCASH=libzcash.a +WALLET_TOOL_BIN=zcashd-wallet-tool$(EXEEXT) +WALLET_TOOL_BUILD=$(top_builddir)/target/$(RUST_TARGET)/release/zcashd-wallet-tool$(EXEEXT) + if ENABLE_ZMQ LIBBITCOIN_ZMQ=libbitcoin_zmq.a endif @@ -47,7 +50,7 @@ endif # - Note that this does not prevent the secp256k1-sys vendored code from being built; this # requires https://github.com/rust-bitcoin/rust-secp256k1/issues/380 to be addressed. RUST_ENV_VARS = RUSTC="$(RUSTC)" TERM=dumb RUSTFLAGS="--cfg=rust_secp_no_symbol_renaming" -RUST_BUILD_OPTS = --lib --release --target $(RUST_TARGET) +RUST_BUILD_OPTS = --release --target $(RUST_TARGET) --manifest-path $(top_srcdir)/Cargo.toml rust_verbose = $(rust_verbose_@AM_V@) rust_verbose_ = $(rust_verbose_@AM_DEFAULT_V@) @@ -72,10 +75,16 @@ $(CARGO_CONFIGURED): $(top_srcdir)/.cargo/config.offline $(AM_V_at)touch $@ endif -cargo-build: $(CARGO_CONFIGURED) - $(RUST_ENV_VARS) $(CARGO) build $(RUST_BUILD_OPTS) $(rust_verbose) --manifest-path $(top_srcdir)/Cargo.toml +cargo-build-lib: $(CARGO_CONFIGURED) + $(RUST_ENV_VARS) $(CARGO) build --lib $(RUST_BUILD_OPTS) $(rust_verbose) -$(LIBRUSTZCASH): cargo-build +cargo-build-bins: $(CARGO_CONFIGURED) + $(RUST_ENV_VARS) $(CARGO) build --bins $(RUST_BUILD_OPTS) $(rust_verbose) + +$(WALLET_TOOL_BIN): cargo-build-bins + $(AM_V_at)cp $(WALLET_TOOL_BUILD) $@ + +$(LIBRUSTZCASH): cargo-build-lib $(LIBSECP256K1): $(wildcard secp256k1/src/*) $(wildcard secp256k1/include/*) $(AM_V_at)$(MAKE) $(AM_MAKEFLAGS) -C $(@D) $(@F) @@ -99,6 +108,7 @@ lib_LTLIBRARIES = $(LIBZCASH_SCRIPT) bin_PROGRAMS = noinst_PROGRAMS = +bin_SCRIPTS = TESTS = BENCHMARKS = @@ -108,6 +118,7 @@ endif if BUILD_BITCOIN_UTILS bin_PROGRAMS += zcash-cli zcash-tx + bin_SCRIPTS += $(WALLET_TOOL_BIN) endif LIBZCASH_H = \ @@ -129,7 +140,7 @@ LIBZCASH_H = \ zcash/util.h \ zcash/Zcash.h -.PHONY: FORCE cargo-build check-symbols check-security +.PHONY: FORCE cargo-build-lib cargo-build-bins check-symbols check-security # bitcoin core # BITCOIN_CORE_H = \ addrdb.h \ diff --git a/src/rust/src/wallet_tool.rs b/src/rust/src/wallet_tool.rs new file mode 100644 index 000000000..ba64f681b --- /dev/null +++ b/src/rust/src/wallet_tool.rs @@ -0,0 +1,524 @@ +use std::cmp::min; +use std::env::{self, consts::EXE_EXTENSION}; +use std::ffi::OsStr; +use std::fs::File; +use std::io::{self, BufRead, ErrorKind, Stdin}; +use std::iter; +use std::panic; +use std::path::{Path, PathBuf}; +use std::process::{self, Command, Output}; +use std::str::from_utf8; +use std::time::SystemTime; + +use backtrace::Backtrace; +use gumdrop::{Options, ParsingStyle}; +use rand::{thread_rng, Rng}; +use time::macros::format_description; +use time::OffsetDateTime; + +#[derive(Debug, Options)] +struct CliOptions { + #[options(no_short, help = "Print this help output")] + help: bool, + + #[options(no_short, help = "Display debugging output")] + debug: bool, + + #[options( + no_short, + help = "Specify configuration file (default: zcash.conf)", + meta = "PATH" + )] + conf: Option, + + #[options(no_short, help = "Specify data directory")] + datadir: Option, + + #[options(no_short, help = "Use the test chain")] + testnet: bool, + + #[options( + no_short, + help = "Send commands to node running on IPADDR (default: 127.0.0.1)", + meta = "IPADDR" + )] + rpcconnect: Option, + + #[options( + no_short, + help = "Connect to JSON-RPC on PORT (default: 8232 or testnet: 18232)", + meta = "PORT" + )] + rpcport: Option, + + #[options( + no_short, + help = "Username for JSON-RPC connections", + meta = "USERNAME" + )] + rpcuser: Option, + + #[options( + no_short, + help = "Password for JSON-RPC connections", + meta = "PASSWORD" + )] + rpcpassword: Option, + + #[options( + no_short, + help = "Timeout in seconds during HTTP requests, or 0 for no timeout. (default: 900)", + meta = "SECONDS" + )] + rpcclienttimeout: Option, +} + +impl CliOptions { + fn to_zcash_cli_options(&self) -> Vec { + iter::empty::() + .chain(self.conf.as_ref().map(|o| format!("-conf={}", o))) + .chain(self.datadir.as_ref().map(|o| format!("-datadir={}", o))) + .chain(if self.testnet { + Some("-testnet".into()) + } else { + None + }) + .chain( + self.rpcconnect + .as_ref() + .map(|o| format!("-rpcconnect={}", o)), + ) + .chain(self.rpcport.as_ref().map(|o| format!("-rpcport={}", o))) + .chain(self.rpcuser.as_ref().map(|o| format!("-rpcuser={}", o))) + .chain( + self.rpcpassword + .as_ref() + .map(|o| format!("-rpcpassword={}", o)), + ) + .chain( + self.rpcclienttimeout + .as_ref() + .map(|o| format!("-rpcclienttimeout={}", o)), + ) + .collect() + } +} + +pub fn main() { + // Allow either Bitcoin-style or GNU-style arguments. + let mut args = env::args(); + let command = args.next().expect("argv[0] should exist"); + let args: Vec<_> = args + .map(|s| { + if s.starts_with('-') && !s.starts_with("--") { + format!("-{}", s) + } else { + s + } + }) + .collect(); + + const USAGE_NOTE: &str = concat!( + "Options can be given in GNU style (`--conf=CONF` or `--conf CONF`),\n", + "or in Bitcoin style with a single hyphen (`-conf=CONF`).\n" + ); + + let opts = CliOptions::parse_args(&args, ParsingStyle::default()).unwrap_or_else(|e| { + eprintln!( + "{}: {}\n\nUsage: {} [OPTIONS]\n\n{}\n\n{}", + command, + e, + command, + CliOptions::usage(), + USAGE_NOTE, + ); + process::exit(2); + }); + + if opts.help_requested() { + println!( + "Usage: {} [OPTIONS]\n\n{}\n\n{}", + command, + opts.self_usage(), + USAGE_NOTE + ); + process::exit(0); + } + + if let Err(e) = run(&opts) { + eprintln!("{}: {}", command, e); + process::exit(1); + } +} + +fn run(opts: &CliOptions) -> Result<(), io::Error> { + let cli_options: Vec = opts.to_zcash_cli_options(); + + println!(concat!( + "To reduce the risk of loss of funds, we're going to confirm that the\n", + "zcashd wallet is backed up reliably.\n\n", + " 👛 ➜ 🗃️ \n" + )); + + println!("Checking that we can connect to zcashd..."); + let zcash_cli = zcash_cli_path(opts.debug)?; + + // Pass an invalid filename, "\x01", and use the error message to distinguish + // whether zcashd is running with the -exportdir option, running without that + // option, or not running / cannot connect. + let mut cli_args = cli_options.clone(); + cli_args.extend_from_slice(&["z_exportwallet".to_string(), "\x01".to_string()]); + let out = exec(&zcash_cli, &cli_args, opts.debug)?; + let cli_err: Vec<_> = from_utf8(&out.stderr) + .map_err(|_| io::Error::from(ErrorKind::InvalidData))? + .lines() + .map(|s| s.trim_end_matches('\r')) + .collect(); + + if cli_err.len() < 3 || cli_err[0] != "error code: -4" || cli_err[1] != "error message:" { + // TODO: distinguish not running case more precisely + println!(concat!( + "\nNo, we could not connect. zcashd might not be running; in that case\n", + "please start it. The '-exportdir' option should be set to the absolute\n", + "path of the directory you want to save the wallet export file to.\n\n", + "(Don't forget to restart zcashd without '-exportdir' after finishing\n", + "the backup, if running it long-term with that option is not desired\n", + "or would be a security hazard in your environment.)\n\n", + "If you believe zcashd is running, it might be using an unexpected port,\n", + "address, or authentication options for the RPC interface, for example.\n", + "In that case try to connect to it using zcash-cli, and if successful,\n", + "use the same connection options for zcashd-wallet-tool (see '--help' for\n", + "accepted options) as for zcash-cli.\n" + )); + return Err(io::Error::from(ErrorKind::Other)); + } + if cli_err[2].contains("zcashd -exportdir") { + println!(concat!( + "\nIt looks like zcashd is running without the '-exportdir' option.\n\n", + "Please start or restart zcashd with '-exportdir' set to the absolute\n", + "path of the directory you want to save the wallet export file to.\n", + "(Don't forget to restart zcashd without '-exportdir' after finishing\n", + "the backup, if running it long-term with that option is not desired\n", + "or would be a security hazard in your environment.)" + )); + return Err(io::Error::from(ErrorKind::Other)); + } + println!("Yes, and it is running with the '-exportdir' option as required."); + + let mut stdin = io::stdin(); + + let base = default_filename_base(); + let mut r = 0u32; + let out = loop { + let default_filename = if r != 0 { + format!("{}r{}", base, r) + } else { + base.to_string() + }; + println!( + concat!( + "\nEnter the filename for the wallet export file, using only characters\n", + "a-z, A-Z and 0-9 (default '{}')." + ), + default_filename + ); + let mut buf = String::new(); + let response = prompt(&mut stdin, &mut buf)?; + let filename = if response.is_empty() { + r = r.saturating_add(1); + &default_filename + } else { + response + }; + if opts.debug { + eprintln!("DEBUG: Using filename {:?}", filename); + } + + let mut cli_args = cli_options.clone(); + cli_args.extend_from_slice(&["z_exportwallet".to_string(), filename.to_string()]); + let out = exec(&zcash_cli, &cli_args, opts.debug)?; + let cli_err: Vec<_> = from_utf8(&out.stderr) + .map_err(|_| io::Error::from(ErrorKind::InvalidData))? + .lines() + .map(|s| s.trim_end_matches('\r')) + .collect(); + if opts.debug { + eprintln!("DEBUG: stderr {:?}", cli_err); + } + if cli_err.len() >= 3 + && cli_err[0] == "error code: -8" + && cli_err[1] == "error message:" + && cli_err[2].contains("overwrite existing") + { + println!(concat!( + "That file already exists. Please pick a unique filename in the\n", + "directory specified by the '-exportdir' option to zcashd." + )); + continue; + } else { + break out; + } + }; + + let cli_out: Vec<_> = from_utf8(&out.stdout) + .map_err(|_| io::Error::from(ErrorKind::InvalidData))? + .lines() + .map(|s| s.trim_end_matches('\r')) + .collect(); + if opts.debug { + eprintln!("DEBUG: stdout {:?}", cli_out); + } + if cli_out.is_empty() { + return Err(io::Error::from(ErrorKind::InvalidData)); + } + let export_path = cli_out[0]; + + println!("\nSaved the export file to '{}'.", export_path); + println!("IMPORTANT: This file contains secrets that allow spending all wallet funds.\n"); + + let export_file = File::open(export_path)?; + let phrase_line: Vec<_> = io::BufReader::new(export_file) + .lines() + .filter(|s| { + s.as_ref() + .map(|t| t.starts_with("# - recovery_phrase=\"")) + .unwrap_or(false) + }) + .collect(); + if phrase_line.len() != 1 || phrase_line[0].is_err() { + return Err(io::Error::from(ErrorKind::InvalidData)); + } + let phrase = phrase_line[0] + .as_ref() + .unwrap() + .trim_start_matches("# - recovery_phrase=\"") + .trim_end_matches('"'); + + // This panic hook allows us to make a best effort to clear the screen (and then print + // another reminder about secrets in the export file) even if a panic occurs. + + let moved_export_path = export_path.to_string(); // borrow checker workaround + let old_hook = panic::take_hook(); + panic::set_hook(Box::new(move |panic_info| { + clear_and_show_cautions(&moved_export_path); + + let s = panic_info.payload().downcast_ref::<&str>().unwrap_or(&""); + eprintln!("\nPanic: {}\n{:?}", s, Backtrace::new()); + })); + + let res = (|| -> io::Result<()> { + println!("The recovery phrase is:\n"); + + const WORDS_PER_LINE: usize = 3; + + let words: Vec<_> = phrase.split(' ').collect(); + let max_len = words.iter().map(|w| w.len()).max().unwrap_or(0); + + for (i, word) in words.iter().enumerate() { + print!("{0:2}: {1:2$}", i + 1, word, max_len + 2); + if (i + 1) % WORDS_PER_LINE == 0 { + println!(); + } + } + if words.len() % WORDS_PER_LINE != 0 { + println!(); + } + + println!(concat!( + "\nPlease write down this phrase on something durable that you will keep\n", + "in a secure location.\n", + "Press Enter when finished; then the phrase will disappear and you'll be\n", + "asked to re-enter a selection of words from it." + )); + + let mut stdin = io::stdin(); + let mut buf = String::new(); + prompt(&mut stdin, &mut buf)?; + + // The only reliable and portable way to make sure the recovery phrase + // is no longer displayed is to clear the whole terminal (including + // scrollback, if possible). The text is only printed if clearing fails. + try_to_clear(concat!( + "\n\n\n\n\n\n\n\n\n\n\n\n", + "Please adjust the terminal window so that you can't see the\n", + "recovery phrase above. After finishing the backup, clear the\n", + "terminal window" + )); + + println!("\nNow we're going to confirm that you backed up the recovery phrase."); + + let mut rng = thread_rng(); + let mut unconfirmed: Vec = (0..words.len()).collect(); + for _ in 0..min(3, words.len()) { + let index: usize = rng.gen_range(0..unconfirmed.len()); + let n = unconfirmed[index]; + unconfirmed[index] = unconfirmed[unconfirmed.len() - 1]; + unconfirmed.pop().expect("should be nonempty"); + + loop { + let mut buf = String::new(); + println!("\nPlease enter the {} word:", ordinal(n + 1)); + let line = prompt(&mut stdin, &mut buf)?; + if words[n] == line { + break; + } + println!("That's not correct, please try again."); + } + } + Ok(()) + })(); + + panic::set_hook(old_hook); + clear_and_show_cautions(export_path); + res?; + + let mut cli_args = cli_options; + cli_args.extend_from_slice(&["walletconfirmbackup".to_string(), phrase.to_string()]); + + // Always pass false for debug to avoid printing the recovery phrase. + exec(&zcash_cli, &cli_args, false) + .and_then(|out| { + let cli_err: Vec<_> = from_utf8(&out.stderr) + .map_err(|_| io::Error::from(ErrorKind::InvalidData))? + .lines() + .map(|s| s.trim_end_matches('\r')) + .collect(); + if opts.debug { + eprintln!("DEBUG: stderr {:?}", cli_err); + } + if !cli_err.is_empty() { + // TODO: distinguish errors: cannot connect, vs incorrect passphrase + // (RPC_WALLET_PASSPHRASE_INCORRECT), vs other errors. + Err(io::Error::from(ErrorKind::Other)) + } else { + println!(concat!( + "\nThe backup of the emergency recovery phrase for the zcashd\n", + "wallet has been successfully confirmed 🙂. You can now use the\n", + "zcashd RPC methods that create keys and addresses in that wallet.\n\n", + "If you use other wallets, their recovery information will need\n", + "to be backed up separately.\n" + )); + Ok(()) + } + }) + .map_err(|e| { + println!(concat!( + "\nzcash-wallet-tool was unable to communicate to zcashd that the\n", + "backup was confirmed. This can happen if zcashd stopped, in which\n", + "case you should try again. If zcashd is still running, please seek\n", + "help or try to use 'zcash-cli -stdin walletconfirmbackup' manually.\n" + )); + e + }) +} + +fn prompt<'a>(input: &mut Stdin, buf: &'a mut String) -> io::Result<&'a str> { + let n = input.read_line(buf)?; + if n == 0 { + return Err(io::Error::from(ErrorKind::UnexpectedEof)); + } + Ok(buf.trim_end_matches(|c| c == '\r' || c == '\n').trim()) +} + +fn ordinal(num: usize) -> String { + let suffix = if (11..=13).contains(&(num % 100)) { + "th" + } else { + match num % 10 { + 1 => "st", + 2 => "nd", + 3 => "rd", + _ => "th", + } + }; + format!("{}{}", num, suffix) +} + +fn zcash_cli_path(debug: bool) -> io::Result { + // First look for `zcash_cli[.exe]` as a sibling of the executable. + let mut exe = env::current_exe()?; + exe.set_file_name("zcash-cli"); + exe.set_extension(EXE_EXTENSION); + if debug { + eprintln!("DEBUG: Testing for zcash-cli at {:?}", exe); + } + if exe.exists() { + return Ok(exe); + } + // If not found there, look in `../src/zcash_cli[.exe]` provided + // that `src` is a sibling of `target`. + exe.pop(); // strip filename + exe.pop(); // .. + if exe.file_name() != Some(OsStr::new("target")) { + // or in `../../src/zcash_cli[.exe]` under the same proviso + exe.pop(); // ../.. + if exe.file_name() != Some(OsStr::new("target")) { + return Err(io::Error::from(ErrorKind::NotFound)); + } + } + // Replace 'target/' with 'src/'. + exe.set_file_name("src"); + exe.push("zcash-cli"); + exe.set_extension(EXE_EXTENSION); + if debug { + eprintln!("DEBUG: Testing for zcash-cli at {:?}", exe); + } + if exe.exists() { + Ok(exe) + } else { + Err(io::Error::from(ErrorKind::NotFound)) + } +} + +fn exec(exe_path: &Path, args: &[String], debug: bool) -> Result { + if debug { + eprintln!("DEBUG: Running {:?} {:?}", exe_path, args); + } + Command::new(exe_path).args(args).output() +} + +fn default_filename_base() -> String { + let format = format_description!("export[year][month][day]"); + + // We use the UTC date because there is a security issue in obtaining the local date + // from either `chrono` or `time`: . + // We could use the approach in + // + // if it were important, but it isn't worth the dependency on `tz-rs`. + + OffsetDateTime::from(SystemTime::now()) + .format(&format) + .unwrap_or_else(|_| "export".to_string()) +} + +fn clear_and_show_cautions(export_path: &str) { + try_to_clear(concat!( + "\nCAUTION: This terminal window might be showing secrets (or have\n", + "them in the scrollback). Please copy any useful information and\n", + "then clear it" + )); + + println!( + concat!( + "\nIMPORTANT: Secrets that allow spending all zcashd wallet funds\n", + "have been left in the file '{}'.\n\n", + "Don't forget to restart zcashd without '-exportdir', if running it\n", + "long-term with that option is not desired or would be a security\n", + "hazard in your environment." + ), + export_path, + ); +} + +fn try_to_clear(error_blurb: &str) { + if let Err(e) = clearscreen::clear() { + eprintln!("Unable to clear screen: {}.", e); + + #[cfg(target_os = "windows")] + const CLEAR: &str = "cls"; + #[cfg(not(target_os = "windows"))] + const CLEAR: &str = "clear"; + + println!("{} using '{}'.", error_blurb, CLEAR); + } +} diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 81400f4e0..248f031a1 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1750,7 +1750,11 @@ UniValue walletconfirmbackup(const UniValue& params, bool fHelp) if (fHelp || params.size() != 1) throw runtime_error( "walletconfirmbackup \"emergency recovery phrase\"\n" - "\nNotify the wallet that the user has backed up the emergency recovery phrase,\n" + "\nCAUTION: This is an internal method that is not intended to be called directly by\n" + "users. Please use the zcashd-wallet-tool utility (built or installed in the same directory\n" + "as zcashd) instead. In particular, this method should not be used from zcash-cli, in order\n" + "to avoid exposing the recovery phrase on the command line.\n\n" + "Notify the wallet that the user has backed up the emergency recovery phrase,\n" "which can be obtained by making a call to z_exportwallet. The zcashd embedded wallet\n" "requires confirmation that the emergency recovery phrase has been backed up before it\n" "will permit new spending keys or addresses to be generated.\n" diff --git a/zcutil/clean.sh b/zcutil/clean.sh index 01738dcb0..596cb3e09 100755 --- a/zcutil/clean.sh +++ b/zcutil/clean.sh @@ -80,6 +80,7 @@ clean_dirs __pycache__ clean_exe src/bench/bench_bitcoin clean_exe src/zcash-cli clean_exe src/zcashd +clean_exe src/zcashd-wallet-tool clean_exe src/zcash-gtest clean_exe src/zcash-tx clean_exe src/test/test_bitcoin From de933c1cb5296505ac13dd604fd12abf25148cf0 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Sun, 27 Feb 2022 23:06:05 +0000 Subject: [PATCH 02/21] Attempt to fix linking problem on ARM. Signed-off-by: Daira Hopwood --- .cargo/config.offline | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.cargo/config.offline b/.cargo/config.offline index 1f8659ab3..03e4c12ab 100644 --- a/.cargo/config.offline +++ b/.cargo/config.offline @@ -1,3 +1,6 @@ +[target.aarch64-unknown-linux-gnu] +linker = "aarch64-linux-gnu-gcc" + [source.crates-io] replace-with = "vendored-sources" From 8ec0d854b9ca66bcc7f76f8b0dc317d613bb12b9 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Mon, 28 Feb 2022 10:15:58 +0000 Subject: [PATCH 03/21] Add some text about choosing location of the physical backup. Add TODO for better handling of file not found and permission errors. Signed-off-by: Daira Hopwood --- src/rust/src/wallet_tool.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/rust/src/wallet_tool.rs b/src/rust/src/wallet_tool.rs index ba64f681b..7b4e8b1a9 100644 --- a/src/rust/src/wallet_tool.rs +++ b/src/rust/src/wallet_tool.rs @@ -276,6 +276,7 @@ fn run(opts: &CliOptions) -> Result<(), io::Error> { println!("\nSaved the export file to '{}'.", export_path); println!("IMPORTANT: This file contains secrets that allow spending all wallet funds.\n"); + // TODO: better handling of file not found and permission errors. let export_file = File::open(export_path)?; let phrase_line: Vec<_> = io::BufReader::new(export_file) .lines() @@ -504,7 +505,10 @@ fn clear_and_show_cautions(export_path: &str) { "have been left in the file '{}'.\n\n", "Don't forget to restart zcashd without '-exportdir', if running it\n", "long-term with that option is not desired or would be a security\n", - "hazard in your environment." + "hazard in your environment.\n\n", + "When choosing a location for the physical backup of your emergency\n", + "recovery phrase, please make sure to consider both risk of theft,\n", + "and your long-term ability to remember where it is kept." ), export_path, ); From 25792cba93d0aa48b7760d95da32d5246c06eb19 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Tue, 1 Mar 2022 00:47:03 +0000 Subject: [PATCH 04/21] Move `wallet_tool.rs` from `src/rust/src` into `src/rust/bin`. Also add a brief description of `zcashd-wallet-tool` to `src/rust/README.md`. Signed-off-by: Daira Hopwood --- Cargo.toml | 2 +- src/rust/README.md | 5 +++++ src/rust/{src => bin}/wallet_tool.rs | 0 3 files changed, 6 insertions(+), 1 deletion(-) rename src/rust/{src => bin}/wallet_tool.rs (100%) diff --git a/Cargo.toml b/Cargo.toml index e544a2d4a..02b61f5ba 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,7 +27,7 @@ crate-type = ["staticlib"] [[bin]] name = "zcashd-wallet-tool" -path = "src/rust/src/wallet_tool.rs" +path = "src/rust/bin/wallet_tool.rs" [dependencies] bellman = "0.11" diff --git a/src/rust/README.md b/src/rust/README.md index a6e8e97f2..790db5d11 100644 --- a/src/rust/README.md +++ b/src/rust/README.md @@ -6,6 +6,11 @@ the `zcashd` full node. The FFI API does not have any stability guarantees, and will change as required by `zcashd`. +# zcashd-wallet-tool + +`zcashd-wallet-tool` is a command-line tool that allows confirming to a `zcashd` +node that the emergency recovery phrase of the node's wallet has been backed up. + ## License Licensed under either of diff --git a/src/rust/src/wallet_tool.rs b/src/rust/bin/wallet_tool.rs similarity index 100% rename from src/rust/src/wallet_tool.rs rename to src/rust/bin/wallet_tool.rs From 6f5efcbb0fefec49031319b8f980047150ddb7f2 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Mon, 7 Mar 2022 15:00:59 +0000 Subject: [PATCH 05/21] Improved error handling. Signed-off-by: Daira Hopwood --- Cargo.lock | 7 ++ Cargo.toml | 2 + src/rust/bin/wallet_tool.rs | 140 ++++++++++++++++++++++++------------ 3 files changed, 104 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 46eb0fb5e..bef8caef4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -67,6 +67,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "anyhow" +version = "1.0.55" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "159bb86af3a200e19a068f4224eae4c8bb2d0fa054c7e5d1cacd5cef95e684cd" + [[package]] name = "arrayref" version = "0.3.6" @@ -921,6 +927,7 @@ checksum = "33a33a362ce288760ec6a508b94caaec573ae7d3bbbd91b87aa0bad4456839db" name = "librustzcash" version = "0.2.0" dependencies = [ + "anyhow", "backtrace", "bellman", "blake2b_simd 1.0.0", diff --git a/Cargo.toml b/Cargo.toml index 02b61f5ba..083b58b7b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -65,6 +65,8 @@ thiserror = "1" tokio = { version = "1.0", features = ["rt", "net", "time", "macros"] } # Wallet tool +# (also depends on thiserror) +anyhow = "1.0" backtrace = "0.3" clearscreen = "1.0" gumdrop = "0.8" diff --git a/src/rust/bin/wallet_tool.rs b/src/rust/bin/wallet_tool.rs index 7b4e8b1a9..e42bdd3df 100644 --- a/src/rust/bin/wallet_tool.rs +++ b/src/rust/bin/wallet_tool.rs @@ -10,9 +10,11 @@ use std::process::{self, Command, Output}; use std::str::from_utf8; use std::time::SystemTime; +use anyhow::{self, Context}; use backtrace::Backtrace; use gumdrop::{Options, ParsingStyle}; use rand::{thread_rng, Rng}; +use thiserror::Error; use time::macros::format_description; use time::OffsetDateTime; @@ -104,6 +106,24 @@ impl CliOptions { } } +#[derive(Debug, Error)] +enum WalletToolError { + #[error("zcash-cli executable not found")] + ZcashCliNotFound, + + #[error("Unexpected response from zcash-cli or zcashd")] + UnexpectedResponse, + + #[error("Could not connect to zcashd")] + ZcashdConnection, + + #[error("zcashd -exportdir option not set")] + ExportDirNotSet, + + #[error("Could not parse a recovery phrase from the export file")] + RecoveryPhraseNotFound, +} + pub fn main() { // Allow either Bitcoin-style or GNU-style arguments. let mut args = env::args(); @@ -151,7 +171,7 @@ pub fn main() { } } -fn run(opts: &CliOptions) -> Result<(), io::Error> { +fn run(opts: &CliOptions) -> anyhow::Result<()> { let cli_options: Vec = opts.to_zcash_cli_options(); println!(concat!( @@ -170,13 +190,12 @@ fn run(opts: &CliOptions) -> Result<(), io::Error> { cli_args.extend_from_slice(&["z_exportwallet".to_string(), "\x01".to_string()]); let out = exec(&zcash_cli, &cli_args, opts.debug)?; let cli_err: Vec<_> = from_utf8(&out.stderr) - .map_err(|_| io::Error::from(ErrorKind::InvalidData))? + .with_context(|| "Output from zcash-cli was not UTF-8")? .lines() .map(|s| s.trim_end_matches('\r')) .collect(); - if cli_err.len() < 3 || cli_err[0] != "error code: -4" || cli_err[1] != "error message:" { - // TODO: distinguish not running case more precisely + if !cli_err.is_empty() && cli_err[0].starts_with("error: couldn't connect") { println!(concat!( "\nNo, we could not connect. zcashd might not be running; in that case\n", "please start it. The '-exportdir' option should be set to the absolute\n", @@ -190,18 +209,37 @@ fn run(opts: &CliOptions) -> Result<(), io::Error> { "use the same connection options for zcashd-wallet-tool (see '--help' for\n", "accepted options) as for zcash-cli.\n" )); - return Err(io::Error::from(ErrorKind::Other)); + return Err(WalletToolError::ZcashdConnection.into()); } - if cli_err[2].contains("zcashd -exportdir") { + if !cli_err.is_empty() && cli_err[0] == "error code: -28" { println!(concat!( - "\nIt looks like zcashd is running without the '-exportdir' option.\n\n", - "Please start or restart zcashd with '-exportdir' set to the absolute\n", + "\nNo, we could not connect. zcashd seems to be initializing; please try\n", + "again once it has finished.\n", + )); + return Err(WalletToolError::ZcashdConnection.into()); + } + if cli_err.len() < 3 + || cli_err[0] != "error code: -4" + || !cli_err[2].starts_with("Filename is invalid") + { + let e = if cli_err[2].contains("zcashd -exportdir") { + println!("\nIt looks like zcashd is running without the '-exportdir' option."); + WalletToolError::ExportDirNotSet + } else { + println!( + "\nThere was an unexpected response from zcashd:\n> {}", + cli_err.join("\n> ") + ); + WalletToolError::UnexpectedResponse + }; + println!(concat!( + "\nPlease start or restart zcashd with '-exportdir' set to the absolute\n", "path of the directory you want to save the wallet export file to.\n", "(Don't forget to restart zcashd without '-exportdir' after finishing\n", "the backup, if running it long-term with that option is not desired\n", - "or would be a security hazard in your environment.)" + "or would be a security hazard in your environment.)\n", )); - return Err(io::Error::from(ErrorKind::Other)); + return Err(e.into()); } println!("Yes, and it is running with the '-exportdir' option as required."); @@ -238,7 +276,7 @@ fn run(opts: &CliOptions) -> Result<(), io::Error> { cli_args.extend_from_slice(&["z_exportwallet".to_string(), filename.to_string()]); let out = exec(&zcash_cli, &cli_args, opts.debug)?; let cli_err: Vec<_> = from_utf8(&out.stderr) - .map_err(|_| io::Error::from(ErrorKind::InvalidData))? + .with_context(|| "Output from zcash-cli was not UTF-8")? .lines() .map(|s| s.trim_end_matches('\r')) .collect(); @@ -261,7 +299,7 @@ fn run(opts: &CliOptions) -> Result<(), io::Error> { }; let cli_out: Vec<_> = from_utf8(&out.stdout) - .map_err(|_| io::Error::from(ErrorKind::InvalidData))? + .with_context(|| "Output from zcash-cli was not UTF-8")? .lines() .map(|s| s.trim_end_matches('\r')) .collect(); @@ -269,15 +307,15 @@ fn run(opts: &CliOptions) -> Result<(), io::Error> { eprintln!("DEBUG: stdout {:?}", cli_out); } if cli_out.is_empty() { - return Err(io::Error::from(ErrorKind::InvalidData)); + return Err(WalletToolError::UnexpectedResponse.into()); } let export_path = cli_out[0]; println!("\nSaved the export file to '{}'.", export_path); println!("IMPORTANT: This file contains secrets that allow spending all wallet funds.\n"); - // TODO: better handling of file not found and permission errors. - let export_file = File::open(export_path)?; + let export_file = File::open(export_path) + .with_context(|| format!("Could not open {:?} for reading", export_path))?; let phrase_line: Vec<_> = io::BufReader::new(export_file) .lines() .filter(|s| { @@ -287,7 +325,7 @@ fn run(opts: &CliOptions) -> Result<(), io::Error> { }) .collect(); if phrase_line.len() != 1 || phrase_line[0].is_err() { - return Err(io::Error::from(ErrorKind::InvalidData)); + return Err(WalletToolError::RecoveryPhraseNotFound.into()); } let phrase = phrase_line[0] .as_ref() @@ -307,7 +345,7 @@ fn run(opts: &CliOptions) -> Result<(), io::Error> { eprintln!("\nPanic: {}\n{:?}", s, Backtrace::new()); })); - let res = (|| -> io::Result<()> { + let res = (|| -> anyhow::Result<()> { println!("The recovery phrase is:\n"); const WORDS_PER_LINE: usize = 3; @@ -380,7 +418,7 @@ fn run(opts: &CliOptions) -> Result<(), io::Error> { exec(&zcash_cli, &cli_args, false) .and_then(|out| { let cli_err: Vec<_> = from_utf8(&out.stderr) - .map_err(|_| io::Error::from(ErrorKind::InvalidData))? + .with_context(|| "Output from zcash-cli was not UTF-8")? .lines() .map(|s| s.trim_end_matches('\r')) .collect(); @@ -388,19 +426,25 @@ fn run(opts: &CliOptions) -> Result<(), io::Error> { eprintln!("DEBUG: stderr {:?}", cli_err); } if !cli_err.is_empty() { - // TODO: distinguish errors: cannot connect, vs incorrect passphrase - // (RPC_WALLET_PASSPHRASE_INCORRECT), vs other errors. - Err(io::Error::from(ErrorKind::Other)) - } else { - println!(concat!( - "\nThe backup of the emergency recovery phrase for the zcashd\n", - "wallet has been successfully confirmed 🙂. You can now use the\n", - "zcashd RPC methods that create keys and addresses in that wallet.\n\n", - "If you use other wallets, their recovery information will need\n", - "to be backed up separately.\n" - )); - Ok(()) + if cli_err[0].starts_with("error: couldn't connect") { + println!("\nWe could not connect to zcashd; it may have exited."); + return Err(WalletToolError::ZcashdConnection.into()); + } else { + println!( + "\nThere was an unexpected response from zcashd:\n> {}", + cli_err.join("\n> "), + ); + return Err(WalletToolError::UnexpectedResponse.into()); + } } + println!(concat!( + "\nThe backup of the emergency recovery phrase for the zcashd\n", + "wallet has been successfully confirmed 🙂. You can now use the\n", + "zcashd RPC methods that create keys and addresses in that wallet.\n\n", + "If you use other wallets, their recovery information will need\n", + "to be backed up separately.\n" + )); + Ok(()) }) .map_err(|e| { println!(concat!( @@ -410,15 +454,21 @@ fn run(opts: &CliOptions) -> Result<(), io::Error> { "help or try to use 'zcash-cli -stdin walletconfirmbackup' manually.\n" )); e - }) + })?; + Ok(()) } -fn prompt<'a>(input: &mut Stdin, buf: &'a mut String) -> io::Result<&'a str> { - let n = input.read_line(buf)?; - if n == 0 { - return Err(io::Error::from(ErrorKind::UnexpectedEof)); +fn prompt<'a>(input: &mut Stdin, buf: &'a mut String) -> anyhow::Result<&'a str> { + input + .read_line(buf) + .with_context(|| "Error reading from stdin")?; + + if !buf.ends_with('\n') { + Err(io::Error::from(ErrorKind::UnexpectedEof)) + .with_context(|| "End of file reading from stdin") + } else { + Ok(buf.trim_end_matches(|c| c == '\r' || c == '\n').trim()) } - Ok(buf.trim_end_matches(|c| c == '\r' || c == '\n').trim()) } fn ordinal(num: usize) -> String { @@ -435,9 +485,10 @@ fn ordinal(num: usize) -> String { format!("{}{}", num, suffix) } -fn zcash_cli_path(debug: bool) -> io::Result { +fn zcash_cli_path(debug: bool) -> anyhow::Result { // First look for `zcash_cli[.exe]` as a sibling of the executable. - let mut exe = env::current_exe()?; + let mut exe = env::current_exe() + .with_context(|| "Cannot determine the path of the running executable")?; exe.set_file_name("zcash-cli"); exe.set_extension(EXE_EXTENSION); if debug { @@ -454,7 +505,7 @@ fn zcash_cli_path(debug: bool) -> io::Result { // or in `../../src/zcash_cli[.exe]` under the same proviso exe.pop(); // ../.. if exe.file_name() != Some(OsStr::new("target")) { - return Err(io::Error::from(ErrorKind::NotFound)); + return Err(WalletToolError::ZcashCliNotFound.into()); } } // Replace 'target/' with 'src/'. @@ -464,18 +515,17 @@ fn zcash_cli_path(debug: bool) -> io::Result { if debug { eprintln!("DEBUG: Testing for zcash-cli at {:?}", exe); } - if exe.exists() { - Ok(exe) - } else { - Err(io::Error::from(ErrorKind::NotFound)) + if !exe.exists() { + return Err(WalletToolError::ZcashCliNotFound.into()); } + Ok(exe) } -fn exec(exe_path: &Path, args: &[String], debug: bool) -> Result { +fn exec(exe_path: &Path, args: &[String], debug: bool) -> anyhow::Result { if debug { eprintln!("DEBUG: Running {:?} {:?}", exe_path, args); } - Command::new(exe_path).args(args).output() + Ok(Command::new(exe_path).args(args).output()?) } fn default_filename_base() -> String { From e94082931345777cb2f3ac21d19f33cb39c5a6a8 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Mon, 7 Mar 2022 19:31:05 +0000 Subject: [PATCH 06/21] The recovery phrase confirmation and `zcashd-wallet-tool` are being introduced in zcashd v4.7.0. Co-authored-by: Kris Nuttycombe Signed-off-by: Daira Hopwood --- doc/release-notes.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/release-notes.md b/doc/release-notes.md index f70d71263..76dd3fffb 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -13,10 +13,10 @@ be generated on load of the wallet, or the first time the wallet is unlocked, and is available via the `z_exportwallet` RPC call. All new addresses produced by the wallet are now derived from this seed using the HD wallet functionality described in ZIP 32 and ZIP 316. For users upgrading an existing Zcashd wallet, -it is recommended that the wallet be backed up prior to upgrading to the 4.5.2 +it is recommended that the wallet be backed up prior to upgrading to the 4.7.0 Zcashd release. -Following the upgrade to 4.5.2, Zcashd will require that the user confirm that +Following the upgrade to 4.7.0, Zcashd will require that the user confirm that they have backed up their new emergency recovery phrase, which may be obtained from the output of the `z_exportwallet` RPC call. This confirmation can be performed manually using the `zcashd-wallet-tool` utility that is supplied @@ -29,7 +29,7 @@ recovery phrase going forward. If you choose not to migrate funds in this fashion, you will continue to need to securely back up the entire `wallet.dat` file to ensure that you do not lose access to existing funds; EXISTING FUNDS WILL NOT BE RECOVERABLE USING THE EMERGENCY RECOVERY PHRASE UNLESS THEY HAVE -BEEN MOVED TO A NEWLY GENERATED ADDRESS FOLLOWING THE 4.5.2 UPGRADE. +BEEN MOVED TO A NEWLY GENERATED ADDRESS FOLLOWING THE 4.7.0 UPGRADE. New RPC Methods --------------- From 6fb943d0f6e284c1e8fd8cfa13b656ae3646579c Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Mon, 7 Mar 2022 19:33:27 +0000 Subject: [PATCH 07/21] Refactor use of `export_path` as suggested. Co-authored-by: Sean Bowe Signed-off-by: Daira Hopwood --- src/rust/bin/wallet_tool.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/rust/bin/wallet_tool.rs b/src/rust/bin/wallet_tool.rs index e42bdd3df..db94e8787 100644 --- a/src/rust/bin/wallet_tool.rs +++ b/src/rust/bin/wallet_tool.rs @@ -336,14 +336,16 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { // This panic hook allows us to make a best effort to clear the screen (and then print // another reminder about secrets in the export file) even if a panic occurs. - let moved_export_path = export_path.to_string(); // borrow checker workaround let old_hook = panic::take_hook(); - panic::set_hook(Box::new(move |panic_info| { - clear_and_show_cautions(&moved_export_path); + { + let export_path = export_path.to_string(); + panic::set_hook(Box::new(move |panic_info| { + clear_and_show_cautions(&export_path); - let s = panic_info.payload().downcast_ref::<&str>().unwrap_or(&""); - eprintln!("\nPanic: {}\n{:?}", s, Backtrace::new()); - })); + let s = panic_info.payload().downcast_ref::<&str>().unwrap_or(&""); + eprintln!("\nPanic: {}\n{:?}", s, Backtrace::new()); + })); + } let res = (|| -> anyhow::Result<()> { println!("The recovery phrase is:\n"); From 9b201054137ae34582e676baa7872b5171488856 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Mon, 7 Mar 2022 19:34:31 +0000 Subject: [PATCH 08/21] Tweak the wording of the fallback messages when the terminal cannot be automatically cleared. Co-authored-by: Kris Nuttycombe Signed-off-by: Daira Hopwood --- src/rust/bin/wallet_tool.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rust/bin/wallet_tool.rs b/src/rust/bin/wallet_tool.rs index db94e8787..260f327fd 100644 --- a/src/rust/bin/wallet_tool.rs +++ b/src/rust/bin/wallet_tool.rs @@ -382,8 +382,8 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { try_to_clear(concat!( "\n\n\n\n\n\n\n\n\n\n\n\n", "Please adjust the terminal window so that you can't see the\n", - "recovery phrase above. After finishing the backup, clear the\n", - "terminal window" + "recovery phrase above. After finishing the backup, close the\n", + "terminal window or clear it" )); println!("\nNow we're going to confirm that you backed up the recovery phrase."); @@ -548,7 +548,7 @@ fn clear_and_show_cautions(export_path: &str) { try_to_clear(concat!( "\nCAUTION: This terminal window might be showing secrets (or have\n", "them in the scrollback). Please copy any useful information and\n", - "then clear it" + "then close it, or clear it" )); println!( From d4405feddf5b06881b59216a2f8bd782d2933967 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Wed, 9 Mar 2022 18:04:27 +0000 Subject: [PATCH 09/21] Simplify extraction of recovery phrase. Co-authored-by: Jack Grigg Signed-off-by: Daira Hopwood --- src/rust/bin/wallet_tool.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/rust/bin/wallet_tool.rs b/src/rust/bin/wallet_tool.rs index 260f327fd..7cd211647 100644 --- a/src/rust/bin/wallet_tool.rs +++ b/src/rust/bin/wallet_tool.rs @@ -324,14 +324,13 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { .unwrap_or(false) }) .collect(); - if phrase_line.len() != 1 || phrase_line[0].is_err() { - return Err(WalletToolError::RecoveryPhraseNotFound.into()); - } - let phrase = phrase_line[0] - .as_ref() - .unwrap() - .trim_start_matches("# - recovery_phrase=\"") - .trim_end_matches('"'); + + let phrase = match &phrase_line[..] { + [Ok(line)] => line + .trim_start_matches("# - recovery_phrase=\"") + .trim_end_matches('"'), + _ => return Err(WalletToolError::RecoveryPhraseNotFound.into()), + }; // This panic hook allows us to make a best effort to clear the screen (and then print // another reminder about secrets in the export file) even if a panic occurs. From 999b3953601870fc9080f637ba52e4050545dc00 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Thu, 10 Mar 2022 17:17:06 +0000 Subject: [PATCH 10/21] Improve memory hygiene, and use -stdin to pass the recovery phrase to the child zcash-cli process. Co-authored-by: Jack Grigg Signed-off-by: Daira Hopwood --- Cargo.lock | 10 ++++ Cargo.toml | 1 + src/rust/bin/wallet_tool.rs | 92 ++++++++++++++++++++++++++----------- 3 files changed, 77 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bef8caef4..5e515b710 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -951,6 +951,7 @@ dependencies = [ "rand 0.8.5", "rand_core 0.6.3", "secp256k1", + "secrecy", "subtle", "thiserror", "time", @@ -1713,6 +1714,15 @@ dependencies = [ "cc", ] +[[package]] +name = "secrecy" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9bd1c54ea06cfd2f6b63219704de0b9b4f72dcc2b8fdef820be6cd799780e91e" +dependencies = [ + "zeroize", +] + [[package]] name = "serde" version = "1.0.136" diff --git a/Cargo.toml b/Cargo.toml index 083b58b7b..d323c6025 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -71,6 +71,7 @@ backtrace = "0.3" clearscreen = "1.0" gumdrop = "0.8" rand = "0.8" +secrecy = "0.8" time = { version = "0.3", features = ["formatting", "macros"] } [dependencies.tracing-subscriber] diff --git a/src/rust/bin/wallet_tool.rs b/src/rust/bin/wallet_tool.rs index 7cd211647..d6230572d 100644 --- a/src/rust/bin/wallet_tool.rs +++ b/src/rust/bin/wallet_tool.rs @@ -2,11 +2,11 @@ use std::cmp::min; use std::env::{self, consts::EXE_EXTENSION}; use std::ffi::OsStr; use std::fs::File; -use std::io::{self, BufRead, ErrorKind, Stdin}; +use std::io::{self, BufRead, Stdin, Write}; use std::iter; use std::panic; use std::path::{Path, PathBuf}; -use std::process::{self, Command, Output}; +use std::process::{self, ChildStdin, Command, Output, Stdio}; use std::str::from_utf8; use std::time::SystemTime; @@ -14,6 +14,7 @@ use anyhow::{self, Context}; use backtrace::Backtrace; use gumdrop::{Options, ParsingStyle}; use rand::{thread_rng, Rng}; +use secrecy::{ExposeSecret, SecretString}; use thiserror::Error; use time::macros::format_description; use time::OffsetDateTime; @@ -122,6 +123,9 @@ enum WalletToolError { #[error("Could not parse a recovery phrase from the export file")] RecoveryPhraseNotFound, + + #[error("Unexpected EOF in input")] + UnexpectedEof, } pub fn main() { @@ -188,7 +192,7 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { // option, or not running / cannot connect. let mut cli_args = cli_options.clone(); cli_args.extend_from_slice(&["z_exportwallet".to_string(), "\x01".to_string()]); - let out = exec(&zcash_cli, &cli_args, opts.debug)?; + let out = exec(&zcash_cli, &cli_args, None, opts.debug)?; let cli_err: Vec<_> = from_utf8(&out.stderr) .with_context(|| "Output from zcash-cli was not UTF-8")? .lines() @@ -260,8 +264,8 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { ), default_filename ); - let mut buf = String::new(); - let response = prompt(&mut stdin, &mut buf)?; + let response = prompt(&mut stdin)?; + let response = strip(&response); let filename = if response.is_empty() { r = r.saturating_add(1); &default_filename @@ -274,7 +278,7 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { let mut cli_args = cli_options.clone(); cli_args.extend_from_slice(&["z_exportwallet".to_string(), filename.to_string()]); - let out = exec(&zcash_cli, &cli_args, opts.debug)?; + let out = exec(&zcash_cli, &cli_args, None, opts.debug)?; let cli_err: Vec<_> = from_utf8(&out.stderr) .with_context(|| "Output from zcash-cli was not UTF-8")? .lines() @@ -316,17 +320,21 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { let export_file = File::open(export_path) .with_context(|| format!("Could not open {:?} for reading", export_path))?; + + // TODO: ensure the buffer will be zeroized (#5650) let phrase_line: Vec<_> = io::BufReader::new(export_file) .lines() + .map(|line| line.map(SecretString::new)) .filter(|s| { s.as_ref() - .map(|t| t.starts_with("# - recovery_phrase=\"")) + .map(|t| t.expose_secret().starts_with("# - recovery_phrase=\"")) .unwrap_or(false) }) .collect(); let phrase = match &phrase_line[..] { [Ok(line)] => line + .expose_secret() .trim_start_matches("# - recovery_phrase=\"") .trim_end_matches('"'), _ => return Err(WalletToolError::RecoveryPhraseNotFound.into()), @@ -372,8 +380,7 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { )); let mut stdin = io::stdin(); - let mut buf = String::new(); - prompt(&mut stdin, &mut buf)?; + prompt(&mut stdin)?; // The only reliable and portable way to make sure the recovery phrase // is no longer displayed is to clear the whole terminal (including @@ -396,10 +403,9 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { unconfirmed.pop().expect("should be nonempty"); loop { - let mut buf = String::new(); println!("\nPlease enter the {} word:", ordinal(n + 1)); - let line = prompt(&mut stdin, &mut buf)?; - if words[n] == line { + let line = prompt(&mut stdin)?; + if words[n] == strip(&line) { break; } println!("That's not correct, please try again."); @@ -413,10 +419,8 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { res?; let mut cli_args = cli_options; - cli_args.extend_from_slice(&["walletconfirmbackup".to_string(), phrase.to_string()]); - - // Always pass false for debug to avoid printing the recovery phrase. - exec(&zcash_cli, &cli_args, false) + cli_args.extend_from_slice(&["-stdin".to_string(), "walletconfirmbackup".to_string()]); + exec(&zcash_cli, &cli_args, Some(phrase), opts.debug) .and_then(|out| { let cli_err: Vec<_> = from_utf8(&out.stderr) .with_context(|| "Output from zcash-cli was not UTF-8")? @@ -459,17 +463,26 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { Ok(()) } -fn prompt<'a>(input: &mut Stdin, buf: &'a mut String) -> anyhow::Result<&'a str> { - input - .read_line(buf) - .with_context(|| "Error reading from stdin")?; +const MAX_USER_INPUT_LEN: usize = 100; +fn prompt(input: &mut Stdin) -> anyhow::Result { + let mut buf = String::with_capacity(MAX_USER_INPUT_LEN); + let res = input + .read_line(&mut buf) + .with_context(|| "Error reading from stdin"); if !buf.ends_with('\n') { - Err(io::Error::from(ErrorKind::UnexpectedEof)) - .with_context(|| "End of file reading from stdin") - } else { - Ok(buf.trim_end_matches(|c| c == '\r' || c == '\n').trim()) + return Err(WalletToolError::UnexpectedEof.into()); } + // TODO: Ensure the buffer is zeroized even on error. + let line = SecretString::new(buf); + res.map(|_| line) +} + +fn strip(input: &SecretString) -> &str { + input + .expose_secret() + .trim_end_matches(|c| c == '\r' || c == '\n') + .trim() } fn ordinal(num: usize) -> String { @@ -522,11 +535,38 @@ fn zcash_cli_path(debug: bool) -> anyhow::Result { Ok(exe) } -fn exec(exe_path: &Path, args: &[String], debug: bool) -> anyhow::Result { +fn exec( + exe_path: &Path, + args: &[String], + stdin: Option<&str>, + debug: bool, +) -> anyhow::Result { if debug { eprintln!("DEBUG: Running {:?} {:?}", exe_path, args); } - Ok(Command::new(exe_path).args(args).output()?) + let mut cmd = Command::new(exe_path); + let cli = cmd.args(args); + match stdin { + None => Ok(cli.output()?), + Some(data) => { + let mut cli_process = cli + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + cli_process + .stdin + .take() + .with_context(|| "Could not open pipe to zcash-cli's stdin") + .and_then(|mut pipe: ChildStdin| -> anyhow::Result<()> { + pipe.write_all(data.as_bytes())?; + pipe.write_all("\n".as_bytes())?; + Ok(()) + }) + .with_context(|| "Could not write to zcash-cli's stdin")?; + Ok(cli_process.wait_with_output()?) + } + } } fn default_filename_base() -> String { From 799b46b90a76e5a7f01fe651c6235b0261b6aba5 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Thu, 10 Mar 2022 17:18:26 +0000 Subject: [PATCH 11/21] Cleanups to error handling for the first invocation of zcash-cli. Signed-off-by: Daira Hopwood --- src/rust/bin/wallet_tool.rs | 107 +++++++++++++++++++++--------------- 1 file changed, 64 insertions(+), 43 deletions(-) diff --git a/src/rust/bin/wallet_tool.rs b/src/rust/bin/wallet_tool.rs index d6230572d..8f15471e4 100644 --- a/src/rust/bin/wallet_tool.rs +++ b/src/rust/bin/wallet_tool.rs @@ -198,52 +198,73 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { .lines() .map(|s| s.trim_end_matches('\r')) .collect(); + if opts.debug { + eprintln!("DEBUG: stderr {:?}", cli_err); + } - if !cli_err.is_empty() && cli_err[0].starts_with("error: couldn't connect") { - println!(concat!( - "\nNo, we could not connect. zcashd might not be running; in that case\n", - "please start it. The '-exportdir' option should be set to the absolute\n", - "path of the directory you want to save the wallet export file to.\n\n", - "(Don't forget to restart zcashd without '-exportdir' after finishing\n", - "the backup, if running it long-term with that option is not desired\n", - "or would be a security hazard in your environment.)\n\n", - "If you believe zcashd is running, it might be using an unexpected port,\n", - "address, or authentication options for the RPC interface, for example.\n", - "In that case try to connect to it using zcash-cli, and if successful,\n", - "use the same connection options for zcashd-wallet-tool (see '--help' for\n", - "accepted options) as for zcash-cli.\n" - )); - return Err(WalletToolError::ZcashdConnection.into()); + if !cli_err.is_empty() { + if cli_err[0].starts_with("Error reading configuration file") { + println!(concat!( + "\nNo, we could not read 'zcash.conf'. If this file is not in the default\n", + "location (~/.zcash/zcash.conf), please try again with the '-datadir='\n", + "option set to the directory containing this file. Also make sure that\n", + "the current user has permission to read that directory and 'zcash.conf'.\n", + )); + return Err(WalletToolError::ZcashdConnection.into()); + } + if cli_err[0].starts_with("error: couldn't connect") { + println!(concat!( + "\nNo, we could not connect. zcashd might not be running; in that case\n", + "please start it. The '-exportdir' option should be set to the absolute\n", + "path of the directory you want to save the wallet export file to.\n\n", + "(Don't forget to restart zcashd without '-exportdir' after finishing\n", + "the backup, if running it long-term with that option is not desired\n", + "or would be a security hazard in your environment.)\n\n", + "If you believe zcashd is running, it might be using an unexpected port,\n", + "address, or authentication options for the RPC interface, for example.\n", + "In that case try to connect to it using zcash-cli, and if successful,\n", + "use the same connection options for zcashd-wallet-tool (see '--help' for\n", + "accepted options) as for zcash-cli.\n" + )); + return Err(WalletToolError::ZcashdConnection.into()); + } + if cli_err[0] == "error code: -28" { + println!(concat!( + "\nNo, we could not connect. zcashd seems to be initializing; please try\n", + "again once it has finished.\n", + )); + return Err(WalletToolError::ZcashdConnection.into()); + } } - if !cli_err.is_empty() && cli_err[0] == "error code: -28" { - println!(concat!( - "\nNo, we could not connect. zcashd seems to be initializing; please try\n", - "again once it has finished.\n", - )); - return Err(WalletToolError::ZcashdConnection.into()); - } - if cli_err.len() < 3 - || cli_err[0] != "error code: -4" - || !cli_err[2].starts_with("Filename is invalid") + + const REMINDER_MSG: &str = concat!( + "\nPlease start or restart zcashd with '-exportdir' set to the absolute\n", + "path of the directory you want to save the wallet export file to.\n", + "(Don't forget to restart zcashd without '-exportdir' after finishing\n", + "the backup, if running it long-term with that option is not desired\n", + "or would be a security hazard in your environment.)\n", + ); + + if cli_err.len() >= 3 + && cli_err[0] == "error code: -4" + && cli_err[2].contains("zcashd -exportdir") { - let e = if cli_err[2].contains("zcashd -exportdir") { - println!("\nIt looks like zcashd is running without the '-exportdir' option."); - WalletToolError::ExportDirNotSet - } else { - println!( - "\nThere was an unexpected response from zcashd:\n> {}", - cli_err.join("\n> ") - ); - WalletToolError::UnexpectedResponse - }; - println!(concat!( - "\nPlease start or restart zcashd with '-exportdir' set to the absolute\n", - "path of the directory you want to save the wallet export file to.\n", - "(Don't forget to restart zcashd without '-exportdir' after finishing\n", - "the backup, if running it long-term with that option is not desired\n", - "or would be a security hazard in your environment.)\n", - )); - return Err(e.into()); + println!( + "\nIt looks like zcashd is running without the '-exportdir' option.{}", + REMINDER_MSG + ); + return Err(WalletToolError::ExportDirNotSet.into()); + } + if !(cli_err.len() >= 3 + && cli_err[0] == "error code: -4" + && cli_err[2].starts_with("Filename is invalid")) + { + println!( + "\nThere was an unexpected response from zcash-cli or zcashd:\n> {}{}", + cli_err.join("\n> "), + REMINDER_MSG, + ); + return Err(WalletToolError::UnexpectedResponse.into()); } println!("Yes, and it is running with the '-exportdir' option as required."); From cb2c89eedf48ca862c3ab19d652ff882060aa1a6 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Fri, 11 Mar 2022 17:17:56 +0000 Subject: [PATCH 12/21] Use the tracing crate for debugging. Signed-off-by: Daira Hopwood --- Cargo.toml | 4 +-- src/rust/bin/wallet_tool.rs | 69 ++++++++++++++++--------------------- 2 files changed, 32 insertions(+), 41 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d323c6025..2d9f476f7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -65,7 +65,7 @@ thiserror = "1" tokio = { version = "1.0", features = ["rt", "net", "time", "macros"] } # Wallet tool -# (also depends on thiserror) +# (also depends on thiserror, tracing, and tracing-subscriber with "env-filter" and "fmt" features) anyhow = "1.0" backtrace = "0.3" clearscreen = "1.0" @@ -77,7 +77,7 @@ time = { version = "0.3", features = ["formatting", "macros"] } [dependencies.tracing-subscriber] version = "0.3" default-features = false -features = ["ansi", "env-filter", "time"] +features = ["ansi", "env-filter", "fmt", "time"] [profile.release] lto = true diff --git a/src/rust/bin/wallet_tool.rs b/src/rust/bin/wallet_tool.rs index 8f15471e4..17659ec25 100644 --- a/src/rust/bin/wallet_tool.rs +++ b/src/rust/bin/wallet_tool.rs @@ -18,15 +18,14 @@ use secrecy::{ExposeSecret, SecretString}; use thiserror::Error; use time::macros::format_description; use time::OffsetDateTime; +use tracing::{event, Level}; +use tracing_subscriber::{fmt, EnvFilter}; #[derive(Debug, Options)] struct CliOptions { #[options(no_short, help = "Print this help output")] help: bool, - #[options(no_short, help = "Display debugging output")] - debug: bool, - #[options( no_short, help = "Specify configuration file (default: zcash.conf)", @@ -129,6 +128,12 @@ enum WalletToolError { } pub fn main() { + // Log to stderr, configured by the RUST_LOG environment variable. + fmt() + .with_writer(io::stderr) + .with_env_filter(EnvFilter::from_default_env()) + .init(); + // Allow either Bitcoin-style or GNU-style arguments. let mut args = env::args(); let command = args.next().expect("argv[0] should exist"); @@ -144,7 +149,9 @@ pub fn main() { const USAGE_NOTE: &str = concat!( "Options can be given in GNU style (`--conf=CONF` or `--conf CONF`),\n", - "or in Bitcoin style with a single hyphen (`-conf=CONF`).\n" + "or in Bitcoin style with a single hyphen (`-conf=CONF`).\n\n", + "The environment variable RUST_LOG controls debug output, e.g.\n", + "`RUST_LOG=debug`.\n", ); let opts = CliOptions::parse_args(&args, ParsingStyle::default()).unwrap_or_else(|e| { @@ -185,22 +192,20 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { )); println!("Checking that we can connect to zcashd..."); - let zcash_cli = zcash_cli_path(opts.debug)?; + let zcash_cli = zcash_cli_path()?; // Pass an invalid filename, "\x01", and use the error message to distinguish // whether zcashd is running with the -exportdir option, running without that // option, or not running / cannot connect. let mut cli_args = cli_options.clone(); cli_args.extend_from_slice(&["z_exportwallet".to_string(), "\x01".to_string()]); - let out = exec(&zcash_cli, &cli_args, None, opts.debug)?; + let out = exec(&zcash_cli, &cli_args, None)?; let cli_err: Vec<_> = from_utf8(&out.stderr) .with_context(|| "Output from zcash-cli was not UTF-8")? .lines() .map(|s| s.trim_end_matches('\r')) .collect(); - if opts.debug { - eprintln!("DEBUG: stderr {:?}", cli_err); - } + event!(Level::DEBUG, "stderr {:?}", cli_err); if !cli_err.is_empty() { if cli_err[0].starts_with("Error reading configuration file") { @@ -293,21 +298,18 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { } else { response }; - if opts.debug { - eprintln!("DEBUG: Using filename {:?}", filename); - } + event!(Level::DEBUG, "Using filename {:?}", filename); let mut cli_args = cli_options.clone(); cli_args.extend_from_slice(&["z_exportwallet".to_string(), filename.to_string()]); - let out = exec(&zcash_cli, &cli_args, None, opts.debug)?; + let out = exec(&zcash_cli, &cli_args, None)?; let cli_err: Vec<_> = from_utf8(&out.stderr) .with_context(|| "Output from zcash-cli was not UTF-8")? .lines() .map(|s| s.trim_end_matches('\r')) .collect(); - if opts.debug { - eprintln!("DEBUG: stderr {:?}", cli_err); - } + event!(Level::DEBUG, "stderr {:?}", cli_err); + if cli_err.len() >= 3 && cli_err[0] == "error code: -8" && cli_err[1] == "error message:" @@ -328,9 +330,8 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { .lines() .map(|s| s.trim_end_matches('\r')) .collect(); - if opts.debug { - eprintln!("DEBUG: stdout {:?}", cli_out); - } + event!(Level::DEBUG, "stdout {:?}", cli_out); + if cli_out.is_empty() { return Err(WalletToolError::UnexpectedResponse.into()); } @@ -441,16 +442,15 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { let mut cli_args = cli_options; cli_args.extend_from_slice(&["-stdin".to_string(), "walletconfirmbackup".to_string()]); - exec(&zcash_cli, &cli_args, Some(phrase), opts.debug) + exec(&zcash_cli, &cli_args, Some(phrase)) .and_then(|out| { let cli_err: Vec<_> = from_utf8(&out.stderr) .with_context(|| "Output from zcash-cli was not UTF-8")? .lines() .map(|s| s.trim_end_matches('\r')) .collect(); - if opts.debug { - eprintln!("DEBUG: stderr {:?}", cli_err); - } + event!(Level::DEBUG, "stderr {:?}", cli_err); + if !cli_err.is_empty() { if cli_err[0].starts_with("error: couldn't connect") { println!("\nWe could not connect to zcashd; it may have exited."); @@ -520,15 +520,14 @@ fn ordinal(num: usize) -> String { format!("{}{}", num, suffix) } -fn zcash_cli_path(debug: bool) -> anyhow::Result { +fn zcash_cli_path() -> anyhow::Result { // First look for `zcash_cli[.exe]` as a sibling of the executable. let mut exe = env::current_exe() .with_context(|| "Cannot determine the path of the running executable")?; exe.set_file_name("zcash-cli"); exe.set_extension(EXE_EXTENSION); - if debug { - eprintln!("DEBUG: Testing for zcash-cli at {:?}", exe); - } + + event!(Level::DEBUG, "Testing for zcash-cli at {:?}", exe); if exe.exists() { return Ok(exe); } @@ -547,24 +546,16 @@ fn zcash_cli_path(debug: bool) -> anyhow::Result { exe.set_file_name("src"); exe.push("zcash-cli"); exe.set_extension(EXE_EXTENSION); - if debug { - eprintln!("DEBUG: Testing for zcash-cli at {:?}", exe); - } + + event!(Level::DEBUG, "Testing for zcash-cli at {:?}", exe); if !exe.exists() { return Err(WalletToolError::ZcashCliNotFound.into()); } Ok(exe) } -fn exec( - exe_path: &Path, - args: &[String], - stdin: Option<&str>, - debug: bool, -) -> anyhow::Result { - if debug { - eprintln!("DEBUG: Running {:?} {:?}", exe_path, args); - } +fn exec(exe_path: &Path, args: &[String], stdin: Option<&str>) -> anyhow::Result { + event!(Level::DEBUG, "Running {:?} {:?}", exe_path, args); let mut cmd = Command::new(exe_path); let cli = cmd.args(args); match stdin { From 7a98644ff27b702a73021235c3f952099ce7543c Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Fri, 11 Mar 2022 17:54:34 +0000 Subject: [PATCH 13/21] Improve error message when the config file cannot be found, taking into account -conf and -datadir. Signed-off-by: Daira Hopwood --- src/rust/bin/wallet_tool.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/rust/bin/wallet_tool.rs b/src/rust/bin/wallet_tool.rs index 17659ec25..3c7d3f629 100644 --- a/src/rust/bin/wallet_tool.rs +++ b/src/rust/bin/wallet_tool.rs @@ -28,8 +28,8 @@ struct CliOptions { #[options( no_short, - help = "Specify configuration file (default: zcash.conf)", - meta = "PATH" + help = "Specify configuration filename, relative to the data directory (default: zcash.conf)", + meta = "FILENAME" )] conf: Option, @@ -209,11 +209,16 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { if !cli_err.is_empty() { if cli_err[0].starts_with("Error reading configuration file") { + println!( + "\nNo, we could not read the zcashd configuration file, expected to be at\n{:?}.", + Path::new(opts.datadir.as_ref().map_or("~/.zcash", |s| &s[..])).join(Path::new( + opts.conf.as_ref().map_or("zcash.conf", |s| &s[..]) + )), + ); println!(concat!( - "\nNo, we could not read 'zcash.conf'. If this file is not in the default\n", - "location (~/.zcash/zcash.conf), please try again with the '-datadir='\n", - "option set to the directory containing this file. Also make sure that\n", - "the current user has permission to read that directory and 'zcash.conf'.\n", + "If it is not at that path, please try again with the '-datadir' and/or\n", + "'-conf' options set correctly (see '--help' for details). Also make sure\n", + "that the current user has permission to read the configuration file.\n", )); return Err(WalletToolError::ZcashdConnection.into()); } @@ -243,7 +248,7 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { } const REMINDER_MSG: &str = concat!( - "\nPlease start or restart zcashd with '-exportdir' set to the absolute\n", + "\n\nPlease start or restart zcashd with '-exportdir' set to the absolute\n", "path of the directory you want to save the wallet export file to.\n", "(Don't forget to restart zcashd without '-exportdir' after finishing\n", "the backup, if running it long-term with that option is not desired\n", From d5b6e226f013caa8a5669631dd25932eeaff3d20 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Fri, 11 Mar 2022 18:21:11 +0000 Subject: [PATCH 14/21] Ensure the buffer used in `prompt` is zeroized even on error. Co-authored-by: Jack Grigg Signed-off-by: Daira Hopwood --- src/rust/bin/wallet_tool.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/rust/bin/wallet_tool.rs b/src/rust/bin/wallet_tool.rs index 3c7d3f629..71cc78edf 100644 --- a/src/rust/bin/wallet_tool.rs +++ b/src/rust/bin/wallet_tool.rs @@ -496,12 +496,16 @@ fn prompt(input: &mut Stdin) -> anyhow::Result { let res = input .read_line(&mut buf) .with_context(|| "Error reading from stdin"); - if !buf.ends_with('\n') { - return Err(WalletToolError::UnexpectedEof.into()); - } - // TODO: Ensure the buffer is zeroized even on error. + + // Ensure the buffer is zeroized even on error. let line = SecretString::new(buf); - res.map(|_| line) + res.and_then(|_| { + if line.expose_secret().ends_with('\n') { + Ok(line) + } else { + Err(WalletToolError::UnexpectedEof.into()) + } + }) } fn strip(input: &SecretString) -> &str { From 3e36252d9ab67ac99c04a3d2b4a1af9182c55430 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Fri, 11 Mar 2022 18:35:52 +0000 Subject: [PATCH 15/21] Document that '~' cannot be used in `-datadir` (see #5661). Signed-off-by: Daira Hopwood --- src/bitcoin-cli.cpp | 2 +- src/init.cpp | 2 +- src/rust/bin/wallet_tool.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 0d9492c52..b78fb5461 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -39,7 +39,7 @@ std::string HelpMessageCli() strUsage += HelpMessageGroup(_("Options:")); strUsage += HelpMessageOpt("-?", _("This help message")); strUsage += HelpMessageOpt("-conf=", strprintf(_("Specify configuration file (default: %s)"), BITCOIN_CONF_FILENAME)); - strUsage += HelpMessageOpt("-datadir=", _("Specify data directory")); + strUsage += HelpMessageOpt("-datadir=", _("Specify data directory (this path cannot use '~')")); strUsage += HelpMessageOpt("-stdin", _("Read extra arguments from standard input, one per line until EOF/Ctrl-D (recommended for sensitive information such as passphrases). If first extra argument is `walletpassphrase` then the first line(password) will not be echoed.")); AppendParamsHelpMessages(strUsage); strUsage += HelpMessageOpt("-rpcconnect=", strprintf(_("Send commands to node running on (default: %s)"), DEFAULT_RPCCONNECT)); diff --git a/src/init.cpp b/src/init.cpp index e070bf944..1929fdab2 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -341,7 +341,7 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-daemon", _("Run in the background as a daemon and accept commands")); #endif } - strUsage += HelpMessageOpt("-datadir=", _("Specify data directory")); + strUsage += HelpMessageOpt("-datadir=", _("Specify data directory (this path cannot use '~')")); strUsage += HelpMessageOpt("-paramsdir=", _("Specify Zcash network parameters directory")); strUsage += HelpMessageOpt("-dbcache=", strprintf(_("Set database cache size in megabytes (%d to %d, default: %d)"), nMinDbCache, nMaxDbCache, nDefaultDbCache)); strUsage += HelpMessageOpt("-debuglogfile=", strprintf(_("Specify location of debug log file: this can be an absolute path or a path relative to the data directory (default: %s)"), DEFAULT_DEBUGLOGFILE)); diff --git a/src/rust/bin/wallet_tool.rs b/src/rust/bin/wallet_tool.rs index 71cc78edf..e0fc21dc2 100644 --- a/src/rust/bin/wallet_tool.rs +++ b/src/rust/bin/wallet_tool.rs @@ -33,7 +33,7 @@ struct CliOptions { )] conf: Option, - #[options(no_short, help = "Specify data directory")] + #[options(no_short, help = "Specify data directory (this path cannot use '~')")] datadir: Option, #[options(no_short, help = "Use the test chain")] From 577f695af5124473561e58f0e9aaf531f2846f48 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Fri, 11 Mar 2022 19:02:31 +0000 Subject: [PATCH 16/21] Set `meta` for `-datadir` option. Signed-off-by: Daira Hopwood --- src/rust/bin/wallet_tool.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/rust/bin/wallet_tool.rs b/src/rust/bin/wallet_tool.rs index e0fc21dc2..73593c8e0 100644 --- a/src/rust/bin/wallet_tool.rs +++ b/src/rust/bin/wallet_tool.rs @@ -33,7 +33,11 @@ struct CliOptions { )] conf: Option, - #[options(no_short, help = "Specify data directory (this path cannot use '~')")] + #[options( + no_short, + help = "Specify data directory (this path cannot use '~')", + meta = "PATH" + )] datadir: Option, #[options(no_short, help = "Use the test chain")] From ccada6632411035d76f8e2376ccfea6fdcb2568d Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Sat, 12 Mar 2022 12:56:32 +0000 Subject: [PATCH 17/21] Simplify debug tracing. Signed-off-by: Daira Hopwood --- src/rust/bin/wallet_tool.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/rust/bin/wallet_tool.rs b/src/rust/bin/wallet_tool.rs index 73593c8e0..0bccc62e9 100644 --- a/src/rust/bin/wallet_tool.rs +++ b/src/rust/bin/wallet_tool.rs @@ -18,7 +18,7 @@ use secrecy::{ExposeSecret, SecretString}; use thiserror::Error; use time::macros::format_description; use time::OffsetDateTime; -use tracing::{event, Level}; +use tracing::debug; use tracing_subscriber::{fmt, EnvFilter}; #[derive(Debug, Options)] @@ -209,7 +209,7 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { .lines() .map(|s| s.trim_end_matches('\r')) .collect(); - event!(Level::DEBUG, "stderr {:?}", cli_err); + debug!("stderr {:?}", cli_err); if !cli_err.is_empty() { if cli_err[0].starts_with("Error reading configuration file") { @@ -307,7 +307,7 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { } else { response }; - event!(Level::DEBUG, "Using filename {:?}", filename); + debug!("Using filename {:?}", filename); let mut cli_args = cli_options.clone(); cli_args.extend_from_slice(&["z_exportwallet".to_string(), filename.to_string()]); @@ -317,7 +317,7 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { .lines() .map(|s| s.trim_end_matches('\r')) .collect(); - event!(Level::DEBUG, "stderr {:?}", cli_err); + debug!("stderr {:?}", cli_err); if cli_err.len() >= 3 && cli_err[0] == "error code: -8" @@ -339,7 +339,7 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { .lines() .map(|s| s.trim_end_matches('\r')) .collect(); - event!(Level::DEBUG, "stdout {:?}", cli_out); + debug!("stdout {:?}", cli_out); if cli_out.is_empty() { return Err(WalletToolError::UnexpectedResponse.into()); @@ -458,7 +458,7 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { .lines() .map(|s| s.trim_end_matches('\r')) .collect(); - event!(Level::DEBUG, "stderr {:?}", cli_err); + debug!("stderr {:?}", cli_err); if !cli_err.is_empty() { if cli_err[0].starts_with("error: couldn't connect") { @@ -540,7 +540,7 @@ fn zcash_cli_path() -> anyhow::Result { exe.set_file_name("zcash-cli"); exe.set_extension(EXE_EXTENSION); - event!(Level::DEBUG, "Testing for zcash-cli at {:?}", exe); + debug!("Testing for zcash-cli at {:?}", exe); if exe.exists() { return Ok(exe); } @@ -560,7 +560,7 @@ fn zcash_cli_path() -> anyhow::Result { exe.push("zcash-cli"); exe.set_extension(EXE_EXTENSION); - event!(Level::DEBUG, "Testing for zcash-cli at {:?}", exe); + debug!("Testing for zcash-cli at {:?}", exe); if !exe.exists() { return Err(WalletToolError::ZcashCliNotFound.into()); } @@ -568,7 +568,7 @@ fn zcash_cli_path() -> anyhow::Result { } fn exec(exe_path: &Path, args: &[String], stdin: Option<&str>) -> anyhow::Result { - event!(Level::DEBUG, "Running {:?} {:?}", exe_path, args); + debug!("Running {:?} {:?}", exe_path, args); let mut cmd = Command::new(exe_path); let cli = cmd.args(args); match stdin { From 55d14faebf17fd4c22569cdfe52762fbfced482e Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Sun, 13 Mar 2022 18:25:40 +0000 Subject: [PATCH 18/21] Correct the fallback instruction for how to clear the terminal on macOS: pressing Command + K also clears scrollback. Signed-off-by: Daira Hopwood --- src/rust/bin/wallet_tool.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/rust/bin/wallet_tool.rs b/src/rust/bin/wallet_tool.rs index 0bccc62e9..61fe80b33 100644 --- a/src/rust/bin/wallet_tool.rs +++ b/src/rust/bin/wallet_tool.rs @@ -635,10 +635,12 @@ fn try_to_clear(error_blurb: &str) { eprintln!("Unable to clear screen: {}.", e); #[cfg(target_os = "windows")] - const CLEAR: &str = "cls"; - #[cfg(not(target_os = "windows"))] - const CLEAR: &str = "clear"; + const HOW_TO_CLEAR: &str = "using 'cls'"; + #[cfg(target_os = "macos")] + const HOW_TO_CLEAR: &str = "by pressing Command + K"; + #[cfg(not(any(target_os = "windows", target_os = "macos")))] + const HOW_TO_CLEAR: &str = "using 'clear'"; - println!("{} using '{}'.", error_blurb, CLEAR); + println!("{} {}.", error_blurb, HOW_TO_CLEAR); } } From 1d31d4bc7c18493e9ec5ad7e28a477c5c0c89af3 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Sun, 13 Mar 2022 21:08:46 +0000 Subject: [PATCH 19/21] Include $(bin_SCRIPTS) in `check-symbols`, `check-security`, and `clean` targets. Checking for stack canaries in `check-security` is disabled for Rust executables (Rust does support `-Z stack-protector=all` but only for the nightly compiler). Signed-off-by: Daira Hopwood --- Makefile.am | 6 ++++++ contrib/devtools/security-check.py | 8 +++++++- src/Makefile.am | 13 +++++++------ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/Makefile.am b/Makefile.am index a77f7e980..d5a8eac63 100644 --- a/Makefile.am +++ b/Makefile.am @@ -48,6 +48,12 @@ $(BITCOIND_BIN): FORCE $(BITCOIN_CLI_BIN): FORCE $(MAKE) -C src $(@F) +check-security: FORCE + $(MAKE) -C src check-security + +check-symbols: FORCE + $(MAKE) -C src check-symbols + if USE_LCOV baseline.info: diff --git a/contrib/devtools/security-check.py b/contrib/devtools/security-check.py index 43c825bde..ca1a6e0ea 100755 --- a/contrib/devtools/security-check.py +++ b/contrib/devtools/security-check.py @@ -189,8 +189,12 @@ def identify_executable(executable): return None if __name__ == '__main__': + args = sys.argv[1:] + allow_no_canary = "--allow-no-canary" in args + files = [arg for arg in args if not arg.startswith("--")] + retval = 0 - for filename in sys.argv[1:]: + for filename in files: try: etype = identify_executable(filename) if etype is None: @@ -201,6 +205,8 @@ if __name__ == '__main__': failed = [] warning = [] for (name, func) in CHECKS[etype]: + if name == "Canary" and allow_no_canary: + continue if not func(filename): if name in NONFATAL: warning.append(name) diff --git a/src/Makefile.am b/src/Makefile.am index e3ce37c62..bf3bd9f77 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -612,7 +612,7 @@ CTAES_DIST += crypto/ctaes/ctaes.h CTAES_DIST += crypto/ctaes/README.md CTAES_DIST += crypto/ctaes/test.c -CLEANFILES = *.gcda *.gcno */*.gcno wallet/*/*.gcno +CLEANFILES = *.gcda *.gcno */*.gcno wallet/*/*.gcno $(bin_SCRIPTS) DISTCLEANFILES = obj/build.h @@ -636,16 +636,17 @@ clean-local: $(AM_V_CXX) $(OBJCXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) \ $(CPPFLAGS) $(AM_CXXFLAGS) $(AM_CXXFLAGS) $(PIE_FLAGS) $(CXXFLAGS) -c -o $@ $< -check-symbols: $(bin_PROGRAMS) +check-symbols: $(bin_PROGRAMS) $(bin_SCRIPTS) if GLIBC_BACK_COMPAT - @echo "Checking glibc back compat of [$(bin_PROGRAMS)]..." - $(AM_V_at) READELF=$(READELF) CPPFILT=$(CPPFILT) $(top_srcdir)/contrib/devtools/symbol-check.py $(bin_PROGRAMS) + @echo "Checking glibc back compat of [$(bin_PROGRAMS) $(bin_SCRIPTS)]..." + $(AM_V_at) READELF=$(READELF) CPPFILT=$(CPPFILT) $(top_srcdir)/contrib/devtools/symbol-check.py $(bin_PROGRAMS) $(bin_SCRIPTS) endif -check-security: $(bin_PROGRAMS) +check-security: $(bin_PROGRAMS) $(bin_SCRIPTS) if HARDEN - @echo "Checking binary security of [$(bin_PROGRAMS)]..." + @echo "Checking binary security of [$(bin_PROGRAMS) $(bin_SCRIPTS)]..." $(AM_V_at) READELF=$(READELF) OBJDUMP=$(OBJDUMP) $(top_srcdir)/contrib/devtools/security-check.py $(bin_PROGRAMS) + $(AM_V_at) READELF=$(READELF) OBJDUMP=$(OBJDUMP) $(top_srcdir)/contrib/devtools/security-check.py --allow-no-canary $(bin_SCRIPTS) endif %.pb.cc %.pb.h: %.proto From 6ee6692c83754119c7531e6662dd11e1f202e57a Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Mon, 14 Mar 2022 08:05:59 +0000 Subject: [PATCH 20/21] qa/zcash/full_test_suite.py: enable `test_rpath_runpath` for Rust binaries, and reenable `test_fortify_source` for C++ binaries. Signed-off-by: Daira Hopwood --- qa/zcash/full_test_suite.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/qa/zcash/full_test_suite.py b/qa/zcash/full_test_suite.py index 173e07f09..fd67a6945 100755 --- a/qa/zcash/full_test_suite.py +++ b/qa/zcash/full_test_suite.py @@ -46,6 +46,17 @@ RE_RPATH_RUNPATH = re.compile('No RPATH.*No RUNPATH') RE_FORTIFY_AVAILABLE = re.compile('FORTIFY_SOURCE support available.*Yes') RE_FORTIFY_USED = re.compile('Binary compiled with FORTIFY_SOURCE support.*Yes') +CXX_BINARIES = [ + 'src/zcashd', + 'src/zcash-cli', + 'src/zcash-gtest', + 'src/zcash-tx', + 'src/test/test_bitcoin', +] +RUST_BINARIES = [ + 'src/zcashd-wallet-tool', +] + def test_rpath_runpath(filename): output = subprocess.check_output( [repofile('qa/zcash/checksec.sh'), '--file=' + repofile(filename)] @@ -86,21 +97,14 @@ def check_security_hardening(): if not magic.startswith(b'\x7fELF'): return ret - ret &= test_rpath_runpath('src/zcashd') - ret &= test_rpath_runpath('src/zcash-cli') - ret &= test_rpath_runpath('src/zcash-gtest') - ret &= test_rpath_runpath('src/zcash-tx') - ret &= test_rpath_runpath('src/test/test_bitcoin') + for bin in CXX_BINARIES + RUST_BINARIES: + ret &= test_rpath_runpath(bin) # NOTE: checksec.sh does not reliably determine whether FORTIFY_SOURCE # is enabled for the entire binary. See issue #915. - # FORTIFY_SOURCE does mostly nothing for Clang before 10, which we don't - # pin yet, so we disable these tests. - # ret &= test_fortify_source('src/zcashd') - # ret &= test_fortify_source('src/zcash-cli') - # ret &= test_fortify_source('src/zcash-gtest') - # ret &= test_fortify_source('src/zcash-tx') - # ret &= test_fortify_source('src/test/test_bitcoin') + # FORTIFY_SOURCE is not applicable to Rust binaries. + for bin in CXX_BINARIES: + ret &= test_fortify_source(bin) return ret From 340840894aa88a6ba9499e22aae08c4659641f8b Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Mon, 14 Mar 2022 08:23:24 +0000 Subject: [PATCH 21/21] Tweaks to message text. Signed-off-by: Daira Hopwood --- src/rust/bin/wallet_tool.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rust/bin/wallet_tool.rs b/src/rust/bin/wallet_tool.rs index 61fe80b33..832ec9073 100644 --- a/src/rust/bin/wallet_tool.rs +++ b/src/rust/bin/wallet_tool.rs @@ -404,8 +404,8 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { } println!(concat!( - "\nPlease write down this phrase on something durable that you will keep\n", - "in a secure location.\n", + "\nPlease write down this phrase (including the numbering of words) on\n", + "something durable that you will keep in a secure location.\n", "Press Enter when finished; then the phrase will disappear and you'll be\n", "asked to re-enter a selection of words from it." )); @@ -466,7 +466,7 @@ fn run(opts: &CliOptions) -> anyhow::Result<()> { return Err(WalletToolError::ZcashdConnection.into()); } else { println!( - "\nThere was an unexpected response from zcashd:\n> {}", + "\nThere was an unexpected response from zcash-cli or zcashd:\n> {}", cli_err.join("\n> "), ); return Err(WalletToolError::UnexpectedResponse.into());