From 662dfef436a3a1738f0ffe4be3b66e40a31644bc Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 22 May 2017 20:56:51 +0930 Subject: [PATCH] lightningd/gossip: Move INIT message handling to handshake daemon. We need to do this on every connection, whether reconnecting or not, so it makes sense for the handshake daemon to handle it and return the feature fields. Longer term I'm considering having the handshake daemon handle the listening and connecting, and simply hand the fds back once the peers are ready. Signed-off-by: Rusty Russell --- lightningd/gossip/gossip.c | 78 +------------------------ lightningd/gossip/gossip_wire.csv | 4 -- lightningd/gossip_control.c | 23 -------- lightningd/handshake/handshake.c | 44 +++++++++++++- lightningd/handshake/handshake_wire.csv | 11 +++- lightningd/new_connection.c | 64 +++++++++++++++++++- tests/test_lightningd.py | 6 +- 7 files changed, 120 insertions(+), 110 deletions(-) diff --git a/lightningd/gossip/gossip.c b/lightningd/gossip/gossip.c index e290f0d12..c16ad81e5 100644 --- a/lightningd/gossip/gossip.c +++ b/lightningd/gossip/gossip.c @@ -229,10 +229,6 @@ static struct io_plan *peer_msgin(struct io_conn *conn, handle_gossip_msg(peer->daemon->rstate, msg); return peer_read_message(conn, &peer->pcs, peer_msgin); - case WIRE_INIT: - peer->error = "Duplicate INIT message received"; - return io_close(conn); - case WIRE_PING: if (!handle_ping(peer, msg)) return io_close(conn); @@ -258,6 +254,7 @@ static struct io_plan *peer_msgin(struct io_conn *conn, case WIRE_UPDATE_FAIL_MALFORMED_HTLC: case WIRE_COMMITMENT_SIGNED: case WIRE_REVOKE_AND_ACK: + case WIRE_INIT: /* Not our place to handle this, so we punt */ s = towire_gossipstatus_peer_nongossip(msg, peer->unique_id, &peer->pcs.cs, msg); @@ -322,19 +319,6 @@ static struct io_plan *peer_pkt_out(struct io_conn *conn, struct peer *peer) return msg_queue_wait(conn, &peer->peer_out, peer_pkt_out, peer); } -static bool has_even_bit(const u8 *bitmap) -{ - size_t len = tal_count(bitmap); - - while (len) { - if (*bitmap & 0xAA) - return true; - len--; - bitmap++; - } - return false; -} - /** * owner_msg_in - Called by the `peer->owner_conn` upon receiving a * message @@ -380,43 +364,8 @@ static struct io_plan *nonlocal_dump_gossip(struct io_conn *conn, struct daemon_ } } -static struct io_plan *peer_parse_init(struct io_conn *conn, - struct peer *peer, u8 *msg) +static struct io_plan *peer_start_gossip(struct io_conn *conn, struct peer *peer) { - u8 *gfeatures, *lfeatures; - - if (!fromwire_init(msg, msg, NULL, &gfeatures, &lfeatures)) { - peer->error = tal_fmt(msg, "Bad init: %s", tal_hex(msg, msg)); - return io_close(conn); - } - - /* BOLT #1: - * - * The receiving node MUST fail the channels if it receives a - * `globalfeatures` or `localfeatures` with an even bit set which it - * does not understand. - */ - if (has_even_bit(gfeatures)) { - peer->error = tal_fmt(msg, "Bad globalfeatures: %s", - tal_hex(msg, gfeatures)); - return io_close(conn); - } - - if (has_even_bit(lfeatures)) { - peer->error = tal_fmt(msg, "Bad localfeatures: %s", - tal_hex(msg, lfeatures)); - return io_close(conn); - } - - /* BOLT #1: - * - * Each node MUST wait to receive `init` before sending any other - * messages. - */ - daemon_conn_send(&peer->daemon->master, - take(towire_gossipstatus_peer_ready(msg, - peer->unique_id))); - /* Need to go duplex here, otherwise backpressure would mean * we both wait indefinitely */ return io_duplex(conn, @@ -424,29 +373,9 @@ static struct io_plan *peer_parse_init(struct io_conn *conn, peer_pkt_out(conn, peer)); } -static struct io_plan *peer_init_sent(struct io_conn *conn, struct peer *peer) -{ - return peer_read_message(conn, &peer->pcs, peer_parse_init); -} - -static struct io_plan *peer_send_init(struct io_conn *conn, struct peer *peer) -{ - /* BOLT #1: - * - * The sending node SHOULD use the minimum lengths required to - * represent the feature fields. The sending node MUST set feature - * bits corresponding to features it requires the peer to support, and - * SHOULD set feature bits corresponding to features it optionally - * supports. - */ - return peer_write_message(conn, &peer->pcs, - take(towire_init(peer, NULL, NULL)), - peer_init_sent); -} - static struct io_plan *new_peer_got_fd(struct io_conn *conn, struct peer *peer) { - peer->conn = io_new_conn(conn, peer->fd, peer_send_init, peer); + peer->conn = io_new_conn(conn, peer->fd, peer_start_gossip, peer); if (!peer->conn) { peer->error = "Could not create connection"; tal_free(peer); @@ -716,7 +645,6 @@ static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master case WIRE_GOSSIPSTATUS_FDPASS_FAILED: case WIRE_GOSSIPSTATUS_PEER_BAD_MSG: case WIRE_GOSSIPSTATUS_PEER_FAILED: - case WIRE_GOSSIPSTATUS_PEER_READY: case WIRE_GOSSIPSTATUS_PEER_NONGOSSIP: break; } diff --git a/lightningd/gossip/gossip_wire.csv b/lightningd/gossip/gossip_wire.csv index 4b44384bb..db487a70c 100644 --- a/lightningd/gossip/gossip_wire.csv +++ b/lightningd/gossip/gossip_wire.csv @@ -46,10 +46,6 @@ gossipctl_release_peer_reply,8,crypto_state,struct crypto_state #gossipstatus_peer_features,10+gflen,lflen,2 #gossipstatus_peer_features,12+gflen,localfeatures,lflen -# Peer init handshake complete (now you can release_peer if you want) -gossipstatus_peer_ready,3 -gossipstatus_peer_ready,0,unique_id,8 - # Peer can send non-gossip packet (usually an open_channel) (followed two fds: peer and gossip) gossipstatus_peer_nongossip,4 gossipstatus_peer_nongossip,0,unique_id,8 diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index 3e65a75e4..6041cdccd 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -95,26 +95,6 @@ static void peer_nongossip(struct subd *gossip, const u8 *msg, peer_accept_open(peer, &cs, inner); } -static void peer_ready(struct subd *gossip, const u8 *msg) -{ - u64 unique_id; - struct peer *peer; - - if (!fromwire_gossipstatus_peer_ready(msg, NULL, &unique_id)) - fatal("Gossip gave bad PEER_READY message %s", - tal_hex(msg, msg)); - - peer = peer_by_unique_id(gossip->ld, unique_id); - if (!peer) - fatal("Gossip gave bad peerid %"PRIu64, unique_id); - - log_debug(gossip->log, "Peer %s (%"PRIu64") ready for channel open", - type_to_string(msg, struct pubkey, peer->id), - unique_id); - - peer_set_condition(peer, GOSSIPING); -} - static int gossip_msg(struct subd *gossip, const u8 *msg, const int *fds) { enum gossip_wire_type t = fromwire_peektype(msg); @@ -155,9 +135,6 @@ static int gossip_msg(struct subd *gossip, const u8 *msg, const int *fds) return 2; peer_nongossip(gossip, msg, fds[0], fds[1]); break; - case WIRE_GOSSIPSTATUS_PEER_READY: - peer_ready(gossip, msg); - break; } return 0; } diff --git a/lightningd/handshake/handshake.c b/lightningd/handshake/handshake.c index 3f8aed0f9..e35d61e80 100644 --- a/lightningd/handshake/handshake.c +++ b/lightningd/handshake/handshake.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -20,6 +21,7 @@ #include #include #include +#include #include #include @@ -956,6 +958,36 @@ static void responder(int fd, } #ifndef TESTING +static void exchange_init(int fd, struct crypto_state *cs, + u8 **gfeatures, u8 **lfeatures) +{ + /* BOLT #1: + * + * The sending node SHOULD use the minimum lengths required to + * represent the feature fields. The sending node MUST set feature + * bits corresponding to features it requires the peer to support, and + * SHOULD set feature bits corresponding to features it optionally + * supports. + */ + u8 *msg = towire_init(NULL, NULL, NULL); + + if (!sync_crypto_write(cs, fd, msg)) + status_failed(WIRE_INITMSG_WRITE_FAILED, "%s", strerror(errno)); + + /* BOLT #1: + * + * Each node MUST wait to receive `init` before sending any other + * messages. + */ + msg = sync_crypto_read(NULL, cs, fd); + if (!msg) + status_failed(WIRE_INITMSG_READ_FAILED, "%s", strerror(errno)); + + if (!fromwire_init(msg, msg, NULL, gfeatures, lfeatures)) + status_failed(WIRE_INITMSG_READ_FAILED, "bad init: %s", + tal_hex(msg, msg)); +} + /* We expect hsmfd as fd 3, clientfd as 4 */ int main(int argc, char *argv[]) { @@ -964,6 +996,7 @@ int main(int argc, char *argv[]) int hsmfd = 3, clientfd = 4; struct secret ck, rk, sk; struct crypto_state cs; + u8 *gfeatures, *lfeatures; if (argc == 2 && streq(argv[1], "--version")) { printf("%s\n", version()); @@ -983,14 +1016,18 @@ int main(int argc, char *argv[]) if (fromwire_handshake_responder(msg, NULL, &my_id)) { responder(clientfd, &my_id, &their_id, &ck, &sk, &rk); + cs.rn = cs.sn = 0; cs.sk = sk; cs.rk = rk; cs.r_ck = cs.s_ck = ck; + exchange_init(clientfd, &cs, &gfeatures, &lfeatures); wire_sync_write(REQ_FD, towire_handshake_responder_reply(msg, &their_id, - &cs)); + &cs, + gfeatures, + lfeatures)); } else if (fromwire_handshake_initiator(msg, NULL, &my_id, &their_id)) { initiator(clientfd, &my_id, &their_id, &ck, &sk, &rk); @@ -998,8 +1035,11 @@ int main(int argc, char *argv[]) cs.sk = sk; cs.rk = rk; cs.r_ck = cs.s_ck = ck; + exchange_init(clientfd, &cs, &gfeatures, &lfeatures); wire_sync_write(REQ_FD, - towire_handshake_initiator_reply(msg, &cs)); + towire_handshake_initiator_reply(msg, &cs, + gfeatures, + lfeatures)); } else status_failed(WIRE_HANDSHAKE_BAD_COMMAND, "%i", fromwire_peektype(msg)); diff --git a/lightningd/handshake/handshake_wire.csv b/lightningd/handshake/handshake_wire.csv index 8937a26bb..c8b3100b0 100644 --- a/lightningd/handshake/handshake_wire.csv +++ b/lightningd/handshake/handshake_wire.csv @@ -22,6 +22,8 @@ respr_act3_bad_ciphertext,0x8023 respr_act3_bad_pubkey,0x8024 respr_act3_bad_ecdh_for_ss,0x8025 respr_act3_bad_tag,0x8026 +initmsg_write_failed,0x8030 +initmsg_read_failed,0x8031 # FIXME: This is probably too finegrained. initr_act_one,1001 @@ -37,6 +39,10 @@ handshake_responder,1,my_id,33 handshake_responder_reply,101 handshake_responder_reply,0,initiator_id,33 handshake_responder_reply,33,cs,struct crypto_state +handshake_responder_reply,0,gflen,2 +handshake_responder_reply,0,globalfeatures,gflen +handshake_responder_reply,0,lflen,2 +handshake_responder_reply,0,localfeatures,lflen handshake_initiator,2 handshake_initiator,0,my_id,33 @@ -44,4 +50,7 @@ handshake_initiator,33,responder_id,33 handshake_initiator_reply,102 handshake_initiator_reply,0,cs,struct crypto_state - +handshake_initiator_reply,0,gflen,2 +handshake_initiator_reply,0,globalfeatures,gflen +handshake_initiator_reply,0,lflen,2 +handshake_initiator_reply,0,localfeatures,lflen diff --git a/lightningd/new_connection.c b/lightningd/new_connection.c index d2eba741d..4b2b32610 100644 --- a/lightningd/new_connection.c +++ b/lightningd/new_connection.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -37,6 +38,25 @@ static void connection_destroy(struct connection *c) command_fail(c->cmd, "Failed to connect to peer"); } +static void +PRINTF_FMT(3,4) connection_failed(struct connection *c, struct log *log, + const char *fmt, ...) +{ + const char *msg; + va_list ap; + + va_start(ap, fmt); + msg = tal_vfmt(c, fmt, ap); + va_end(ap); + log_info(log, "%s", msg); + if (c->cmd) { + command_fail(c->cmd, "%s", msg); + /* Don't fail in destructor, too. */ + c->cmd = NULL; + } + tal_free(c); +} + struct connection *new_connection(const tal_t *ctx, struct lightningd *ld, struct command *cmd, @@ -58,12 +78,26 @@ struct connection *new_connection(const tal_t *ctx, return c; } +static bool has_even_bit(const u8 *bitmap) +{ + size_t len = tal_count(bitmap); + + while (len) { + if (*bitmap & 0xAA) + return true; + len--; + bitmap++; + } + return false; +} + static bool handshake_succeeded(struct subd *handshaked, const u8 *msg, const int *fds, struct connection *c) { struct crypto_state cs; struct pubkey *id; + u8 *globalfeatures, *localfeatures; assert(tal_count(fds) == 1); @@ -71,18 +105,44 @@ static bool handshake_succeeded(struct subd *handshaked, if (!c->known_id) { id = tal(msg, struct pubkey); - if (!fromwire_handshake_responder_reply(msg, NULL, id, &cs)) + if (!fromwire_handshake_responder_reply(c, msg, NULL, id, &cs, + &globalfeatures, + &localfeatures)) goto err; log_info_struct(handshaked->log, "Peer in from %s", struct pubkey, id); } else { id = c->known_id; - if (!fromwire_handshake_initiator_reply(msg, NULL, &cs)) + if (!fromwire_handshake_initiator_reply(c, msg, NULL, &cs, + &globalfeatures, + &localfeatures)) goto err; log_info_struct(handshaked->log, "Peer out to %s", struct pubkey, id); } + /* BOLT #1: + * + * The receiving node MUST fail the channels if it receives a + * `globalfeatures` or `localfeatures` with an even bit set which it + * does not understand. + */ + if (has_even_bit(globalfeatures)) { + connection_failed(c, handshaked->log, + "peer %s: bad globalfeatures: %s", + type_to_string(c, struct pubkey, id), + tal_hex(msg, globalfeatures)); + return true; + } + + if (has_even_bit(localfeatures)) { + connection_failed(c, handshaked->log, + "peer %s: bad localfeatures: %s", + type_to_string(c, struct pubkey, id), + tal_hex(msg, localfeatures)); + return true; + } + if (c->cmd) { struct json_result *response; response = new_json_result(c->cmd); diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index ad5e3439f..c895df42d 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -163,8 +163,8 @@ class LightningDTests(BaseLightningDTests): assert ret['id'] == l2.info['id'] - l1.daemon.wait_for_log('WIRE_GOSSIPSTATUS_PEER_READY') - l2.daemon.wait_for_log('WIRE_GOSSIPSTATUS_PEER_READY') + l1.daemon.wait_for_log('WIRE_GOSSIPCTL_NEW_PEER') + l2.daemon.wait_for_log('WIRE_GOSSIPCTL_NEW_PEER') return l1,l2 def fund_channel(self,l1,l2,amount): @@ -398,7 +398,7 @@ class LightningDTests(BaseLightningDTests): assert ret['id'] == l3.info['id'] - l3.daemon.wait_for_log('WIRE_GOSSIPSTATUS_PEER_READY') + l3.daemon.wait_for_log('WIRE_GOSSIPCTL_NEW_PEER') self.fund_channel(l1, l2, 10**6) self.fund_channel(l2, l3, 10**6)