lightningd: check channel_announcement signatures we get from peer.

We hoise check_signed_hash_nodeid from gossipd's internals, into common/.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

common/node_id.c

@@ -1,5 +1,6 @@
 #include "config.h"
 #include <assert.h>
+#include <bitcoin/signature.h>
 #include <ccan/array_size/array_size.h>
 #include <ccan/mem/mem.h>
 #include <ccan/str/hex/hex.h>
@@ -66,3 +67,13 @@ void towire_node_id(u8 **pptr, const struct node_id *id)
 #endif
 	towire(pptr, id->k, sizeof(id->k));
 }
+
+bool check_signed_hash_nodeid(const struct sha256_double *hash,
+			      const secp256k1_ecdsa_signature *signature,
+			      const struct node_id *id)
+{
+	struct pubkey key;
+
+	return pubkey_from_node_id(&key, id)
+		&& check_signed_hash(hash, signature, &key);
+}

common/node_id.h

@@ -7,6 +7,8 @@
 #include <ccan/htable/htable_type.h>
 #include <common/pseudorand.h>
 
+struct sha256_double;
+
 struct node_id {
 	u8 k[PUBKEY_CMPR_LEN];
 };
@@ -63,4 +65,18 @@ static inline size_t node_id_hash(const struct node_id *id)
 {
 	return siphash24(siphash_seed(), id->k, sizeof(id->k));
 }
+
+/**
+ * check_signed_hash_nodeid - check a raw secp256k1 signature.
+ * @h: hash which was signed.
+ * @signature: signature.
+ * @id: node_id corresponding to private key used to sign.
+ *
+ * Returns true if the id, hash and signature are correct.  Changing any
+ * one of these will make it fail.
+ */
+bool check_signed_hash_nodeid(const struct sha256_double *hash,
+			      const secp256k1_ecdsa_signature *signature,
+			      const struct node_id *id);
+
 #endif /* LIGHTNING_COMMON_NODE_ID_H */

gossipd/routing.c

@@ -618,17 +618,6 @@ struct chan *new_chan(struct routing_state *rstate,
 	return chan;
 }
 
