lightningd: move bip32_pubkey here from common/, add hsm check.

At the moment only lightingd needs it, and this avoids missing any places where we do bip32 derivation. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Added: Protocol: we now always double-check bitcoin addresses are correct (no memory errors!) before issuing them.
ElementsProject · Nov 11, 2022 · a281242 · a281242
1 parent d5d4036
commit a281242
Show file tree

Hide file tree

Showing 13 changed files with 62 additions and 48 deletions.
diff --git a/common/key_derive.c b/common/key_derive.c
@@ -248,22 +248,3 @@ bool derive_revocation_privkey(const struct secret *base_secret,
 #endif
 	return true;
 }
-
-
-bool bip32_pubkey(const struct ext_key *bip32_base,
-		  struct pubkey *pubkey, u32 index)
-{
-	const uint32_t flags = BIP32_FLAG_KEY_PUBLIC | BIP32_FLAG_SKIP_HASH;
-	struct ext_key ext;
-
-	if (index >= BIP32_INITIAL_HARDENED_CHILD)
-		return false;
-
-	if (bip32_key_from_parent(bip32_base, index, flags, &ext) != WALLY_OK)
-		return false;
-
-	if (!secp256k1_ec_pubkey_parse(secp256k1_ctx, &pubkey->pubkey,
-				       ext.pub_key, sizeof(ext.pub_key)))
-		return false;
-	return true;
-}
diff --git a/common/key_derive.h b/common/key_derive.h
@@ -28,9 +28,4 @@ bool derive_revocation_privkey(const struct secret *base_secret,
 			       const struct pubkey *basepoint,
 			       const struct pubkey *per_commitment_point,
 			       struct privkey *key);
-
-
-struct ext_key;
-bool bip32_pubkey(const struct ext_key *bip32_base,
-		  struct pubkey *pubkey, u32 index);
 #endif /* LIGHTNING_COMMON_KEY_DERIVE_H */
diff --git a/lightningd/hsm_control.c b/lightningd/hsm_control.c
@@ -164,6 +164,37 @@ struct ext_key *hsm_init(struct lightningd *ld)
 	return bip32_base;
 }
 
