From 8994b8dade01c11b73ab452651b562ae477e82ca Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Wed, 16 Mar 2022 12:45:29 -0700 Subject: [PATCH] hsmd: Add validate_commitment_tx --- channeld/channeld.c | 81 +++++++++++++++++++++++++++++++++----------- hsmd/Makefile | 2 +- hsmd/hsmd.c | 2 ++ hsmd/hsmd_wire.csv | 15 ++++++++ hsmd/libhsmd.c | 55 ++++++++++++++++++++++++++++++ openingd/common.c | 38 +++++++++++++++++++++ openingd/common.h | 6 ++++ openingd/dualopend.c | 4 +++ openingd/openingd.c | 4 +++ 9 files changed, 187 insertions(+), 20 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 98cff384f..edf4c7215 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -994,22 +994,12 @@ static u8 *master_wait_sync_reply(const tal_t *ctx, return reply; } -/* Returns HTLC sigs, sets commit_sig */ -static struct bitcoin_signature *calc_commitsigs(const tal_t *ctx, - const struct peer *peer, - struct bitcoin_tx **txs, - const u8 *funding_wscript, - const struct htlc **htlc_map, - u64 commit_index, - struct bitcoin_signature *commit_sig) +/* Collect the htlcs for call to hsmd. */ +static struct simple_htlc **collect_htlcs(const tal_t *ctx, const struct htlc **htlc_map) { - size_t i; - struct pubkey local_htlckey; - const u8 *msg; - struct bitcoin_signature *htlc_sigs; + struct simple_htlc **htlcs; - /* Collect the htlcs for call to hsmd. */ - struct simple_htlc **htlcs = tal_arr(tmpctx, struct simple_htlc *, 0); + htlcs = tal_arr(ctx, struct simple_htlc *, 0); size_t num_entries = tal_count(htlc_map); for (size_t ndx = 0; ndx < num_entries; ++ndx) { struct htlc const *hh = htlc_map[ndx]; @@ -1023,7 +1013,25 @@ static struct bitcoin_signature *calc_commitsigs(const tal_t *ctx, tal_arr_expand(&htlcs, simple); } } + return htlcs; +} +/* Returns HTLC sigs, sets commit_sig */ +static struct bitcoin_signature *calc_commitsigs(const tal_t *ctx, + const struct peer *peer, + struct bitcoin_tx **txs, + const u8 *funding_wscript, + const struct htlc **htlc_map, + u64 commit_index, + struct bitcoin_signature *commit_sig) +{ + struct simple_htlc **htlcs; + size_t i; + struct pubkey local_htlckey; + const u8 *msg; + struct bitcoin_signature *htlc_sigs; + + htlcs = collect_htlcs(tmpctx, htlc_map); msg = towire_hsmd_sign_remote_commitment_tx(NULL, txs[0], &peer->channel->funding_pubkey[REMOTE], &peer->remote_per_commit, @@ -1442,6 +1450,17 @@ static u8 *make_revocation_msg(const struct peer *peer, u64 revoke_index, point); } +static u8 *make_revocation_msg_from_secret(const struct peer *peer, + u64 revoke_index, + struct pubkey *point, + const struct secret *old_commit_secret, + const struct pubkey *next_point) +{ + *point = *next_point; + return towire_revoke_and_ack(peer, &peer->channel_id, + old_commit_secret, next_point); +} + /* Convert changed htlcs into parts which lightningd expects. */ static void marshall_htlc_info(const tal_t *ctx, const struct htlc **changed_htlcs, @@ -1501,12 +1520,15 @@ static void send_revocation(struct peer *peer, const struct bitcoin_signature *commit_sig, const struct bitcoin_signature *htlc_sigs, const struct htlc **changed_htlcs, - const struct bitcoin_tx *committx) + const struct bitcoin_tx *committx, + const struct secret *old_secret, + const struct pubkey *next_point) { struct changed_htlc *changed; struct fulfilled_htlc *fulfilled; const struct failed_htlc **failed; struct added_htlc *added; + const u8 *msg; const u8 *msg_for_master; /* Marshall it now before channel_sending_revoke_and_ack changes htlcs */ @@ -1519,8 +1541,9 @@ static void send_revocation(struct peer *peer, &added); /* Revoke previous commit, get new point. */ - u8 *msg = make_revocation_msg(peer, peer->next_index[LOCAL]-1, - &peer->next_local_per_commit); + msg = make_revocation_msg_from_secret(peer, peer->next_index[LOCAL]-1, + &peer->next_local_per_commit, + old_secret, next_point); /* From now on we apply changes to the next commitment */ peer->next_index[LOCAL]++; @@ -1564,6 +1587,8 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) const struct htlc **htlc_map, **changed_htlcs; const u8 *funding_wscript; size_t i; + struct simple_htlc **htlcs; + const u8 * msg2; changed_htlcs = tal_arr(msg, const struct htlc *, 0); if (!channel_rcvd_commit(peer->channel, &changed_htlcs)) { @@ -1685,8 +1710,26 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) status_debug("Received commit_sig with %zu htlc sigs", tal_count(htlc_sigs)); - send_revocation(peer, - &commit_sig, htlc_sigs, changed_htlcs, txs[0]); + /* Validate the counterparty's signatures, returns prior per_commitment_secret. */ + htlcs = collect_htlcs(NULL, htlc_map); + msg2 = towire_hsmd_validate_commitment_tx(NULL, + txs[0], + (const struct simple_htlc **) htlcs, + peer->next_index[LOCAL], + channel_feerate(peer->channel, LOCAL), + &commit_sig, + htlc_sigs); + tal_free(htlcs); + msg2 = hsm_req(tmpctx, take(msg2)); + struct secret *old_secret; + struct pubkey next_point; + if (!fromwire_hsmd_validate_commitment_tx_reply(tmpctx, msg2, &old_secret, &next_point)) + status_failed(STATUS_FAIL_HSM_IO, + "Reading validate_commitment_tx reply: %s", + tal_hex(tmpctx, msg2)); + + send_revocation(peer, &commit_sig, htlc_sigs, changed_htlcs, txs[0], + old_secret, &next_point); /* We may now be quiescent on our side. */ maybe_send_stfu(peer); diff --git a/hsmd/Makefile b/hsmd/Makefile index bc396cd07..6fd4dac59 100644 --- a/hsmd/Makefile +++ b/hsmd/Makefile @@ -31,7 +31,6 @@ HSMD_COMMON_OBJS := \ common/daemon.o \ common/daemon_conn.o \ common/derive_basepoints.o \ - common/status_wiregen.o \ common/hash_u5.o \ common/hsm_encryption.o \ common/htlc_wire.o \ @@ -47,6 +46,7 @@ HSMD_COMMON_OBJS := \ common/setup.o \ common/status.o \ common/status_wire.o \ + common/status_wiregen.o \ common/subdaemon.o \ common/type_to_string.o \ common/utils.o \ diff --git a/hsmd/hsmd.c b/hsmd/hsmd.c index ff1b889e4..d5dd898f8 100644 --- a/hsmd/hsmd.c +++ b/hsmd/hsmd.c @@ -644,6 +644,7 @@ static struct io_plan *handle_client(struct io_conn *conn, struct client *c) case WIRE_HSMD_NEW_CHANNEL: case WIRE_HSMD_READY_CHANNEL: case WIRE_HSMD_SIGN_COMMITMENT_TX: + case WIRE_HSMD_VALIDATE_COMMITMENT_TX: case WIRE_HSMD_VALIDATE_REVOCATION: case WIRE_HSMD_SIGN_PENALTY_TO_US: case WIRE_HSMD_SIGN_REMOTE_COMMITMENT_TX: @@ -682,6 +683,7 @@ static struct io_plan *handle_client(struct io_conn *conn, struct client *c) case WIRE_HSMD_INIT_REPLY: case WIRE_HSMSTATUS_CLIENT_BAD_REQUEST: case WIRE_HSMD_SIGN_COMMITMENT_TX_REPLY: + case WIRE_HSMD_VALIDATE_COMMITMENT_TX_REPLY: case WIRE_HSMD_VALIDATE_REVOCATION_REPLY: case WIRE_HSMD_SIGN_TX_REPLY: case WIRE_HSMD_SIGN_OPTION_WILL_FUND_OFFER_REPLY: diff --git a/hsmd/hsmd_wire.csv b/hsmd/hsmd_wire.csv index a92402eb5..a62edce8c 100644 --- a/hsmd/hsmd_wire.csv +++ b/hsmd/hsmd_wire.csv @@ -138,6 +138,21 @@ msgdata,hsmd_sign_commitment_tx,commit_num,u64, msgtype,hsmd_sign_commitment_tx_reply,105 msgdata,hsmd_sign_commitment_tx_reply,sig,bitcoin_signature, +# Validate the counterparty's commitment signatures. +msgtype,hsmd_validate_commitment_tx,35 +msgdata,hsmd_validate_commitment_tx,tx,bitcoin_tx, +msgdata,hsmd_validate_commitment_tx,num_htlcs,u16, +msgdata,hsmd_validate_commitment_tx,htlcs,simple_htlc,num_htlcs +msgdata,hsmd_validate_commitment_tx,commit_num,u64, +msgdata,hsmd_validate_commitment_tx,feerate,u32, +msgdata,hsmd_validate_commitment_tx,sig,bitcoin_signature, +msgdata,hsmd_validate_commitment_tx,num_htlc_sigs,u16, +msgdata,hsmd_validate_commitment_tx,htlc_sigs,bitcoin_signature,num_htlc_sigs + +msgtype,hsmd_validate_commitment_tx_reply,135 +msgdata,hsmd_validate_commitment_tx_reply,old_commitment_secret,?secret, +msgdata,hsmd_validate_commitment_tx_reply,next_per_commitment_point,pubkey, + # Vaidate the counterparty's revocation secret msgtype,hsmd_validate_revocation,36 msgdata,hsmd_validate_revocation,revoke_num,u64, diff --git a/hsmd/libhsmd.c b/hsmd/libhsmd.c index fa488724b..b8e230718 100644 --- a/hsmd/libhsmd.c +++ b/hsmd/libhsmd.c @@ -96,6 +96,7 @@ bool hsmd_check_client_capabilities(struct hsmd_client *client, case WIRE_HSMD_SIGN_REMOTE_COMMITMENT_TX: case WIRE_HSMD_SIGN_REMOTE_HTLC_TX: + case WIRE_HSMD_VALIDATE_COMMITMENT_TX: case WIRE_HSMD_VALIDATE_REVOCATION: return (client->capabilities & HSM_CAP_SIGN_REMOTE_TX) != 0; @@ -133,6 +134,7 @@ bool hsmd_check_client_capabilities(struct hsmd_client *client, case WIRE_HSMD_INIT_REPLY: case WIRE_HSMSTATUS_CLIENT_BAD_REQUEST: case WIRE_HSMD_SIGN_COMMITMENT_TX_REPLY: + case WIRE_HSMD_VALIDATE_COMMITMENT_TX_REPLY: case WIRE_HSMD_VALIDATE_REVOCATION_REPLY: case WIRE_HSMD_SIGN_TX_REPLY: case WIRE_HSMD_SIGN_OPTION_WILL_FUND_OFFER_REPLY: @@ -1339,6 +1341,56 @@ static u8 *handle_sign_commitment_tx(struct hsmd_client *c, const u8 *msg_in) return towire_hsmd_sign_commitment_tx_reply(NULL, &sig); } +/* ~This stub implementation is overriden by fully validating signers + * that need to independently verify the peer's signatures. */ +static u8 *handle_validate_commitment_tx(struct hsmd_client *c, const u8 *msg_in) +{ + struct bitcoin_tx *tx; + struct simple_htlc **htlc; + u64 commit_num; + u32 feerate; + struct bitcoin_signature sig; + struct bitcoin_signature *htlc_sigs; + struct secret channel_seed; + struct sha256 shaseed; + struct secret *old_secret; + struct pubkey next_per_commitment_point; + + if (!fromwire_hsmd_validate_commitment_tx(tmpctx, msg_in, + &tx, &htlc, + &commit_num, &feerate, + &sig, &htlc_sigs)) + return hsmd_status_malformed_request(c, msg_in); + + /* Stub implementation */ + + /* The signatures are not checked in this stub because they + * are already checked by the caller. However, the returned + * old_secret and next_per_commitment_point are used. + */ + + get_channel_seed(&c->id, c->dbid, &channel_seed); + if (!derive_shaseed(&channel_seed, &shaseed)) + return hsmd_status_bad_request(c, msg_in, "bad derive_shaseed"); + + if (!per_commit_point(&shaseed, &next_per_commitment_point, commit_num + 1)) + return hsmd_status_bad_request_fmt( + c, msg_in, "bad per_commit_point %" PRIu64, commit_num + 1); + + if (commit_num >= 1) { + old_secret = tal(tmpctx, struct secret); + if (!per_commit_secret(&shaseed, old_secret, commit_num - 1)) { + return hsmd_status_bad_request_fmt( + c, msg_in, "Cannot derive secret %" PRIu64, commit_num - 1); + } + } else { + old_secret = NULL; + } + + return towire_hsmd_validate_commitment_tx_reply( + NULL, old_secret, &next_per_commitment_point); +} + /* This stub implementation is overriden by fully validating signers * that need to independently verify that the latest state is * commited. */ @@ -1533,6 +1585,8 @@ u8 *hsmd_handle_client_message(const tal_t *ctx, struct hsmd_client *client, return handle_sign_penalty_to_us(client, msg); case WIRE_HSMD_SIGN_COMMITMENT_TX: return handle_sign_commitment_tx(client, msg); + case WIRE_HSMD_VALIDATE_COMMITMENT_TX: + return handle_validate_commitment_tx(client, msg); case WIRE_HSMD_VALIDATE_REVOCATION: return handle_validate_revocation(client, msg); case WIRE_HSMD_SIGN_REMOTE_HTLC_TO_US: @@ -1553,6 +1607,7 @@ u8 *hsmd_handle_client_message(const tal_t *ctx, struct hsmd_client *client, case WIRE_HSMD_INIT_REPLY: case WIRE_HSMSTATUS_CLIENT_BAD_REQUEST: case WIRE_HSMD_SIGN_COMMITMENT_TX_REPLY: + case WIRE_HSMD_VALIDATE_COMMITMENT_TX_REPLY: case WIRE_HSMD_VALIDATE_REVOCATION_REPLY: case WIRE_HSMD_SIGN_TX_REPLY: case WIRE_HSMD_SIGN_OPTION_WILL_FUND_OFFER_REPLY: diff --git a/openingd/common.c b/openingd/common.c index a5d6ff412..c31bb7774 100644 --- a/openingd/common.c +++ b/openingd/common.c @@ -3,8 +3,11 @@ #include #include #include +#include #include +#include #include +#include /*~ This is the key function that checks that their configuration is reasonable: * it applied for both the case where they're trying to open a channel, and when @@ -225,3 +228,38 @@ u8 *no_upfront_shutdown_script(const tal_t *ctx, return NULL; } + +void validate_initial_commitment_signature(int hsm_fd, + struct bitcoin_tx *tx, + struct bitcoin_signature *sig) +{ + struct existing_htlc **htlcs; + struct bitcoin_signature *htlc_sigs; + u32 feerate; + u64 commit_num; + const u8 *msg; + struct secret *old_secret; + struct pubkey next_point; + + /* Validate the counterparty's signature. */ + htlcs = tal_arr(NULL, struct existing_htlc *, 0); + htlc_sigs = tal_arr(NULL, struct bitcoin_signature, 0); + feerate = 0; /* unused since there are no htlcs */ + commit_num = 0; + msg = towire_hsmd_validate_commitment_tx(NULL, + tx, + (const struct simple_htlc **) htlcs, + commit_num, + feerate, + sig, + htlc_sigs); + tal_free(htlc_sigs); + tal_free(htlcs); + wire_sync_write(hsm_fd, take(msg)); + msg = wire_sync_read(tmpctx, hsm_fd); + if (!fromwire_hsmd_validate_commitment_tx_reply(tmpctx, msg, &old_secret, &next_point)) + status_failed(STATUS_FAIL_HSM_IO, + "Reading validate_commitment_tx reply: %s", + tal_hex(tmpctx, msg)); +} + diff --git a/openingd/common.h b/openingd/common.h index 8edbd9905..838321867 100644 --- a/openingd/common.h +++ b/openingd/common.h @@ -4,6 +4,8 @@ #include "config.h" struct amount_sat; +struct bitcoin_tx; +struct bitcoin_signature; struct channel_config; @@ -21,4 +23,8 @@ bool check_config_bounds(const tal_t *ctx, u8 *no_upfront_shutdown_script(const tal_t *ctx, struct feature_set *our_features, const u8 *their_features); + +void validate_initial_commitment_signature(int hsm_fd, + struct bitcoin_tx *tx, + struct bitcoin_signature *sig); #endif /* LIGHTNING_OPENINGD_COMMON_H */ diff --git a/openingd/dualopend.c b/openingd/dualopend.c index 261b3d9c1..afc0d68fa 100644 --- a/openingd/dualopend.c +++ b/openingd/dualopend.c @@ -1843,6 +1843,8 @@ static u8 *accepter_commits(struct state *state, return NULL; } + validate_initial_commitment_signature(HSM_FD, local_commit, &remote_sig); + /* BOLT #2: * * The recipient: @@ -2567,6 +2569,8 @@ static u8 *opener_commits(struct state *state, return NULL; } + validate_initial_commitment_signature(HSM_FD, local_commit, &remote_sig); + /* BOLT #2: * * The recipient: diff --git a/openingd/openingd.c b/openingd/openingd.c index 82c097249..ce6db7eee 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -701,6 +701,8 @@ static bool funder_finalize_channel_setup(struct state *state, goto fail; } + validate_initial_commitment_signature(HSM_FD, *tx, sig); + if (!check_tx_sig(*tx, 0, NULL, wscript, &state->their_funding_pubkey, sig)) { peer_failed_err(state->pps, &state->channel_id, "Bad signature %s on tx %s using key %s (channel_type=%s)", @@ -1133,6 +1135,8 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) return NULL; } + validate_initial_commitment_signature(HSM_FD, local_commit, &theirsig); + if (!check_tx_sig(local_commit, 0, NULL, wscript, &their_funding_pubkey, &theirsig)) { /* BOLT #1: