diff --git a/common/hsm_secret.c b/common/hsm_secret.c index f14c7e19f..fd32c516d 100644 --- a/common/hsm_secret.c +++ b/common/hsm_secret.c @@ -220,16 +220,13 @@ static struct hsm_secret *extract_plain_secret(const tal_t *ctx, { struct hsm_secret *hsms = tal(ctx, struct hsm_secret); - assert(len == sizeof(hsms->secret)); + assert(len == HSM_SECRET_PLAIN_SIZE); hsms->type = HSM_SECRET_PLAIN; hsms->mnemonic = NULL; /* Allocate and populate secret_data (new field) */ hsms->secret_data = tal_dup_arr(hsms, u8, hsm_secret, HSM_SECRET_PLAIN_SIZE, 0); - /* Also populate legacy secret field for compatibility */ - memcpy(&hsms->secret, hsm_secret, sizeof(hsms->secret)); - *err = HSM_SECRET_OK; return hsms; } @@ -254,24 +251,20 @@ static struct hsm_secret *extract_encrypted_secret(const tal_t *ctx, return tal_free(hsms); } - /* Clear secret data first in case of partial decryption */ - memset(&hsms->secret, 0, sizeof(hsms->secret)); - /* Attempt decryption */ - decrypt_success = decrypt_hsm_secret(encryption_key, hsm_secret, &hsms->secret); + 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) { - /* Clear any partial decryption data */ - memset(&hsms->secret, 0, sizeof(hsms->secret)); *err = HSM_SECRET_ERR_WRONG_PASSPHRASE; return tal_free(hsms); } - /* Allocate and populate secret_data (new field) */ - hsms->secret_data = tal_dup_arr(hsms, u8, hsms->secret.data, HSM_SECRET_PLAIN_SIZE, 0); + /* Duplicate decrypted secret data */ + hsms->secret_data = tal_dup_arr(hsms, u8, temp_secret.data, HSM_SECRET_PLAIN_SIZE, 0); hsms->type = HSM_SECRET_ENCRYPTED; hsms->mnemonic = NULL; @@ -342,9 +335,6 @@ static struct hsm_secret *extract_mnemonic_secret(const tal_t *ctx, /* Allocate and populate secret_data with full 64-byte seed */ hsms->secret_data = tal_dup_arr(hsms, u8, bip32_seed.seed, sizeof(bip32_seed.seed), 0); - /* Also populate legacy secret field with first 32 bytes for compatibility */ - memcpy(hsms->secret.data, bip32_seed.seed, sizeof(hsms->secret.data)); - *err = HSM_SECRET_OK; return hsms; } @@ -544,20 +534,6 @@ u8 *grab_file_contents(const tal_t *ctx, const char *filename, size_t *len) return contents; } -const u8 *hsm_secret_bytes(const struct hsm_secret *hsm) -{ - if (hsm->secret_data) - return hsm->secret_data; - return hsm->secret.data; -} - -size_t hsm_secret_size(const struct hsm_secret *hsm) -{ - if (hsm->secret_data) - return tal_bytelen(hsm->secret_data); - return sizeof(hsm->secret); -} - bool is_mnemonic_secret(size_t secret_len) { return secret_len == HSM_SECRET_MNEMONIC_SIZE; diff --git a/common/hsm_secret.h b/common/hsm_secret.h index 66c158f3a..f874f0422 100644 --- a/common/hsm_secret.h +++ b/common/hsm_secret.h @@ -41,21 +41,9 @@ enum hsm_secret_error { struct hsm_secret { enum hsm_secret_type type; u8 *secret_data; /* Variable length: 32 bytes (legacy) or 64 bytes (mnemonic) */ - struct secret secret; /* Legacy 32-byte field for compatibility */ const char *mnemonic; /* NULL if not derived from mnemonic */ }; -/** - * Get the secret bytes from an hsm_secret. - * Returns secret_data if available, otherwise falls back to legacy secret.data. - */ -const u8 *hsm_secret_bytes(const struct hsm_secret *hsm); - -/** - * Get the secret size from an hsm_secret. - * Returns tal_bytelen of secret_data if available, otherwise 32 bytes for legacy. - */ -size_t hsm_secret_size(const struct hsm_secret *hsm); /** * Check if this HSM secret is mnemonic-based (64-byte seed). diff --git a/common/hsm_version.h b/common/hsm_version.h index d6a9000da..5b7df62db 100644 --- a/common/hsm_version.h +++ b/common/hsm_version.h @@ -30,6 +30,7 @@ * v6 with dev_warn_on_overgrind: a273b68e19336073e551c01a78bcd1e1f8cc510da7d0dde3afc45e249f9830cc * v6 with bip137_sign_message: 4bfe28b02e92aae276b8eca2228e32f32d5dee8d5381639e7364939fa2fa1370 * v6 with hsm_passphrase changes: c646d557d7561dd885df3cad5b99c82895cda4b040699f3853980ec61b2873fa + * v6 with hsm_secret struct cleanup: 06c56396fe42f4f47911d7f865dd0004d264fc1348f89547743755b6b33fec90 */ #define HSM_MIN_VERSION 5 #define HSM_MAX_VERSION 6 diff --git a/hsmd/hsmd.c b/hsmd/hsmd.c index 5e2306534..1205d142d 100644 --- a/hsmd/hsmd.c +++ b/hsmd/hsmd.c @@ -30,6 +30,7 @@ #include #include #include +#include #include /*~ Each subdaemon is started with stdin connected to lightningd (for status @@ -39,7 +40,7 @@ #define REQ_FD 3 /* Temporary storage for the secret until we pass it to `hsmd_init` */ -struct hsm_secret hsm_secret; +struct hsm_secret *hsm_secret; /*~ We keep track of clients, but there's not much to keep. */ struct client { @@ -356,9 +357,6 @@ static void create_hsm(int fd, const char *passphrase) "Failed to derive seed from mnemonic"); } - /* Use first 32 bytes for hsm_secret */ - memcpy(&hsm_secret.secret, bip32_seed, sizeof(hsm_secret.secret)); - /* Write the hsm_secret data to file */ if (!write_all(fd, hsm_secret_data, hsm_secret_len)) { unlink_noerr("hsm_secret"); @@ -450,10 +448,14 @@ static void load_hsm(const char *passphrase) "Failed to load hsm_secret: %s", hsm_secret_error_str(err)); } - /* Copy only the secret field to our global hsm_secret */ - hsm_secret.secret = hsms->secret; - hsm_secret.type = hsms->type; - hsm_secret.mnemonic = hsms->mnemonic; + /* Allocate and populate our global hsm_secret */ + hsm_secret = tal(NULL, struct hsm_secret); + hsm_secret->secret_data = tal_steal(hsm_secret, hsms->secret_data); + hsm_secret->type = hsms->type; + 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)); } /*~ We have a pre-init call in developer mode, to set dev flags */ @@ -531,9 +533,6 @@ static struct io_plan *init_hsm(struct io_conn *conn, if (tlvs->hsm_passphrase) hsm_passphrase = tlvs->hsm_passphrase; - /*~ Don't swap this. */ - sodium_mlock(hsm_secret.secret.data, sizeof(hsm_secret.secret.data)); - if (!developer) { assert(!dev_force_privkey); assert(!dev_force_bip32_seed); @@ -552,8 +551,8 @@ static struct io_plan *init_hsm(struct io_conn *conn, /* This was tallocated off NULL, and memleak complains if we don't free it */ tal_free(tlvs); - return req_reply(conn, c, hsmd_init(hsm_secret_bytes(&hsm_secret), - hsm_secret_size(&hsm_secret), + return req_reply(conn, c, hsmd_init(hsm_secret->secret_data, + tal_bytelen(hsm_secret->secret_data), hsmd_mutual_version, bip32_key_version)); } @@ -635,10 +634,10 @@ static struct io_plan *handle_memleak(struct io_conn *conn, memleak_scan_region(memtable, dbid_zero_clients, sizeof(dbid_zero_clients)); memleak_scan_uintmap(memtable, &clients); memleak_scan_obj(memtable, status_conn); + memleak_scan_obj(memtable, hsm_secret); 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)); @@ -696,7 +695,7 @@ static struct io_plan *handle_derive_bip86_key(struct io_conn *conn, return bad_req(conn, c, msg_in); /* Check if we have a mnemonic-based HSM secret */ - if (!use_bip86_derivation(hsm_secret_size(&hsm_secret))) { + if (!use_bip86_derivation(tal_bytelen(hsm_secret->secret_data))) { return bad_req_fmt(conn, c, msg_in, "BIP86 derivation requires mnemonic-based HSM secret"); } @@ -724,7 +723,7 @@ static struct io_plan *handle_check_bip86_pubkey(struct io_conn *conn, return bad_req(conn, c, msg_in); /* Check if we have a mnemonic-based HSM secret */ - if (!use_bip86_derivation(hsm_secret_size(&hsm_secret))) { + if (!use_bip86_derivation(tal_bytelen(hsm_secret->secret_data))) { return bad_req_fmt(conn, c, msg_in, "BIP86 derivation requires mnemonic-based HSM secret"); } @@ -774,6 +773,7 @@ static struct io_plan *handle_client(struct io_conn *conn, struct client *c) 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: diff --git a/hsmd/libhsmd.c b/hsmd/libhsmd.c index b48c15d18..1f1096b45 100644 --- a/hsmd/libhsmd.c +++ b/hsmd/libhsmd.c @@ -1,4 +1,5 @@ #include "config.h" +#include #include #include #include @@ -11,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -18,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -34,13 +37,7 @@ struct secret *dev_force_bip32_seed; * tree, bolt12 payer_id keys and derived_secret are derived from that, and * cached here. */ struct { - /* keep for legacy callers that read 32B directly today */ - struct secret hsm_secret; - - /* new: full root bytes we received (32 or 64) */ - u8 *bip32_seed; - size_t bip32_seed_len; - + u8 *bip32_seed; /* Variable length: 32 bytes (legacy) or 64 bytes (mnemonic) */ struct ext_key bip32; struct secret bolt12; struct secret derived_secret; @@ -52,6 +49,7 @@ bool initialized = false; /* BIP32 key version for network compatibility */ static struct bip32_key_version network_bip32_key_version; + /* Do we fail all preapprove requests? */ bool dev_fail_preapprove = false; bool dev_no_preapprove_check = false; @@ -269,8 +267,8 @@ static void node_key(struct privkey *node_privkey, struct pubkey *node_id) * leaks somehow, the other keys are not compromised. */ hkdf_sha256(node_privkey, sizeof(*node_privkey), &salt, sizeof(salt), - &secretstuff.hsm_secret, - sizeof(secretstuff.hsm_secret), + secretstuff.bip32_seed, + 32, /* Use first 32 bytes for node key derivation */ "nodeid", 6); salt++; } while (!secp256k1_ec_pubkey_create(secp256k1_ctx, &node_id->pubkey, @@ -303,7 +301,7 @@ static void node_schnorrkey(secp256k1_keypair *node_keypair) static void hsm_channel_secret_base(struct secret *channel_seed_base) { hkdf_sha256(channel_seed_base, sizeof(struct secret), NULL, 0, - &secretstuff.hsm_secret, sizeof(secretstuff.hsm_secret), + secretstuff.bip32_seed, 32, /* Use first 32 bytes */ /*~ Initially, we didn't support multiple channels per * peer at all: a channel had to be completely forgotten * before another could exist. That was slightly relaxed, @@ -551,7 +549,7 @@ static void hsm_key_for_utxo(struct privkey *privkey, struct pubkey *pubkey, /* For P2TR scripts, we need to determine if it's BIP86 or regular P2TR * But BIP86 derivation requires mnemonic-based secrets */ if (is_p2tr(utxo->scriptPubkey, script_len, NULL) && - use_bip86_derivation(secretstuff.bip32_seed_len)) { + use_bip86_derivation(tal_bytelen(secretstuff.bip32_seed))) { /* Try BIP86 derivation first and see if it matches */ struct pubkey test_pubkey; bip86_key(NULL, &test_pubkey, utxo->keyindex); @@ -2390,7 +2388,7 @@ u8 *hsmd_handle_client_message(const tal_t *ctx, struct hsmd_client *client, void derive_bip86_base_key(struct ext_key *bip86_base) { /* Check if we have the full BIP32 seed available */ - if (secretstuff.bip32_seed_len < BIP39_SEED_LEN_512) { + if (!use_bip86_derivation(tal_bytelen(secretstuff.bip32_seed))) { hsmd_status_failed(STATUS_FAIL_INTERNAL_ERROR, "BIP86 derivation requires full 64-byte BIP32 seed (not available in legacy format)"); } @@ -2398,7 +2396,7 @@ void derive_bip86_base_key(struct ext_key *bip86_base) /* First create the master key from the seed */ struct ext_key master_key; - if (bip32_key_from_seed(secretstuff.bip32_seed, secretstuff.bip32_seed_len, network_bip32_key_version.bip32_privkey_version, 0, &master_key) != WALLY_OK) { + if (bip32_key_from_seed(secretstuff.bip32_seed, tal_bytelen(secretstuff.bip32_seed), network_bip32_key_version.bip32_privkey_version, 0, &master_key) != WALLY_OK) { hsmd_status_failed(STATUS_FAIL_INTERNAL_ERROR, "Failed to create master key from BIP32 seed"); } @@ -2477,13 +2475,9 @@ u8 *hsmd_init(const u8 *secret_data, size_t secret_len, const u64 hsmd_version, /*~ Store the BIP32 key version for network compatibility */ network_bip32_key_version = bip32_key_version; - /* new: keep the full 32/64B root */ - secretstuff.bip32_seed_len = secret_len; + /*~ 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, secret_len); - - /* legacy mirror: first 32 bytes for existing code paths */ - memcpy(secretstuff.hsm_secret.data, secret_data, 32); + sodium_mlock(secretstuff.bip32_seed, tal_bytelen(secretstuff.bip32_seed)); assert(bip32_key_version.bip32_pubkey_version == BIP32_VER_MAIN_PUBLIC || bip32_key_version.bip32_pubkey_version == BIP32_VER_TEST_PUBLIC); @@ -2498,8 +2492,8 @@ u8 *hsmd_init(const u8 *secret_data, size_t secret_len, const u64 hsmd_version, do { hkdf_sha256(bip32_seed, sizeof(bip32_seed), &salt, sizeof(salt), - &secretstuff.hsm_secret, - sizeof(secretstuff.hsm_secret), + secretstuff.bip32_seed, + tal_bytelen(secretstuff.bip32_seed), "bip32 seed", strlen("bip32 seed")); salt++; } while (bip32_key_from_seed(bip32_seed, sizeof(bip32_seed), @@ -2597,7 +2591,7 @@ u8 *hsmd_init(const u8 *secret_data, size_t secret_len, const u64 hsmd_version, /* We derive the derived_secret key for generating pseudorandom keys * by taking input string from the makesecret RPC */ hkdf_sha256(&secretstuff.derived_secret, sizeof(struct secret), NULL, 0, - &secretstuff.hsm_secret, sizeof(secretstuff.hsm_secret), + secretstuff.bip32_seed, tal_bytelen(secretstuff.bip32_seed), "derived secrets", strlen("derived secrets")); /* Capabilities arg needs to be a tal array */ diff --git a/hsmd/libhsmd.h b/hsmd/libhsmd.h index 3ebb3eb8d..fe6aa47c6 100644 --- a/hsmd/libhsmd.h +++ b/hsmd/libhsmd.h @@ -88,6 +88,7 @@ void hsmd_status_failed(enum status_failreason code, bool hsmd_check_client_capabilities(struct hsmd_client *client, enum hsmd_wire t); + /* BIP86 key derivation functions */ void derive_bip86_base_key(struct ext_key *bip86_base); void bip86_key(struct privkey *privkey, struct pubkey *pubkey, u32 index); diff --git a/plugins/exposesecret.c b/plugins/exposesecret.c index f514630dc..1303254f4 100644 --- a/plugins/exposesecret.c +++ b/plugins/exposesecret.c @@ -90,8 +90,8 @@ static struct command_result *json_exposesecret(struct command *cmd, /* Before we expose it, check it's correct! */ hkdf_sha256(&node_privkey, sizeof(node_privkey), &salt, sizeof(salt), - &hsms->secret, - sizeof(hsms->secret), + hsms->secret_data, + 32, "nodeid", 6); /* Should not happen! */ @@ -125,7 +125,7 @@ static struct command_result *json_exposesecret(struct command *cmd, } /* This also cannot fail! */ - const char *encode_err = codex32_secret_encode(tmpctx, "cl", id, 0, hsms->secret.data, 32, &bip93); + const char *encode_err = codex32_secret_encode(tmpctx, "cl", id, 0, hsms->secret_data, 32, &bip93); if (encode_err) return command_fail(cmd, LIGHTNINGD, "Unexpected failure encoding hsm_secret: %s", encode_err); diff --git a/tools/hsmtool.c b/tools/hsmtool.c index c9cd720b1..2ec8934e3 100644 --- a/tools/hsmtool.c +++ b/tools/hsmtool.c @@ -146,7 +146,7 @@ static void decrypt_hsm(const char *hsm_secret_path) if (fd < 0) err(EXITCODE_ERROR_HSM_FILE, "Could not open new hsm_secret"); - if (!write_all(fd, &hsms->secret, sizeof(hsms->secret))) { + if (!write_all(fd, hsms->secret_data, tal_bytelen(hsms->secret_data))) { unlink_noerr(hsm_secret_path); close(fd); rename("hsm_secret.backup", hsm_secret_path); @@ -218,7 +218,9 @@ static void encrypt_hsm(const char *hsm_secret_path) if (!encryption_key) errx(ERROR_LIBSODIUM, "Could not derive encryption key"); - if (!encrypt_legacy_hsm_secret(encryption_key, &hsms->secret, encrypted_hsm_secret)) + struct secret legacy_secret; + memcpy(legacy_secret.data, hsms->secret_data, 32); + 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 */ @@ -279,7 +281,8 @@ static void print_codexsecret(const char *hsm_secret_path, const char *id) char *bip93; const char *err; struct hsm_secret *hsms = load_hsm_secret(tmpctx, hsm_secret_path); - hsm_secret = hsms->secret; + /* Extract first 32 bytes for legacy compatibility */ + memcpy(hsm_secret.data, hsms->secret_data, 32); err = codex32_secret_encode(tmpctx, "cl", id, 0, hsm_secret.data, 32, &bip93); if (err) @@ -314,7 +317,8 @@ static void dump_commitments_infos(struct node_id *node_id, u64 channel_id, struct secret hsm_secret, channel_seed, per_commitment_secret; struct pubkey per_commitment_point; struct hsm_secret *hsms = load_hsm_secret(tmpctx, hsm_secret_path); - hsm_secret = hsms->secret; + /* Extract first 32 bytes for legacy compatibility */ + memcpy(hsm_secret.data, hsms->secret_data, 32); get_channel_seed(&channel_seed, node_id, channel_id, &hsm_secret); derive_shaseed(&channel_seed, &shaseed); @@ -350,7 +354,7 @@ static void guess_to_remote(const char *address, struct node_id *node_id, errx(ERROR_USAGE, "Wrong bech32 address"); struct hsm_secret *hsms = load_hsm_secret(tmpctx, hsm_secret_path); - hsm_secret = hsms->secret; + memcpy(hsm_secret.data, hsms->secret_data, 32); for (u64 dbid = 1; dbid < tries ; dbid++) { get_channel_seed(&channel_seed, node_id, dbid, &hsm_secret); @@ -444,7 +448,8 @@ static void derive_to_remote(const struct unilateral_close_info *info, const cha | SECP256K1_CONTEXT_SIGN); struct hsm_secret *hsms = load_hsm_secret(tmpctx, hsm_secret_path); - hsm_secret = hsms->secret; + /* Copy first 32 bytes of secret_data to the local secret struct */ + memcpy(hsm_secret.data, hsms->secret_data, sizeof(hsm_secret.data)); get_channel_seed(&channel_seed, &info->peer_id, info->channel_id, &hsm_secret); if (!derive_payment_basepoint(&channel_seed, &basepoint, &basepoint_secret)) errx(ERROR_KEYDERIV, "Could not derive basepoints for dbid %"PRIu64 @@ -469,7 +474,8 @@ static void dumponchaindescriptors(const char *hsm_secret_path, char *enc_xkey, *descriptor; struct descriptor_checksum checksum; struct hsm_secret *hsms = load_hsm_secret(tmpctx, hsm_secret_path); - hsm_secret = hsms->secret; + /* Extract first 32 bytes for legacy compatibility */ + memcpy(hsm_secret.data, hsms->secret_data, 32); /* The root seed is derived from hsm_secret using hkdf.. */ do { @@ -527,7 +533,8 @@ static void check_hsm(const char *hsm_secret_path) /* Load the hsm_secret (handles decryption automatically if needed) */ struct hsm_secret *hsms = load_hsm_secret(tmpctx, hsm_secret_path); - file_secret = hsms->secret; + /* Extract first 32 bytes for legacy compatibility */ + memcpy(file_secret.data, hsms->secret_data, 32); /* Ask user for their backup mnemonic passphrase */ printf("Warning: remember that different passphrases yield different " @@ -566,7 +573,8 @@ static void make_rune(const char *hsm_secret_path) struct secret hsm_secret, derived_secret, rune_secret; struct rune *master_rune, *rune; struct hsm_secret *hsms = load_hsm_secret(tmpctx, hsm_secret_path); - hsm_secret = hsms->secret; + /* Extract first 32 bytes for legacy compatibility */ + memcpy(hsm_secret.data, hsms->secret_data, 32); /* HSM derives a root secret for `makesecret` */ hkdf_sha256(&derived_secret, sizeof(struct secret), NULL, 0, @@ -593,7 +601,8 @@ static void print_node_id(const char *hsm_secret_path) struct privkey node_privkey; struct pubkey node_id; struct hsm_secret *hsms = load_hsm_secret(tmpctx, hsm_secret_path); - hsm_secret = hsms->secret; + /* Extract first 32 bytes for legacy compatibility */ + memcpy(hsm_secret.data, hsms->secret_data, 32); /*~ So, there is apparently a 1 in 2^127 chance that a random value is * not a valid private key, so this never actually loops. */