utils: add a generic mlock function with a destructor
What changed, and why it matters
This commit is a code cleanup that replaces several scattered copies of a memory-locking pattern with one shared helper function. It does not add new security features, but it reduces the chance that future code forgets to unlock sensitive memory when it is freed. The change also makes memory locking mandatory (the program aborts if locking fails) in more places, which is a small hardening improvement.
Treat as a defensive hardening/refactoring commit. Review that all converted call sites pass a tal-allocated pointer and that the destructor ordering does not interfere with early-return or error paths. No urgent action required.
Security signals we found
Refactors sensitive-memory handling into a single helper with mandatory mlock and automatic munlock destructor
Converts HSM secret encryption key and passphrase handling to use shared mlock_tal_memory helper
Removes explicit destroy_secret() calls where tal destructor now covers cleanup
Adds automatic sodium_munlock destructor to hsmd secret_data and bip32_seed paths
No new cryptographic operations or protocol changes
Evidence from the diff
The patch introduces mlock_tal_memory() in common/utils.c, which combines sodium_mlock() with a tal destructor that calls sodium_munlock() on free. Existing call sites in common/hsm_secret.c, hsmd/hsmd.c, hsmd/libhsmd.c, and tools/hsmtool.c are converted to use the helper. Notable side effects: (1) get_encryption_key() now aborts on mlock failure instead of returning NULL; (2) read_stdin_pass() already aborted on failure and keeps that behavior; (3) two explicit destroy_secret()/destroy_passphrase() calls are removed because the tal destructor now handles cleanup automatically. The hsmd.c and libhsmd.c conversions add an automatic munlock destructor where previously only a sodium_mlock() was performed, so those secrets are now explicitly zeroed/unlocked on free.
Changed components
common/utils.ccommon/utils.hcommon/hsm_secret.chsmd/hsmd.chsmd/libhsmd.ctools/hsmtool.cInspect captured patch +27 / −33
diff --git a/common/hsm_secret.c b/common/hsm_secret.c
index fd32c516..9acb044f 100644
--- a/common/hsm_secret.c
+++ b/common/hsm_secret.c
@@ -28,11 +28,6 @@
#define PASSPHRASE_HASH_LEN 32
#define HSM_SECRET_PLAIN_SIZE 32
-void destroy_secret(struct secret *secret)
-{
- sodium_munlock(secret->data, sizeof(secret->data));
-}
-
/* Helper function to validate a mnemonic string */
static bool validate_mnemonic(const char *mnemonic, enum hsm_secret_error *err)
{
@@ -70,9 +65,7 @@ struct secret *get_encryption_key(const tal_t *ctx, const char *passphrase)
}
/* Don't swap the encryption key ! */
- if (sodium_mlock(secret->data, sizeof(secret->data)) != 0)
- return tal_free(secret);
- tal_add_destructor(secret, destroy_secret);
+ mlock_tal_memory(secret);
/* Now derive the key. */
if (crypto_pwhash(secret->data, sizeof(secret->data), passphrase, strlen(passphrase), salt,
@@ -254,10 +247,6 @@ static struct hsm_secret *extract_encrypted_secret(const tal_t *ctx,
/* Attempt decryption */
struct secret temp_secret;
decrypt_success = decrypt_hsm_secret(encryption_key, hsm_secret, &temp_secret);
-
- /* Clear encryption key immediately after use */
- destroy_secret(encryption_key);
-
if (!decrypt_success) {
*err = HSM_SECRET_ERR_WRONG_PASSPHRASE;
return tal_free(hsms);
@@ -389,11 +378,6 @@ bool encrypt_legacy_hsm_secret(const struct secret *encryption_key,
return true;
}
-static void destroy_passphrase(char *passphrase)
-{
- sodium_munlock(passphrase, tal_bytelen(passphrase));
-}
-
/* Disable terminal echo if needed */
static bool disable_echo(struct termios *saved_term)
{
@@ -457,11 +441,7 @@ const char *read_stdin_pass(const tal_t *ctx, enum hsm_secret_error *err)
return NULL;
}
- /* Memory locking is mandatory: failure means we're on an insecure system */
- if (sodium_mlock(input, tal_bytelen(input)) != 0)
- abort();
-
- tal_add_destructor(input, destroy_passphrase);
+ mlock_tal_memory(input);
if (echo_disabled)
restore_echo(&saved_term);
diff --git a/common/hsm_secret.h b/common/hsm_secret.h
index f874f042..74a11a32 100644
--- a/common/hsm_secret.h
+++ b/common/hsm_secret.h
@@ -158,12 +158,6 @@ bool derive_seed_hash(const char *mnemonic, const char *passphrase, struct sha25
*/
int is_legacy_hsm_secret_encrypted(const char *path);
-/**
- * Zero and unlock a secret's memory.
- * @secret - the secret to destroy
- */
-void destroy_secret(struct secret *secret);
-
/**
* Convert hsm_secret_type enum to human-readable string.
* @type - the hsm_secret_type to convert
diff --git a/common/utils.c b/common/utils.c
index 053e09a7..aba47452 100644
--- a/common/utils.c
+++ b/common/utils.c
@@ -10,6 +10,7 @@
#include <common/utils.h>
#include <errno.h>
#include <locale.h>
+#include <sodium.h>
const tal_t *wally_tal_ctx = NULL;
secp256k1_context *secp256k1_ctx;
@@ -92,6 +93,18 @@ u8 *tal_hexdata(const tal_t *ctx, const void *str, size_t len)
return data;
}
+static void destroy_munlock(const tal_t *ptr)
+{
+ sodium_munlock((void *)ptr, tal_bytelen(ptr));
+}
+
+void mlock_tal_memory(const tal_t *ptr)
+{
+ if (sodium_mlock((void *)ptr, tal_bytelen(ptr)) != 0)
+ abort();
+ tal_add_destructor(ptr, destroy_munlock);
+}
+
bool tal_arr_eq_(const void *a, const void *b, size_t unused)
{
return memeq(a, tal_bytelen(a), b, tal_bytelen(b));
diff --git a/common/utils.h b/common/utils.h
index 550748b2..d9b441e4 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -53,6 +53,16 @@ char *tal_hex(const tal_t *ctx, const tal_t *data);
/* Allocate and fill a buffer with the data of this hex string. */
u8 *tal_hexdata(const tal_t *ctx, const void *str, size_t len);
+/**
+ * mlock_tal_memory - lock a tal-allocated memory with sodium_mlock.
+ * @ptr - the tal-allocated memory to lock
+ *
+ * This is a generic function to replace the pattern of sodium_mlock + tal_add_destructor.
+ *
+ * Aborts on failure (memory locking is mandatory for security).
+ */
+void mlock_tal_memory(const tal_t *ptr);
+
/* Note: p is never a complex expression, otherwise this multi-evaluates! */
#define tal_arr_expand(p, s) \
do { \
diff --git a/hsmd/hsmd.c b/hsmd/hsmd.c
index 11bcfe27..78750908 100644
--- a/hsmd/hsmd.c
+++ b/hsmd/hsmd.c
@@ -455,7 +455,7 @@ static void load_hsm(const char *passphrase)
hsm_secret->mnemonic = tal_steal(hsm_secret, hsms->mnemonic);
/*~ Don't swap this secret data to disk for security. */
- sodium_mlock(hsm_secret->secret_data, tal_bytelen(hsm_secret->secret_data));
+ mlock_tal_memory(hsm_secret->secret_data);
}
/*~ We have a pre-init call in developer mode, to set dev flags */
diff --git a/hsmd/libhsmd.c b/hsmd/libhsmd.c
index bff142b2..b362c812 100644
--- a/hsmd/libhsmd.c
+++ b/hsmd/libhsmd.c
@@ -2477,7 +2477,7 @@ u8 *hsmd_init(const u8 *secret_data, size_t secret_len, const u64 hsmd_version,
/*~ Store the secret (32 or 64 bytes) - use NULL context for persistence */
secretstuff.bip32_seed = notleak(tal_dup_arr(NULL, u8, secret_data, secret_len, 0));
- sodium_mlock(secretstuff.bip32_seed, tal_bytelen(secretstuff.bip32_seed));
+ mlock_tal_memory(secretstuff.bip32_seed);
assert(bip32_key_version.bip32_pubkey_version == BIP32_VER_MAIN_PUBLIC
|| bip32_key_version.bip32_pubkey_version == BIP32_VER_TEST_PUBLIC);
diff --git a/tools/hsmtool.c b/tools/hsmtool.c
index 2ec8934e..5d93b740 100644
--- a/tools/hsmtool.c
+++ b/tools/hsmtool.c
@@ -223,9 +223,6 @@ static void encrypt_hsm(const char *hsm_secret_path)
if (!encrypt_legacy_hsm_secret(encryption_key, &legacy_secret, encrypted_hsm_secret))
errx(ERROR_LIBSODIUM, "Could not encrypt the hsm_secret seed.");
- /* Securely discard the encryption key */
- destroy_secret(encryption_key);
-
/* Create a backup file, "just in case". */
rename(hsm_secret_path, backup);
fd = open(hsm_secret_path, O_CREAT|O_EXCL|O_WRONLY, 0400);
Why this scored 24/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.