lightningd: group hsm_secret encryption key derivation

This avoids duplication of both logic and error-prone values, such as
the salt. Grouping all hsm encryption logic into a public API will also
allow us to fuzz it.

Signed-off-by: Antoine Poinsot <darosior@protonmail.com>

common/Makefile

@@ -32,6 +32,7 @@ COMMON_SRC_NOGEN :=				\
 	common/gossip_store.c			\
 	common/hash_u5.c			\
 	common/hmac.c				\
+	common/hsm_encryption.c			\
 	common/htlc_state.c			\
 	common/htlc_trim.c			\
 	common/htlc_tx.c			\

common/hsm_encryption.c

@@ -0,0 +1,38 @@
+#include <common/hsm_encryption.h>
+#include <sodium.h>
+#include <sodium/utils.h>
+
+
+char *hsm_secret_encryption_key(const char *pass, struct secret *key)
+{
+	u8 salt[16] = "c-lightning\0\0\0\0\0";
+
+	/* Don't swap the encryption key ! */
+	if (sodium_mlock(key->data, sizeof(key->data)) != 0)
+		return "Could not lock hsm_secret encryption key memory.";
+
+	/* Check bounds. */
+	if (strlen(pass) < crypto_pwhash_argon2id_PASSWD_MIN)
+		return "Password too short to be able to derive a key from it.";
+	if (strlen(pass) > crypto_pwhash_argon2id_PASSWD_MAX)
+		return "Password too long to be able to derive a key from it.";
+
+	/* Now derive the key. */
+	if (crypto_pwhash(key->data, sizeof(key->data), pass, strlen(pass), salt,
+			  /* INTERACTIVE needs 64 MiB of RAM, MODERATE needs 256,
+			   * and SENSITIVE needs 1024. */
+			  crypto_pwhash_argon2id_OPSLIMIT_MODERATE,
+			  crypto_pwhash_argon2id_MEMLIMIT_MODERATE,
+			  crypto_pwhash_ALG_ARGON2ID13) != 0)
+		return "Could not derive a key from the password.";
+
+	return NULL;
+}
+
+void discard_key(struct secret *key TAKES)
+{
+	/* sodium_munlock() also zeroes the memory. */
+	sodium_munlock(key->data, sizeof(key->data));
+	if (taken(key))
+		tal_free(key);
+}

common/hsm_encryption.h

@@ -0,0 +1,22 @@
+#ifndef LIGHTNING_COMMON_HSM_ENCRYPTION_H
+#define LIGHTNING_COMMON_HSM_ENCRYPTION_H
+#include "config.h"
+#include <bitcoin/privkey.h>
+#include <ccan/short_types/short_types.h>
+#include <ccan/tal/tal.h>
+
+
+/** Derive the hsm_secret encryption key from a passphrase.
+ * @pass: the passphrase string.
+ * @encryption_key: the output key derived from the passphrase.
+ *
+ * On success, NULL is returned. On error, a human-readable error is.
+ */
+char *hsm_secret_encryption_key(const char *pass, struct secret *encryption_key);
+
+/** Unlock and zeroize the encryption key memory after use.
+ * @key: the encryption key. If taken, it will be tal_free'd
+ */
+void discard_key(struct secret *key TAKES);
+
+#endif /* LIGHTNING_COMMON_HSM_ENCRYPTION_H */

hsmd/Makefile

@@ -26,6 +26,7 @@ HSMD_COMMON_OBJS :=				\
 	common/derive_basepoints.o		\
 	common/status_wiregen.o			\
 	common/hash_u5.o			\
+	common/hsm_encryption.o			\
 	common/key_derive.o			\
 	common/memleak.o			\
 	common/msg_queue.o			\

hsmd/hsmd.c

@@ -31,6 +31,7 @@
 #include <common/daemon_conn.h>
 #include <common/derive_basepoints.h>
 #include <common/hash_u5.h>
+#include <common/hsm_encryption.h>
 #include <common/key_derive.h>
 #include <common/memleak.h>
 #include <common/node_id.h>
