From 7d0fc03f4910c64fff521d5dfd23d8990fd4ea5d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 8 Mar 2018 13:54:14 +1030 Subject: [PATCH] gossip: send error messages on grossly malformed node_announcement. As per BOLT #7. Signed-off-by: Rusty Russell --- gossipd/gossip.c | 26 ++++++---- gossipd/routing.c | 68 +++++++++++++++++++++----- gossipd/routing.h | 4 +- gossipd/test/run-bench-find_route.c | 5 ++ gossipd/test/run-find_route-specific.c | 5 ++ gossipd/test/run-find_route.c | 5 ++ wallet/test/run-wallet.c | 9 ++-- 7 files changed, 97 insertions(+), 25 deletions(-) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index 7d2d0f99641a..1c2c1a7eeb11 100644 --- a/gossipd/gossip.c +++ b/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,14 +470,20 @@ 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: { @@ -485,14 +491,16 @@ static void handle_gossip_msg(struct daemon *daemon, u8 *msg) /* 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) { diff --git a/gossipd/routing.c b/gossipd/routing.c index 336ec9f51ee3..331e3673a6cf 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -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, ×tamp, &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,20 +1022,27 @@ 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", @@ -1015,9 +1050,19 @@ void handle_node_announcement( 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, diff --git a/gossipd/routing.h b/gossipd/routing.h index ce476c8fc0ba..0977fdd4aebe 100644 --- a/gossipd/routing.h +++ b/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, diff --git a/gossipd/test/run-bench-find_route.c b/gossipd/test/run-bench-find_route.c index ac037e3cfdd8..115aa7c52794 100644 --- a/gossipd/test/run-bench-find_route.c +++ b/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(); } diff --git a/gossipd/test/run-find_route-specific.c b/gossipd/test/run-find_route-specific.c index 509bb28c9f3e..aecf2de22a95 100644 --- a/gossipd/test/run-find_route-specific.c +++ b/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(); } diff --git a/gossipd/test/run-find_route.c b/gossipd/test/run-find_route.c index 0550b86e6e58..2c8694e13527 100644 --- a/gossipd/test/run-find_route.c +++ b/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(); } diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 55d8d2bae8e0..e05ac7620967 100644 --- a/wallet/test/run-wallet.c +++ b/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