From 15f54878e45dca87b477f2d77d2d5e3c46eb8927 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 2 Apr 2020 14:33:47 +1030 Subject: [PATCH] connectd: do feature bits check after init exchange. This will help with the next patch, where we wean off using a global for features: connectd.c has access to the feature bits. Since connectd might now want to send a message, it needs the crypto_state non-const, which makes this less trivial than it would otherwise be. Signed-off-by: Rusty Russell --- connectd/connectd.c | 30 +++++++++++++++++++++------ connectd/connectd.h | 2 +- connectd/handshake.c | 8 +++---- connectd/handshake.h | 8 +++---- connectd/peer_exchange_initmsg.c | 18 ---------------- connectd/test/run-initiator-success.c | 2 +- connectd/test/run-responder-success.c | 2 +- devtools/gossipwith.c | 2 +- 8 files changed, 36 insertions(+), 36 deletions(-) diff --git a/connectd/connectd.c b/connectd/connectd.c index e389e032e..aec7bcb23 100644 --- a/connectd/connectd.c +++ b/connectd/connectd.c @@ -413,22 +413,40 @@ struct io_plan *peer_connected(struct io_conn *conn, struct daemon *daemon, const struct node_id *id, const struct wireaddr_internal *addr, - const struct crypto_state *cs, + struct crypto_state *cs, const u8 *features TAKES) { u8 *msg; struct per_peer_state *pps; + int unsup; if (node_set_get(&daemon->peers, id)) return peer_reconnected(conn, daemon, id, addr, cs, features); - /* We've successfully connected. */ - connected_to_peer(daemon, conn, id); - /* We promised we'd take it by marking it TAKEN above; prepare to free it. */ if (taken(features)) tal_steal(tmpctx, features); + /* BOLT #1: + * + * The receiving node: + * ... + * - upon receiving unknown _odd_ feature bits that are non-zero: + * - MUST ignore the bit. + * - upon receiving unknown _even_ feature bits that are non-zero: + * - MUST fail the connection. + */ + unsup = features_unsupported(features); + if (unsup != -1) { + msg = towire_errorfmt(NULL, NULL, "Unsupported feature %u", + unsup); + msg = cryptomsg_encrypt_msg(tmpctx, cs, take(msg)); + return io_write(conn, msg, tal_count(msg), io_close_cb, NULL); + } + + /* We've successfully connected. */ + connected_to_peer(daemon, conn, id); + /* This contains the per-peer state info; gossipd fills in pps->gs */ pps = new_per_peer_state(tmpctx, cs); @@ -466,7 +484,7 @@ struct io_plan *peer_connected(struct io_conn *conn, static struct io_plan *handshake_in_success(struct io_conn *conn, const struct pubkey *id_key, const struct wireaddr_internal *addr, - const struct crypto_state *cs, + struct crypto_state *cs, struct daemon *daemon) { struct node_id id; @@ -524,7 +542,7 @@ static struct io_plan *connection_in(struct io_conn *conn, struct daemon *daemon static struct io_plan *handshake_out_success(struct io_conn *conn, const struct pubkey *key, const struct wireaddr_internal *addr, - const struct crypto_state *cs, + struct crypto_state *cs, struct connecting *connect) { struct node_id id; diff --git a/connectd/connectd.h b/connectd/connectd.h index 8e16e3ab7..a66b768da 100644 --- a/connectd/connectd.h +++ b/connectd/connectd.h @@ -18,7 +18,7 @@ struct io_plan *peer_connected(struct io_conn *conn, struct daemon *daemon, const struct node_id *id, const struct wireaddr_internal *addr, - const struct crypto_state *cs, + struct crypto_state *cs, const u8 *features TAKES); #endif /* LIGHTNING_CONNECTD_CONNECTD_H */ diff --git a/connectd/handshake.c b/connectd/handshake.c index 66480c2bd..8cc1fd51b 100644 --- a/connectd/handshake.c +++ b/connectd/handshake.c @@ -181,7 +181,7 @@ struct handshake { struct io_plan *(*cb)(struct io_conn *conn, const struct pubkey *their_id, const struct wireaddr_internal *wireaddr, - const struct crypto_state *cs, + struct crypto_state *cs, void *cbarg); void *cbarg; }; @@ -351,7 +351,7 @@ static struct io_plan *handshake_succeeded(struct io_conn *conn, struct io_plan *(*cb)(struct io_conn *conn, const struct pubkey *their_id, const struct wireaddr_internal *addr, - const struct crypto_state *cs, + struct crypto_state *cs, void *cbarg); void *cbarg; struct pubkey their_id; @@ -968,7 +968,7 @@ struct io_plan *responder_handshake_(struct io_conn *conn, struct io_plan *(*cb)(struct io_conn *, const struct pubkey *, const struct wireaddr_internal *, - const struct crypto_state *, + struct crypto_state *, void *cbarg), void *cbarg) { @@ -990,7 +990,7 @@ struct io_plan *initiator_handshake_(struct io_conn *conn, struct io_plan *(*cb)(struct io_conn *, const struct pubkey *, const struct wireaddr_internal *, - const struct crypto_state *, + struct crypto_state *, void *cbarg), void *cbarg) { diff --git a/connectd/handshake.h b/connectd/handshake.h index 970782eac..1e860e090 100644 --- a/connectd/handshake.h +++ b/connectd/handshake.h @@ -16,7 +16,7 @@ struct pubkey; struct io_conn *, \ const struct pubkey *, \ const struct wireaddr_internal *, \ - const struct crypto_state *), \ + struct crypto_state *), \ (cbarg)) @@ -27,7 +27,7 @@ struct io_plan *initiator_handshake_(struct io_conn *conn, struct io_plan *(*cb)(struct io_conn *, const struct pubkey *, const struct wireaddr_internal *, - const struct crypto_state *, + struct crypto_state *, void *cbarg), void *cbarg); @@ -39,7 +39,7 @@ struct io_plan *initiator_handshake_(struct io_conn *conn, struct io_conn *, \ const struct pubkey *, \ const struct wireaddr_internal *, \ - const struct crypto_state *), \ + struct crypto_state *), \ (cbarg)) struct io_plan *responder_handshake_(struct io_conn *conn, @@ -48,7 +48,7 @@ struct io_plan *responder_handshake_(struct io_conn *conn, struct io_plan *(*cb)(struct io_conn *, const struct pubkey *, const struct wireaddr_internal *, - const struct crypto_state *, + struct crypto_state *, void *cbarg), void *cbarg); diff --git a/connectd/peer_exchange_initmsg.c b/connectd/peer_exchange_initmsg.c index 20c924424..a9c425873 100644 --- a/connectd/peer_exchange_initmsg.c +++ b/connectd/peer_exchange_initmsg.c @@ -44,7 +44,6 @@ static struct io_plan *peer_init_received(struct io_conn *conn, { u8 *msg = cryptomsg_decrypt_body(tmpctx, &peer->cs, peer->msg); u8 *globalfeatures, *features; - int unsup; struct tlv_init_tlvs *tlvs = tlv_init_tlvs_new(msg); if (!msg) @@ -89,23 +88,6 @@ static struct io_plan *peer_init_received(struct io_conn *conn, * window where it was: combine the two. */ features = featurebits_or(tmpctx, take(features), globalfeatures); - /* BOLT #1: - * - * The receiving node: - * ... - * - upon receiving unknown _odd_ feature bits that are non-zero: - * - MUST ignore the bit. - * - upon receiving unknown _even_ feature bits that are non-zero: - * - MUST fail the connection. - */ - unsup = features_unsupported(features); - if (unsup != -1) { - msg = towire_errorfmt(NULL, NULL, "Unsupported feature %u", - unsup); - msg = cryptomsg_encrypt_msg(NULL, &peer->cs, take(msg)); - return io_write(conn, msg, tal_count(msg), io_close_cb, NULL); - } - /* Usually return io_close_taken_fd, but may wait for old peer to * be disconnected if it's a reconnect. */ return peer_connected(conn, peer->daemon, &peer->id, diff --git a/connectd/test/run-initiator-success.c b/connectd/test/run-initiator-success.c index 96e102b50..a7fa6fa0a 100644 --- a/connectd/test/run-initiator-success.c +++ b/connectd/test/run-initiator-success.c @@ -218,7 +218,7 @@ static struct io_plan *test_read(struct io_conn *conn, static struct io_plan *success(struct io_conn *conn UNUSED, const struct pubkey *them, const struct wireaddr_internal *addr UNUSED, - const struct crypto_state *cs, + struct crypto_state *cs, void *unused UNUSED) { assert(pubkey_eq(them, &rs_pub)); diff --git a/connectd/test/run-responder-success.c b/connectd/test/run-responder-success.c index 5a6977140..c35400164 100644 --- a/connectd/test/run-responder-success.c +++ b/connectd/test/run-responder-success.c @@ -217,7 +217,7 @@ static struct io_plan *test_read(struct io_conn *conn, static struct io_plan *success(struct io_conn *conn UNUSED, const struct pubkey *them UNUSED, const struct wireaddr_internal *addr UNUSED, - const struct crypto_state *cs, + struct crypto_state *cs, void *unused UNUSED) { assert(secret_eq_str(&cs->sk, expect_sk)); diff --git a/devtools/gossipwith.c b/devtools/gossipwith.c index a0ce66f1f..1ad73cbc7 100644 --- a/devtools/gossipwith.c +++ b/devtools/gossipwith.c @@ -142,7 +142,7 @@ static struct io_plan *simple_read(struct io_conn *conn, static struct io_plan *handshake_success(struct io_conn *conn, const struct pubkey *them, const struct wireaddr_internal *addr, - const struct crypto_state *orig_cs, + struct crypto_state *orig_cs, char **args) { u8 *msg;