diff --git a/common/test/run-wireaddr.c b/common/test/run-wireaddr.c index 3602ae2c7057..c6e19d8b752b 100644 --- a/common/test/run-wireaddr.c +++ b/common/test/run-wireaddr.c @@ -112,39 +112,6 @@ void towire_u8_array(u8 **pptr UNNEEDED, const u8 *arr UNNEEDED, size_t num UNNE { fprintf(stderr, "towire_u8_array called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ -static bool wireaddr_internal_eq(const struct wireaddr_internal *a, - const struct wireaddr_internal *b, - bool cmp_torservice_blob) -{ - if (a->itype != b->itype) - return false; - - switch (a->itype) { - case ADDR_INTERNAL_SOCKNAME: - return streq(a->u.sockname, b->u.sockname); - case ADDR_INTERNAL_ALLPROTO: - return a->u.port == b->u.port; - case ADDR_INTERNAL_AUTOTOR: - case ADDR_INTERNAL_STATICTOR: - if (!wireaddr_eq(&a->u.torservice.address, - &b->u.torservice.address)) - return false; - if (a->u.torservice.port != b->u.torservice.port) - return false; - if (!cmp_torservice_blob) - return true; - return memeq(a->u.torservice.blob, sizeof(a->u.torservice.blob), - b->u.torservice.blob, sizeof(b->u.torservice.blob)); - case ADDR_INTERNAL_FORPROXY: - if (!streq(a->u.unresolved.name, b->u.unresolved.name)) - return false; - return a->u.unresolved.port == b->u.unresolved.port; - case ADDR_INTERNAL_WIREADDR: - return wireaddr_eq(&a->u.wireaddr, &b->u.wireaddr); - } - abort(); -} - int main(int argc, char *argv[]) { const char *err; @@ -155,87 +122,98 @@ int main(int argc, char *argv[]) assert(parse_wireaddr_internal("127.0.0.1", &addr, DEFAULT_PORT, false, false, false, false, &err)); expect->itype = ADDR_INTERNAL_WIREADDR; assert(parse_wireaddr("127.0.0.1:9735", &expect->u.wireaddr, 0, NULL, &err)); - assert(wireaddr_internal_eq(&addr, expect, false)); + assert(wireaddr_internal_eq(&addr, expect)); /* IPv4 address with port. */ assert(parse_wireaddr_internal("127.0.0.1:1", &addr, DEFAULT_PORT, false, false, false, false, &err)); expect->itype = ADDR_INTERNAL_WIREADDR; assert(parse_wireaddr("127.0.0.1:1", &expect->u.wireaddr, 0, NULL, &err)); - assert(wireaddr_internal_eq(&addr, expect, false)); + assert(wireaddr_internal_eq(&addr, expect)); /* Simple IPv6 address. */ assert(parse_wireaddr_internal("::1", &addr, DEFAULT_PORT, false, false, false, false, &err)); expect->itype = ADDR_INTERNAL_WIREADDR; assert(parse_wireaddr("::1", &expect->u.wireaddr, DEFAULT_PORT, NULL, &err)); - assert(wireaddr_internal_eq(&addr, expect, false)); + assert(wireaddr_internal_eq(&addr, expect)); /* IPv6 address with port. */ assert(parse_wireaddr_internal("[::1]:1", &addr, DEFAULT_PORT, false, false, false, false, &err)); expect->itype = ADDR_INTERNAL_WIREADDR; assert(parse_wireaddr("::1", &expect->u.wireaddr, 1, NULL, &err)); - assert(wireaddr_internal_eq(&addr, expect, false)); + assert(wireaddr_internal_eq(&addr, expect)); /* autotor address */ assert(parse_wireaddr_internal("autotor:127.0.0.1", &addr, DEFAULT_PORT, false, false, false, false, &err)); expect->itype = ADDR_INTERNAL_AUTOTOR; expect->u.torservice.port = DEFAULT_PORT; assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9051, NULL, &err)); - assert(wireaddr_internal_eq(&addr, expect, false)); + assert(wireaddr_internal_eq(&addr, expect)); /* autotor address with port */ assert(parse_wireaddr_internal("autotor:127.0.0.1:9055", &addr, DEFAULT_PORT, false, false, false, false, &err)); expect->itype = ADDR_INTERNAL_AUTOTOR; expect->u.torservice.port = DEFAULT_PORT; assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9055, NULL, &err)); - assert(wireaddr_internal_eq(&addr, expect, false)); + assert(wireaddr_internal_eq(&addr, expect)); /* autotor address with torport */ assert(parse_wireaddr_internal("autotor:127.0.0.1/torport=9055", &addr, DEFAULT_PORT, false, false, false, false, &err)); expect->itype = ADDR_INTERNAL_AUTOTOR; expect->u.torservice.port = 9055; assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9051, NULL, &err)); - assert(wireaddr_internal_eq(&addr, expect, false)); + assert(wireaddr_internal_eq(&addr, expect)); /* autotor address with port and torport */ assert(parse_wireaddr_internal("autotor:127.0.0.1:9055/torport=10055", &addr, DEFAULT_PORT, false, false, false, false, &err)); expect->itype = ADDR_INTERNAL_AUTOTOR; expect->u.torservice.port = 10055; assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9055, NULL, &err)); - assert(wireaddr_internal_eq(&addr, expect, false)); + assert(wireaddr_internal_eq(&addr, expect)); /* statictor address */ assert(parse_wireaddr_internal("statictor:127.0.0.1", &addr, DEFAULT_PORT, false, false, false, false, &err)); expect->itype = ADDR_INTERNAL_STATICTOR; expect->u.torservice.port = DEFAULT_PORT; + memset(expect->u.torservice.blob, 0, sizeof(expect->u.torservice.blob)); + strcpy((char *)expect->u.torservice.blob, STATIC_TOR_MAGIC_STRING); assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9051, NULL, &err)); - assert(wireaddr_internal_eq(&addr, expect, false)); + assert(wireaddr_internal_eq(&addr, expect)); /* statictor address with port */ assert(parse_wireaddr_internal("statictor:127.0.0.1:9055", &addr, DEFAULT_PORT, false, false, false, false, &err)); expect->itype = ADDR_INTERNAL_STATICTOR; expect->u.torservice.port = DEFAULT_PORT; assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9055, NULL, &err)); - assert(wireaddr_internal_eq(&addr, expect, false)); + assert(wireaddr_internal_eq(&addr, expect)); /* statictor address with torport */ assert(parse_wireaddr_internal("statictor:127.0.0.1/torport=9055", &addr, DEFAULT_PORT, false, false, false, false, &err)); expect->itype = ADDR_INTERNAL_STATICTOR; expect->u.torservice.port = 9055; assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9051, NULL, &err)); - assert(wireaddr_internal_eq(&addr, expect, false)); + assert(wireaddr_internal_eq(&addr, expect)); /* statictor address with port and torport */ assert(parse_wireaddr_internal("statictor:127.0.0.1:9055/torport=10055", &addr, DEFAULT_PORT, false, false, false, false, &err)); expect->itype = ADDR_INTERNAL_STATICTOR; expect->u.torservice.port = 10055; assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9055, NULL, &err)); - assert(wireaddr_internal_eq(&addr, expect, false)); + assert(wireaddr_internal_eq(&addr, expect)); + + /* statictor address with port and torport and torblob */ + assert(parse_wireaddr_internal("statictor:127.0.0.1:9055/torport=10055/torblob=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", &addr, DEFAULT_PORT, false, false, false, false, &err)); + expect->itype = ADDR_INTERNAL_STATICTOR; + expect->u.torservice.port = 10055; + /* This is actually nul terminated */ + memset(expect->u.torservice.blob, 'x', sizeof(expect->u.torservice.blob)-1); + assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9055, NULL, &err)); + assert(wireaddr_internal_eq(&addr, expect)); /* local socket path */ assert(parse_wireaddr_internal("/tmp/foo.sock", &addr, DEFAULT_PORT, false, false, false, false, &err)); expect->itype = ADDR_INTERNAL_SOCKNAME; strcpy(expect->u.sockname, "/tmp/foo.sock"); - assert(wireaddr_internal_eq(&addr, expect, false)); + assert(wireaddr_internal_eq(&addr, expect)); /* Unresolved */ assert(!parse_wireaddr_internal("ozlabs.org", &addr, DEFAULT_PORT, false, false, false, false, &err)); @@ -244,7 +222,7 @@ int main(int argc, char *argv[]) expect->itype = ADDR_INTERNAL_FORPROXY; strcpy(expect->u.unresolved.name, "ozlabs.org"); expect->u.unresolved.port = DEFAULT_PORT; - assert(wireaddr_internal_eq(&addr, expect, false)); + assert(wireaddr_internal_eq(&addr, expect)); /* Unresolved with port */ assert(!parse_wireaddr_internal("ozlabs.org:1234", &addr, DEFAULT_PORT, false, false, false, false, &err)); @@ -253,7 +231,7 @@ int main(int argc, char *argv[]) expect->itype = ADDR_INTERNAL_FORPROXY; strcpy(expect->u.unresolved.name, "ozlabs.org"); expect->u.unresolved.port = 1234; - assert(wireaddr_internal_eq(&addr, expect, false)); + assert(wireaddr_internal_eq(&addr, expect)); tal_free(expect); common_shutdown(); diff --git a/common/wireaddr.c b/common/wireaddr.c index 08b0683fb738..4f4deac832e8 100644 --- a/common/wireaddr.c +++ b/common/wireaddr.c @@ -454,6 +454,37 @@ bool parse_wireaddr(const char *arg, struct wireaddr *addr, u16 defport, return res; } +bool wireaddr_internal_eq(const struct wireaddr_internal *a, + const struct wireaddr_internal *b) +{ + if (a->itype != b->itype) + return false; + + switch (a->itype) { + case ADDR_INTERNAL_SOCKNAME: + return streq(a->u.sockname, b->u.sockname); + case ADDR_INTERNAL_ALLPROTO: + return a->u.port == b->u.port; + case ADDR_INTERNAL_STATICTOR: + if (!memeq(a->u.torservice.blob, sizeof(a->u.torservice.blob), + b->u.torservice.blob, sizeof(b->u.torservice.blob))) + return false; + /* fall thru */ + case ADDR_INTERNAL_AUTOTOR: + if (!wireaddr_eq(&a->u.torservice.address, + &b->u.torservice.address)) + return false; + return a->u.torservice.port == b->u.torservice.port; + case ADDR_INTERNAL_FORPROXY: + if (!streq(a->u.unresolved.name, b->u.unresolved.name)) + return false; + return a->u.unresolved.port == b->u.unresolved.port; + case ADDR_INTERNAL_WIREADDR: + return wireaddr_eq(&a->u.wireaddr, &b->u.wireaddr); + } + abort(); +} + bool parse_wireaddr_internal(const char *arg, struct wireaddr_internal *addr, u16 port, bool wildcard_ok, bool dns_ok, bool unresolved_ok, bool allow_deprecated, diff --git a/common/wireaddr.h b/common/wireaddr.h index b484be47feb4..cf16914d7098 100644 --- a/common/wireaddr.h +++ b/common/wireaddr.h @@ -146,6 +146,9 @@ struct wireaddr_internal { char sockname[sizeof(((struct sockaddr_un *)0)->sun_path)]; } u; }; + +bool wireaddr_internal_eq(const struct wireaddr_internal *a, + const struct wireaddr_internal *b); bool parse_wireaddr_internal(const char *arg, struct wireaddr_internal *addr, u16 port, bool wildcard_ok, bool dns_ok, bool unresolved_ok, bool allow_deprecated, diff --git a/lightningd/options.c b/lightningd/options.c index 152575254f87..b666a26ee17b 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -198,6 +198,18 @@ static char *opt_add_addr_withtype(const char *arg, deprecated_apis, &err_msg)) { return tal_fmt(NULL, "Unable to parse address '%s': %s", arg, err_msg); } + + /* Sanity check for exact duplicates. */ + for (size_t i = 0; i < tal_count(ld->proposed_wireaddr); i++) { + /* Only compare announce vs announce and bind vs bind */ + if ((ld->proposed_listen_announce[i] & ala) == 0) + continue; + + if (wireaddr_internal_eq(&ld->proposed_wireaddr[i], &wi)) + return tal_fmt(NULL, "Duplicate %s address %s", + ala & ADDR_ANNOUNCE ? "announce" : "listen", + type_to_string(tmpctx, struct wireaddr_internal, &wi)); + } tal_arr_expand(&ld->proposed_wireaddr, wi); return NULL; @@ -205,7 +217,6 @@ static char *opt_add_addr_withtype(const char *arg, static char *opt_add_announce_addr(const char *arg, struct lightningd *ld) { - const struct wireaddr *wn; size_t n = tal_count(ld->proposed_wireaddr); char *err; @@ -226,24 +237,6 @@ static char *opt_add_announce_addr(const char *arg, struct lightningd *ld) return tal_fmt(NULL, "address '%s' is not announcable", arg); - /* gossipd will refuse to announce the second one, sure, but it's - * better to check and fail now if they've explicitly asked for it. */ - wn = &ld->proposed_wireaddr[n].u.wireaddr; - for (size_t i = 0; i < n; i++) { - const struct wireaddr *wi; - - if (ld->proposed_listen_announce[i] != ADDR_ANNOUNCE) - continue; - assert(ld->proposed_wireaddr[i].itype == ADDR_INTERNAL_WIREADDR); - wi = &ld->proposed_wireaddr[i].u.wireaddr; - - if (wn->type != wi->type) - continue; - return tal_fmt(NULL, "Cannot announce address %s;" - " already have %s which is the same type", - type_to_string(tmpctx, struct wireaddr, wn), - type_to_string(tmpctx, struct wireaddr, wi)); - } return NULL; } diff --git a/tests/test_gossip.py b/tests/test_gossip.py index b45e430311b9..648606faaf87 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -916,8 +916,11 @@ def test_gossip_addresses(node_factory, bitcoind): l1 = node_factory.get_node(options={ 'announce-addr': [ '[::]:3', + '[::]', '127.0.0.1:2', + '127.0.0.1', 'vww6ybal4bd7szmgncyruucpgfkqahzddi37ktceo3ah7ngmcopnpyyd.onion', + '4acth47i6kxnvkewtm6q7ib2s3ufpo5sqbsnzjpbi7utijcltosqemad.onion:1234' ], }) l2 = node_factory.get_node() @@ -931,8 +934,11 @@ def test_gossip_addresses(node_factory, bitcoind): nodes = l2.rpc.listnodes(l1.info['id'])['nodes'] assert len(nodes) == 1 and nodes[0]['addresses'] == [ {'type': 'ipv4', 'address': '127.0.0.1', 'port': 2}, + {'type': 'ipv4', 'address': '127.0.0.1', 'port': 9735}, {'type': 'ipv6', 'address': '::', 'port': 3}, + {'type': 'ipv6', 'address': '::', 'port': 9735}, {'type': 'torv3', 'address': 'vww6ybal4bd7szmgncyruucpgfkqahzddi37ktceo3ah7ngmcopnpyyd.onion', 'port': 9735}, + {'type': 'torv3', 'address': '4acth47i6kxnvkewtm6q7ib2s3ufpo5sqbsnzjpbi7utijcltosqemad.onion', 'port': 1234}, ]