remote-hsmd: Add validate_commitment_tx

Author: ksedgwic

channeld/channeld.c

@@ -1442,6 +1442,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,7 +1512,9 @@ 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;
@@ -1519,8 +1532,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);
+	u8 *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]++;
@@ -1685,8 +1699,44 @@ 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]);
+	/* Collect the htlcs for call to hsmd validate. */
+	struct simple_htlc **htlcs = tal_arr(NULL, 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];
+		if (hh) {
+			status_debug("HTLC[%lu]=%" PRIu64 ", %s",
+				     ndx, hh->id, htlc_state_name(hh->state));
+			struct simple_htlc *simple =
+			    new_simple_htlc(NULL,
+					    htlc_state_owner(hh->state),
+					    hh->amount,
+					    &hh->rhash,
+					    hh->expiry.locktime);
+			tal_arr_expand(&htlcs, tal_steal(htlcs, simple));
+		}
+	}
+
+	/* Validate the counterparty's signatures, returns old_secret. */
+	const u8 * 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);

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				\

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:

hsmd/hsmd_wire.csv

@@ -138,6 +138,22 @@ 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.
+#include <common/htlc_wire.h>
+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,

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,50 @@ static u8 *handle_sign_commitment_tx(struct hsmd_client *c, const u8 *msg_in)
 	return towire_hsmd_sign_commitment_tx_reply(NULL, &sig);
 }
 
+/* Validate the peer's signatures for our commitment and htlc txs. */
+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);
+
+	/* TODO - move the signature validation code from channeld to here */
+
+	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 +1579,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 +1601,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:

openingd/common.c

@@ -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
@@ -225,3 +228,33 @@ 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)
+{
+	/* Validate the counterparty's signature. */
+	struct existing_htlc **htlcs = tal_arr(NULL, struct existing_htlc *, 0);
+	struct bitcoin_signature *htlc_sigs = tal_arr(NULL, struct bitcoin_signature, 0);
+	u32 feerate = 0; /* unused since there are no htlcs */
+	u64 commit_num = 0;
+	const u8 * 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);
+	struct secret *old_secret;
+	struct pubkey next_point;
+	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));
+}
+

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 */

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:

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: