Browse Source

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 <rusty@rustcorp.com.au>
ppa-0.6.1
Rusty Russell 7 years ago
committed by Christian Decker
parent
commit
e76a0b4ddc
  1. 7
      common/cryptomsg.c
  2. 4
      common/cryptomsg.h
  3. 12
      gossipd/gossip.c

7
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); 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. */ /* We read in two parts, so we might have started body. */
bool peer_in_started(const struct io_conn *conn, bool peer_in_started(const struct io_conn *conn,
const struct peer_crypto_state *cs) const struct peer_crypto_state *cs)

4
common/cryptomsg.h

@ -33,9 +33,7 @@ struct io_plan *peer_read_message(struct io_conn *conn,
struct peer *, struct peer *,
u8 *msg)); u8 *msg));
/* Have we already started writing/reading a message? */ /* Have we already started reading a message? */
bool peer_out_started(const struct io_conn *conn,
const struct peer_crypto_state *cs);
bool peer_in_started(const struct io_conn *conn, bool peer_in_started(const struct io_conn *conn,
const struct peer_crypto_state *cs); const struct peer_crypto_state *cs);

12
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) { if (peer->local->return_to_master) {
assert(!peer_in_started(conn, &peer->local->pcs)); assert(!peer_in_started(conn, &peer->local->pcs));
if (!peer_out_started(conn, &peer->local->pcs)) /* Wake writer. */
return ready_for_master(conn, peer); msg_wake(&peer->local->peer_out);
return io_wait(conn, peer, peer_next_in, peer); 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? */ /* Do we want to send this peer to the master daemon? */
if (peer->local->return_to_master) { if (peer->local->return_to_master) {
assert(!peer_out_started(conn, &peer->local->pcs));
if (!peer_in_started(conn, &peer->local->pcs)) if (!peer_in_started(conn, &peer->local->pcs))
return ready_for_master(conn, peer); return ready_for_master(conn, peer);
return io_out_wait(conn, peer, peer_pkt_out, peer); } else if (peer->gossip_sync) {
} /* If we're supposed to be sending gossip, do so now. */
/* If we're supposed to be sending gossip, do so now. */
if (peer->gossip_sync) {
struct queued_message *next; struct queued_message *next;
next = next_broadcast_message(peer->daemon->rstate->broadcasts, next = next_broadcast_message(peer->daemon->rstate->broadcasts,

Loading…
Cancel
Save