From 4098f47cfc0f687db132ae5a769db3bac1a45da7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 23 Jul 2018 11:53:02 +0930 Subject: [PATCH] onchaind: use HSM to sign "to-us" transactions. Signed-off-by: Rusty Russell --- lightningd/onchain_control.c | 9 ++ onchaind/Makefile | 4 +- onchaind/onchain.c | 153 ++++++++++++++++-------------- onchaind/test/run-grind_feerate.c | 19 ++-- 4 files changed, 104 insertions(+), 81 deletions(-) diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index 887c25de4..2fa221969 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -1,14 +1,17 @@ #include #include #include +#include #include #include +#include #include #include #include #include #include #include +#include /* We dump all the known preimages when onchaind starts up. */ static void onchaind_tell_fulfill(struct channel *channel) @@ -388,12 +391,17 @@ enum watch_result onchaind_funding_spent(struct channel *channel, struct htlc_stub *stubs; struct lightningd *ld = channel->peer->ld; struct pubkey final_key; + int hsmfd; channel_fail_permanent(channel, "Funding transaction spent"); /* We could come from almost any state. */ channel_set_state(channel, channel->state, FUNDING_SPEND_SEEN); + hsmfd = hsm_get_client_fd(ld, &channel->peer->id, + channel->dbid, + HSM_CAP_SIGN_ONCHAIN_TX); + channel_set_owner(channel, new_channel_subd(ld, "lightning_onchaind", channel, @@ -402,6 +410,7 @@ enum watch_result onchaind_funding_spent(struct channel *channel, onchain_msg, onchain_error, channel_set_billboard, + take(&hsmfd), NULL)); if (!channel->owner) { diff --git a/onchaind/Makefile b/onchaind/Makefile index 90c79b97e..7a663e479 100644 --- a/onchaind/Makefile +++ b/onchaind/Makefile @@ -47,6 +47,7 @@ $(LIGHTNINGD_ONCHAIN_OBJS): $(LIGHTNINGD_HEADERS) # Common source we use. ONCHAIND_COMMON_OBJS := \ + common/bip32.o \ common/daemon.o \ common/daemon_conn.o \ common/derive_basepoints.o \ @@ -66,6 +67,7 @@ ONCHAIND_COMMON_OBJS := \ common/subdaemon.o \ common/type_to_string.o \ common/utils.o \ + common/utxo.o \ common/version.o onchaind/gen_onchain_wire.h: $(WIRE_GEN) onchaind/onchain_wire.csv @@ -81,7 +83,7 @@ ALL_OBJS += $(LIGHTNINGD_ONCHAIN_OBJS) ALL_PROGRAMS += lightningd/lightning_onchaind ALL_GEN_HEADERS += $(LIGHTNINGD_ONCHAIN_HEADERS_GEN) -lightningd/lightning_onchaind: $(LIGHTNINGD_ONCHAIN_OBJS) $(WIRE_ONION_OBJS) $(ONCHAIND_COMMON_OBJS) $(WIRE_OBJS) $(BITCOIN_OBJS) +lightningd/lightning_onchaind: $(LIGHTNINGD_ONCHAIN_OBJS) $(WIRE_ONION_OBJS) $(ONCHAIND_COMMON_OBJS) $(WIRE_OBJS) $(BITCOIN_OBJS) $(LIGHTNINGD_HSM_CLIENT_OBJS) check-source: $(LIGHTNINGD_ONCHAIN_SRC_NOGEN:%=check-src-include-order/%) check-source-bolt: $(LIGHTNINGD_ONCHAIN_SRC:%=bolt-check/%) $(LIGHTNINGD_ONCHAIN_HEADERS:%=bolt-check/%) diff --git a/onchaind/onchain.c b/onchaind/onchain.c index ea3b12524..44a6d82ef 100644 --- a/onchaind/onchain.c +++ b/onchaind/onchain.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -26,10 +27,14 @@ /* stdin == requests */ #define REQ_FD STDIN_FILENO +#define HSM_FD 3 /* Required in various places: keys for commitment transaction. */ static const struct keyset *keyset; +/* IFF it's their commitment tx: HSM can't derive their per-commitment point! */ +static const struct pubkey *remote_per_commitment_point; + /* The commitment number we're dealing with (if not mutual close) */ static u64 commit_num; @@ -48,11 +53,11 @@ static u32 to_self_delay[NUM_SIDES]; /* Where we send money to (our wallet) */ static struct pubkey our_wallet_pubkey; -/* Private keys for spending HTLC outputs via HTLC txs, delayed, and directly. */ -static struct privkey htlc_privkey, delayed_payment_privkey, payment_privkey; +/* Private key for spending HTLC outputs via HTLC txs. */ +static struct privkey htlc_privkey; -/* Private keys for spending HTLC for penalty (only if they cheated). */ -static struct privkey *revocation_privkey; +/* Their revocation secret (only if they cheated). */ +static const struct secret *remote_per_commitment_secret; /* one value is useful for a few witness scripts */ static const u8 ONE = 0x1; @@ -237,6 +242,33 @@ static const char *output_type_name(enum output_type output_type) return "unknown"; } +static u8 *delayed_payment_to_us(const tal_t *ctx, + struct bitcoin_tx *tx, + const u8 *wscript) +{ + return towire_hsm_sign_delayed_payment_to_us(ctx, commit_num, + tx, wscript, + *tx->input[0].amount); +} + +static u8 *remote_htlc_to_us(const tal_t *ctx, + struct bitcoin_tx *tx, + const u8 *wscript) +{ + return towire_hsm_sign_remote_htlc_to_us(ctx, + remote_per_commitment_point, + tx, wscript, + *tx->input[0].amount); +} + +static u8 *penalty_to_us(const tal_t *ctx, + struct bitcoin_tx *tx, + const u8 *wscript) +{ + return towire_hsm_sign_penalty_to_us(ctx, remote_per_commitment_secret, + tx, wscript, *tx->input[0].amount); +} + /* * This covers: * 1. to-us output spend (` 0`) @@ -248,18 +280,20 @@ static const char *output_type_name(enum output_type output_type) * Overrides *tx_type if it all turns to dust. */ static struct bitcoin_tx *tx_to_us(const tal_t *ctx, + u8 *(*hsm_sign_msg)(const tal_t *ctx, + struct bitcoin_tx *tx, + const u8 *wscript), struct tracked_output *out, u32 to_self_delay, u32 locktime, const void *elem, size_t elemsize, const u8 *wscript, - const struct privkey *privkey, - const struct pubkey *pubkey, enum tx_type *tx_type) { struct bitcoin_tx *tx; u64 fee; secp256k1_ecdsa_signature sig; + u8 *msg; tx = bitcoin_tx(ctx, 1, 1); tx->lock_time = locktime; @@ -303,7 +337,15 @@ static struct bitcoin_tx *tx_to_us(const tal_t *ctx, } tx->output[0].amount -= fee; - sign_tx_input(tx, 0, NULL, wscript, privkey, pubkey, &sig); + if (!wire_sync_write(HSM_FD, take(hsm_sign_msg(NULL, tx, wscript)))) + status_failed(STATUS_FAIL_HSM_IO, "Writing sign request to hsm"); + msg = wire_sync_read(tmpctx, HSM_FD); + if (!msg || !fromwire_hsm_sign_tx_reply(msg, &sig)) { + status_failed(STATUS_FAIL_HSM_IO, + "Reading sign_tx_reply: %s", + tal_hex(tmpctx, msg)); + } + tx->input[0].witness = bitcoin_witness_sig_and_element(tx->input, &sig, elem, elemsize, @@ -815,10 +857,9 @@ static void resolve_htlc_tx(struct tracked_output ***outs, * nSequence `to_self_delay` and a witness stack ` * 0` */ - tx = tx_to_us(*outs, out, to_self_delay[LOCAL], 0, NULL, 0, + tx = tx_to_us(*outs, delayed_payment_to_us, + out, to_self_delay[LOCAL], 0, NULL, 0, wscript, - &delayed_payment_privkey, - &keyset->self_delayed_payment_key, &tx_type); propose_resolution(out, tx, to_self_delay[LOCAL], tx_type); @@ -841,11 +882,9 @@ static void steal_htlc_tx(struct tracked_output *out) * To spend this via penalty, the remote node uses a witness stack * ` 1` */ - tx = tx_to_us(out, out, 0xFFFFFFFF, 0, + tx = tx_to_us(out, penalty_to_us, out, 0xFFFFFFFF, 0, &ONE, sizeof(ONE), out->wscript, - revocation_privkey, - &keyset->self_revocation_key, &tx_type); propose_resolution(out, tx, 0, tx_type); } @@ -1135,11 +1174,10 @@ static void handle_preimage(struct tracked_output **outs, * - MUST *resolve* the output by spending it to a * convenient address. */ - tx = tx_to_us(outs[i], outs[i], 0, 0, + tx = tx_to_us(outs[i], remote_htlc_to_us, + outs[i], 0, 0, preimage, sizeof(*preimage), outs[i]->wscript, - &htlc_privkey, - &keyset->other_htlc_key, &tx_type); propose_resolution(outs[i], tx, 0, tx_type); } @@ -1288,10 +1326,9 @@ static void resolve_our_htlc_theircommit(struct tracked_output *out) * - MUST *resolve* the output, by spending it to a convenient * address. */ - tx = tx_to_us(out, out, 0, out->htlc->cltv_expiry, NULL, 0, + tx = tx_to_us(out, remote_htlc_to_us, + out, 0, out->htlc->cltv_expiry, NULL, 0, out->wscript, - &htlc_privkey, - &keyset->other_htlc_key, &tx_type); propose_resolution_at_block(out, tx, out->htlc->cltv_expiry, tx_type); @@ -1428,22 +1465,6 @@ static void handle_our_unilateral(const struct bitcoin_tx *tx, type_to_string(tmpctx, struct pubkey, &keyset->other_htlc_key)); - if (!derive_simple_privkey(&secrets->delayed_payment_basepoint_secret, - &basepoints[LOCAL].delayed_payment, - &local_per_commitment_point, - &delayed_payment_privkey)) - status_failed(STATUS_FAIL_INTERNAL_ERROR, - "Deriving delayed_payment_privkey for %"PRIu64, - commit_num); - - if (!derive_simple_privkey(&secrets->payment_basepoint_secret, - &basepoints[LOCAL].payment, - &local_per_commitment_point, - &payment_privkey)) - status_failed(STATUS_FAIL_INTERNAL_ERROR, - "Deriving payment_privkey for %"PRIu64, - commit_num); - if (!derive_simple_privkey(&secrets->htlc_basepoint_secret, &basepoints[LOCAL].htlc, &local_per_commitment_point, @@ -1510,11 +1531,10 @@ static void handle_our_unilateral(const struct bitcoin_tx *tx, * * 0 */ - to_us = tx_to_us(out, out, to_self_delay[LOCAL], 0, + to_us = tx_to_us(out, delayed_payment_to_us, + out, to_self_delay[LOCAL], 0, NULL, 0, local_wscript, - &delayed_payment_privkey, - &keyset->self_delayed_payment_key, &tx_type); /* BOLT #5: @@ -1619,11 +1639,11 @@ static void steal_to_them_output(struct tracked_output *out) &keyset->self_revocation_key, &keyset->self_delayed_payment_key); - tx = tx_to_us(tmpctx, out, 0xFFFFFFFF, 0, + tx = tx_to_us(tmpctx, + penalty_to_us, + out, 0xFFFFFFFF, 0, &ONE, sizeof(ONE), wscript, - revocation_privkey, - &keyset->self_revocation_key, &tx_type); propose_resolution(out, tx, 0, tx_type); @@ -1643,11 +1663,11 @@ static void steal_htlc(struct tracked_output *out) * */ pubkey_to_der(der, &keyset->self_revocation_key); - tx = tx_to_us(out, out, 0xFFFFFFFF, 0, + tx = tx_to_us(out, + penalty_to_us, + out, 0xFFFFFFFF, 0, der, sizeof(der), out->wscript, - revocation_privkey, - &keyset->self_revocation_key, &tx_type); propose_resolution(out, tx, 0, tx_type); @@ -1674,9 +1694,8 @@ static void handle_their_cheat(const struct bitcoin_tx *tx, u8 **htlc_scripts; u8 *remote_wscript, *script[NUM_SIDES]; struct keyset *ks; + struct pubkey *k; size_t i; - struct secret per_commitment_secret; - struct pubkey per_commitment_point; init_reply("Tracking their illegal close: taking all funds"); @@ -1688,15 +1707,18 @@ static void handle_their_cheat(const struct bitcoin_tx *tx, resolved_by_other(outs[0], txid, THEIR_REVOKED_UNILATERAL); /* FIXME: Types. */ - BUILD_ASSERT(sizeof(per_commitment_secret) - == sizeof(*revocation_preimage)); - memcpy(&per_commitment_secret, revocation_preimage, - sizeof(per_commitment_secret)); - if (!pubkey_from_secret(&per_commitment_secret, &per_commitment_point)) + BUILD_ASSERT(sizeof(struct secret) == sizeof(*revocation_preimage)); + remote_per_commitment_secret = tal_dup(tx, struct secret, + (struct secret *) + revocation_preimage); + + /* Need tmpvar for non-const. */ + remote_per_commitment_point = k = tal(tx, struct pubkey); + if (!pubkey_from_secret(remote_per_commitment_secret, k)) status_failed(STATUS_FAIL_INTERNAL_ERROR, "Failed derive from per_commitment_secret %s", type_to_string(tmpctx, struct secret, - &per_commitment_secret)); + remote_per_commitment_secret)); status_trace("Deriving keyset %"PRIu64 ": per_commit_point=%s" @@ -1708,7 +1730,7 @@ static void handle_their_cheat(const struct bitcoin_tx *tx, " other_revocation_basepoint=%s", commit_num, type_to_string(tmpctx, struct pubkey, - &per_commitment_point), + remote_per_commitment_point), type_to_string(tmpctx, struct pubkey, &basepoints[REMOTE].payment), type_to_string(tmpctx, struct pubkey, @@ -1724,7 +1746,7 @@ static void handle_their_cheat(const struct bitcoin_tx *tx, /* keyset is const, we need a non-const ptr to set it up */ keyset = ks = tal(tx, struct keyset); - if (!derive_keyset(&per_commitment_point, + if (!derive_keyset(remote_per_commitment_point, &basepoints[REMOTE], &basepoints[LOCAL], ks)) @@ -1753,16 +1775,6 @@ static void handle_their_cheat(const struct bitcoin_tx *tx, type_to_string(tmpctx, struct pubkey, &keyset->other_htlc_key)); - revocation_privkey = tal(tx, struct privkey); - if (!derive_revocation_privkey(&secrets->revocation_basepoint_secret, - &per_commitment_secret, - &basepoints[LOCAL].revocation, - &per_commitment_point, - revocation_privkey)) - status_failed(STATUS_FAIL_INTERNAL_ERROR, - "Deriving revocation_privkey for %"PRIu64, - commit_num); - remote_wscript = to_self_wscript(tmpctx, to_self_delay[REMOTE], keyset); /* Figure out what to-them output looks like. */ @@ -1882,7 +1894,7 @@ static void handle_their_unilateral(const struct bitcoin_tx *tx, u32 tx_blockheight, const struct bitcoin_txid *txid, const struct secrets *secrets, - const struct pubkey *remote_per_commitment_point, + const struct pubkey *this_remote_per_commitment_point, const struct basepoints basepoints[NUM_SIDES], const struct htlc_stub *htlcs, const bool *tell_if_missing, @@ -1896,6 +1908,9 @@ static void handle_their_unilateral(const struct bitcoin_tx *tx, init_reply("Tracking their unilateral close"); + /* HSM can't derive this. */ + remote_per_commitment_point = this_remote_per_commitment_point; + /* BOLT #5: * * # Unilateral Close Handling: Remote Commitment Transaction @@ -1965,14 +1980,6 @@ static void handle_their_unilateral(const struct bitcoin_tx *tx, type_to_string(tmpctx, struct pubkey, &keyset->other_htlc_key)); - if (!derive_simple_privkey(&secrets->payment_basepoint_secret, - &basepoints[LOCAL].payment, - remote_per_commitment_point, - &payment_privkey)) - status_failed(STATUS_FAIL_INTERNAL_ERROR, - "Deriving local_delayeprivkey for %"PRIu64, - commit_num); - if (!derive_simple_privkey(&secrets->htlc_basepoint_secret, &basepoints[LOCAL].htlc, remote_per_commitment_point, diff --git a/onchaind/test/run-grind_feerate.c b/onchaind/test/run-grind_feerate.c index 726f6a110..74716718f 100644 --- a/onchaind/test/run-grind_feerate.c +++ b/onchaind/test/run-grind_feerate.c @@ -31,19 +31,15 @@ bool derive_keyset(const struct pubkey *per_commitment_point UNNEEDED, const struct basepoints *other UNNEEDED, struct keyset *keyset UNNEEDED) { fprintf(stderr, "derive_keyset called!\n"); abort(); } -/* Generated stub for derive_revocation_privkey */ -bool derive_revocation_privkey(const struct secret *base_secret UNNEEDED, - const struct secret *per_commitment_secret UNNEEDED, - const struct pubkey *basepoint UNNEEDED, - const struct pubkey *per_commitment_point UNNEEDED, - struct privkey *key UNNEEDED) -{ fprintf(stderr, "derive_revocation_privkey called!\n"); abort(); } /* Generated stub for derive_simple_privkey */ bool derive_simple_privkey(const struct secret *base_secret UNNEEDED, const struct pubkey *basepoint UNNEEDED, const struct pubkey *per_commitment_point UNNEEDED, struct privkey *key UNNEEDED) { fprintf(stderr, "derive_simple_privkey called!\n"); abort(); } +/* Generated stub for fromwire_hsm_sign_tx_reply */ +bool fromwire_hsm_sign_tx_reply(const void *p UNNEEDED, secp256k1_ecdsa_signature *sig UNNEEDED) +{ fprintf(stderr, "fromwire_hsm_sign_tx_reply called!\n"); abort(); } /* Generated stub for fromwire_onchain_depth */ bool fromwire_onchain_depth(const void *p UNNEEDED, struct bitcoin_txid *txid UNNEEDED, u32 *depth UNNEEDED) { fprintf(stderr, "fromwire_onchain_depth called!\n"); abort(); } @@ -119,6 +115,15 @@ u8 *to_self_wscript(const tal_t *ctx UNNEEDED, u16 to_self_delay UNNEEDED, const struct keyset *keyset UNNEEDED) { fprintf(stderr, "to_self_wscript called!\n"); abort(); } +/* Generated stub for towire_hsm_sign_delayed_payment_to_us */ +u8 *towire_hsm_sign_delayed_payment_to_us(const tal_t *ctx UNNEEDED, u64 commit_num UNNEEDED, const struct bitcoin_tx *tx UNNEEDED, const u8 *wscript UNNEEDED, u64 input_amount UNNEEDED) +{ fprintf(stderr, "towire_hsm_sign_delayed_payment_to_us called!\n"); abort(); } +/* Generated stub for towire_hsm_sign_penalty_to_us */ +u8 *towire_hsm_sign_penalty_to_us(const tal_t *ctx UNNEEDED, const struct secret *revocation_secret UNNEEDED, const struct bitcoin_tx *tx UNNEEDED, const u8 *wscript UNNEEDED, u64 input_amount UNNEEDED) +{ fprintf(stderr, "towire_hsm_sign_penalty_to_us called!\n"); abort(); } +/* Generated stub for towire_hsm_sign_remote_htlc_to_us */ +u8 *towire_hsm_sign_remote_htlc_to_us(const tal_t *ctx UNNEEDED, const struct pubkey *remote_per_commitment_point UNNEEDED, const struct bitcoin_tx *tx UNNEEDED, const u8 *wscript UNNEEDED, u64 input_amount UNNEEDED) +{ fprintf(stderr, "towire_hsm_sign_remote_htlc_to_us called!\n"); abort(); } /* Generated stub for towire_onchain_add_utxo */ u8 *towire_onchain_add_utxo(const tal_t *ctx UNNEEDED, const struct bitcoin_txid *prev_out_tx UNNEEDED, u32 prev_out_index UNNEEDED, const struct pubkey *per_commit_point UNNEEDED, u64 value UNNEEDED, u32 blockheight UNNEEDED) { fprintf(stderr, "towire_onchain_add_utxo called!\n"); abort(); }