Skip to content

Commit

Permalink
hsmd: Add validate_commitment_tx
Browse files Browse the repository at this point in the history
  • Loading branch information
ksedgwic authored and rustyrussell committed Mar 20, 2022
1 parent 8b04bf1 commit 8994b8d
Show file tree
Hide file tree
Showing 9 changed files with 187 additions and 20 deletions.
81 changes: 62 additions & 19 deletions channeld/channeld.c
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 */
Expand All @@ -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]++;
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion hsmd/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand All @@ -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 \
Expand Down
2 changes: 2 additions & 0 deletions hsmd/hsmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
15 changes: 15 additions & 0 deletions hsmd/hsmd_wire.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
55 changes: 55 additions & 0 deletions hsmd/libhsmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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. */
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down
38 changes: 38 additions & 0 deletions openingd/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
#include <common/channel_config.h>
#include <common/features.h>
#include <common/initial_commit_tx.h>
#include <common/status.h>
#include <common/type_to_string.h>
#include <hsmd/hsmd_wiregen.h>
#include <openingd/common.h>
#include <wire/wire_sync.h>

/*~ 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
Expand Down Expand Up @@ -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));
}

6 changes: 6 additions & 0 deletions openingd/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#include "config.h"

struct amount_sat;
struct bitcoin_tx;
struct bitcoin_signature;
struct channel_config;


Expand All @@ -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 */
4 changes: 4 additions & 0 deletions openingd/dualopend.c
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 4 additions & 0 deletions openingd/openingd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit 8994b8d

Please sign in to comment.