channeld: use revoke_commitment_tx if hsmd supports

ChangeLog-Added: Added hsmd_revoke_commitment_tx to protect against local commitment persistance synchronzation errors with remote signers.

channeld/Makefile

@@ -60,6 +60,7 @@ CHANNELD_COMMON_OBJS :=				\
 	common/status_wiregen.o			\
 	common/gossip_store.o			\
 	common/hmac.o				\
+	common/hsm_capable.o				\
 	common/interactivetx.o			\
 	common/htlc_state.o			\
 	common/htlc_trim.o			\

channeld/channeld.c

@@ -25,6 +25,7 @@
 #include <common/billboard.h>
 #include <common/ecdh_hsmd.h>
 #include <common/gossip_store.h>
+#include <common/hsm_capable.h>
 #include <common/interactivetx.h>
 #include <common/key_derive.h>
 #include <common/memleak.h>
@@ -1923,6 +1924,7 @@ static void send_revocation(struct peer *peer,
 	const struct failed_htlc **failed;
 	struct added_htlc *added;
 	const u8 *msg;
+	const u8 *msg2;
 	const u8 *msg_for_master;
 
 	/* Marshall it now before channel_sending_revoke_and_ack changes htlcs */
@@ -1964,10 +1966,28 @@ static void send_revocation(struct peer *peer,
 
 	peer->splice_state->await_commitment_succcess = false;
 
+	/* Now that the master has persisted the new commitment advance the HSMD
+	 * and fetch the revocation secret for the old one. */
+	struct secret old_secret2;
+	struct pubkey next_point2;
+	if (!hsm_is_capable(peer->hsm_capabilities, WIRE_HSMD_REVOKE_COMMITMENT_TX)) {
+		/* Prior to HSM_VERSION 5 we use the old_secret
+		 * received earlier from validate_commitment_tx. */
+		memcpy(&old_secret2, old_secret, sizeof(old_secret2));
+		memcpy(&next_point2, next_point, sizeof(next_point2));
+	} else {
+		msg2 = towire_hsmd_revoke_commitment_tx(tmpctx, peer->next_index[LOCAL] - 2);
+		msg2 = hsm_req(tmpctx, take(msg2));
+		if (!fromwire_hsmd_revoke_commitment_tx_reply(msg2, &old_secret2, &next_point2))
+			status_failed(STATUS_FAIL_HSM_IO,
+				      "Reading revoke_commitment_tx reply: %s",
+				      tal_hex(tmpctx, msg2));
+	}
+
 	/* Revoke previous commit, get new point. */
 	msg = make_revocation_msg_from_secret(peer, peer->next_index[LOCAL]-2,
 					      &peer->next_local_per_commit,
-					      old_secret, next_point);
+					      &old_secret2, &next_point2);
 
 	/* Now we can finally send revoke_and_ack to peer */
 	peer_write(peer->pps, take(msg));
@@ -2273,7 +2293,10 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer,
 		tal_steal(commitsigs, result);
 	}
 
-	assert(old_secret);
+	// If the HSM doesn't support WIRE_HSMD_REVOKE_COMMITMENT_TX we'd better
+	// have the old_secret at this point.
+	if (!hsm_is_capable(peer->hsm_capabilities, WIRE_HSMD_REVOKE_COMMITMENT_TX))
+		assert(old_secret);
 
 	send_revocation(peer, &commit_sig, htlc_sigs, changed_htlcs, txs[0],
 			old_secret, &next_point, commitsigs);

hsmd/hsmd.c

@@ -650,6 +650,7 @@ static struct io_plan *handle_client(struct io_conn *conn, struct client *c)
  	case WIRE_HSMD_FORGET_CHANNEL:
 	case WIRE_HSMD_SIGN_COMMITMENT_TX:
 	case WIRE_HSMD_VALIDATE_COMMITMENT_TX:
+	case WIRE_HSMD_REVOKE_COMMITMENT_TX:
 	case WIRE_HSMD_VALIDATE_REVOCATION:
 	case WIRE_HSMD_SIGN_PENALTY_TO_US:
 	case WIRE_HSMD_SIGN_REMOTE_COMMITMENT_TX:
