From 1dccbb30f9e79351640d72c173c8ed962196f920 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 8 Mar 2018 14:40:33 +1030 Subject: [PATCH] gossip: send error messages on grossly malformed channel_update. As per BOLT #7. We don't do this for channel_update which are queued because the channel_announcement is pending though. Signed-off-by: Rusty Russell --- gossipd/gossip.c | 19 ++++++++++--- gossipd/routing.c | 72 ++++++++++++++++++++++++++++++++--------------- gossipd/routing.h | 4 ++- 3 files changed, 67 insertions(+), 28 deletions(-) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index f1b530b01..309076043 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -506,7 +506,9 @@ static void handle_gossip_msg(struct peer *peer, u8 *msg) break; case WIRE_CHANNEL_UPDATE: - handle_channel_update(rstate, msg); + err = handle_channel_update(rstate, msg); + if (err) + queue_peer_msg(peer, take(err)); break; } } @@ -1335,7 +1337,7 @@ static void gossip_send_keepalive_update(struct routing_state *rstate, u32 timestamp, fee_base_msat, fee_proportional_millionths; u64 htlc_minimum_msat; u16 flags, cltv_expiry_delta; - u8 *update, *msg; + u8 *update, *msg, *err; /* Parse old update */ if (!fromwire_channel_update( @@ -1370,7 +1372,11 @@ static void gossip_send_keepalive_update(struct routing_state *rstate, status_trace("Sending keepalive channel_update for %s", type_to_string(tmpctx, struct short_channel_id, &scid)); - handle_channel_update(rstate, update); + err = handle_channel_update(rstate, update); + if (err) + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "rejected keepalive channel_update: %s", + tal_hex(trc, err)); tal_free(tmpctx); } @@ -1865,6 +1871,7 @@ static struct io_plan *handle_disable_channel(struct io_conn *conn, struct bitcoin_blkid chain_hash; secp256k1_ecdsa_signature sig; u64 htlc_minimum_msat; + u8 *err; if (!fromwire_gossip_disable_channel(msg, &scid, &direction, &active) ) { status_unusual("Unable to parse %s", @@ -1929,7 +1936,11 @@ static struct io_plan *handle_disable_channel(struct io_conn *conn, strerror(errno)); } - handle_channel_update(daemon->rstate, msg); + err = handle_channel_update(daemon->rstate, msg); + if (err) + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "rejected disabling channel_update: %s", + tal_hex(trc, err)); fail: tal_free(tmpctx); diff --git a/gossipd/routing.c b/gossipd/routing.c index c8678ecd6..887559df5 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -452,16 +452,28 @@ find_route(const tal_t *ctx, struct routing_state *rstate, } /* Verify the signature of a channel_update message */ -static bool check_channel_update(const struct pubkey *node_key, - const secp256k1_ecdsa_signature *node_sig, - const u8 *update) +static u8 *check_channel_update(const tal_t *ctx, + const struct pubkey *node_key, + const secp256k1_ecdsa_signature *node_sig, + const u8 *update) { /* 2 byte msg type + 64 byte signatures */ int offset = 66; struct sha256_double hash; sha256_double(&hash, update + offset, tal_len(update) - offset); - return check_signed_hash(&hash, node_sig, node_key); + if (!check_signed_hash(&hash, node_sig, node_key)) + return towire_errorfmt(ctx, NULL, + "Bad signature for %s hash %s" + " on channel_update %s", + type_to_string(ctx, + secp256k1_ecdsa_signature, + node_sig), + type_to_string(ctx, + struct sha256_double, + &hash), + tal_hex(ctx, update)); + return NULL; } static u8 *check_channel_announcement(const tal_t *ctx, @@ -796,10 +808,11 @@ bool handle_pending_cannouncement(struct routing_state *rstate, pubkey_eq(&pending->node_id_2, &rstate->local_id); /* Did we have an update waiting? If so, apply now. */ + /* FIXME: We don't remember who sent us updates, so we can't error them */ if (pending->updates[0]) - handle_channel_update(rstate, pending->updates[0]); + tal_free(handle_channel_update(rstate, pending->updates[0])); if (pending->updates[1]) - handle_channel_update(rstate, pending->updates[1]); + tal_free(handle_channel_update(rstate, pending->updates[1])); process_pending_node_announcement(rstate, &pending->node_id_1); process_pending_node_announcement(rstate, &pending->node_id_2); @@ -863,7 +876,7 @@ void set_connection_values(struct chan *chan, } } -void handle_channel_update(struct routing_state *rstate, const u8 *update) +u8 *handle_channel_update(struct routing_state *rstate, const u8 *update) { u8 *serialized; struct half_chan *c; @@ -880,6 +893,7 @@ void handle_channel_update(struct routing_state *rstate, const u8 *update) struct chan *chan; u8 direction; size_t len = tal_len(update); + u8 *err; serialized = tal_dup_arr(tmpctx, u8, update, len, 0); if (!fromwire_channel_update(serialized, &signature, @@ -887,8 +901,11 @@ void handle_channel_update(struct routing_state *rstate, const u8 *update) ×tamp, &flags, &expiry, &htlc_minimum_msat, &fee_base_msat, &fee_proportional_millionths)) { + err = towire_errorfmt(rstate, NULL, + "Malformed channel_update %s", + tal_hex(tmpctx, serialized)); tal_free(tmpctx); - return; + return err; } direction = flags & 0x1; @@ -902,7 +919,7 @@ void handle_channel_update(struct routing_state *rstate, const u8 *update) type_to_string(tmpctx, struct bitcoin_blkid, &chain_hash)); tal_free(tmpctx); - return; + return NULL; } chan = get_channel(rstate, &short_channel_id); @@ -916,7 +933,7 @@ void handle_channel_update(struct routing_state *rstate, const u8 *update) update_pending(pending, timestamp, serialized, direction); tal_free(tmpctx); - return; + return NULL; } if (!chan) { @@ -924,7 +941,7 @@ void handle_channel_update(struct routing_state *rstate, const u8 *update) type_to_string(trc, struct short_channel_id, &short_channel_id)); tal_free(tmpctx); - return; + return NULL; } } @@ -933,14 +950,23 @@ void handle_channel_update(struct routing_state *rstate, const u8 *update) if (c->last_timestamp >= timestamp) { SUPERVERBOSE("Ignoring outdated update."); tal_free(tmpctx); - return; + return NULL; } - if (!check_channel_update(&chan->nodes[direction]->id, - &signature, serialized)) { - status_trace("Signature verification failed."); + err = check_channel_update(rstate, &chan->nodes[direction]->id, + &signature, serialized); + if (err) { + /* 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 + * `fee_proportional_millionths`): + * - MUST NOT process the message further. + * - SHOULD fail the connection. + */ tal_free(tmpctx); - return; + return err; } status_trace("Received channel_update for channel %s(%d) now %s", @@ -969,6 +995,7 @@ void handle_channel_update(struct routing_state *rstate, const u8 *update) tal_free(c->channel_update); c->channel_update = tal_steal(chan, serialized); tal_free(tmpctx); + return NULL; } static struct wireaddr *read_addresses(const tal_t *ctx, const u8 *ser) @@ -1242,7 +1269,6 @@ void routing_failure(struct routing_state *rstate, const tal_t *tmpctx = tal_tmpctx(rstate); struct node *node; int i; - enum wire_type t; time_t now = time_now().ts.tv_sec; status_trace("Received routing failure 0x%04x (%s), " @@ -1302,6 +1328,7 @@ void routing_failure(struct routing_state *rstate, * channel_update is newer it will be * reactivated. */ if (failcode & UPDATE) { + u8 *err; if (tal_len(channel_update) == 0) { /* Suppress UNUSUAL log if local failure */ if (structeq(&erring_node_pubkey->pubkey, @@ -1313,15 +1340,14 @@ void routing_failure(struct routing_state *rstate, (int) failcode); goto out; } - t = fromwire_peektype(channel_update); - if (t != WIRE_CHANNEL_UPDATE) { + err = handle_channel_update(rstate, channel_update); + if (err) { status_unusual("routing_failure: " - "not a channel_update. " - "type: %d", - (int) t); + "bad channel_update %s", + sanitize_error(err, err, NULL)); + tal_free(err); goto out; } - handle_channel_update(rstate, channel_update); } else { if (tal_len(channel_update) != 0) status_unusual("routing_failure: " diff --git a/gossipd/routing.h b/gossipd/routing.h index 896fb1810..0d2e5d9e6 100644 --- a/gossipd/routing.h +++ b/gossipd/routing.h @@ -215,7 +215,9 @@ bool handle_pending_cannouncement(struct routing_state *rstate, const struct short_channel_id *scid, const u64 satoshis, const u8 *txscript); -void handle_channel_update(struct routing_state *rstate, const u8 *update); + +/* Returns NULL if all OK, otherwise an error for the peer which sent. */ +u8 *handle_channel_update(struct routing_state *rstate, const u8 *update); /* Returns NULL if all OK, otherwise an error for the peer which sent. */ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node);