fixup! channeld: Verify the signature sent by the counterparty

channeld/channeld.c

@@ -10,6 +10,7 @@
  *    reading and writing synchronously we could deadlock if we hit buffer
  *    limits, unlikely as that is.
  */
+#include "common/utils.h"
 #include "config.h"
 #include <ccan/asort/asort.h>
 #include <ccan/cast/cast.h>
@@ -647,11 +648,24 @@ static void handle_peer_channel_ready(struct peer *peer, const u8 *msg)
 	billboard_update(peer);
 }
 
+/* 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);
+}
+
 static void handle_peer_announcement_signatures(struct peer *peer, const u8 *msg)
 {
 	struct channel_id chanid;
-	struct pubkey peer_pubkey;
-	struct sha256 peer_announcement_signature;
+	const u8 *cannounce;
+	/* 2 byte msg type + 256 byte signatures */
+	int offset = 258;
+	struct sha256_double hash;
 
 	if (!fromwire_announcement_signatures(msg,
 					      &chanid,
@@ -678,22 +692,34 @@ static void handle_peer_announcement_signatures(struct peer *peer, const u8 *msg
 	 * In our case, we send an error and stop the open channel procedure. This approach is
 	 * considered overly strict since the peer can recover from it. However, this step is
 	 * optional. If the peer sends it, we assume that the signature must be correct.*/
-	assert(pubkey_from_node_id(&peer_pubkey, &peer->node_ids[REMOTE]));
-	sha256(&peer_announcement_signature, msg, tal_count(msg));
-	if (!secp256k1_ecdsa_verify(secp256k1_ctx,
-				    &peer->announcement_node_sigs[REMOTE],
-				    (const u8 *)&peer_announcement_signature,
-				    &peer_pubkey.pubkey))
-		peer_failed_err(peer->pps, &chanid,
-				"Failed to verify node_signature.");
+	 cannounce = create_channel_announcement(tmpctx, peer);
+	 sha256_double(&hash, cannounce + offset,
+		       tal_count(cannounce) - offset);
 
-	if (!secp256k1_ecdsa_verify(secp256k1_ctx,
-				    &peer->announcement_bitcoin_sigs[REMOTE],
-				    (const u8 *)&peer_announcement_signature,
-				    &peer->channel->funding_pubkey[REMOTE].pubkey))
+	 if (!check_signed_hash_nodeid(&hash, &peer->announcement_node_sigs[REMOTE], &peer->node_ids[REMOTE])) {
 		peer_failed_err(peer->pps, &chanid,
-				"Failed to verify bitcoin_signature.");
-
+					 "Bad node_signature %s hash %s"
+					 " on announcement_signatures %s",
+					 type_to_string(tmpctx,
+							secp256k1_ecdsa_signature,
+							&peer->announcement_node_sigs[REMOTE]),
+					 type_to_string(tmpctx,
+							struct sha256_double,
+							&hash),
+					 tal_hex(tmpctx, cannounce));
+	}
+	if (!check_signed_hash(&hash, &peer->announcement_bitcoin_sigs[REMOTE], &peer->channel->funding_pubkey[REMOTE])) {
+		peer_failed_err(peer->pps, &chanid,
+					 "Bad bitcoin_signature %s hash %s"
+					 " on announcement_signatures %s",
+					 type_to_string(tmpctx,
+							secp256k1_ecdsa_signature,
+							&peer->announcement_bitcoin_sigs[REMOTE]),
+					 type_to_string(tmpctx,
+							struct sha256_double,
+							&hash),
+					 tal_hex(tmpctx, cannounce));
+	}
 	peer->have_sigs[REMOTE] = true;
 	billboard_update(peer);