From 53806d1abdc0859795176a94ac59a9fbf61520c0 Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Thu, 10 Mar 2022 22:26:20 +0100 Subject: [PATCH] cli: make the command line more user friendly. Also has to fix up tests. Changelog-Fixed: cli doesn't required anymore to confirm the password if the `hsm_secret` is already encrypted. Signed-off-by: Vincenzo Palazzo --- common/errcode.h | 1 + common/hsm_encryption.c | 13 +++++++++++++ common/hsm_encryption.h | 3 +++ lightningd/hsm_control.c | 6 ++---- lightningd/options.c | 27 +++++++++++++++++---------- tests/test_wallet.py | 7 ------- tools/hsmtool.c | 29 ++++++++++++++++++----------- 7 files changed, 54 insertions(+), 32 deletions(-) diff --git a/common/errcode.h b/common/errcode.h index a4ac49e17..d9784e962 100644 --- a/common/errcode.h +++ b/common/errcode.h @@ -14,5 +14,6 @@ typedef s32 errcode_t; #define HSM_ERROR_IS_ENCRYPT 21 #define HSM_BAD_PASSWORD 22 #define HSM_PASSWORD_INPUT_ERR 23 +#define ERROR_HSM_FILE 24 #endif /* LIGHTNING_COMMON_ERRCODE_H */ diff --git a/common/hsm_encryption.c b/common/hsm_encryption.c index 8a433998b..b2f7c813f 100644 --- a/common/hsm_encryption.c +++ b/common/hsm_encryption.c @@ -1,6 +1,8 @@ #include "config.h" +#include #include #include +#include #include #include @@ -79,6 +81,17 @@ bool decrypt_hsm_secret(const struct secret *encryption_key, return true; } +/* Returns -1 on error (and sets errno), 0 if not encrypted, 1 if it is */ +int is_hsm_secret_encrypted(const char *path) +{ + struct stat st; + + if (stat(path, &st) != 0) + return -1; + + return st.st_size == ENCRYPTED_HSM_SECRET_LEN; +} + void discard_key(struct secret *key TAKES) { /* sodium_munlock() also zeroes the memory. */ diff --git a/common/hsm_encryption.h b/common/hsm_encryption.h index 1534cb4ff..867202661 100644 --- a/common/hsm_encryption.h +++ b/common/hsm_encryption.h @@ -4,6 +4,7 @@ #include #include #include +#include /* Length of the encrypted hsm secret header. */ #define HS_HEADER_LEN crypto_secretstream_xchacha20poly1305_HEADERBYTES @@ -63,4 +64,6 @@ void discard_key(struct secret *key TAKES); */ char *read_stdin_pass_with_exit_code(char **reason, int *exit_code); +/** Returns -1 on error (and sets errno), 0 if not encrypted, 1 if it is */ +int is_hsm_secret_encrypted(const char *path); #endif /* LIGHTNING_COMMON_HSM_ENCRYPTION_H */ diff --git a/lightningd/hsm_control.c b/lightningd/hsm_control.c index 4efd57b19..9a49c03df 100644 --- a/lightningd/hsm_control.c +++ b/lightningd/hsm_control.c @@ -96,11 +96,9 @@ struct ext_key *hsm_init(struct lightningd *ld) * not passed, don't let hsmd use the first 32 bytes of the cypher as the * actual secret. */ if (!ld->config.keypass) { - struct stat st; - if (stat("hsm_secret", &st) == 0 && - st.st_size == ENCRYPTED_HSM_SECRET_LEN) + if (is_hsm_secret_encrypted("hsm_secret") == 1) errx(HSM_ERROR_IS_ENCRYPT, "hsm_secret is encrypted, you need to pass the " - "--encrypted-hsm startup option."); + "--encrypted-hsm startup option."); } ld->hsm_fd = fds[0]; diff --git a/lightningd/options.c b/lightningd/options.c index 66c7ca951..e75685492 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -502,6 +502,12 @@ static char *opt_important_plugin(const char *arg, struct lightningd *ld) static char *opt_set_hsm_password(struct lightningd *ld) { char *passwd, *passwd_confirmation, *err_msg; + int is_encrypted; + + is_encrypted = is_hsm_secret_encrypted("hsm_secret"); + if (is_encrypted == -1) + return tal_fmt(NULL, "Could not access 'hsm_secret': %s", + strerror(errno)); printf("The hsm_secret is encrypted with a password. In order to " "decrypt it and start the node you must provide the password.\n"); @@ -513,17 +519,19 @@ static char *opt_set_hsm_password(struct lightningd *ld) passwd = read_stdin_pass_with_exit_code(&err_msg, &opt_exitcode); if (!passwd) return err_msg; - printf("Confirm hsm_secret password:\n"); - fflush(stdout); - passwd_confirmation = read_stdin_pass_with_exit_code(&err_msg, &opt_exitcode); - if (!passwd_confirmation) - return err_msg; + if (!is_encrypted) { + printf("Confirm hsm_secret password:\n"); + fflush(stdout); + passwd_confirmation = read_stdin_pass_with_exit_code(&err_msg, &opt_exitcode); + if (!passwd_confirmation) + return err_msg; - if (!streq(passwd, passwd_confirmation)) { - opt_exitcode = HSM_BAD_PASSWORD; - return "Passwords confirmation mismatch."; + if (!streq(passwd, passwd_confirmation)) { + opt_exitcode = HSM_BAD_PASSWORD; + return "Passwords confirmation mismatch."; + } + free(passwd_confirmation); } - printf("\n"); ld->config.keypass = tal(NULL, struct secret); @@ -534,7 +542,6 @@ static char *opt_set_hsm_password(struct lightningd *ld) ld->encrypted_hsm = true; free(passwd); - free(passwd_confirmation); return NULL; } diff --git a/tests/test_wallet.py b/tests/test_wallet.py index ef5e8184e..05cf54319 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -1039,8 +1039,6 @@ def test_hsm_secret_encryption(node_factory): wait_for_initialized=False) l1.daemon.wait_for_log(r'Enter hsm_secret password') write_all(master_fd, password[2:].encode("utf-8")) - l1.daemon.wait_for_log(r'Confirm hsm_secret password') - write_all(master_fd, password[2:].encode("utf-8")) assert(l1.daemon.proc.wait(WAIT_TIMEOUT) == HSM_BAD_PASSWORD) assert(l1.daemon.is_in_log("Wrong password for encrypted hsm_secret.")) @@ -1048,8 +1046,6 @@ def test_hsm_secret_encryption(node_factory): l1.daemon.start(stdin=slave_fd, wait_for_initialized=False) l1.daemon.wait_for_log(r'The hsm_secret is encrypted') write_all(master_fd, password.encode("utf-8")) - l1.daemon.wait_for_log(r'Confirm hsm_secret password') - write_all(master_fd, password.encode("utf-8")) l1.daemon.wait_for_log("Server started with public key") assert id == l1.rpc.getinfo()["id"] l1.stop() @@ -1057,7 +1053,6 @@ def test_hsm_secret_encryption(node_factory): # We can restore the same wallet with the same password provided through stdin l1.daemon.start(stdin=subprocess.PIPE, wait_for_initialized=False) l1.daemon.proc.stdin.write(password.encode("utf-8")) - l1.daemon.proc.stdin.write(password.encode("utf-8")) l1.daemon.proc.stdin.flush() l1.daemon.wait_for_log("Server started with public key") assert id == l1.rpc.getinfo()["id"] @@ -1138,8 +1133,6 @@ def test_hsmtool_secret_decryption(node_factory): l1.daemon.wait_for_log(r'The hsm_secret is encrypted') write_all(master_fd, password.encode("utf-8")) - l1.daemon.wait_for_log(r'Confirm hsm_secret password') - write_all(master_fd, password.encode("utf-8")) l1.daemon.wait_for_log("Server started with public key") print(node_id, l1.rpc.getinfo()["id"]) assert node_id == l1.rpc.getinfo()["id"] diff --git a/tools/hsmtool.c b/tools/hsmtool.c index f6a426443..41eb514ab 100644 --- a/tools/hsmtool.c +++ b/tools/hsmtool.c @@ -10,18 +10,17 @@ #include #include #include +#include #include #include #include #include #include #include -#include #include #include #include -#define ERROR_HSM_FILE errno #define ERROR_USAGE 2 #define ERROR_LIBSODIUM 3 #define ERROR_LIBWALLY 4 @@ -147,16 +146,24 @@ static void get_channel_seed(struct secret *channel_seed, struct node_id *peer_i /* We detect an encrypted hsm_secret as a hsm_secret which is 73-bytes long. */ static bool hsm_secret_is_encrypted(const char *hsm_secret_path) { - struct stat st; + switch (is_hsm_secret_encrypted(hsm_secret_path)) { + case -1: + err(ERROR_HSM_FILE, "Cannot open '%s'", hsm_secret_path); + case 1: + return true; + case 0: { + /* Extra sanity check on HSM file! */ + struct stat st; + stat(hsm_secret_path, &st); + if (st.st_size != 32) + errx(ERROR_HSM_FILE, + "Invalid hsm_secret '%s' (neither plaintext " + "nor encrypted).", hsm_secret_path); + return false; + } + } - if (stat(hsm_secret_path, &st) != 0) - errx(ERROR_HSM_FILE, "Could not stat hsm_secret"); - - if (st.st_size != 32 && st.st_size != ENCRYPTED_HSM_SECRET_LEN) - errx(ERROR_HSM_FILE, "Invalid hsm_secret (neither plaintext " - "nor encrypted)."); - - return st.st_size == ENCRYPTED_HSM_SECRET_LEN; + abort(); } static int decrypt_hsm(const char *hsm_secret_path)