diff --git a/connectd/connectd.c b/connectd/connectd.c index 6685c471f9b4..343a59ec4ad0 100644 --- a/connectd/connectd.c +++ b/connectd/connectd.c @@ -113,7 +113,7 @@ static void destroy_peer(struct peer *peer) static struct peer *new_peer(struct daemon *daemon, const struct node_id *id, const struct crypto_state *cs, - const u8 *their_features, + const u8 *their_features TAKES, enum is_websocket is_websocket, struct timemono connect_starttime, struct io_conn *conn STEALS, @@ -154,6 +154,7 @@ static struct peer *new_peer(struct daemon *daemon, peer->onionmsg_incoming_tokens = ONION_MSG_TOKENS_MAX; peer->onionmsg_last_incoming = time_mono(); peer->onionmsg_limit_warned = false; + peer->their_features = tal_dup_talarr(peer, u8, their_features); peer->to_peer = conn; @@ -320,10 +321,6 @@ struct io_plan *peer_connected(struct io_conn *conn, destroy_peer_immediately(oldpeer); } - /* We promised we'd take it by marking it TAKEN above; prepare to free it. */ - if (taken(their_features)) - tal_steal(tmpctx, their_features); - /* BOLT #1: * * The receiving node: @@ -336,6 +333,9 @@ struct io_plan *peer_connected(struct io_conn *conn, unsup = features_unsupported(daemon->our_features, their_features, INIT_FEATURE); if (unsup != -1) { + if (taken(their_features)) + tal_free(their_features); + /* We were going to send a reconnect message, but not now! */ if (oldpeer) send_disconnected(daemon, id, prev_connectd_counter, @@ -348,6 +348,9 @@ struct io_plan *peer_connected(struct io_conn *conn, } if (!feature_check_depends(their_features, &depender, &missing)) { + if (taken(their_features)) + tal_free(their_features); + /* We were going to send a reconnect message, but not now! */ if (oldpeer) send_disconnected(daemon, id, prev_connectd_counter, @@ -407,11 +410,11 @@ struct io_plan *peer_connected(struct io_conn *conn, /* Tell gossipd it can ask query this new peer for gossip */ option_gossip_queries = feature_negotiated(daemon->our_features, - their_features, + peer->their_features, OPT_GOSSIP_QUERIES); /* Get ready for streaming gossip from the store */ - setup_peer_gossip_store(peer, daemon->our_features, their_features); + setup_peer_gossip_store(peer, daemon->our_features, peer->their_features); /* Create message to tell master peer has connected/reconnected. */ if (oldpeer) { @@ -419,7 +422,7 @@ struct io_plan *peer_connected(struct io_conn *conn, prev_connectd_counter, peer->counter, addr, remote_addr, - incoming, their_features, + incoming, peer->their_features, time_to_nsec(timemono_since(prev_connect_start))); } else { /* Tell gossipd about new peer */ @@ -428,7 +431,7 @@ struct io_plan *peer_connected(struct io_conn *conn, msg = towire_connectd_peer_connected(NULL, id, peer->counter, addr, remote_addr, - incoming, their_features, + incoming, peer->their_features, connect_reason, connect_time_nsec); } @@ -1692,8 +1695,7 @@ static void connect_init(struct daemon *daemon, const u8 *msg) &daemon->dev_no_reconnect, &daemon->dev_fast_reconnect, &dev_limit_connections_inflight, - &daemon->dev_keep_nagle, - &daemon->dev_uniform_padding)) { + &daemon->dev_keep_nagle)) { /* This is a helper which prints the type expected and the actual * message, then exits (it should never be called!). */ master_badmsg(WIRE_CONNECTD_INIT, msg); @@ -2529,7 +2531,6 @@ int main(int argc, char *argv[]) daemon->dev_exhausted_fds = false; daemon->dev_lightningd_is_slow = false; daemon->dev_keep_nagle = false; - daemon->dev_uniform_padding = false; /* We generally allow 1MB per second per peer, except for dev testing */ daemon->gossip_stream_limit = 1000000; daemon->incoming_stream_limit = 1000000; diff --git a/connectd/connectd.h b/connectd/connectd.h index 25d254839075..d3885c176022 100644 --- a/connectd/connectd.h +++ b/connectd/connectd.h @@ -67,6 +67,8 @@ struct peer { struct crypto_state cs; /* Time when we first connected */ struct timemono connect_starttime; + /* Features they told us about */ + const u8 *their_features; /* Connection to the peer (NULL if it's disconnected and we're flushing) */ struct io_conn *to_peer; @@ -318,9 +320,6 @@ struct daemon { /* Allow localhost to be considered "public", only with --developer */ bool dev_allow_localhost; - /* Pad outgoing messages to uniform 1460-byte segments (traffic analysis defence) */ - bool dev_uniform_padding; - /* How much to gossip allow a peer every second (bytes) */ size_t gossip_stream_limit; diff --git a/connectd/connectd_wire.csv b/connectd/connectd_wire.csv index 700e68296e23..d5f6aee33b01 100644 --- a/connectd/connectd_wire.csv +++ b/connectd/connectd_wire.csv @@ -29,8 +29,6 @@ msgdata,connectd_init,dev_no_reconnect,bool, msgdata,connectd_init,dev_fast_reconnect,bool, msgdata,connectd_init,dev_limit_connections_inflight,bool, msgdata,connectd_init,dev_keep_nagle,bool, -# Pad outgoing messages to uniform 1460-byte segments (traffic analysis defence) -msgdata,connectd_init,dev_uniform_padding,bool, # Connectd->master, here are the addresses I bound, can announce. msgtype,connectd_init_reply,2100 diff --git a/connectd/multiplex.c b/connectd/multiplex.c index 8cfaa8852105..48df6401aec3 100644 --- a/connectd/multiplex.c +++ b/connectd/multiplex.c @@ -481,14 +481,22 @@ static bool have_empty_encrypted_queue(const struct peer *peer) return membuf_num_elems(&peer->encrypted_peer_out) == 0; } +/* Funny story: we discovered an LND bug, where they hung up if we sent + * "no reply" ping messages. This was fixed in (the upcoming) v21, which + * Laolu pointed out also supports onion messages. Hence we use that + * to detect if we should pad packets. */ +static bool use_uniform_writes(const struct peer *peer) +{ + return feature_offered(peer->their_features, OPT_ONION_MESSAGES); +} + /* (Continue) writing the encrypted_peer_out array */ static struct io_plan *write_encrypted_to_peer(struct peer *peer) { size_t avail = membuf_num_elems(&peer->encrypted_peer_out); /* With padding: always a full uniform-size chunk. * Without: flush whatever we have (caller ensures non-zero). */ - size_t write_size = peer->daemon->dev_uniform_padding - ? UNIFORM_MESSAGE_SIZE : avail; + size_t write_size = use_uniform_writes(peer) ? UNIFORM_MESSAGE_SIZE : avail; assert(avail >= write_size && write_size > 0); return io_write_partial(peer->to_peer, @@ -1250,8 +1258,8 @@ static struct io_plan *write_to_peer(struct io_conn *peer_conn, /* Wait for them to wake us */ return msg_queue_wait(peer_conn, peer->peer_outq, write_to_peer, peer); } - /* OK, add padding (only if --dev-uniform-padding enabled). */ - if (peer->daemon->dev_uniform_padding) + /* OK, add padding (only if supported). */ + if (use_uniform_writes(peer)) pad_encrypted_queue(peer); else break; diff --git a/lightningd/connect_control.c b/lightningd/connect_control.c index 58b3b44684f0..e95a04db9167 100644 --- a/lightningd/connect_control.c +++ b/lightningd/connect_control.c @@ -727,8 +727,7 @@ int connectd_init(struct lightningd *ld) !ld->reconnect, ld->dev_fast_reconnect, ld->dev_limit_connections_inflight, - ld->dev_keep_nagle, - ld->dev_uniform_padding); + ld->dev_keep_nagle); subd_req(ld->connectd, ld->connectd, take(msg), -1, 0, connect_init_done, NULL); diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 3a669e1b3102..0241b992da20 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -150,7 +150,6 @@ static struct lightningd *new_lightningd(const tal_t *ctx) ld->dev_strict_forwarding = false; ld->dev_limit_connections_inflight = false; ld->dev_keep_nagle = false; - ld->dev_uniform_padding = false; /*~ We try to ensure enough fds for twice the number of channels * we start with. We have a developer option to change that factor diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index 3a00f9f3e4be..9edba38d09fa 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -369,9 +369,6 @@ struct lightningd { /* Tell connectd we don't want TCP_NODELAY */ bool dev_keep_nagle; - /* Pad outgoing messages to uniform 1460-byte segments (traffic analysis defence) */ - bool dev_uniform_padding; - /* tor support */ struct wireaddr *proxyaddr; bool always_use_proxy; diff --git a/lightningd/options.c b/lightningd/options.c index 88148f3e6d12..d9ef8e445a0e 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -952,10 +952,6 @@ static void dev_register_opts(struct lightningd *ld) opt_set_bool, &ld->dev_keep_nagle, "Tell connectd not to set TCP_NODELAY."); - clnopt_noarg("--dev-uniform-padding", OPT_DEV, - opt_set_bool, - &ld->dev_uniform_padding, - "Pad all outgoing peer messages to uniform 1460-byte segments"); /* This is handled directly in daemon_developer_mode(), so we ignore it here */ clnopt_noarg("--dev-debug-self", OPT_DEV, opt_ignore, diff --git a/tests/test_connection.py b/tests/test_connection.py index db51651cc492..2aef856860ff 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -4823,7 +4823,7 @@ def test_constant_packet_size(node_factory, tcp_capture): Test that TCP packets between nodes are constant size. This will be skipped unless you can run `dumpcap` (usually means you have to be in the `wireshark` group). """ - l1, l2, l3, l4 = node_factory.get_nodes(4, opts={'dev-uniform-padding': None}) + l1, l2, l3, l4 = node_factory.get_nodes(4) # Encrypted setup BOLT 8 has some short packets. l1.connect(l2)