+/*~ There was a nasty LND bug report where the user issued an address which it
+ * couldn't spend, presumably due to a bitflip.  We check every address using our
+ * hsm, to be sure it's valid.  Expensive, but not as expensive as losing BTC! */
+void bip32_pubkey(struct lightningd *ld,
+		  struct pubkey *pubkey, u32 index)
+{
+	const uint32_t flags = BIP32_FLAG_KEY_PUBLIC | BIP32_FLAG_SKIP_HASH;
+	struct ext_key ext;
+
+	if (index >= BIP32_INITIAL_HARDENED_CHILD)
+		fatal("Can't derive keu %u (too large!)", index);
+
+	if (bip32_key_from_parent(ld->wallet->bip32_base, index, flags, &ext)
+	    != WALLY_OK)
+		fatal("Can't derive key %u", index);
+
+	if (!secp256k1_ec_pubkey_parse(secp256k1_ctx, &pubkey->pubkey,
+				       ext.pub_key, sizeof(ext.pub_key)))
+		fatal("Can't parse derived key %u", index);
+
+	/* If the pubkey is wrong, the hsm exits, so the reply is to just make
+	 * sure we check before proceeding!  Only supported in v3+ */
+	if (ld->hsm_version >= 3) {
+		u8 *msg = towire_hsmd_check_pubkey(NULL, index, pubkey);
+		wire_sync_write(ld->hsm_fd, take(msg));
+		msg = wire_sync_read(tmpctx, ld->hsm_fd);
+		if (!fromwire_hsmd_check_pubkey_reply(msg))
+			fatal("Invalid check_pubkey_reply from hsm");
+	}
+}
+
 static struct command_result *json_makesecret(struct command *cmd,
 					   const char *buffer,
 					   const jsmntok_t *obj UNNEEDED,

diff --git a/lightningd/hsm_control.h b/lightningd/hsm_control.h
@@ -17,4 +17,9 @@ int hsm_get_client_fd(struct lightningd *ld,
 int hsm_get_global_fd(struct lightningd *ld, int capabilities);
 
 struct ext_key *hsm_init(struct lightningd *ld);
+
+/* Get (and check!) a bip32 derived pubkey */
+void bip32_pubkey(struct lightningd *ld,
+		  struct pubkey *pubkey, u32 index);
+
 #endif /* LIGHTNING_LIGHTNINGD_HSM_CONTROL_H */
diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c
@@ -59,6 +59,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <header_versions_gen.h>
+#include <hsmd/hsmd_wiregen.h>
 #include <lightningd/chaintopology.h>
 #include <lightningd/channel.h>
 #include <lightningd/channel_control.h>

diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c
@@ -666,12 +666,8 @@ enum watch_result onchaind_funding_spent(struct channel *channel,
 		return KEEP_WATCHING;
 	}
 
-	if (!bip32_pubkey(ld->wallet->bip32_base, &final_key,
-			  channel->final_key_idx)) {
-		log_broken(channel->log, "Could not derive onchain key %"PRIu64,
-			   channel->final_key_idx);
-		return KEEP_WATCHING;
-	}
+	bip32_pubkey(ld, &final_key, channel->final_key_idx);
+
 	struct ext_key final_wallet_ext_key;
 	if (bip32_key_from_parent(
 		    ld->wallet->bip32_base,

diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c
@@ -211,9 +211,7 @@ u8 *p2wpkh_for_keyidx(const tal_t *ctx, struct lightningd *ld, u64 keyidx)
 {
 	struct pubkey shutdownkey;
 
-	if (!bip32_pubkey(ld->wallet->bip32_base, &shutdownkey, keyidx))
-		return NULL;
-
+	bip32_pubkey(ld, &shutdownkey, keyidx);
 	return scriptpubkey_p2wpkh(ctx, &shutdownkey);
 }
 

diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c
@@ -13,6 +13,10 @@ struct channel *any_channel_by_scid(struct lightningd *ld UNNEEDED,
 				    const struct short_channel_id *scid UNNEEDED,
 				    bool privacy_leak_ok UNNEEDED)
 { fprintf(stderr, "any_channel_by_scid called!\n"); abort(); }
+/* Generated stub for bip32_pubkey */
+void bip32_pubkey(struct lightningd *ld UNNEEDED,
+		  struct pubkey *pubkey UNNEEDED, u32 index UNNEEDED)
+{ fprintf(stderr, "bip32_pubkey called!\n"); abort(); }
 /* Generated stub for bitcoind_getutxout_ */
 void bitcoind_getutxout_(struct bitcoind *bitcoind UNNEEDED,
 			 const struct bitcoin_outpoint *outpoint UNNEEDED,

diff --git a/wallet/db.c b/wallet/db.c
@@ -13,6 +13,7 @@
 #include <errno.h>
 #include <hsmd/hsmd_wiregen.h>
 #include <lightningd/channel.h>
+#include <lightningd/hsm_control.h>
 #include <lightningd/plugin_hook.h>
 #include <wallet/db.h>
 #include <wire/wire_sync.h>
@@ -1109,7 +1110,7 @@ void fillin_missing_scriptpubkeys(struct lightningd *ld, struct db *db)
 			db_col_ignore(stmt, "peer_id");
 			db_col_ignore(stmt, "commitment_point");
 			/* Build from bip32_base */
-			bip32_pubkey(ld->wallet->bip32_base, &key, keyindex);
+			bip32_pubkey(ld, &key, keyindex);
 			if (type == p2sh_wpkh) {
 				u8 *redeemscript = bitcoin_redeem_p2sh_p2wpkh(stmt, &key);
 				scriptPubkey = scriptpubkey_p2sh(tmpctx, redeemscript);

diff --git a/wallet/reservation.c b/wallet/reservation.c
@@ -10,6 +10,7 @@
 #include <common/key_derive.h>
 #include <common/type_to_string.h>
 #include <lightningd/chaintopology.h>
+#include <lightningd/hsm_control.h>
 #include <lightningd/jsonrpc.h>
 #include <lightningd/lightningd.h>
 #include <wallet/txfilter.h>
@@ -246,7 +247,6 @@ static bool inputs_sufficient(struct amount_sat input,
 static struct wally_psbt *psbt_using_utxos(const tal_t *ctx,
 					   struct wallet *wallet,
 					   struct utxo **utxos,
-					   const struct ext_key *bip32_base,
 					   u32 nlocktime,
 					   u32 nsequence)
 {
@@ -261,7 +261,7 @@ static struct wally_psbt *psbt_using_utxos(const tal_t *ctx,
 		struct bitcoin_tx *tx;
 
 		if (utxos[i]->is_p2sh) {
-			bip32_pubkey(bip32_base, &key, utxos[i]->keyindex);
+			bip32_pubkey(wallet->ld, &key, utxos[i]->keyindex);
 			scriptSig = bitcoin_scriptsig_p2sh_p2wpkh(tmpctx, &key);
 			redeemscript = bitcoin_redeem_p2sh_p2wpkh(tmpctx, &key);
 			scriptPubkey = scriptpubkey_p2sh(tmpctx, redeemscript);
@@ -357,7 +357,6 @@ static struct command_result *finish_psbt(struct command *cmd,
 	}
 
 	psbt = psbt_using_utxos(cmd, cmd->ld->wallet, utxos,
-				cmd->ld->wallet->bip32_base,
 				*locktime, BITCOIN_TX_RBF_SEQUENCE);
 
 	/* Should we add a change output for the excess? */
@@ -381,10 +380,7 @@ static struct command_result *finish_psbt(struct command *cmd,
 					    "Failed to generate change address."
 					    " Keys exhausted.");
 
-		if (!bip32_pubkey(cmd->ld->wallet->bip32_base, &pubkey, keyidx))
-			return command_fail(cmd, LIGHTNINGD,
-					    "Failed to generate change address."
-					    " Keys generation failure");
+		bip32_pubkey(cmd->ld, &pubkey, keyidx);
 		b32script = scriptpubkey_p2wpkh(tmpctx, &pubkey);
 		txfilter_add_scriptpubkey(cmd->ld->owned_txfilter, b32script);
 

diff --git a/wallet/test/run-db.c b/wallet/test/run-db.c
@@ -20,6 +20,10 @@ static void db_log_(struct log *log UNUSED, enum log_level level UNUSED, const s
 #include <unistd.h>
 
 /* AUTOGENERATED MOCKS START */
+/* Generated stub for bip32_pubkey */
+void bip32_pubkey(struct lightningd *ld UNNEEDED,
+		  struct pubkey *pubkey UNNEEDED, u32 index UNNEEDED)
+{ fprintf(stderr, "bip32_pubkey called!\n"); abort(); }
 /* Generated stub for derive_channel_id */
 void derive_channel_id(struct channel_id *channel_id UNNEEDED,
 		       const struct bitcoin_outpoint *outpoint UNNEEDED)

diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c
@@ -49,6 +49,10 @@ void db_fatal(const char *fmt, ...)
 /* Generated stub for bigsize_put */
 size_t bigsize_put(u8 buf[BIGSIZE_MAX_LEN] UNNEEDED, bigsize_t v UNNEEDED)
 { fprintf(stderr, "bigsize_put called!\n"); abort(); }
+/* Generated stub for bip32_pubkey */
+void bip32_pubkey(struct lightningd *ld UNNEEDED,
+		  struct pubkey *pubkey UNNEEDED, u32 index UNNEEDED)
+{ fprintf(stderr, "bip32_pubkey called!\n"); abort(); }
 /* Generated stub for bitcoind_getutxout_ */
 void bitcoind_getutxout_(struct bitcoind *bitcoind UNNEEDED,
 			 const struct bitcoin_outpoint *outpoint UNNEEDED,

diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c
@@ -17,6 +17,7 @@
 #include <lightningd/chaintopology.h>
 #include <lightningd/channel.h>
 #include <lightningd/coin_mvts.h>
+#include <lightningd/hsm_control.h>
 #include <lightningd/jsonrpc.h>
 #include <lightningd/lightningd.h>
 #include <lightningd/peer_control.h>
@@ -125,8 +126,7 @@ static struct command_result *json_newaddr(struct command *cmd,
 		return command_fail(cmd, LIGHTNINGD, "Keys exhausted ");
 	}
 
-	if (!bip32_pubkey(cmd->ld->wallet->bip32_base, &pubkey, keyidx))
-		return command_fail(cmd, LIGHTNINGD, "Keys generation failure");
+	bip32_pubkey(cmd->ld, &pubkey, keyidx);
 
 	b32script = scriptpubkey_p2wpkh(tmpctx, &pubkey);
 	if (*addrtype & ADDR_BECH32)
@@ -187,8 +187,7 @@ static struct command_result *json_listaddrs(struct command *cmd,
 			break;
 		}
 
-		if (!bip32_pubkey(cmd->ld->wallet->bip32_base, &pubkey, keyidx))
-			abort();
+		bip32_pubkey(cmd->ld, &pubkey, keyidx);
 
 		// p2sh
 		u8 *redeemscript_p2sh;
@@ -250,7 +249,7 @@ static void json_add_utxo(struct json_stream *response,
 
 	if (utxo->is_p2sh) {
 		struct pubkey key;
-		bip32_pubkey(wallet->bip32_base, &key, utxo->keyindex);
+		bip32_pubkey(wallet->ld, &key, utxo->keyindex);
 
 		json_add_hex_talarr(response, "redeemscript",
 				    bitcoin_redeem_p2sh_p2wpkh(tmpctx, &key));
@@ -663,8 +662,7 @@ static struct command_result *match_psbt_inputs_to_utxos(struct command *cmd,
 				u8 *redeemscript;
 				int wally_err;
 
-				bip32_pubkey(cmd->ld->wallet->bip32_base, &key,
-					     utxo->keyindex);
+				bip32_pubkey(cmd->ld, &key, utxo->keyindex);
 				redeemscript = bitcoin_redeem_p2sh_p2wpkh(tmpctx, &key);
 				scriptPubKey = scriptpubkey_p2sh(tmpctx, redeemscript);