-/* Checks that key is valid, and signed this hash */
-static bool check_signed_hash_nodeid(const struct sha256_double *hash,
-				     const secp256k1_ecdsa_signature *signature,
-				     const struct node_id *id)
-{
-	struct pubkey key;
-
-	return pubkey_from_node_id(&key, id)
-		&& check_signed_hash(hash, signature, &key);
-}
-
 /* Verify the signature of a channel_update message */
 static u8 *check_channel_update(const tal_t *ctx,
 				const struct node_id *node_id,

lightningd/channel_gossip.c

@@ -351,6 +351,21 @@ static void stash_remote_announce_sigs(struct channel *channel,
 				       const secp256k1_ecdsa_signature *bitcoin_sig)
 {
 	struct channel_gossip *cg = channel->channel_gossip;
+	const char *err;
+
+	/* BOLT #7:
+	 * - if the `node_signature` OR the `bitcoin_signature` is NOT correct:
+	 *   - MAY send a `warning` and close the connection, or send an
+         *     `error` and fail the channel.
+	 */
+	err = check_announce_sigs(channel, scid, node_sig, bitcoin_sig);
+	if (err) {
+		channel_fail_transient(channel, true,
+				       "Bad gossip announcement_signatures for scid %s: %s",
+				       short_channel_id_to_str(tmpctx, &scid),
+				       err);
+		return;
+	}
 
 	tal_free(cg->remote_sigs);
 	cg->remote_sigs = tal(cg, struct remote_announce_sigs);
@@ -375,7 +390,6 @@ static bool apply_remote_sigs(struct channel *channel)
 		return false;
 	}
 
-	/* FIXME: check sigs are valid! */
 	wallet_announcement_save(channel->peer->ld->wallet,
 				 channel->dbid,
 				 &cg->remote_sigs->node_sig,
@@ -397,7 +411,7 @@ static void send_channel_announce_sigs(struct channel *channel)
 	if (!channel_state_can_add_htlc(channel->state))
 		return;
 
-	ca = create_channel_announcement(tmpctx, channel,
+	ca = create_channel_announcement(tmpctx, channel, *channel->scid,
 					 NULL, NULL, NULL, NULL);
 
 	msg = hsm_sync_req(tmpctx, ld,
@@ -453,7 +467,7 @@ static void send_channel_announcement(struct channel *channel)
 	const u8 *ca, *msg;
 	struct channel_gossip *cg = channel->channel_gossip;
 
- 	ca = create_channel_announcement(tmpctx, channel,
+ 	ca = create_channel_announcement(tmpctx, channel, *channel->scid,
 					 NULL, NULL,
 					 &cg->remote_sigs->node_sig,
 					 &cg->remote_sigs->bitcoin_sig);
@@ -471,7 +485,7 @@ static void send_channel_announcement(struct channel *channel)
 	if (!ld->gossip)
 		return;
 
-	ca = create_channel_announcement(tmpctx, channel,
+	ca = create_channel_announcement(tmpctx, channel, *channel->scid,
 					 &local_node_sig,
 					 &local_bitcoin_sig,
 					 &cg->remote_sigs->node_sig,

lightningd/gossip_generation.c

@@ -51,6 +51,7 @@ static void copysig_or_zero(secp256k1_ecdsa_signature *dst,
 
 u8 *create_channel_announcement(const tal_t *ctx,
 				const struct channel *channel,
+				struct short_channel_id scid,
 				const secp256k1_ecdsa_signature *local_node_signature,
 				const secp256k1_ecdsa_signature *local_bitcoin_signature,
 				const secp256k1_ecdsa_signature *remote_node_signature,
@@ -72,7 +73,7 @@ u8 *create_channel_announcement(const tal_t *ctx,
 	node_id[REMOTE] = channel->peer->id;
 	funding_pubkey[LOCAL] = channel->local_funding_pubkey;
 	funding_pubkey[REMOTE] = channel->channel_info.remote_fundingkey;
-	return create_channel_announcement_dir(ctx, features, *channel->scid,
+	return create_channel_announcement_dir(ctx, features, scid,
 					       node_signature, bitcoin_signature, node_id, funding_pubkey);
 }
 
@@ -205,3 +206,36 @@ bool channel_update_details(const u8 *channel_update,
 		*enabled = !(channel_flags & ROUTING_FLAGS_DISABLED);
 	return true;
 }
+
+const char *check_announce_sigs(const struct channel *channel,
+				struct short_channel_id scid,
+				const secp256k1_ecdsa_signature *remote_node_signature,
+				const secp256k1_ecdsa_signature *remote_bitcoin_signature)
+{
+	struct sha256_double hash;
+	const u8 *cannounce;
+
+	cannounce = create_channel_announcement(tmpctx, channel, scid,
+						NULL, NULL, NULL, NULL);
+
+	/* BOLT #7:
+	 *
+	 * - MUST compute the double-SHA256 hash `h` of the message, beginning
+	 *   at offset 256, up to the end of the message.
+	 *     - Note: the hash skips the 4 signatures but hashes the rest of the
+	 *       message, including any future fields appended to the end.
+	 */
+	/* First two bytes are the msg type */
+	int offset = 258;
+	sha256_double(&hash, cannounce + offset, tal_count(cannounce) - offset);
+
+	if (!check_signed_hash_nodeid(&hash, remote_node_signature,
+				      &channel->peer->id))
+		return "invalid node_signature";
+
+	if (!check_signed_hash(&hash, remote_bitcoin_signature,
+			       &channel->channel_info.remote_fundingkey))
+		return "invalid bitcoin_signature";
+
+	return NULL;
+}

lightningd/gossip_generation.h

@@ -10,6 +10,7 @@ struct channel;
  * create_channel_announcement: create a channel_announcement message
  * @ctx: the tal context to allocate return from
  * @channel: the channel to announce
+ * @scid: channel->scid
  * @local_node_signature: optional local node signature
  * @local_bitcoin_signature: optional local node signature
  * @remote_node_signature: optional peer node signature
@@ -20,6 +21,7 @@ struct channel;
  */
 u8 *create_channel_announcement(const tal_t *ctx,
 				const struct channel *channel,
+				struct short_channel_id scid,
 				const secp256k1_ecdsa_signature *local_node_signature,
 				const secp256k1_ecdsa_signature *local_bitcoin_signature,
 				const secp256k1_ecdsa_signature *remote_node_signature,
@@ -61,4 +63,18 @@ bool channel_update_details(const u8 *channel_update,
 			    u32 *timestamp,
 			    bool *enabled);
 
+/**
+ * check_announce_sigs: check that signatures are correct for this scid
+ * @channel: the channel (for peer's id / bitcoin key and the channel features)
+ * @scid: the short_channel_id it's proposing
+ * @remote_node_signature: node signature
+ * @remote_bitcoin_signature: bitcoin signature
+ *
+ * Returns a string literal if one signature is bad.
+ */
+const char *check_announce_sigs(const struct channel *channel,
+				struct short_channel_id scid,
+				const secp256k1_ecdsa_signature *remote_node_signature,
+				const secp256k1_ecdsa_signature *remote_bitcoin_signature);
+
 #endif /* LIGHTNING_LIGHTNINGD_GOSSIP_GENERATION_H */

wallet/test/run-wallet.c

@@ -107,6 +107,12 @@ void channeld_tell_depth(struct channel *channel UNNEEDED,
 			 const struct bitcoin_txid *txid UNNEEDED,
 			 u32 depth UNNEEDED)
 { fprintf(stderr, "channeld_tell_depth called!\n"); abort(); }
+/* Generated stub for check_announce_sigs */
+const char *check_announce_sigs(const struct channel *channel UNNEEDED,
+				struct short_channel_id scid UNNEEDED,
+				const secp256k1_ecdsa_signature *remote_node_signature UNNEEDED,
+				const secp256k1_ecdsa_signature *remote_bitcoin_signature UNNEEDED)
+{ fprintf(stderr, "check_announce_sigs called!\n"); abort(); }
 /* Generated stub for cmd_id_from_close_command */
 const char *cmd_id_from_close_command(const tal_t *ctx UNNEEDED,
 				      struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED)
@@ -174,6 +180,7 @@ struct anchor_details *create_anchor_details(const tal_t *ctx UNNEEDED,
 /* Generated stub for create_channel_announcement */
 u8 *create_channel_announcement(const tal_t *ctx UNNEEDED,
 				const struct channel *channel UNNEEDED,
+				struct short_channel_id scid UNNEEDED,
 				const secp256k1_ecdsa_signature *local_node_signature UNNEEDED,
 				const secp256k1_ecdsa_signature *local_bitcoin_signature UNNEEDED,
 				const secp256k1_ecdsa_signature *remote_node_signature UNNEEDED,