@@ -779,12 +780,9 @@ static struct io_plan *init_hsm(struct io_conn *conn,
 	maybe_create_new_hsm(hsm_encryption_key, true);
 	load_hsm(hsm_encryption_key);
 
-	/*~ We don't need the hsm_secret encryption key anymore.
+	/*~ We don't need the hsm_secret encryption key anymore. */
-	 * Note that sodium_munlock() also zeroes the memory. */
+	if (hsm_encryption_key)
-	if (hsm_encryption_key) {
+		discard_key(take(hsm_encryption_key));
-		sodium_munlock(hsm_encryption_key, sizeof(*hsm_encryption_key));
-		tal_free(hsm_encryption_key);
-	}
 
 	/*~ We tell lightning our node id and (public) bip32 seed. */
 	node_key(NULL, &key);

lightningd/Makefile

@@ -92,6 +92,7 @@ LIGHTNINGD_COMMON_OBJS :=			\
 	common/gossip_rcvd_filter.o		\
 	common/hash_u5.o			\
 	common/hmac.o				\
+	common/hsm_encryption.o			\
 	common/htlc_state.o			\
 	common/htlc_trim.o			\
 	common/htlc_wire.o			\

lightningd/lightningd.c

@@ -59,6 +59,7 @@
 #include <common/daemon.h>
 #include <common/ecdh_hsmd.h>
 #include <common/features.h>
+#include <common/hsm_encryption.h>
 #include <common/memleak.h>
 #include <common/timeout.h>
 #include <common/utils.h>
@@ -878,7 +879,7 @@ int main(int argc, char *argv[])
 	/*~ If hsm_secret is encrypted, we don't need its encryption key
 	 * anymore. Note that sodium_munlock() also zeroes the memory.*/
 	if (ld->config.keypass)
-		sodium_munlock(ld->config.keypass->data, sizeof(ld->config.keypass->data));
+		discard_key(take(ld->config.keypass));
 
 	/*~ Our default color and alias are derived from our node id, so we
 	 * can only set those now (if not set by config options). */

lightningd/options.c

@@ -13,6 +13,7 @@
 #include <common/channel_id.h>
 #include <common/derive_basepoints.h>
 #include <common/features.h>
+#include <common/hsm_encryption.h>
 #include <common/json_command.h>
 #include <common/jsonrpc_errors.h>
 #include <common/memleak.h>
@@ -388,16 +389,8 @@ static char *opt_important_plugin(const char *arg, struct lightningd *ld)
 static char *opt_set_hsm_password(struct lightningd *ld)
 {
 	struct termios current_term, temp_term;
-	char *passwd = NULL, *passwd_confirmation = NULL;
+	char *passwd = NULL, *passwd_confirmation = NULL, *err;
 	size_t passwd_size = 0;
-	u8 salt[16] = "c-lightning\0\0\0\0\0";
-	ld->encrypted_hsm = true;
-
-	ld->config.keypass = tal(NULL, struct secret);
-	/* Don't swap the encryption key ! */
-	if (sodium_mlock(ld->config.keypass->data,
-	                 sizeof(ld->config.keypass->data)) != 0)
-		return "Could not lock hsm_secret encryption key memory.";
 
 	/* Get the password from stdin, but don't echo it. */
 	if (tcgetattr(fileno(stdin), &current_term) != 0)
@@ -426,21 +419,14 @@ static char *opt_set_hsm_password(struct lightningd *ld)
 		return "Could not restore terminal options.";
 	printf("\n");
 
-	/* Derive the key from the password. */
+	ld->config.keypass = tal(NULL, struct secret);
-	if (strlen(passwd) < crypto_pwhash_argon2id_PASSWD_MIN)
+	err = hsm_secret_encryption_key(passwd, ld->config.keypass);
-		return "Password too short to be able to derive a key from it.";
+	if (err)
-	if (strlen(passwd) > crypto_pwhash_argon2id_PASSWD_MAX)
+		return err;
-		return "Password too long to be able to derive a key from it.";
+	ld->encrypted_hsm = true;
-	if (crypto_pwhash(ld->config.keypass->data, sizeof(ld->config.keypass->data),
-	                  passwd, strlen(passwd), salt,
-	                  /* INTERACTIVE needs 64 MiB of RAM, MODERATE needs 256,
-	                   * and SENSITIVE needs 1024. */
-	                  crypto_pwhash_argon2id_OPSLIMIT_MODERATE,
-	                  crypto_pwhash_argon2id_MEMLIMIT_MODERATE,
-	                  crypto_pwhash_ALG_ARGON2ID13) != 0)
-		return "Could not derive a key from the password.";
 	free(passwd);
 	free(passwd_confirmation);
+
 	return NULL;
 }
 

lightningd/test/run-find_my_abspath.c

@@ -50,6 +50,9 @@ s64 db_get_intvar(struct db *db UNNEEDED, char *varname UNNEEDED, s64 defval UNN
 /* Generated stub for db_in_transaction */
 bool db_in_transaction(struct db *db UNNEEDED)
 { fprintf(stderr, "db_in_transaction called!\n"); abort(); }
+/* Generated stub for discard_key */
+void discard_key(struct secret *key TAKES UNNEEDED)
+{ fprintf(stderr, "discard_key called!\n"); abort(); }
 /* Generated stub for ecdh_hsmd_setup */
 void ecdh_hsmd_setup(int hsm_fd UNNEEDED,
 		     void (*failed)(enum status_failreason UNNEEDED,

tools/Makefile

@@ -17,7 +17,7 @@ tools/headerversions: FORCE tools/headerversions.o $(CCAN_OBJS)
 
 tools/check-bolt: tools/check-bolt.o $(CCAN_OBJS) $(TOOLS_COMMON_OBJS)
 
-tools/hsmtool: tools/hsmtool.o $(CCAN_OBJS) $(TOOLS_COMMON_OBJS) $(BITCOIN_OBJS) common/amount.o common/bech32.o common/bigsize.o common/configdir.o common/derive_basepoints.o common/descriptor_checksum.o common/node_id.o common/type_to_string.o common/version.o wire/fromwire.o wire/towire.o
+tools/hsmtool: tools/hsmtool.o $(CCAN_OBJS) $(TOOLS_COMMON_OBJS) $(BITCOIN_OBJS) common/amount.o common/bech32.o common/bigsize.o common/configdir.o common/derive_basepoints.o common/descriptor_checksum.o common/hsm_encryption.o common/node_id.o common/type_to_string.o common/version.o wire/fromwire.o wire/towire.o
 
 tools/lightning-hsmtool: tools/hsmtool
 	cp $< $@

tools/hsmtool.c

@@ -11,6 +11,7 @@
 #include <common/configdir.h>
 #include <common/derive_basepoints.h>
 #include <common/descriptor_checksum.h>
+#include <common/hsm_encryption.h>
 #include <common/node_id.h>
 #include <common/type_to_string.h>
 #include <common/utils.h>
@@ -91,7 +92,7 @@ static void get_encrypted_hsm_secret(struct secret *hsm_secret,
 {
 	int fd;
 	struct secret key;
-	u8 salt[16] = "c-lightning\0\0\0\0\0";
+	char *err;
 	crypto_secretstream_xchacha20poly1305_state crypto_state;
 	u8 header[crypto_secretstream_xchacha20poly1305_HEADERBYTES];
 	/* The cipher size is static with xchacha20poly1305. */
@@ -106,14 +107,13 @@ static void get_encrypted_hsm_secret(struct secret *hsm_secret,
 	if (!read_all(fd, cipher, sizeof(cipher)))
 		errx(ERROR_HSM_FILE, "Could not read cipher body");
 
-	if (crypto_pwhash(key.data, sizeof(key.data), passwd, strlen(passwd), salt,
+	err = hsm_secret_encryption_key(passwd, &key);
-	                  crypto_pwhash_argon2id_OPSLIMIT_MODERATE,
+	if (err)
-	                  crypto_pwhash_argon2id_MEMLIMIT_MODERATE,
+		errx(ERROR_LIBSODIUM, "%s", err);
-	                  crypto_pwhash_ALG_ARGON2ID13) != 0)
-		errx(ERROR_LIBSODIUM, "Could not derive a key from the password.");
 	if (crypto_secretstream_xchacha20poly1305_init_pull(&crypto_state, header,
 							    key.data) != 0)
 		errx(ERROR_LIBSODIUM, "Could not initialize the crypto state");
+	discard_key(&key);
 	if (crypto_secretstream_xchacha20poly1305_pull(&crypto_state, hsm_secret->data,
 	                                               NULL, 0, cipher, sizeof(cipher),
 	                                               NULL, 0) != 0)
@@ -253,8 +253,7 @@ static int encrypt_hsm(const char *hsm_secret_path)
 {
 	int fd;
 	struct secret key, hsm_secret;
-	char *passwd, *passwd_confirmation;
+	char *passwd, *passwd_confirmation, *err;
-	u8 salt[16] = "c-lightning\0\0\0\0\0";
 	crypto_secretstream_xchacha20poly1305_state crypto_state;
 	u8 header[crypto_secretstream_xchacha20poly1305_HEADERBYTES];
 	/* The cipher size is static with xchacha20poly1305. */
@@ -282,14 +281,13 @@ static int encrypt_hsm(const char *hsm_secret_path)
 
 	/* Derive the encryption key from the password provided, and try to encrypt
 	 * the seed. */
-	if (crypto_pwhash(key.data, sizeof(key.data), passwd, strlen(passwd), salt,
+	err = hsm_secret_encryption_key(passwd, &key);
-	                  crypto_pwhash_argon2id_OPSLIMIT_MODERATE,
+	if (err)
-	                  crypto_pwhash_argon2id_MEMLIMIT_MODERATE,
+		errx(ERROR_LIBSODIUM, "%s", err);
-	                  crypto_pwhash_ALG_ARGON2ID13) != 0)
-		errx(ERROR_LIBSODIUM, "Could not derive a key from the password.");
 	if (crypto_secretstream_xchacha20poly1305_init_push(&crypto_state, header,
 	                                                    key.data) != 0)
 		errx(ERROR_LIBSODIUM, "Could not initialize the crypto state");
+	discard_key(&key);
 	if (crypto_secretstream_xchacha20poly1305_push(&crypto_state, cipher,
 	                                               NULL, hsm_secret.data,
 	                                               sizeof(hsm_secret.data),