From 39914251113d74d8bc563649841045dec67a2147 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 15 Oct 2018 15:27:22 +1030 Subject: [PATCH] gossipd: don't accept forwarding short_channel_ids we don't own. Gossipd provided a generic "get endpoints of this scid" and we only use it in one place: to look up htlc forwards. But lightningd just assumed that one would be us. Instead, provide a simpler API which only returns the peer node if any, and now we handle it much more gracefully. Signed-off-by: Rusty Russell --- gossipd/gossip_wire.csv | 11 +++++------ gossipd/gossipd.c | 34 +++++++++++++++++----------------- lightningd/gossip_control.c | 4 ++-- lightningd/peer_htlcs.c | 23 +++++------------------ wallet/test/run-wallet.c | 12 ++++++------ 5 files changed, 35 insertions(+), 49 deletions(-) diff --git a/gossipd/gossip_wire.csv b/gossipd/gossip_wire.csv index 1f893df3f..6bfe07762 100644 --- a/gossipd/gossip_wire.csv +++ b/gossipd/gossip_wire.csv @@ -93,13 +93,12 @@ gossip_query_channel_range_reply,,scids,num*struct short_channel_id gossip_dev_set_max_scids_encode_size,3030 gossip_dev_set_max_scids_encode_size,,max,u32 -# Given a short_channel_id, return the endpoints -gossip_resolve_channel_request,3009 -gossip_resolve_channel_request,,channel_id,struct short_channel_id +# Given a short_channel_id, return the other endpoint (or none if DNE) +gossip_get_channel_peer,3009 +gossip_get_channel_peer,,channel_id,struct short_channel_id -gossip_resolve_channel_reply,3109 -gossip_resolve_channel_reply,,num_keys,u16 -gossip_resolve_channel_reply,,keys,num_keys*struct pubkey +gossip_get_channel_peer_reply,3109 +gossip_get_channel_peer_reply,,peer_id,?struct pubkey # Channel daemon can ask for updates for a specific channel, for sending # errors. Must be distinct from WIRE_CHANNEL_ANNOUNCEMENT etc. gossip msgs! diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index f74f5efdf..b0da6e14a 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -1846,32 +1846,32 @@ static struct io_plan *gossip_init(struct daemon_conn *master, return daemon_conn_read_next(master->conn, master); } -static struct io_plan *resolve_channel_req(struct io_conn *conn, - struct daemon *daemon, const u8 *msg) +static struct io_plan *get_channel_peer(struct io_conn *conn, + struct daemon *daemon, const u8 *msg) { struct short_channel_id scid; struct chan *chan; - struct pubkey *keys; + const struct pubkey *key; + int direction; - if (!fromwire_gossip_resolve_channel_request(msg, &scid)) - master_badmsg(WIRE_GOSSIP_RESOLVE_CHANNEL_REQUEST, msg); + if (!fromwire_gossip_get_channel_peer(msg, &scid)) + master_badmsg(WIRE_GOSSIP_GET_CHANNEL_PEER, msg); chan = get_channel(daemon->rstate, &scid); if (!chan) { status_trace("Failed to resolve channel %s", type_to_string(tmpctx, struct short_channel_id, &scid)); - keys = NULL; + key = NULL; + } else if (local_direction(daemon, chan, &direction)) { + key = &chan->nodes[!direction]->id; } else { - keys = tal_arr(msg, struct pubkey, 2); - keys[0] = chan->nodes[0]->id; - keys[1] = chan->nodes[1]->id; - status_trace("Resolved channel %s %s<->%s", - type_to_string(tmpctx, struct short_channel_id, &scid), - type_to_string(tmpctx, struct pubkey, &keys[0]), - type_to_string(tmpctx, struct pubkey, &keys[1])); + status_trace("Resolved channel %s was not local", + type_to_string(tmpctx, struct short_channel_id, + &scid)); + key = NULL; } daemon_conn_send(&daemon->master, - take(towire_gossip_resolve_channel_reply(NULL, keys))); + take(towire_gossip_get_channel_peer_reply(NULL, key))); return daemon_conn_read_next(conn, &daemon->master); } @@ -2000,8 +2000,8 @@ static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master case WIRE_GOSSIP_GETCHANNELS_REQUEST: return getchannels_req(conn, daemon, daemon->master.msg_in); - case WIRE_GOSSIP_RESOLVE_CHANNEL_REQUEST: - return resolve_channel_req(conn, daemon, daemon->master.msg_in); + case WIRE_GOSSIP_GET_CHANNEL_PEER: + return get_channel_peer(conn, daemon, daemon->master.msg_in); case WIRE_GOSSIP_GET_TXOUT_REPLY: return handle_txout_reply(conn, daemon, master->msg_in); @@ -2057,7 +2057,7 @@ static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master case WIRE_GOSSIP_PING_REPLY: case WIRE_GOSSIP_SCIDS_REPLY: case WIRE_GOSSIP_QUERY_CHANNEL_RANGE_REPLY: - case WIRE_GOSSIP_RESOLVE_CHANNEL_REPLY: + case WIRE_GOSSIP_GET_CHANNEL_PEER_REPLY: case WIRE_GOSSIP_GET_INCOMING_CHANNELS_REPLY: case WIRE_GOSSIP_GET_UPDATE: case WIRE_GOSSIP_GET_UPDATE_REPLY: diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index 44dc26740..e31d5cbf9 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -106,7 +106,7 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds) case WIRE_GOSSIP_GETROUTE_REQUEST: case WIRE_GOSSIP_GETCHANNELS_REQUEST: case WIRE_GOSSIP_PING: - case WIRE_GOSSIP_RESOLVE_CHANNEL_REQUEST: + case WIRE_GOSSIP_GET_CHANNEL_PEER: case WIRE_GOSSIP_GET_UPDATE: case WIRE_GOSSIP_SEND_GOSSIP: case WIRE_GOSSIP_GET_TXOUT_REPLY: @@ -126,7 +126,7 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds) case WIRE_GOSSIP_GETCHANNELS_REPLY: case WIRE_GOSSIP_SCIDS_REPLY: case WIRE_GOSSIP_QUERY_CHANNEL_RANGE_REPLY: - case WIRE_GOSSIP_RESOLVE_CHANNEL_REPLY: + case WIRE_GOSSIP_GET_CHANNEL_PEER_REPLY: case WIRE_GOSSIP_GET_INCOMING_CHANNELS_REPLY: /* These are inter-daemon messages, not received by us */ case WIRE_GOSSIP_LOCAL_ADD_CHANNEL: diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 93bf5d4a1..aada3470a 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -569,30 +569,18 @@ struct gossip_resolve { static void channel_resolve_reply(struct subd *gossip, const u8 *msg, const int *fds UNUSED, struct gossip_resolve *gr) { - struct pubkey *nodes, *peer_id; + struct pubkey *peer_id; - if (!fromwire_gossip_resolve_channel_reply(msg, msg, &nodes)) { + if (!fromwire_gossip_get_channel_peer_reply(msg, msg, &peer_id)) { log_broken(gossip->log, - "bad fromwire_gossip_resolve_channel_reply %s", + "bad fromwire_gossip_get_channel_peer_reply %s", tal_hex(msg, msg)); return; } - if (tal_count(nodes) == 0) { + if (!peer_id) { local_fail_htlc(gr->hin, WIRE_UNKNOWN_NEXT_PEER, NULL); return; - } else if (tal_count(nodes) != 2) { - log_broken(gossip->log, - "fromwire_gossip_resolve_channel_reply has %zu nodes", - tal_count(nodes)); - return; - } - - /* Get the other peer matching the id that is not us */ - if (pubkey_cmp(&nodes[0], &gossip->ld->id) == 0) { - peer_id = &nodes[1]; - } else { - peer_id = &nodes[0]; } forward_htlc(gr->hin, gr->hin->cltv_expiry, @@ -697,8 +685,7 @@ static bool peer_accepted_htlc(struct channel *channel, gr->outgoing_cltv_value = rs->hop_data.outgoing_cltv; gr->hin = hin; - req = towire_gossip_resolve_channel_request(tmpctx, - &gr->next_channel); + req = towire_gossip_get_channel_peer(tmpctx, &gr->next_channel); log_debug(channel->log, "Asking gossip to resolve channel %s", type_to_string(tmpctx, struct short_channel_id, &gr->next_channel)); diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 433d0c343..9dc03c090 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -85,9 +85,9 @@ bool fromwire_channel_sending_commitsig(const tal_t *ctx UNNEEDED, const void *p /* Generated stub for fromwire_connect_peer_connected */ bool fromwire_connect_peer_connected(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct pubkey *id UNNEEDED, struct wireaddr_internal *addr UNNEEDED, struct crypto_state *crypto_state UNNEEDED, u8 **globalfeatures UNNEEDED, u8 **localfeatures UNNEEDED) { fprintf(stderr, "fromwire_connect_peer_connected called!\n"); abort(); } -/* Generated stub for fromwire_gossip_resolve_channel_reply */ -bool fromwire_gossip_resolve_channel_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct pubkey **keys UNNEEDED) -{ fprintf(stderr, "fromwire_gossip_resolve_channel_reply called!\n"); abort(); } +/* Generated stub for fromwire_gossip_get_channel_peer_reply */ +bool fromwire_gossip_get_channel_peer_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct pubkey **peer_id UNNEEDED) +{ fprintf(stderr, "fromwire_gossip_get_channel_peer_reply called!\n"); abort(); } /* Generated stub for fromwire_hsm_sign_commitment_tx_reply */ bool fromwire_hsm_sign_commitment_tx_reply(const void *p UNNEEDED, secp256k1_ecdsa_signature *sig UNNEEDED) { fprintf(stderr, "fromwire_hsm_sign_commitment_tx_reply called!\n"); abort(); } @@ -424,9 +424,9 @@ 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_gossip_resolve_channel_request */ -u8 *towire_gossip_resolve_channel_request(const tal_t *ctx UNNEEDED, const struct short_channel_id *channel_id UNNEEDED) -{ fprintf(stderr, "towire_gossip_resolve_channel_request called!\n"); abort(); } +/* Generated stub for towire_gossip_get_channel_peer */ +u8 *towire_gossip_get_channel_peer(const tal_t *ctx UNNEEDED, const struct short_channel_id *channel_id UNNEEDED) +{ fprintf(stderr, "towire_gossip_get_channel_peer called!\n"); abort(); } /* Generated stub for towire_hsm_sign_commitment_tx */ u8 *towire_hsm_sign_commitment_tx(const tal_t *ctx UNNEEDED, const struct pubkey *peer_id UNNEEDED, u64 channel_dbid UNNEEDED, const struct bitcoin_tx *tx UNNEEDED, const struct pubkey *remote_funding_key UNNEEDED, u64 funding_amount UNNEEDED) { fprintf(stderr, "towire_hsm_sign_commitment_tx called!\n"); abort(); }