hsm_encryption: read from STDIN if not in a TTY

[openoms]

fixes: #4570 as discussed.

The temporary terminal with ECHO disabled is not created if the input is piped instead of typed in a terminal.

Passwords are still not visible when typed, but allows for automation.

[vincenzopalazzo]

LGTM.

utack 28e5494

[openoms]

thank you @vincenzopalazzo , just modified the commit to add the changelog as in https://lightning.readthedocs.io/STYLE.html#changelog-entries-in-commit-messages

[niftynei]

concept ACK 1b060d6

[darosior]

Concept ACK.

Nice improvement! This definitely needs a test though, and probably a changelog entry?

[darosior]

Note that if this fails in lightningd/options.c, we'd fail. This is a bit unfortunate as we'd like to use it for lightningd as well i guess?

[openoms]

yes, indeed the piped password still fails for lightningd --encrypted_hsm
It is because the temporary terminal is being created as a duplicate in

lightning/lightningd/options.c

Line 397 in 03cfe0b

if (tcgetattr(fileno(stdin), &current_term) != 0)

before calling the read_stdin_pass.
Should be able to clean that, testing now.

[openoms]

Resolved with: 8e86a57

tested ok with:
$ (echo "test"; echo "test") | lightningd --encrypted-hsm

and when called in the command line the password (echo) is still hidden

lightningd --encrypted-hsm
The hsm_secret is encrypted with a password. In order to decrypt it and start the node you must provide the password.
Enter hsm_secret password:
Confirm hsm_secret password:

[rustyrussell]

Ack 8e86a57

Nice fix! Waiting for CI now...

[rustyrussell]

Hidden whitespace at the end of line makes it fail make check-source:

common/hsm_encryption.c:6:#include <stdio.h>	
Extraneous whitespace found
make: *** [Makefile:455: check-whitespace/common/hsm_encryption.c] Error 1

Damn our pedantic source checks!

(I would normally just push a fix directly, but some people consider that rude :)

[openoms]

thank you, fixed in 07269b0. It's fair, every character counts!

[rustyrussell]

Ack cf5eabc

[openoms]

Now not sure how a CI test failure in

lightning/tests/test_opening.py

Line 1025 in 5e1fadf

def test_funder_contribution_limits(node_factory, bitcoind):

is related to the changes here?

[cdecker]

Likely just a deadlock that was addressed in antoher PR. Restarted CI.

[darosior]

Post-merge ACK cf5eabc.

Tested manually and with:

diff --git a/tests/test_wallet.py b/tests/test_wallet.py
index d7c809da8..dee189ef2 100644
--- a/tests/test_wallet.py
+++ b/tests/test_wallet.py
@@ -993,7 +993,7 @@ def test_transaction_annotations(node_factory, bitcoind):
 @unittest.skipIf(VALGRIND, "It does not play well with prompt and key derivation.")
 def test_hsm_secret_encryption(node_factory):
     l1 = node_factory.get_node(may_fail=True)  # May fail when started without key
-    password = "reckful\n"
+    password = "reckful&é🍕\n"
     # We need to simulate a terminal to use termios in `lightningd`.
     master_fd, slave_fd = os.openpty()
 
@@ -1033,11 +1033,21 @@ def test_hsm_secret_encryption(node_factory):
     os.write(master_fd, password.encode("utf-8"))
     l1.daemon.wait_for_log("Server started with public key")
     assert id == l1.rpc.getinfo()["id"]
+    l1.stop()
+
+    # 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"]
 
 
 class HsmTool(TailableProc):
     """Helper for testing the hsmtool as a subprocess"""
     def __init__(self, *args):
+        self.prefix = "hsmtool"
         TailableProc.__init__(self)
         assert hasattr(self, "env")
         self.cmd_line = ["tools/hsmtool", *args]
@@ -1046,7 +1056,7 @@ class HsmTool(TailableProc):
 @unittest.skipIf(VALGRIND, "It does not play well with prompt and key derivation.")
 def test_hsmtool_secret_decryption(node_factory):
     l1 = node_factory.get_node()
-    password = "reckless\n"
+    password = "reckless123#{ù}\n"
     hsm_path = os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, "hsm_secret")
     # We need to simulate a terminal to use termios in `lightningd`.
     master_fd, slave_fd = os.openpty()
@@ -1127,6 +1137,26 @@ def test_hsmtool_secret_decryption(node_factory):
     l1.daemon.start(stdin=slave_fd, wait_for_initialized=True)
     assert node_id == l1.rpc.getinfo()["id"]
 
+    # We can roundtrip encryption and decryption using a password provided
+    # through stdin.
+    hsmtool = HsmTool("encrypt", hsm_path)
+    hsmtool.start(stdin=subprocess.PIPE, stdout=subprocess.PIPE,
+                  stderr=subprocess.PIPE)
+    hsmtool.proc.stdin.write(password.encode("utf-8"))
+    hsmtool.proc.stdin.write(password.encode("utf-8"))
+    hsmtool.proc.stdin.flush()
+    hsmtool.wait_for_log("Successfully encrypted")
+    assert hsmtool.proc.wait(WAIT_TIMEOUT) == 0
+
+    master_fd, slave_fd = os.openpty()
+    hsmtool = HsmTool("decrypt", hsm_path)
+    hsmtool.start(stdin=slave_fd,
+                  stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    hsmtool.wait_for_log("Enter hsm_secret password:")
+    os.write(master_fd, password.encode("utf-8"))
+    hsmtool.wait_for_log("Successfully decrypted")
+    assert hsmtool.proc.wait(WAIT_TIMEOUT) == 0
+
 
 @unittest.skipIf(TEST_NETWORK == 'liquid-regtest', '')
 def test_hsmtool_dump_descriptors(node_factory, bitcoind):