From c84473f82c4c02590a32c0e86b76e271acc2acdb Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Mon, 25 Nov 2019 17:15:39 +0100 Subject: [PATCH] hsm: Stabilize the hsm encryption and decryption tests We were using sleeps to hope we catch the password prompt. This makes the test flaky. So I added a help text followed by a `fflush` to make sure we catcht he right moment, instead of guessing. The `fflush` is also useful for debugging if a user ever pipes the output to a file it'd get buffered and the user would wait forever. The same applies for automated systems such as `expect` or `pexpect` based scripts that enter the password on prompt. --- lightningd/options.c | 7 ++++++- tests/test_wallet.py | 46 ++++++++++++++++++++++---------------------- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/lightningd/options.c b/lightningd/options.c index 0e10cec9e..8d9b51499 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -331,7 +331,12 @@ static char *opt_set_hsm_password(struct lightningd *ld) temp_term.c_lflag &= ~ECHO; if (tcsetattr(fileno(stdin), TCSAFLUSH, &temp_term) != 0) return "Could not disable password echoing."; - printf("Enter hsm_secret password : "); + printf("The hsm_secret is encrypted with a password. In order to " + "decrypt it and start the node you must provide the password.\n"); + printf("Enter hsm_secret password: "); + /* If we don't flush we might end up being buffered and we might seem + * to hang while we wait for the password. */ + fflush(stdout); if (getline(&passwd, &passwd_size, stdin) < 0) return "Could not read password from stdin."; if(passwd[strlen(passwd) - 1] == '\n') diff --git a/tests/test_wallet.py b/tests/test_wallet.py index c7ba53da1..6aec6fe47 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -3,7 +3,7 @@ from fixtures import * # noqa: F401,F403 from fixtures import TEST_NETWORK from flaky import flaky # noqa: F401 from lightning import RpcError, Millisatoshi -from utils import only_one, wait_for, sync_blockheight, EXPERIMENTAL_FEATURES, COMPAT, VALGRIND, SLOW_MACHINE +from utils import only_one, wait_for, sync_blockheight, EXPERIMENTAL_FEATURES, COMPAT, VALGRIND import os import pytest @@ -574,34 +574,32 @@ def test_hsm_secret_encryption(node_factory): master_fd, slave_fd = os.openpty() # Test we can encrypt an already-existing and not encrypted hsm_secret - l1.rpc.stop() + l1.stop() l1.daemon.opts.update({"encrypted-hsm": None}) l1.daemon.start(stdin=slave_fd, wait_for_initialized=False) - time.sleep(3 if SLOW_MACHINE else 1) + l1.daemon.wait_for_log(r'The hsm_secret is encrypted') + os.write(master_fd, password.encode("utf-8")) l1.daemon.wait_for_log("Server started with public key") id = l1.rpc.getinfo()["id"] + l1.stop() # Test we cannot start the same wallet without specifying --encrypted-hsm - l1.stop() l1.daemon.opts.pop("encrypted-hsm") - l1.daemon.start(stdin=slave_fd, stderr=subprocess.STDOUT, - wait_for_initialized=False) - time.sleep(3 if SLOW_MACHINE else 1) - err = "hsm_secret is encrypted, you need to pass the --encrypted-hsm startup option." - assert l1.daemon.is_in_log(err) + with pytest.raises(subprocess.CalledProcessError, match=r'returned non-zero exit status 1'): + subprocess.check_call(l1.daemon.cmd_line) # Test we cannot restore the same wallet with another password l1.daemon.opts.update({"encrypted-hsm": None}) l1.daemon.start(stdin=slave_fd, stderr=subprocess.STDOUT, wait_for_initialized=False) - time.sleep(3 if SLOW_MACHINE else 1) + l1.daemon.wait_for_log(r'The hsm_secret is encrypted') os.write(master_fd, password[2:].encode("utf-8")) l1.daemon.wait_for_log("Wrong password for encrypted hsm_secret.") # Test we can restore the same wallet with the same password l1.daemon.start(stdin=slave_fd, wait_for_initialized=False) - time.sleep(3 if SLOW_MACHINE else 1) + l1.daemon.wait_for_log(r'The hsm_secret is encrypted') os.write(master_fd, password.encode("utf-8")) l1.daemon.wait_for_log("Server started with public key") assert id == l1.rpc.getinfo()["id"] @@ -616,10 +614,10 @@ def test_hsmtool_secret_decryption(node_factory): master_fd, slave_fd = os.openpty() # Encrypt the master seed - l1.rpc.stop() + l1.stop() l1.daemon.opts.update({"encrypted-hsm": None}) l1.daemon.start(stdin=slave_fd, wait_for_initialized=False) - time.sleep(3 if SLOW_MACHINE else 1) + l1.daemon.wait_for_log(r'The hsm_secret is encrypted') os.write(master_fd, password.encode("utf-8")) l1.daemon.wait_for_log("Server started with public key") node_id = l1.rpc.getinfo()["id"] @@ -627,11 +625,12 @@ def test_hsmtool_secret_decryption(node_factory): # We can't use a wrong password ! cmd_line = ["tools/hsmtool", "decrypt", hsm_path, "A wrong pass"] - assert subprocess.Popen(cmd_line).wait() != 0 + with pytest.raises(subprocess.CalledProcessError): + subprocess.check_call(cmd_line) # Decrypt it with hsmtool cmd_line[3] = password[:-1] - assert subprocess.Popen(cmd_line).wait() == 0 + subprocess.check_call(cmd_line) # Then test we can now start it without password l1.daemon.opts.pop("encrypted-hsm") l1.daemon.start(stdin=slave_fd, wait_for_initialized=True) @@ -640,18 +639,19 @@ def test_hsmtool_secret_decryption(node_factory): # Test we can encrypt it offline cmd_line[1] = "encrypt" - assert subprocess.Popen(cmd_line).wait() == 0 + subprocess.check_call(cmd_line) # Now we need to pass the encrypted-hsm startup option l1.stop() - l1.daemon.start(stdin=slave_fd, stderr=subprocess.STDOUT, - wait_for_initialized=False) - time.sleep(3 if SLOW_MACHINE else 1) - err = "hsm_secret is encrypted, you need to pass the --encrypted-hsm startup option." - assert l1.daemon.is_in_log(err) + + with pytest.raises(subprocess.CalledProcessError, match=r'returned non-zero exit status 1'): + subprocess.check_call(l1.daemon.cmd_line) + l1.daemon.opts.update({"encrypted-hsm": None}) + master_fd, slave_fd = os.openpty() l1.daemon.start(stdin=slave_fd, stderr=subprocess.STDOUT, wait_for_initialized=False) - time.sleep(3 if SLOW_MACHINE else 1) + + l1.daemon.wait_for_log(r'The hsm_secret is encrypted') os.write(master_fd, password.encode("utf-8")) l1.daemon.wait_for_log("Server started with public key") assert node_id == l1.rpc.getinfo()["id"] @@ -659,7 +659,7 @@ def test_hsmtool_secret_decryption(node_factory): # And finally test that we can also decrypt if encrypted with hsmtool cmd_line[1] = "decrypt" - assert subprocess.Popen(cmd_line).wait() == 0 + subprocess.check_call(cmd_line) l1.daemon.opts.pop("encrypted-hsm") l1.daemon.start(stdin=slave_fd, wait_for_initialized=True) assert node_id == l1.rpc.getinfo()["id"]