gossip: send error messages on grossly malformed channel_announcement.

As per BOLT #7.  We also give more exact diagnosis.

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

gossipd/gossip.c

@@ -489,8 +489,10 @@ static void handle_gossip_msg(struct peer *peer, u8 *msg)
 	case WIRE_CHANNEL_ANNOUNCEMENT: {
 		const struct short_channel_id *scid;
 		/* If it's OK, tells us the short_channel_id to lookup */
-		scid = handle_channel_announcement(rstate, msg);
+		err = handle_channel_announcement(rstate, msg, &scid);
-		if (scid)
+		if (err)
+			queue_peer_msg(peer, take(err));
+		else if (scid)
 			daemon_conn_send(&peer->daemon->master,
 					 take(towire_gossip_get_txout(NULL,
 								      scid)));

gossipd/routing.c

@@ -464,7 +464,7 @@ static bool check_channel_update(const struct pubkey *node_key,
 	return check_signed_hash(&hash, node_sig, node_key);
 }
 
-static bool check_channel_announcement(
+static u8 *check_channel_announcement(const tal_t *ctx,
     const struct pubkey *node1_key, const struct pubkey *node2_key,
     const struct pubkey *bitcoin1_key, const struct pubkey *bitcoin2_key,
     const secp256k1_ecdsa_signature *node1_sig,
@@ -478,10 +478,55 @@ static bool check_channel_announcement(
 	sha256_double(&hash, announcement + offset,
 		      tal_len(announcement) - offset);
 
-	return check_signed_hash(&hash, node1_sig, node1_key) &&
+	if (!check_signed_hash(&hash, node1_sig, node1_key)) {
-	       check_signed_hash(&hash, node2_sig, node2_key) &&
+		return towire_errorfmt(ctx, NULL,
-	       check_signed_hash(&hash, bitcoin1_sig, bitcoin1_key) &&
+				       "Bad node_signature_1 %s hash %s"
-	       check_signed_hash(&hash, bitcoin2_sig, bitcoin2_key);
+				       " on node_announcement %s",
+				       type_to_string(ctx,
+						      secp256k1_ecdsa_signature,
+						      node1_sig),
+				       type_to_string(ctx,
+						      struct sha256_double,
+						      &hash),
+				       tal_hex(ctx, announcement));
+	}
+	if (!check_signed_hash(&hash, node2_sig, node2_key)) {
+		return towire_errorfmt(ctx, NULL,
+				       "Bad node_signature_2 %s hash %s"
+				       " on node_announcement %s",
+				       type_to_string(ctx,
+						      secp256k1_ecdsa_signature,
+						      node2_sig),
+				       type_to_string(ctx,
+						      struct sha256_double,
+						      &hash),
+				       tal_hex(ctx, announcement));
+	}
+	if (!check_signed_hash(&hash, bitcoin1_sig, bitcoin1_key)) {
+		return towire_errorfmt(ctx, NULL,
+				       "Bad bitcoin_signature_1 %s hash %s"
+				       " on node_announcement %s",
+				       type_to_string(ctx,
+						      secp256k1_ecdsa_signature,
+						      bitcoin1_sig),
+				       type_to_string(ctx,
+						      struct sha256_double,
+						      &hash),
+				       tal_hex(ctx, announcement));
+	}
+	if (!check_signed_hash(&hash, bitcoin2_sig, bitcoin2_key)) {
+		return towire_errorfmt(ctx, NULL,
+				       "Bad bitcoin_signature_2 %s hash %s"
+				       " on node_announcement %s",
+				       type_to_string(ctx,
+						      secp256k1_ecdsa_signature,
+						      bitcoin2_sig),
+				       type_to_string(ctx,
+						      struct sha256_double,
+						      &hash),
+				       tal_hex(ctx, announcement));
+	}
+	return NULL;
 }
 
 static void add_pending_node_announcement(struct routing_state *rstate, struct pubkey *nodeid)
@@ -537,13 +582,13 @@ static void destroy_pending_cannouncement(struct pending_cannouncement *pending,
 	list_del_from(&rstate->pending_cannouncement, &pending->list);
 }
 
-const struct short_channel_id *handle_channel_announcement(
+u8 *handle_channel_announcement(struct routing_state *rstate,
-	struct routing_state *rstate,
+				const u8 *announce TAKES,
-	const u8 *announce TAKES)
+				const struct short_channel_id **scid)
 {
 	struct pending_cannouncement *pending;
 	struct bitcoin_blkid chain_hash;
-	u8 *features;
+	u8 *features, *err;
 	secp256k1_ecdsa_signature node_signature_1, node_signature_2;
 	secp256k1_ecdsa_signature bitcoin_signature_1, bitcoin_signature_2;
 	struct chan *chan;
@@ -567,8 +612,12 @@ const struct short_channel_id *handle_channel_announcement(
 					   &pending->node_id_2,
 					   &pending->bitcoin_key_1,
 					   &pending->bitcoin_key_2)) {
+		err = towire_errorfmt(rstate, NULL,
+				      "Malformed channel_announcement %s",
+				      tal_hex(pending, pending->announce));
 		tal_free(pending);
-		return NULL;
+		*scid = NULL;
+		return err;
 	}
 
 	/* Check if we know the channel already (no matter in what
@@ -579,7 +628,9 @@ const struct short_channel_id *handle_channel_announcement(
 			     __func__,
 			     type_to_string(trc, struct short_channel_id,
 					    &pending->short_channel_id));
-		return tal_free(pending);
+		tal_free(pending);
+		*scid = NULL;
+		return NULL;
 	}
 
 	/* We don't replace previous ones, since we might validate that and
@@ -589,6 +640,7 @@ const struct short_channel_id *handle_channel_announcement(
 			     __func__,
 			     type_to_string(trc, struct short_channel_id,
 					    &pending->short_channel_id));
+		*scid = NULL;
 		return tal_free(pending);
 	}
 
@@ -605,6 +657,7 @@ const struct short_channel_id *handle_channel_announcement(
 		status_trace("Ignoring channel announcement, unsupported features %s.",
 			     tal_hex(pending, features));
 		tal_free(pending);
+		*scid = NULL;
 		return NULL;
 	}
 
@@ -620,23 +673,30 @@ const struct short_channel_id *handle_channel_announcement(
 				   &pending->short_channel_id),
 		    type_to_string(pending, struct bitcoin_blkid, &chain_hash));
 		tal_free(pending);
+		*scid = NULL;
 		return NULL;
 	}
 
-	if (!check_channel_announcement(&pending->node_id_1, &pending->node_id_2,
+	err = check_channel_announcement(rstate,
-					&pending->bitcoin_key_1,
+					 &pending->node_id_1,
-					&pending->bitcoin_key_2,
+					 &pending->node_id_2,
-					&node_signature_1,
+					 &pending->bitcoin_key_1,
-					&node_signature_2,
+					 &pending->bitcoin_key_2,
-					&bitcoin_signature_1,
+					 &node_signature_1,
-					&bitcoin_signature_2,
+					 &node_signature_2,
-					pending->announce)) {
+					 &bitcoin_signature_1,
-		status_trace("Signature verification of channel_announcement"
+					 &bitcoin_signature_2,
-			     " for %s failed",
+					 pending->announce);
-			     type_to_string(pending, struct short_channel_id,
+	if (err) {
-					    &pending->short_channel_id));
+		/* BOLT #7:
+		 *
+		 * - if `bitcoin_signature_1`, `bitcoin_signature_2`,
+		 *   `node_signature_1` OR `node_signature_2` are invalid OR NOT
+		 *    correct:
+		 *    - SHOULD fail the connection.
+		 */
 		tal_free(pending);
-		return NULL;
+		return err;
 	}
 
 	status_trace("Received channel_announcement for channel %s",
@@ -651,7 +711,8 @@ const struct short_channel_id *handle_channel_announcement(
 	list_add_tail(&rstate->pending_cannouncement, &pending->list);
 	tal_add_destructor2(pending, destroy_pending_cannouncement, rstate);
 
-	return &pending->short_channel_id;
+	*scid = &pending->short_channel_id;
+	return NULL;
 }
 
 bool handle_pending_cannouncement(struct routing_state *rstate,

gossipd/routing.h

@@ -196,11 +196,12 @@ struct chan *new_chan(struct routing_state *rstate,
 /**
  * handle_channel_announcement -- Check channel announcement is valid
  *
- * Returns a short_channel_id to look up if signatures pass.
+ * Returns error message if we should fail channel.  Make *scid non-NULL
+ * (for checking) if we extracted a short_channel_id, otherwise ignore.
  */
-const struct short_channel_id *
+u8 *handle_channel_announcement(struct routing_state *rstate,
-handle_channel_announcement(struct routing_state *rstate,
+				const u8 *announce TAKES,
-			    const u8 *announce TAKES);
+				const struct short_channel_id **scid);
 
 /**
  * handle_pending_cannouncement -- handle channel_announce once we've