common: trivial changes from review.
What changed, and why it matters
This commit is a small cleanup patch in Core Lightning's HSM (hardware security module) secret handling code. It removes an unused error code, makes some pointer types const, fixes an assertion comment, and reorders a developer-only debug command. There is no indication of a security vulnerability being fixed.
No security action required. Treat as normal code-review cleanup.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The diff shows purely cosmetic/refactoring changes: removal of HSM_SECRET_ERR_MEMORY from the enum and its string mapping; const-correctness changes to read_line return value and struct hsm_secret.secret_data; an updated assertion/comment in detect_hsm_secret_type(); removal of an unused hsm_secret_len variable in create_hsm(); explicit else branch initializing hsm_passphrase to NULL; and reordering of the WIRE_HSMD_DEV_MEMLEAK case in handle_client(). None of these changes alter cryptographic behavior, access control, or memory safety in a security-relevant way.
Changed components
common/hsm_secret.ccommon/hsm_secret.hhsmd/hsmd.cInspect captured patch +28 / −26
diff --git a/common/hsm_secret.c b/common/hsm_secret.c
index b24b1f9a..62348627 100644
--- a/common/hsm_secret.c
+++ b/common/hsm_secret.c
@@ -107,9 +107,13 @@ enum hsm_secret_type detect_hsm_secret_type(const u8 *hsm_secret, size_t len)
/* Legacy 73-byte encrypted format */
if (len == ENCRYPTED_HSM_SECRET_LEN)
return HSM_SECRET_ENCRYPTED;
- assert(len > sizeof(struct sha256));
- /* Check if it starts with our type bytes (mnemonic formats) */
- if (memeqzero(hsm_secret, 32))
+
+ /* Since HSM_SECRET_PLAIN_SIZE == 32, this must be true! */
+ assert(len >= sizeof(struct sha256));
+
+ /* First 32 bytes are the hash of the resulting seed: all 0
+ * for "no passphrase" */
+ if (memeqzero(hsm_secret, sizeof(struct sha256)))
return HSM_SECRET_MNEMONIC_NO_PASS;
else
return HSM_SECRET_MNEMONIC_WITH_PASS;
@@ -200,8 +204,6 @@ const char *hsm_secret_error_str(enum hsm_secret_error err)
return "Invalid hsm_secret format";
case HSM_SECRET_ERR_TERMINAL:
return "Terminal error";
- case HSM_SECRET_ERR_MEMORY:
- return "Memory error";
}
return "Unknown error";
}
@@ -403,7 +405,7 @@ static void restore_echo(const struct termios *saved_term)
}
/* Read line from stdin (uses tal allocation) */
-static char *read_line(const tal_t *ctx)
+static const char *read_line(const tal_t *ctx)
{
char *line = NULL;
size_t size = 0;
@@ -433,7 +435,7 @@ const char *read_stdin_pass(const tal_t *ctx, enum hsm_secret_error *err)
return NULL;
}
- char *input = read_line(ctx);
+ const char *input = read_line(ctx);
if (!input) {
if (echo_disabled)
restore_echo(&saved_term);
@@ -455,7 +457,7 @@ const char *read_stdin_mnemonic(const tal_t *ctx, enum hsm_secret_error *err)
printf("Introduce your BIP39 word list separated by space (at least 12 words):\n");
fflush(stdout);
- char *line = read_line(ctx);
+ const char *line = read_line(ctx);
if (!line) {
*err = HSM_SECRET_ERR_INVALID_FORMAT;
return NULL;
diff --git a/common/hsm_secret.h b/common/hsm_secret.h
index 4ec44c9a..740f2ebc 100644
--- a/common/hsm_secret.h
+++ b/common/hsm_secret.h
@@ -32,7 +32,6 @@ enum hsm_secret_error {
HSM_SECRET_ERR_SEED_DERIVATION_FAILED,
HSM_SECRET_ERR_INVALID_FORMAT,
HSM_SECRET_ERR_TERMINAL,
- HSM_SECRET_ERR_MEMORY
};
/**
@@ -40,7 +39,7 @@ enum hsm_secret_error {
*/
struct hsm_secret {
enum hsm_secret_type type;
- u8 *secret_data; /* Variable length: 32 bytes (legacy) or 64 bytes (mnemonic) */
+ const u8 *secret_data; /* Variable length: 32 bytes (legacy) or 64 bytes (mnemonic) */
const char *mnemonic; /* NULL if not derived from mnemonic */
};
@@ -71,7 +70,7 @@ bool hsm_secret_needs_passphrase(const u8 *hsm_secret, size_t len);
* @hsm_secret - raw file contents
* @len - length of file
* @passphrase - passphrase, or NULL if not needed
- * @err - optional pointer to set error code on failure
+ * @err - pointer to set error code on failure
*
* Returns parsed `struct hsm_secret` or NULL on error.
*/
@@ -100,8 +99,8 @@ struct secret *get_encryption_key(const tal_t *ctx, const char *passphrase);
* Returns true on success.
*/
bool encrypt_legacy_hsm_secret(const struct secret *encryption_key,
- const struct secret *hsm_secret,
- u8 *output);
+ const struct secret *hsm_secret,
+ u8 *output);
/**
* Reads a passphrase from stdin, disabling terminal echo.
diff --git a/hsmd/hsmd.c b/hsmd/hsmd.c
index 957e5e84..7266b4f1 100644
--- a/hsmd/hsmd.c
+++ b/hsmd/hsmd.c
@@ -301,7 +301,6 @@ static void hsmd_send_init_reply_failure(enum hsm_secret_error error_code, enum
static void create_hsm(int fd, const char *passphrase)
{
u8 *hsm_secret_data;
- size_t hsm_secret_len;
int ret;
/* Always create a mnemonic-based hsm_secret */
u8 entropy[BIP39_ENTROPY_LEN_128];
@@ -313,7 +312,6 @@ static void create_hsm(int fd, const char *passphrase)
/* Generate random entropy for new mnemonic */
randombytes_buf(entropy, sizeof(entropy));
-
/* Generate mnemonic from entropy */
tal_wally_start();
ret = bip39_mnemonic_from_bytes(NULL, entropy, sizeof(entropy), &mnemonic);
@@ -322,27 +320,26 @@ static void create_hsm(int fd, const char *passphrase)
if (ret != WALLY_OK) {
unlink_noerr("hsm_secret");
hsmd_send_init_reply_failure(HSM_SECRET_ERR_SEED_DERIVATION_FAILED, STATUS_FAIL_INTERNAL_ERROR,
- "Failed to generate mnemonic from entropy");
+ "Failed to generate mnemonic from entropy");
}
if (!mnemonic) {
unlink_noerr("hsm_secret");
hsmd_send_init_reply_failure(HSM_SECRET_ERR_SEED_DERIVATION_FAILED, STATUS_FAIL_INTERNAL_ERROR,
- "Failed to get generated mnemonic");
+ "Failed to get generated mnemonic");
}
/* Derive seed hash from mnemonic + passphrase (or zero if no passphrase) */
if (!derive_seed_hash(mnemonic, passphrase, &seed_hash)) {
unlink_noerr("hsm_secret");
hsmd_send_init_reply_failure(HSM_SECRET_ERR_SEED_DERIVATION_FAILED, STATUS_FAIL_INTERNAL_ERROR,
- "Failed to derive seed hash from mnemonic");
+ "Failed to derive seed hash from mnemonic");
}
/* Create hsm_secret format: seed_hash (32 bytes) + mnemonic */
hsm_secret_data = tal_arr(tmpctx, u8, 0);
towire_sha256(&hsm_secret_data, &seed_hash);
towire(&hsm_secret_data, mnemonic, strlen(mnemonic));
- hsm_secret_len = tal_count(hsm_secret_data);
/* Derive the actual secret from mnemonic + passphrase for our global hsm_secret */
u8 bip32_seed[BIP39_SEED_LEN_512];
@@ -358,7 +355,7 @@ static void create_hsm(int fd, const char *passphrase)
}
/* Write the hsm_secret data to file */
- if (!write_all(fd, hsm_secret_data, hsm_secret_len)) {
+ if (!write_all(fd, hsm_secret_data, tal_count(hsm_secret_data))) {
unlink_noerr("hsm_secret");
status_failed(STATUS_FAIL_INTERNAL_ERROR,
"writing: %s", strerror(errno));
@@ -494,7 +491,7 @@ static struct io_plan *init_hsm(struct io_conn *conn,
u32 minversion, maxversion;
struct tlv_hsmd_init_tlvs *tlvs;
const u32 our_minversion = 4, our_maxversion = 6;
- const char *hsm_passphrase = NULL; /* Initialize to NULL */
+ const char *hsm_passphrase;
/* This must be lightningd. */
assert(is_lightningd(c));
@@ -531,6 +528,8 @@ static struct io_plan *init_hsm(struct io_conn *conn,
* never sets that anymore), and we use the TLV instead. */
if (tlvs->hsm_passphrase)
hsm_passphrase = tlvs->hsm_passphrase;
+ else
+ hsm_passphrase = NULL;
if (!developer) {
assert(!dev_force_privkey);
@@ -637,6 +636,7 @@ static struct io_plan *handle_memleak(struct io_conn *conn,
memleak_ptr(memtable, dev_force_privkey);
memleak_ptr(memtable, dev_force_bip32_seed);
+
found_leak = dump_memleak(memtable, memleak_status_broken, NULL);
reply = towire_hsmd_dev_memleak_reply(NULL, found_leak);
return req_reply(conn, c, take(reply));
@@ -768,15 +768,16 @@ static struct io_plan *handle_client(struct io_conn *conn, struct client *c)
case WIRE_HSMD_CLIENT_HSMFD:
return pass_client_hsmfd(conn, c, c->msg_in);
- case WIRE_HSMD_DEV_MEMLEAK:
- if (developer)
- return handle_memleak(conn, c, c->msg_in);
- /* fall thru */
-
case WIRE_HSMD_DERIVE_BIP86_KEY:
return handle_derive_bip86_key(conn, c, c->msg_in);
case WIRE_HSMD_CHECK_BIP86_PUBKEY:
return handle_check_bip86_pubkey(conn, c, c->msg_in);
+
+ case WIRE_HSMD_DEV_MEMLEAK:
+ if (!developer)
+ break;
+ return handle_memleak(conn, c, c->msg_in);
+
case WIRE_HSMD_NEW_CHANNEL:
case WIRE_HSMD_SETUP_CHANNEL:
case WIRE_HSMD_CHECK_OUTPOINT:
Why this scored 12/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.