gossip: send error messages on grossly malformed node_announcement.

As per BOLT #7.

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

gossipd/gossip.c

@@ -450,7 +450,7 @@ static void send_node_announcement(struct daemon *daemon)
 	tal_t *tmpctx = tal_tmpctx(daemon);
 	u32 timestamp = time_now().ts.tv_sec;
 	secp256k1_ecdsa_signature sig;
-	u8 *msg, *nannounce;
+	u8 *msg, *nannounce, *err;
 
 	/* Timestamps must move forward, or announce will be ignored! */
 	if (timestamp <= daemon->last_announce_timestamp)
@@ -470,29 +470,37 @@ static void send_node_announcement(struct daemon *daemon)
 	 * from the HSM, create the real announcement and forward it to
 	 * gossipd so it can take care of forwarding it. */
 	nannounce = create_node_announcement(tmpctx, daemon, &sig, timestamp);
-	handle_node_announcement(daemon->rstate, take(nannounce));
+	err = handle_node_announcement(daemon->rstate, take(nannounce));
+	if (err)
+		status_failed(STATUS_FAIL_INTERNAL_ERROR,
+			      "rejected own node announcement: %s",
+			      tal_hex(trc, err));
 	tal_free(tmpctx);
 }
 
-static void handle_gossip_msg(struct daemon *daemon, u8 *msg)
+/* Returns error if we should send an error. */
+static void handle_gossip_msg(struct peer *peer, u8 *msg)
 {
-	struct routing_state *rstate = daemon->rstate;
+	struct routing_state *rstate = peer->daemon->rstate;
 	int t = fromwire_peektype(msg);
+	u8 *err;
 
 	switch(t) {
 	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);
 		if (scid)
-			daemon_conn_send(&daemon->master,
-					 take(towire_gossip_get_txout(daemon,
+			daemon_conn_send(&peer->daemon->master,
+					 take(towire_gossip_get_txout(NULL,
 								      scid)));
 		break;
 	}
 
 	case WIRE_NODE_ANNOUNCEMENT:
-		handle_node_announcement(rstate, msg);
+		err = handle_node_announcement(rstate, msg);
+		if (err)
+			queue_peer_msg(peer, take(err));
 		break;
 
 	case WIRE_CHANNEL_UPDATE:
@@ -595,7 +603,7 @@ static struct io_plan *peer_msgin(struct io_conn *conn,
 	case WIRE_CHANNEL_ANNOUNCEMENT:
 	case WIRE_NODE_ANNOUNCEMENT:
 	case WIRE_CHANNEL_UPDATE:
-		handle_gossip_msg(peer->daemon, msg);
+		handle_gossip_msg(peer, msg);
 		return peer_next_in(conn, peer);
 
 	case WIRE_PING:
@@ -828,7 +836,7 @@ static struct io_plan *owner_msg_in(struct io_conn *conn,
 	int type = fromwire_peektype(msg);
 	if (type == WIRE_CHANNEL_ANNOUNCEMENT || type == WIRE_CHANNEL_UPDATE ||
 	    type == WIRE_NODE_ANNOUNCEMENT) {
-		handle_gossip_msg(peer->daemon, dc->msg_in);
+		handle_gossip_msg(peer, dc->msg_in);
 	} else if (type == WIRE_GOSSIP_GET_UPDATE) {
 		handle_get_update(peer, dc->msg_in);
 	} else if (type == WIRE_GOSSIP_LOCAL_ADD_CHANNEL) {

gossipd/routing.c

@@ -11,6 +11,7 @@
 #include <common/status.h>
 #include <common/type_to_string.h>
 #include <common/wireaddr.h>
+#include <common/wire_error.h>
 #include <inttypes.h>
 #include <wire/gen_onion_wire.h>
 #include <wire/gen_peer_wire.h>
@@ -502,6 +503,7 @@ static void process_pending_node_announcement(struct routing_state *rstate,
 		SUPERVERBOSE(
 		    "Processing deferred node_announcement for node %s",
 		    type_to_string(pna, struct pubkey, nodeid));
+		/* FIXME: Do something if this is invalid */
 		handle_node_announcement(rstate, pna->node_announcement);
 	}
 	pending_node_map_del(rstate->pending_node_map, pna);
@@ -932,8 +934,7 @@ static struct wireaddr *read_addresses(const tal_t *ctx, const u8 *ser)
 	return wireaddrs;
 }
 
-void handle_node_announcement(
-	struct routing_state *rstate, const u8 *node_ann)
+u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann)
 {
 	u8 *serialized;
 	struct sha256_double hash;
@@ -954,8 +955,17 @@ void handle_node_announcement(
 					&signature, &features, &timestamp,
 					&node_id, rgb_color, alias,
 					&addresses)) {
+		/* BOLT #7:
+		 *
+		 *   - if `node_id` is NOT a valid compressed public key:
+		 *    - SHOULD fail the connection.
+		 *    - MUST NOT process the message further.
+		 */
+		u8 *err = towire_errorfmt(rstate, NULL,
+					  "Malformed node_announcement %s",
+					  tal_hex(tmpctx, node_ann));
 		tal_free(tmpctx);
-		return;
+		return err;
 	}
 
 	/* BOLT #7:
@@ -969,14 +979,32 @@ void handle_node_announcement(
 			     type_to_string(tmpctx, struct pubkey, &node_id),
 			     tal_hex(tmpctx, features));
 		tal_free(tmpctx);
-		return;
+		return NULL;
 	}
 
 	sha256_double(&hash, serialized + 66, tal_count(serialized) - 66);
 	if (!check_signed_hash(&hash, &signature, &node_id)) {
-		status_trace("Ignoring node announcement, signature verification failed.");
+		/* BOLT #7:
+		 *
+		 * - if `signature` is NOT a valid signature (using `node_id`
+		 *  of the double-SHA256 of the entire message following the
+		 *  `signature` field, including unknown fields following
+		 *  `alias`):
+		 *    - SHOULD fail the connection.
+		 *    - MUST NOT process the message further.
+		 */
+		u8 *err = towire_errorfmt(rstate, NULL,
+					  "Bad signature for %s hash %s"
+					  " on node_announcement %s",
+					  type_to_string(tmpctx,
+							 secp256k1_ecdsa_signature,
+							 &signature),
+					  type_to_string(tmpctx,
+							 struct sha256_double,
+							 &hash),
+					  tal_hex(tmpctx, node_ann));
 		tal_free(tmpctx);
-		return;
+		return err;
 	}
 
 	node = get_node(rstate, &node_id);
@@ -994,30 +1022,47 @@ void handle_node_announcement(
 			pna->node_announcement = tal_dup_arr(pna, u8, node_ann, tal_len(node_ann), 0);
 		}
 		tal_free(tmpctx);
-		return;
+		return NULL;
 	}
 
+	/* BOLT #7:
+	 *
+	 * - if `node_id` is NOT previously known from a
+	 *   `channel_announcement` message, OR if `timestamp` is NOT greater
+	 *   than the last-received `node_announcement` from this `node_id`:
+	 *    - SHOULD ignore the message.
+	 */
 	if (!node) {
 		SUPERVERBOSE("Node not found, was the node_announcement for "
 			     "node %s preceded by at least "
 			     "channel_announcement?",
 			     type_to_string(tmpctx, struct pubkey, &node_id));
 		tal_free(tmpctx);
-		return;
+		return NULL;
 	} else if (node->last_timestamp >= timestamp) {
 		SUPERVERBOSE("Ignoring node announcement, it's outdated.");
 		tal_free(tmpctx);
-		return;
+		return NULL;
 	}
 
 	status_trace("Received node_announcement for node %s",
 		     type_to_string(tmpctx, struct pubkey, &node_id));
 
 	wireaddrs = read_addresses(tmpctx, addresses);
 	if (!wireaddrs) {
-		status_trace("Unable to parse addresses.");
+		/* BOLT #7:
+		 *
+		 * - if `addrlen` is insufficient to hold the address
+		 *  descriptors of the known types:
+		 *    - SHOULD fail the connection.
+		 */
+		u8 *err = towire_errorfmt(rstate, NULL,
+					  "Malformed wireaddrs %s in %s.",
+					  tal_hex(tmpctx, wireaddrs),
+					  tal_hex(tmpctx, node_ann));
 		tal_free(serialized);
-		return;
+		tal_free(tmpctx);
+		return err;
 	}
 	tal_free(node->addresses);
 	node->addresses = tal_steal(node, wireaddrs);
@@ -1038,6 +1083,7 @@ void handle_node_announcement(
 	tal_free(node->node_announcement);
 	node->node_announcement = tal_steal(node, serialized);
 	tal_free(tmpctx);
+	return NULL;
 }
 
 struct route_hop *get_route(tal_t *ctx, struct routing_state *rstate,

gossipd/routing.h

@@ -212,7 +212,9 @@ bool handle_pending_cannouncement(struct routing_state *rstate,
 				  const struct short_channel_id *scid,
 				  const u8 *txscript);
 void handle_channel_update(struct routing_state *rstate, const u8 *update);
-void handle_node_announcement(struct routing_state *rstate, const u8 *node);
+
+/* Returns NULL if all OK, otherwise an error for the peer which sent. */
+u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node);
 
 /* Set values on the struct node_connection */
 void set_connection_values(struct chan *chan,

gossipd/test/run-bench-find_route.c

@@ -87,6 +87,11 @@ bool replace_broadcast(struct broadcast_state *bstate UNNEEDED,
 void status_failed(enum status_failreason code UNNEEDED,
 		   const char *fmt UNNEEDED, ...)
 { fprintf(stderr, "status_failed called!\n"); abort(); }
+/* Generated stub for towire_errorfmt */
+u8 *towire_errorfmt(const tal_t *ctx UNNEEDED,
+		    const struct channel_id *channel UNNEEDED,
+		    const char *fmt UNNEEDED, ...)
+{ fprintf(stderr, "towire_errorfmt called!\n"); abort(); }
 /* Generated stub for towire_pubkey */
 void towire_pubkey(u8 **pptr UNNEEDED, const struct pubkey *pubkey UNNEEDED)
 { fprintf(stderr, "towire_pubkey called!\n"); abort(); }

gossipd/test/run-find_route-specific.c

@@ -51,6 +51,11 @@ bool replace_broadcast(struct broadcast_state *bstate UNNEEDED,
 void status_failed(enum status_failreason code UNNEEDED,
 		   const char *fmt UNNEEDED, ...)
 { fprintf(stderr, "status_failed called!\n"); abort(); }
+/* Generated stub for towire_errorfmt */
+u8 *towire_errorfmt(const tal_t *ctx UNNEEDED,
+		    const struct channel_id *channel UNNEEDED,
+		    const char *fmt UNNEEDED, ...)
+{ fprintf(stderr, "towire_errorfmt called!\n"); abort(); }
 /* Generated stub for towire_pubkey */
 void towire_pubkey(u8 **pptr UNNEEDED, const struct pubkey *pubkey UNNEEDED)
 { fprintf(stderr, "towire_pubkey called!\n"); abort(); }

gossipd/test/run-find_route.c

@@ -49,6 +49,11 @@ bool replace_broadcast(struct broadcast_state *bstate UNNEEDED,
 void status_failed(enum status_failreason code UNNEEDED,
 		   const char *fmt UNNEEDED, ...)
 { fprintf(stderr, "status_failed called!\n"); abort(); }
+/* Generated stub for towire_errorfmt */
+u8 *towire_errorfmt(const tal_t *ctx UNNEEDED,
+		    const struct channel_id *channel UNNEEDED,
+		    const char *fmt UNNEEDED, ...)
+{ fprintf(stderr, "towire_errorfmt called!\n"); abort(); }
 /* Generated stub for towire_pubkey */
 void towire_pubkey(u8 **pptr UNNEEDED, const struct pubkey *pubkey UNNEEDED)
 { fprintf(stderr, "towire_pubkey called!\n"); abort(); }

wallet/test/run-wallet.c

@@ -74,7 +74,7 @@ bool derive_basepoints(const struct privkey *seed UNNEEDED,
 bool extract_channel_id(const u8 *in_pkt UNNEEDED, struct channel_id *channel_id UNNEEDED)
 { fprintf(stderr, "extract_channel_id called!\n"); abort(); }
 /* Generated stub for fromwire_gossip_getpeers_reply */
-bool fromwire_gossip_getpeers_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct pubkey **id UNNEEDED, struct wireaddr **addr UNNEEDED,  struct gossip_getnodes_entry ***nodes UNNEEDED)
+bool fromwire_gossip_getpeers_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct pubkey **id UNNEEDED, struct wireaddr **addr UNNEEDED, struct gossip_getnodes_entry ***nodes UNNEEDED)
 { fprintf(stderr, "fromwire_gossip_getpeers_reply called!\n"); abort(); }
 /* Generated stub for fromwire_gossip_peer_connected */
 bool fromwire_gossip_peer_connected(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct pubkey *id UNNEEDED, struct wireaddr *addr UNNEEDED, struct crypto_state *crypto_state UNNEEDED, u64 *gossip_index UNNEEDED, u8 **gfeatures UNNEEDED, u8 **lfeatures UNNEEDED)
@@ -191,6 +191,10 @@ void json_add_short_channel_id(struct json_result *response UNNEEDED,
 /* Generated stub for json_add_string */
 void json_add_string(struct json_result *result UNNEEDED, const char *fieldname UNNEEDED, const char *value UNNEEDED)
 { fprintf(stderr, "json_add_string called!\n"); abort(); }
+/* Generated stub for json_add_string_escape */
+void json_add_string_escape(struct json_result *result UNNEEDED, const char *fieldname UNNEEDED,
+			    const char *value UNNEEDED)
+{ fprintf(stderr, "json_add_string_escape called!\n"); abort(); }
 /* Generated stub for json_add_txid */
 void json_add_txid(struct json_result *result UNNEEDED, const char *fieldname UNNEEDED,
 		   const struct bitcoin_txid *txid UNNEEDED)
@@ -373,9 +377,6 @@ struct txowatch *watch_txo(const tal_t *ctx UNNEEDED,
 						   size_t input_num UNNEEDED,
 						   const struct block *block))
 { fprintf(stderr, "watch_txo called!\n"); abort(); }
-/* Generated stub for json_add_string_escape */
-void json_add_string_escape(struct json_result *result UNNEEDED, const char *fieldname UNNEEDED, const char *value UNNEEDED)
-{ fprintf(stderr, "json_add_string_escape called!\n"); abort(); }
 /* AUTOGENERATED MOCKS END */
 
 #if DEVELOPER