@@ -705,6 +706,7 @@ static struct io_plan *handle_client(struct io_conn *conn, struct client *c)
 	case WIRE_HSMSTATUS_CLIENT_BAD_REQUEST:
 	case WIRE_HSMD_SIGN_COMMITMENT_TX_REPLY:
 	case WIRE_HSMD_VALIDATE_COMMITMENT_TX_REPLY:
+	case WIRE_HSMD_REVOKE_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

@@ -210,6 +210,14 @@ 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,
 
+# Revoke our local commitment, returns the revocation secret and next point
+msgtype,hsmd_revoke_commitment_tx,40
+msgdata,hsmd_revoke_commitment_tx,commit_num,u64,
+
+msgtype,hsmd_revoke_commitment_tx_reply,140
+msgdata,hsmd_revoke_commitment_tx_reply,old_commitment_secret,secret,
+msgdata,hsmd_revoke_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

@@ -100,6 +100,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_REVOKE_COMMITMENT_TX:
 	case WIRE_HSMD_VALIDATE_REVOCATION:
 		return (client->capabilities & HSM_PERM_SIGN_REMOTE_TX) != 0;
 
@@ -160,6 +161,7 @@ bool hsmd_check_client_capabilities(struct hsmd_client *client,
 	case WIRE_HSMSTATUS_CLIENT_BAD_REQUEST:
 	case WIRE_HSMD_SIGN_COMMITMENT_TX_REPLY:
 	case WIRE_HSMD_VALIDATE_COMMITMENT_TX_REPLY:
+	case WIRE_HSMD_REVOKE_COMMITMENT_TX_REPLY:
 	case WIRE_HSMD_VALIDATE_REVOCATION_REPLY:
 	case WIRE_HSMD_SIGN_TX_REPLY:
 	case WIRE_HSMD_SIGN_OPTION_WILL_FUND_OFFER_REPLY:
@@ -1696,6 +1698,46 @@ static u8 *handle_validate_commitment_tx(struct hsmd_client *c, const u8 *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);
+
+	/* Don't ever return the old_secret here anymore. The node should
+	 * call hsmd_revoke_commitment_tx to transactionally revoke the commitment
+	 * and return the secret ...
+	 */
+	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 revoke the old local commitment tx and
+ * release it's secret
+ */
+static u8 *handle_revoke_commitment_tx(struct hsmd_client *c, const u8 *msg_in)
+{
+	u64 commit_num;
+	struct secret channel_seed;
+	struct sha256 shaseed;
+	struct secret *old_secret;
+	struct pubkey next_per_commitment_point;
+
+	if (!fromwire_hsmd_revoke_commitment_tx(msg_in, &commit_num))
+		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.
@@ -1719,7 +1761,7 @@ static u8 *handle_validate_commitment_tx(struct hsmd_client *c, const u8 *msg_in
 		old_secret = NULL;
 	}
 
-	return towire_hsmd_validate_commitment_tx_reply(
+	return towire_hsmd_revoke_commitment_tx_reply(
 		NULL, old_secret, &next_per_commitment_point);
 }
 
@@ -2010,6 +2052,8 @@ u8 *hsmd_handle_client_message(const tal_t *ctx, struct hsmd_client *client,
 		return handle_sign_commitment_tx(client, msg);
 	case WIRE_HSMD_VALIDATE_COMMITMENT_TX:
 		return handle_validate_commitment_tx(client, msg);
+	case WIRE_HSMD_REVOKE_COMMITMENT_TX:
+		return handle_revoke_commitment_tx(client, msg);
 	case WIRE_HSMD_VALIDATE_REVOCATION:
 		return handle_validate_revocation(client, msg);
 	case WIRE_HSMD_SIGN_REMOTE_HTLC_TO_US:
@@ -2052,6 +2096,7 @@ u8 *hsmd_handle_client_message(const tal_t *ctx, struct hsmd_client *client,
 	case WIRE_HSMSTATUS_CLIENT_BAD_REQUEST:
 	case WIRE_HSMD_SIGN_COMMITMENT_TX_REPLY:
 	case WIRE_HSMD_VALIDATE_COMMITMENT_TX_REPLY:
+	case WIRE_HSMD_REVOKE_COMMITMENT_TX_REPLY:
 	case WIRE_HSMD_VALIDATE_REVOCATION_REPLY:
 	case WIRE_HSMD_SIGN_TX_REPLY:
 	case WIRE_HSMD_SIGN_OPTION_WILL_FUND_OFFER_REPLY: