From 7e7a63a20de2237f5221645b58447d426756bf59 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sat, 8 Jan 2022 23:47:29 +1030 Subject: [PATCH] connectd: keep timeout timer around so we can disable it. connectd will be keeping the conn open, so it needs to free this "conn_timeout" timer. Pass it through, so we can do that. Signed-off-by: Rusty Russell --- connectd/connectd.c | 26 +++++++++++++++----------- connectd/handshake.c | 15 ++++++++++++++- connectd/handshake.h | 23 +++++++++++++++-------- connectd/peer_exchange_initmsg.c | 5 +++++ connectd/peer_exchange_initmsg.h | 2 ++ connectd/test/run-initiator-success.c | 3 ++- connectd/test/run-responder-success.c | 3 ++- devtools/gossipwith.c | 4 +++- 8 files changed, 58 insertions(+), 23 deletions(-) diff --git a/connectd/connectd.c b/connectd/connectd.c index 12adfdc97..c2482df82 100644 --- a/connectd/connectd.c +++ b/connectd/connectd.c @@ -525,13 +525,14 @@ static struct io_plan *handshake_in_success(struct io_conn *conn, const struct pubkey *id_key, const struct wireaddr_internal *addr, struct crypto_state *cs, + struct oneshot *timeout, struct daemon *daemon) { struct node_id id; node_id_from_pubkey(&id, id_key); status_peer_debug(&id, "Connect IN"); return peer_exchange_initmsg(conn, daemon, daemon->our_features, - cs, &id, addr, true); + cs, &id, addr, timeout, true); } /*~ If the timer goes off, we simply free everything, which hangs up. */ @@ -591,11 +592,12 @@ static struct io_plan *conn_in(struct io_conn *conn, struct conn_in *conn_in_arg) { struct daemon *daemon = conn_in_arg->daemon; + struct oneshot *timeout; - /* If they don't complete handshake in reasonable time, hang up */ - notleak(new_reltimer(&daemon->timers, conn, - time_from_sec(daemon->timeout_secs), - conn_timeout, conn)); + /* If they don't complete handshake in reasonable time, we hang up */ + timeout = new_reltimer(&daemon->timers, conn, + time_from_sec(daemon->timeout_secs), + conn_timeout, conn); /*~ The crypto handshake differs depending on whether you received or * initiated the socket connection, so there are two entry points. @@ -603,7 +605,7 @@ static struct io_plan *conn_in(struct io_conn *conn, * code from thinking `conn` (which we don't keep a pointer to) is * leaked */ return responder_handshake(notleak(conn), &daemon->mykey, - &conn_in_arg->addr, + &conn_in_arg->addr, timeout, handshake_in_success, daemon); } @@ -723,6 +725,7 @@ static struct io_plan *handshake_out_success(struct io_conn *conn, const struct pubkey *key, const struct wireaddr_internal *addr, struct crypto_state *cs, + struct oneshot *timeout, struct connecting *connect) { struct node_id id; @@ -732,12 +735,13 @@ static struct io_plan *handshake_out_success(struct io_conn *conn, status_peer_debug(&id, "Connect OUT"); return peer_exchange_initmsg(conn, connect->daemon, connect->daemon->our_features, - cs, &id, addr, false); + cs, &id, addr, timeout, false); } struct io_plan *connection_out(struct io_conn *conn, struct connecting *connect) { struct pubkey outkey; + struct oneshot *timeout; /* This shouldn't happen: lightningd should not give invalid ids! */ if (!pubkey_from_node_id(&outkey, &connect->id)) { @@ -748,15 +752,15 @@ struct io_plan *connection_out(struct io_conn *conn, struct connecting *connect) } /* If they don't complete handshake in reasonable time, hang up */ - notleak(new_reltimer(&connect->daemon->timers, conn, - time_from_sec(connect->daemon->timeout_secs), - conn_timeout, conn)); + timeout = new_reltimer(&connect->daemon->timers, conn, + time_from_sec(connect->daemon->timeout_secs), + conn_timeout, conn); status_peer_debug(&connect->id, "Connected out, starting crypto"); connect->connstate = "Cryptographic handshake"; return initiator_handshake(conn, &connect->daemon->mykey, &outkey, &connect->addrs[connect->addrnum], - handshake_out_success, connect); + timeout, handshake_out_success, connect); } /*~ When we've exhausted all addresses without success, we come here. diff --git a/connectd/handshake.c b/connectd/handshake.c index 768b4a2a6..1097f6f3d 100644 --- a/connectd/handshake.c +++ b/connectd/handshake.c @@ -173,11 +173,15 @@ struct handshake { /* Are we initiator or responder. */ enum bolt8_side side; + /* Timeout timer if we take too long. */ + struct oneshot *timeout; + /* Function to call once handshake complete. */ struct io_plan *(*cb)(struct io_conn *conn, const struct pubkey *their_id, const struct wireaddr_internal *wireaddr, struct crypto_state *cs, + struct oneshot *timeout, void *cbarg); void *cbarg; }; @@ -348,10 +352,12 @@ static struct io_plan *handshake_succeeded(struct io_conn *conn, const struct pubkey *their_id, const struct wireaddr_internal *addr, struct crypto_state *cs, + struct oneshot *timeout, void *cbarg); void *cbarg; struct pubkey their_id; struct wireaddr_internal addr; + struct oneshot *timeout; /* BOLT #8: * @@ -377,9 +383,10 @@ static struct io_plan *handshake_succeeded(struct io_conn *conn, cbarg = h->cbarg; their_id = h->their_id; addr = h->addr; + timeout = h->timeout; tal_free(h); - return cb(conn, &their_id, &addr, &cs, cbarg); + return cb(conn, &their_id, &addr, &cs, timeout, cbarg); } static struct handshake *new_handshake(const tal_t *ctx, @@ -956,10 +963,12 @@ static struct io_plan *act_one_responder(struct io_conn *conn, struct io_plan *responder_handshake_(struct io_conn *conn, const struct pubkey *my_id, const struct wireaddr_internal *addr, + struct oneshot *timeout, struct io_plan *(*cb)(struct io_conn *, const struct pubkey *, const struct wireaddr_internal *, struct crypto_state *, + struct oneshot *, void *cbarg), void *cbarg) { @@ -970,6 +979,7 @@ struct io_plan *responder_handshake_(struct io_conn *conn, h->addr = *addr; h->cbarg = cbarg; h->cb = cb; + h->timeout = timeout; return act_one_responder(conn, h); } @@ -978,10 +988,12 @@ struct io_plan *initiator_handshake_(struct io_conn *conn, const struct pubkey *my_id, const struct pubkey *their_id, const struct wireaddr_internal *addr, + struct oneshot *timeout, struct io_plan *(*cb)(struct io_conn *, const struct pubkey *, const struct wireaddr_internal *, struct crypto_state *, + struct oneshot *timeout, void *cbarg), void *cbarg) { @@ -993,6 +1005,7 @@ struct io_plan *initiator_handshake_(struct io_conn *conn, h->addr = *addr; h->cbarg = cbarg; h->cb = cb; + h->timeout = timeout; return act_one_initiator(conn, h); } diff --git a/connectd/handshake.h b/connectd/handshake.h index 733178bee..facb7f203 100644 --- a/connectd/handshake.h +++ b/connectd/handshake.h @@ -6,15 +6,17 @@ struct crypto_state; struct io_conn; struct wireaddr_internal; struct pubkey; +struct oneshot; -#define initiator_handshake(conn, my_id, their_id, addr, cb, cbarg) \ - initiator_handshake_((conn), (my_id), (their_id), (addr), \ +#define initiator_handshake(conn, my_id, their_id, addr, timeout, cb, cbarg) \ + initiator_handshake_((conn), (my_id), (their_id), (addr), (timeout), \ typesafe_cb_preargs(struct io_plan *, void *, \ (cb), (cbarg), \ struct io_conn *, \ const struct pubkey *, \ - const struct wireaddr_internal *, \ - struct crypto_state *), \ + const struct wireaddr_internal *, \ + struct crypto_state *, \ + struct oneshot *), \ (cbarg)) @@ -22,31 +24,36 @@ struct io_plan *initiator_handshake_(struct io_conn *conn, const struct pubkey *my_id, const struct pubkey *their_id, const struct wireaddr_internal *addr, + struct oneshot *timeout, struct io_plan *(*cb)(struct io_conn *, const struct pubkey *, const struct wireaddr_internal *, struct crypto_state *, + struct oneshot *timeout, void *cbarg), void *cbarg); -#define responder_handshake(conn, my_id, addr, cb, cbarg) \ - responder_handshake_((conn), (my_id), (addr), \ +#define responder_handshake(conn, my_id, addr, timeout, cb, cbarg) \ + responder_handshake_((conn), (my_id), (addr), (timeout), \ typesafe_cb_preargs(struct io_plan *, void *, \ (cb), (cbarg), \ struct io_conn *, \ const struct pubkey *, \ - const struct wireaddr_internal *, \ - struct crypto_state *), \ + const struct wireaddr_internal *, \ + struct crypto_state *, \ + struct oneshot *), \ (cbarg)) struct io_plan *responder_handshake_(struct io_conn *conn, const struct pubkey *my_id, const struct wireaddr_internal *addr, + struct oneshot *timeout, struct io_plan *(*cb)(struct io_conn *, const struct pubkey *, const struct wireaddr_internal *, struct crypto_state *, + struct oneshot *, void *cbarg), void *cbarg); #endif /* LIGHTNING_CONNECTD_HANDSHAKE_H */ diff --git a/connectd/peer_exchange_initmsg.c b/connectd/peer_exchange_initmsg.c index 8110f5a29..8106ab926 100644 --- a/connectd/peer_exchange_initmsg.c +++ b/connectd/peer_exchange_initmsg.c @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -151,6 +152,7 @@ struct io_plan *peer_exchange_initmsg(struct io_conn *conn, const struct crypto_state *cs, const struct node_id *id, const struct wireaddr_internal *addr, + struct oneshot *timeout, bool incoming) { /* If conn is closed, forget peer */ @@ -164,6 +166,9 @@ struct io_plan *peer_exchange_initmsg(struct io_conn *conn, peer->cs = *cs; peer->incoming = incoming; + /* Attach timer to early peer, so it gets freed with it. */ + notleak(tal_steal(peer, timeout)); + /* BOLT #1: * * The sending node: diff --git a/connectd/peer_exchange_initmsg.h b/connectd/peer_exchange_initmsg.h index 6a3c8a243..eb654aaa7 100644 --- a/connectd/peer_exchange_initmsg.h +++ b/connectd/peer_exchange_initmsg.h @@ -8,6 +8,7 @@ struct daemon; struct io_conn; struct node_id; struct wireaddr_internal; +struct oneshot; /* If successful, calls peer_connected() */ struct io_plan *peer_exchange_initmsg(struct io_conn *conn, @@ -16,6 +17,7 @@ struct io_plan *peer_exchange_initmsg(struct io_conn *conn, const struct crypto_state *cs, const struct node_id *id, const struct wireaddr_internal *addr, + struct oneshot *timeout, bool incoming); #endif /* LIGHTNING_CONNECTD_PEER_EXCHANGE_INITMSG_H */ diff --git a/connectd/test/run-initiator-success.c b/connectd/test/run-initiator-success.c index 27bb98bc9..73e35875f 100644 --- a/connectd/test/run-initiator-success.c +++ b/connectd/test/run-initiator-success.c @@ -278,6 +278,7 @@ static struct io_plan *success(struct io_conn *conn UNUSED, const struct pubkey *them, const struct wireaddr_internal *addr UNUSED, struct crypto_state *cs, + struct oneshot *timeout UNUSED, void *unused UNUSED) { assert(pubkey_eq(them, &rs_pub)); @@ -320,7 +321,7 @@ int main(int argc, char *argv[]) dummy.itype = ADDR_INTERNAL_WIREADDR; dummy.u.wireaddr.addrlen = 0; - initiator_handshake((void *)tmpctx, &ls_pub, &rs_pub, &dummy, success, NULL); + initiator_handshake((void *)tmpctx, &ls_pub, &rs_pub, &dummy, NULL, success, NULL); /* Should not exit! */ abort(); } diff --git a/connectd/test/run-responder-success.c b/connectd/test/run-responder-success.c index 1bf317e3f..b5de1da5a 100644 --- a/connectd/test/run-responder-success.c +++ b/connectd/test/run-responder-success.c @@ -277,6 +277,7 @@ static struct io_plan *success(struct io_conn *conn UNUSED, const struct pubkey *them UNUSED, const struct wireaddr_internal *addr UNUSED, struct crypto_state *cs, + struct oneshot *timeout UNUSED, void *unused UNUSED) { assert(secret_eq_str(&cs->sk, expect_sk)); @@ -314,7 +315,7 @@ int main(int argc, char *argv[]) dummy.itype = ADDR_INTERNAL_WIREADDR; dummy.u.wireaddr.addrlen = 0; - responder_handshake((void *)tmpctx, &ls_pub, &dummy, success, NULL); + responder_handshake((void *)tmpctx, &ls_pub, &dummy, NULL, success, NULL); /* Should not exit! */ abort(); } diff --git a/devtools/gossipwith.c b/devtools/gossipwith.c index 0e2093e67..66956ca66 100644 --- a/devtools/gossipwith.c +++ b/devtools/gossipwith.c @@ -144,6 +144,7 @@ static struct io_plan *handshake_success(struct io_conn *conn, const struct pubkey *them, const struct wireaddr_internal *addr, struct crypto_state *orig_cs, + struct oneshot *timer, char **args) { u8 *msg; @@ -351,7 +352,8 @@ int main(int argc, char *argv[]) if (connect(conn->fd, ai->ai_addr, ai->ai_addrlen) != 0) err(1, "Connecting to %s", at+1); - initiator_handshake(conn, &us, &them, &addr, handshake_success, argv+2); + initiator_handshake(conn, &us, &them, &addr, NULL, + handshake_success, argv+2); exit(0); }