From e76a0b4ddcc44424358c0722011b1cf83fa59222 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 8 Feb 2018 13:51:41 +1030 Subject: [PATCH] gossipd: fix race where we can handoff peer with bad cryptostate. DEBUG:root:lightningd(16333): 2018-02-08T02:12:21.158Z lightningd(8262): lightning_openingd(0382ce59ebf18be7d84677c2e35f23294b9992ceca95491fcf8a56c6cb2d9de199): Failed hdr decrypt with rn=2 We only hand off the peer if we've not started writing, but that was insufficient: we increment the sn twice on encrypting packet, so there's a window before we've actually started writing where this is now wrong. The simplest fix is only to hand off from master when we've just written, and have the read-packet path simply wake the write-packet path. Signed-off-by: Rusty Russell --- common/cryptomsg.c | 7 ------- common/cryptomsg.h | 4 +--- gossipd/gossip.c | 12 ++++-------- 3 files changed, 5 insertions(+), 18 deletions(-) diff --git a/common/cryptomsg.c b/common/cryptomsg.c index d82af5887..7ab9b58db 100644 --- a/common/cryptomsg.c +++ b/common/cryptomsg.c @@ -382,13 +382,6 @@ struct io_plan *peer_write_message(struct io_conn *conn, return io_write(conn, pcs->out, tal_count(pcs->out), post, pcs); } -/* We write in one op, so it's all or nothing. */ -bool peer_out_started(const struct io_conn *conn, - const struct peer_crypto_state *cs) -{ - return io_plan_out_started(conn); -} - /* We read in two parts, so we might have started body. */ bool peer_in_started(const struct io_conn *conn, const struct peer_crypto_state *cs) diff --git a/common/cryptomsg.h b/common/cryptomsg.h index d9f1d2ca9..aea4a0833 100644 --- a/common/cryptomsg.h +++ b/common/cryptomsg.h @@ -33,9 +33,7 @@ struct io_plan *peer_read_message(struct io_conn *conn, struct peer *, u8 *msg)); -/* Have we already started writing/reading a message? */ -bool peer_out_started(const struct io_conn *conn, - const struct peer_crypto_state *cs); +/* Have we already started reading a message? */ bool peer_in_started(const struct io_conn *conn, const struct peer_crypto_state *cs); diff --git a/gossipd/gossip.c b/gossipd/gossip.c index 87d3b1142..31ecafd78 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -565,8 +565,8 @@ static struct io_plan *peer_next_in(struct io_conn *conn, struct peer *peer) { if (peer->local->return_to_master) { assert(!peer_in_started(conn, &peer->local->pcs)); - if (!peer_out_started(conn, &peer->local->pcs)) - return ready_for_master(conn, peer); + /* Wake writer. */ + msg_wake(&peer->local->peer_out); return io_wait(conn, peer, peer_next_in, peer); } @@ -683,14 +683,10 @@ static struct io_plan *peer_pkt_out(struct io_conn *conn, struct peer *peer) /* Do we want to send this peer to the master daemon? */ if (peer->local->return_to_master) { - assert(!peer_out_started(conn, &peer->local->pcs)); if (!peer_in_started(conn, &peer->local->pcs)) return ready_for_master(conn, peer); - return io_out_wait(conn, peer, peer_pkt_out, peer); - } - - /* If we're supposed to be sending gossip, do so now. */ - if (peer->gossip_sync) { + } else if (peer->gossip_sync) { + /* If we're supposed to be sending gossip, do so now. */ struct queued_message *next; next = next_broadcast_message(peer->daemon->rstate->broadcasts,