From d40379885d88f463162ccfb1b96301c1dcd5bc00 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 30 May 2023 13:58:18 +0930 Subject: [PATCH] common/wireaddr.h: simplify parse_wireaddr API. 1. Make it the standard "return the error" pattern. 2. Rather than flags to indicate what types are allowed, have the callers check the return explicitly. 3. Document the APIs. Signed-off-by: Rusty Russell --- common/test/run-ip_port_parsing.c | 19 ++-- common/test/run-wireaddr.c | 63 ++++++------- common/wireaddr.c | 144 +++++++++++++----------------- common/wireaddr.h | 42 +++++++-- connectd/test/run-netaddress.c | 30 ++++--- connectd/tor_autoservice.c | 14 +-- devtools/gossipwith.c | 4 +- lightningd/connect_control.c | 23 +++-- lightningd/options.c | 48 ++++++---- wallet/test/run-wallet.c | 9 +- wallet/wallet.c | 15 ++-- 11 files changed, 223 insertions(+), 188 deletions(-) diff --git a/common/test/run-ip_port_parsing.c b/common/test/run-ip_port_parsing.c index cb2f1b27e..4685f50c8 100644 --- a/common/test/run-ip_port_parsing.c +++ b/common/test/run-ip_port_parsing.c @@ -196,37 +196,36 @@ int main(int argc, char *argv[]) assert(!separate_address_and_port(tmpctx, "[::1]:http", &ip, &port)); // localhost hostnames for backward compat - assert(parse_wireaddr("localhost", &addr, 200, false, NULL)); + assert(parse_wireaddr(tmpctx, "localhost", 200, false, &addr) == NULL); assert(addr.port == 200); // string should win the port battle - assert(parse_wireaddr("[::1]:9735", &addr, 500, false, NULL)); + assert(parse_wireaddr(tmpctx, "[::1]:9735", 500, false, &addr) == NULL); assert(addr.port == 9735); ip = fmt_wireaddr(tmpctx, &addr); assert(streq(ip, "[::1]:9735")); // should use argument if we have no port in string - assert(parse_wireaddr("2001:db8:85a3::8a2e:370:7334", &addr, 9777, false, NULL)); + assert(parse_wireaddr(tmpctx, "2001:db8:85a3::8a2e:370:7334", 9777, false, &addr) == NULL); assert(addr.port == 9777); ip = fmt_wireaddr(tmpctx, &addr); assert(streq(ip, "[2001:db8:85a3::8a2e:370:7334]:9777")); - assert(parse_wireaddr("[::ffff:127.0.0.1]:49150", &addr, 1, false, NULL)); + assert(parse_wireaddr(tmpctx, "[::ffff:127.0.0.1]:49150", 1, false, &addr) == NULL); assert(addr.port == 49150); - assert(parse_wireaddr("4ruvswpqec5i2gogopxl4vm5bruzknbvbylov2awbo4rxiq4cimdldad.onion:49150", &addr, 1, false, NULL)); + assert(parse_wireaddr(tmpctx, "4ruvswpqec5i2gogopxl4vm5bruzknbvbylov2awbo4rxiq4cimdldad.onion:49150", 1, false, &addr) == NULL); assert(addr.port == 49150); - assert(parse_wireaddr("4ruvswpqec5i2gogopxl4vm5bruzknbvbylov2awbo4rxiq4cimdldad.onion", &addr, 1, false, NULL)); + assert(parse_wireaddr(tmpctx, "4ruvswpqec5i2gogopxl4vm5bruzknbvbylov2awbo4rxiq4cimdldad.onion", 1, false, &addr) == NULL); assert(addr.port == 1); /* We don't accept torv2 any more */ - assert(!parse_wireaddr("odpzvneidqdf5hdq.onion:49150", &addr, 1, false, NULL)); - assert(!parse_wireaddr("odpzvneidqdf5hdq.onion", &addr, 1, false, NULL)); + assert(parse_wireaddr(tmpctx, "odpzvneidqdf5hdq.onion:49150", 1, false, &addr) != NULL); + assert(parse_wireaddr(tmpctx, "odpzvneidqdf5hdq.onion", 1, false, &addr) != NULL); - assert(!parse_wireaddr_internal("odpzvneidqdf5hdq.onion", &addr_int, 1, - false, false, false, NULL)); + assert(parse_wireaddr_internal(tmpctx, "odpzvneidqdf5hdq.onion", 1, false, &addr_int) != NULL); assert(wireaddr_from_hostname(tmpctx, "odpzvneidqdf5hdq.onion", 1, NULL, NULL, NULL) == NULL); assert(wireaddr_from_hostname(tmpctx, "aaa.onion", 1, NULL, NULL, NULL) == NULL); diff --git a/common/test/run-wireaddr.c b/common/test/run-wireaddr.c index 2b1c852d9..71160f001 100644 --- a/common/test/run-wireaddr.c +++ b/common/test/run-wireaddr.c @@ -128,120 +128,115 @@ void towire_u8_array(u8 **pptr UNNEEDED, const u8 *arr UNNEEDED, size_t num UNNE int main(int argc, char *argv[]) { - const char *err; struct wireaddr_internal addr, *expect = tal(NULL, struct wireaddr_internal); common_setup(argv[0]); /* Simple IPv4 address. */ - assert(parse_wireaddr_internal("127.0.0.1", &addr, DEFAULT_PORT, false, false, false, &err)); + assert(parse_wireaddr_internal(tmpctx, "127.0.0.1", DEFAULT_PORT, false, &addr) == NULL); expect->itype = ADDR_INTERNAL_WIREADDR; - assert(parse_wireaddr("127.0.0.1:9735", &expect->u.wireaddr, 0, NULL, &err)); + assert(parse_wireaddr(tmpctx, "127.0.0.1:9735", 0, NULL, &expect->u.wireaddr) == NULL); 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, &err)); + assert(parse_wireaddr_internal(tmpctx, "127.0.0.1:1", DEFAULT_PORT, false, &addr) == NULL); expect->itype = ADDR_INTERNAL_WIREADDR; - assert(parse_wireaddr("127.0.0.1:1", &expect->u.wireaddr, 0, NULL, &err)); + assert(parse_wireaddr(tmpctx, "127.0.0.1:1", 0, NULL, &expect->u.wireaddr) == NULL); assert(wireaddr_internal_eq(&addr, expect)); /* Simple IPv6 address. */ - assert(parse_wireaddr_internal("::1", &addr, DEFAULT_PORT, false, false, false, &err)); + assert(parse_wireaddr_internal(tmpctx, "::1", DEFAULT_PORT, false, &addr) == NULL); expect->itype = ADDR_INTERNAL_WIREADDR; - assert(parse_wireaddr("::1", &expect->u.wireaddr, DEFAULT_PORT, NULL, &err)); + assert(parse_wireaddr(tmpctx, "::1", DEFAULT_PORT, NULL, &expect->u.wireaddr) == NULL); assert(wireaddr_internal_eq(&addr, expect)); /* IPv6 address with port. */ - assert(parse_wireaddr_internal("[::1]:1", &addr, DEFAULT_PORT, false, false, false, &err)); + assert(parse_wireaddr_internal(tmpctx, "[::1]:1", DEFAULT_PORT, false, &addr) == NULL); expect->itype = ADDR_INTERNAL_WIREADDR; - assert(parse_wireaddr("::1", &expect->u.wireaddr, 1, NULL, &err)); + assert(parse_wireaddr(tmpctx, "::1", 1, NULL, &expect->u.wireaddr) == NULL); assert(wireaddr_internal_eq(&addr, expect)); /* autotor address */ - assert(parse_wireaddr_internal("autotor:127.0.0.1", &addr, DEFAULT_PORT, false, false, false, &err)); + assert(parse_wireaddr_internal(tmpctx, "autotor:127.0.0.1", DEFAULT_PORT, false, &addr) == NULL); 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(parse_wireaddr(tmpctx, "127.0.0.1", 9051, NULL, &expect->u.torservice.address) == NULL); 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, &err)); + assert(parse_wireaddr_internal(tmpctx, "autotor:127.0.0.1:9055", DEFAULT_PORT, false, &addr) == NULL); 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(parse_wireaddr(tmpctx, "127.0.0.1", 9055, NULL, &expect->u.torservice.address) == NULL); 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, &err)); + assert(parse_wireaddr_internal(tmpctx, "autotor:127.0.0.1/torport=9055", DEFAULT_PORT, false, &addr) == NULL); 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(parse_wireaddr(tmpctx, "127.0.0.1", 9051, NULL, &expect->u.torservice.address) == NULL); 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, &err)); + assert(parse_wireaddr_internal(tmpctx, "autotor:127.0.0.1:9055/torport=10055", DEFAULT_PORT, false, &addr) == NULL); 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(parse_wireaddr(tmpctx, "127.0.0.1", 9055, NULL, &expect->u.torservice.address) == NULL); assert(wireaddr_internal_eq(&addr, expect)); /* statictor address */ - assert(parse_wireaddr_internal("statictor:127.0.0.1", &addr, DEFAULT_PORT, false, false, false, &err)); + assert(parse_wireaddr_internal(tmpctx, "statictor:127.0.0.1", DEFAULT_PORT, false, &addr) == NULL); 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(parse_wireaddr(tmpctx, "127.0.0.1", 9051, NULL, &expect->u.torservice.address) == NULL); 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, &err)); + assert(parse_wireaddr_internal(tmpctx, "statictor:127.0.0.1:9055", DEFAULT_PORT, false, &addr) == NULL); 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(parse_wireaddr(tmpctx, "127.0.0.1", 9055, NULL, &expect->u.torservice.address) == NULL); 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, &err)); + assert(parse_wireaddr_internal(tmpctx, "statictor:127.0.0.1/torport=9055", DEFAULT_PORT, false, &addr) == NULL); 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(parse_wireaddr(tmpctx, "127.0.0.1", 9051, NULL, &expect->u.torservice.address) == NULL); 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, &err)); + assert(parse_wireaddr_internal(tmpctx, "statictor:127.0.0.1:9055/torport=10055", DEFAULT_PORT, false, &addr) == NULL); 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(parse_wireaddr(tmpctx, "127.0.0.1", 9055, NULL, &expect->u.torservice.address) == NULL); 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, &err)); + assert(parse_wireaddr_internal(tmpctx, "statictor:127.0.0.1:9055/torport=10055/torblob=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", DEFAULT_PORT, false, &addr) == NULL); 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(parse_wireaddr(tmpctx, "127.0.0.1", 9055, NULL, &expect->u.torservice.address) == NULL); assert(wireaddr_internal_eq(&addr, expect)); /* local socket path */ - assert(parse_wireaddr_internal("/tmp/foo.sock", &addr, DEFAULT_PORT, false, false, false, &err)); + assert(parse_wireaddr_internal(tmpctx, "/tmp/foo.sock", DEFAULT_PORT, false, &addr) == NULL); expect->itype = ADDR_INTERNAL_SOCKNAME; strcpy(expect->u.sockname, "/tmp/foo.sock"); assert(wireaddr_internal_eq(&addr, expect)); /* Unresolved */ - assert(!parse_wireaddr_internal("ozlabs.org", &addr, DEFAULT_PORT, false, false, false, &err)); - assert(streq(err, "Needed DNS, but lookups suppressed")); - assert(parse_wireaddr_internal("ozlabs.org", &addr, DEFAULT_PORT, false, false, true, &err)); + assert(parse_wireaddr_internal(tmpctx, "ozlabs.org", DEFAULT_PORT, false, &addr) == NULL); expect->itype = ADDR_INTERNAL_FORPROXY; strcpy(expect->u.unresolved.name, "ozlabs.org"); expect->u.unresolved.port = DEFAULT_PORT; assert(wireaddr_internal_eq(&addr, expect)); /* Unresolved with port */ - assert(!parse_wireaddr_internal("ozlabs.org:1234", &addr, DEFAULT_PORT, false, false, false, &err)); - assert(streq(err, "Needed DNS, but lookups suppressed")); - assert(parse_wireaddr_internal("ozlabs.org:1234", &addr, DEFAULT_PORT, false, false, true, &err)); + assert(parse_wireaddr_internal(tmpctx, "ozlabs.org:1234", DEFAULT_PORT, false, &addr) == NULL); expect->itype = ADDR_INTERNAL_FORPROXY; strcpy(expect->u.unresolved.name, "ozlabs.org"); expect->u.unresolved.port = 1234; diff --git a/common/wireaddr.c b/common/wireaddr.c index 1c8be2fa8..978fdaca4 100644 --- a/common/wireaddr.c +++ b/common/wireaddr.c @@ -516,23 +516,25 @@ cleanup: return tal_free(addrs); } -bool parse_wireaddr(const char *arg, struct wireaddr *addr, u16 defport, - bool *no_dns, const char **err_msg) +const char *parse_wireaddr(const tal_t *ctx, + const char *arg, + u16 defport, + bool *no_dns, + struct wireaddr *addr) { struct in6_addr v6; struct in_addr v4; u16 port; char *ip; - bool res; + const char *err_msg; + struct wireaddr *addresses; - res = false; port = defport; - if (err_msg) - *err_msg = NULL; if (!separate_address_and_port(tmpctx, arg, &ip, &port)) - goto finish; + return tal_strdup(ctx, "Error parsing hostname"); + /* We resolved these even without DNS */ if (streq(ip, "localhost")) ip = "127.0.0.1"; else if (streq(ip, "ip6-localhost")) @@ -542,27 +544,21 @@ bool parse_wireaddr(const char *arg, struct wireaddr *addr, u16 defport, if (inet_pton(AF_INET, ip, &v4) == 1) { wireaddr_from_ipv4(addr, &v4, port); - res = true; + return NULL; } else if (inet_pton(AF_INET6, ip, &v6) == 1) { wireaddr_from_ipv6(addr, &v6, port); - res = true; + return NULL; } /* Resolve with getaddrinfo */ - if (!res) { - struct wireaddr *addresses = wireaddr_from_hostname(NULL, ip, port, - no_dns, NULL, err_msg); - if (addresses) { - *addr = addresses[0]; - tal_free(addresses); - res = true; - } - } + addresses = wireaddr_from_hostname(NULL, ip, port, no_dns, NULL, &err_msg); + if (!addresses) + return tal_strdup(ctx, err_msg); -finish: - if (!res && err_msg && !*err_msg) - *err_msg = "Error parsing hostname"; - return res; + /* FIXME: Allow return of multiple addresses? */ + *addr = addresses[0]; + tal_free(addresses); + return NULL; } bool wireaddr_internal_eq(const struct wireaddr_internal *a, @@ -596,30 +592,30 @@ bool wireaddr_internal_eq(const struct wireaddr_internal *a, abort(); } -bool parse_wireaddr_internal(const char *arg, struct wireaddr_internal *addr, - u16 port, bool wildcard_ok, bool dns_ok, - bool unresolved_ok, - const char **err_msg) +const char *parse_wireaddr_internal(const tal_t *ctx, + const char *arg, + u16 default_port, + bool dns_lookup_ok, + struct wireaddr_internal *addr) { u16 splitport; char *ip = NULL; char *service_addr; - bool needed_dns = false; + const char *err; + bool needed_dns; /* Addresses starting with '/' are local socket paths */ if (arg[0] == '/') { addr->itype = ADDR_INTERNAL_SOCKNAME; /* Check if the path is too long */ - if (strlen(arg) >= sizeof(addr->u.sockname)) { - if (err_msg) - *err_msg = "Socket name too long"; - return false; - } + if (strlen(arg) >= sizeof(addr->u.sockname)) + return "Socket name too long"; + /* Zero it out for passing across the wire */ memset(addr->u.sockname, 0, sizeof(addr->u.sockname)); strcpy(addr->u.sockname, arg); - return true; + return NULL; } /* 'autotor:' is a special prefix meaning talk to Tor to create @@ -635,22 +631,17 @@ bool parse_wireaddr_internal(const char *arg, struct wireaddr_internal *addr, char *endp = NULL; addr->u.torservice.port = strtol(parts[i]+strlen("torport="), &endp, 10); if (addr->u.torservice.port <= 0 || *endp != '\0') { - if (err_msg) - *err_msg = "Bad :torport: number"; - return false; + return "Bad :torport: number"; } } else { - if (err_msg) - *err_msg = tal_fmt(tmpctx, "unknown tor arg %s", parts[i]); - return false; + return tal_fmt(ctx, "unknown tor arg %s", parts[i]); } } service_addr = tal_fmt(tmpctx, "%s", parts[0] + strlen("autotor:")); - - return parse_wireaddr(service_addr, - &addr->u.torservice.address, 9051, - dns_ok ? NULL : &needed_dns, err_msg); + return parse_wireaddr(ctx, service_addr, 9051, + dns_lookup_ok ? NULL : &needed_dns, + &addr->u.torservice.address); } /* 'statictor:' is a special prefix meaning talk to Tor to create @@ -668,23 +659,17 @@ bool parse_wireaddr_internal(const char *arg, struct wireaddr_internal *addr, char *endp = NULL; addr->u.torservice.port = strtol(parts[i]+strlen("torport="), &endp, 10); if (addr->u.torservice.port <= 0 || *endp != '\0') { - if (err_msg) - *err_msg = "Bad :torport: number"; - return false; + return "Bad :torport: number"; } } else if (strstarts(parts[i], "torblob=")) { const char *blobdata = parts[i] + strlen("torblob="); if (strlen(blobdata) > TOR_V3_BLOBLEN) { - if (err_msg) - *err_msg = "torblob too long"; - return false; + return "torblob too long"; } strcpy(addr->u.torservice.blob, blobdata); use_magic_blob = false; } else { - if (err_msg) - *err_msg = tal_fmt(tmpctx, "unknown tor arg %s", parts[i]); - return false; + return tal_fmt(ctx, "unknown tor arg %s", parts[i]); } } @@ -695,49 +680,42 @@ bool parse_wireaddr_internal(const char *arg, struct wireaddr_internal *addr, service_addr = tal_fmt(tmpctx, "%s", parts[0] + strlen("statictor:")); - return parse_wireaddr(service_addr, - &addr->u.torservice.address, 9051, - dns_ok ? NULL : &needed_dns, err_msg); + return parse_wireaddr(ctx, service_addr, 9051, + dns_lookup_ok ? NULL : &needed_dns, + &addr->u.torservice.address); } - splitport = port; - if (!separate_address_and_port(tmpctx, arg, &ip, &splitport)) { - if (err_msg) { - *err_msg = tal_fmt(tmpctx, "Error parsing hostname %s %s", (char *)arg, ip); - } - return false; - } + splitport = default_port; + if (!separate_address_and_port(tmpctx, arg, &ip, &splitport)) + return tal_fmt(ctx, "Error parsing hostname %s %s", (char *)arg, ip); /* An empty string means IPv4 and IPv6 (which under Linux by default * means just IPv6, and IPv4 gets autobound). */ - if (wildcard_ok && is_wildcardaddr(ip)) { + if (streq(ip, "")) { addr->itype = ADDR_INTERNAL_ALLPROTO; addr->u.port = splitport; - return true; + return NULL; } - addr->itype = ADDR_INTERNAL_WIREADDR; - if (parse_wireaddr(arg, &addr->u.wireaddr, port, - dns_ok ? NULL : &needed_dns, err_msg)) { - if (addr->u.wireaddr.type == ADDR_TYPE_TOR_V2_REMOVED) { - if (err_msg) - *err_msg = "v2 Tor onion services not supported"; - return false; - } - - return true; + needed_dns = false; + err = parse_wireaddr(ctx, arg, default_port, + dns_lookup_ok ? NULL : &needed_dns, + &addr->u.wireaddr); + if (!err) { + addr->itype = ADDR_INTERNAL_WIREADDR; + return NULL; } - if (!needed_dns || !unresolved_ok) - return false; + /* Did we fail because we needed DNS lookup? If not, we just failed. */ + if (!needed_dns) + return err; - /* We can't do DNS, so keep unresolved. */ - if (!wireaddr_from_unresolved(addr, ip, splitport)) { - if (err_msg) - *err_msg = "Name too long"; - return false; - } - return true; + /* Keep unresolved. */ + tal_free(err); + if (!wireaddr_from_unresolved(addr, ip, splitport)) + return "Name too long"; + + return NULL; } bool wireaddr_from_unresolved(struct wireaddr_internal *addr, diff --git a/common/wireaddr.h b/common/wireaddr.h index bdae7788f..ac26999a1 100644 --- a/common/wireaddr.h +++ b/common/wireaddr.h @@ -75,10 +75,25 @@ enum addr_listen_announce fromwire_addr_listen_announce(const u8 **cursor, size_t *max); void towire_addr_listen_announce(u8 **pptr, enum addr_listen_announce ala); -/* If no_dns is non-NULL, we will set it to true and return false if - * we wanted to do a DNS lookup. */ -bool parse_wireaddr(const char *arg, struct wireaddr *addr, u16 port, - bool *no_dns, const char **err_msg); +/** + * parse_wireaddr - parse a string into the various defaults we have. + * @ctx: context to allocate returned error string + * @arg: the string + * @defport: the port to use if none specified in string + * @no_dns: if non-NULL, don't do DNS lookups. + * @addr: the addr to write, set if non-NULL return. + * + * If it returns NULL, check addr->itype to see if it's suitable for + * you! Otherwise, it returns a string allocated off @ctx. If you + * handed @no_dns, it will be set to true if the failure was due to + * the fact we wanted to do an DNS lookup, and false for other + * failures. + */ +const char *parse_wireaddr(const tal_t *ctx, + const char *arg, + u16 defport, + bool *no_dns, + struct wireaddr *addr); char *fmt_wireaddr(const tal_t *ctx, const struct wireaddr *a); char *fmt_wireaddr_without_port(const tal_t *ctx, const struct wireaddr *a); @@ -156,10 +171,21 @@ bool is_wildcardaddr(const char *arg); bool is_dnsaddr(const char *arg); -bool parse_wireaddr_internal(const char *arg, struct wireaddr_internal *addr, - u16 port, bool wildcard_ok, bool dns_ok, - bool unresolved_ok, - const char **err_msg); +/** + * parse_wireaddr_internal - parse a string into the various defaults we have. + * @ctx: context to allocate returned error string + * @arg: the string + * @default_port: the port to use if none specified in string + * @dns_lookup_ok: true if it's OK to do DNS name lookups. + * @addr: the addr to write, set if non-NULL return. + * + * If it returns NULL, you want to check addr->itype to see if it's + * suitable for you! */ +const char *parse_wireaddr_internal(const tal_t *ctx, + const char *arg, + u16 default_port, + bool dns_lookup_ok, + struct wireaddr_internal *addr); void towire_wireaddr_internal(u8 **pptr, const struct wireaddr_internal *addr); diff --git a/connectd/test/run-netaddress.c b/connectd/test/run-netaddress.c index 91fc35926..74f06c3d0 100644 --- a/connectd/test/run-netaddress.c +++ b/connectd/test/run-netaddress.c @@ -168,16 +168,19 @@ int main(int argc, char *argv[]) common_setup(argv[0]); // start with IPv4 - parse_wireaddr("0.0.0.0", &wa, DEFAULT_PORT, NULL, &err); + err = parse_wireaddr(tmpctx, "0.0.0.0", DEFAULT_PORT, NULL, &wa); + assert(!err); assert(!IsValid(&wa)); assert(IsIPv4(&wa)); assert(!IsIPv6(&wa)); - parse_wireaddr("255.255.255.255", &wa, DEFAULT_PORT, NULL, &err); + err = parse_wireaddr(tmpctx, "255.255.255.255", DEFAULT_PORT, NULL, &wa); + assert(!err); assert(!IsValid(&wa)); assert(IsIPv4(&wa)); assert(!IsIPv6(&wa)); - parse_wireaddr("127.0.0.1", &wa, DEFAULT_PORT, NULL, &err); + err = parse_wireaddr(tmpctx, "127.0.0.1", DEFAULT_PORT, NULL, &wa); + assert(!err); assert(IsValid(&wa)); assert(IsIPv4(&wa)); assert(!IsIPv6(&wa)); @@ -185,7 +188,8 @@ int main(int argc, char *argv[]) assert(address_routable(&wa, true)); assert(!address_routable(&wa, false)); - parse_wireaddr("0.1.2.3", &wa, DEFAULT_PORT, NULL, &err); + err = parse_wireaddr(tmpctx, "0.1.2.3", DEFAULT_PORT, NULL, &wa); + assert(!err); assert(IsValid(&wa)); assert(IsIPv4(&wa)); assert(!IsIPv6(&wa)); @@ -193,7 +197,8 @@ int main(int argc, char *argv[]) assert(address_routable(&wa, true)); assert(!address_routable(&wa, false)); - parse_wireaddr("10.0.21.42", &wa, DEFAULT_PORT, NULL, &err); + err = parse_wireaddr(tmpctx, "10.0.21.42", DEFAULT_PORT, NULL, &wa); + assert(!err); assert(IsValid(&wa)); assert(IsIPv4(&wa)); assert(!IsIPv6(&wa)); @@ -202,7 +207,8 @@ int main(int argc, char *argv[]) assert(!address_routable(&wa, true)); assert(!address_routable(&wa, false)); - parse_wireaddr("192.168.123.4", &wa, DEFAULT_PORT, NULL, &err); + err = parse_wireaddr(tmpctx, "192.168.123.4", DEFAULT_PORT, NULL, &wa); + assert(!err); assert(IsValid(&wa)); assert(IsIPv4(&wa)); assert(!IsIPv6(&wa)); @@ -211,7 +217,8 @@ int main(int argc, char *argv[]) assert(!address_routable(&wa, true)); assert(!address_routable(&wa, false)); - parse_wireaddr("1.2.3.4", &wa, DEFAULT_PORT, NULL, &err); + err = parse_wireaddr(tmpctx, "1.2.3.4", DEFAULT_PORT, NULL, &wa); + assert(!err); assert(IsValid(&wa)); assert(IsIPv4(&wa)); assert(!IsIPv6(&wa)); @@ -220,7 +227,8 @@ int main(int argc, char *argv[]) assert(address_routable(&wa, false)); // now IPv6 stuff - parse_wireaddr("2142:DEAD:beef::1", &wa, DEFAULT_PORT, NULL, &err); + err = parse_wireaddr(tmpctx, "2142:DEAD:beef::1", DEFAULT_PORT, NULL, &wa); + assert(!err); assert(IsValid(&wa)); assert(!IsIPv4(&wa)); assert(IsIPv6(&wa)); @@ -228,12 +236,14 @@ int main(int argc, char *argv[]) assert(address_routable(&wa, true)); assert(address_routable(&wa, false)); - parse_wireaddr("0::0", &wa, DEFAULT_PORT, NULL, &err); + err = parse_wireaddr(tmpctx, "0::0", DEFAULT_PORT, NULL, &wa); + assert(!err); assert(!IsValid(&wa)); assert(!IsIPv4(&wa)); assert(IsIPv6(&wa)); - parse_wireaddr("2001:db8::1", &wa, DEFAULT_PORT, NULL, &err); + err = parse_wireaddr(tmpctx, "2001:db8::1", DEFAULT_PORT, NULL, &wa); + assert(!err); assert(!IsValid(&wa)); assert(!IsIPv4(&wa)); assert(IsIPv6(&wa)); diff --git a/connectd/tor_autoservice.c b/connectd/tor_autoservice.c index 054a22015..4b08f9235 100644 --- a/connectd/tor_autoservice.c +++ b/connectd/tor_autoservice.c @@ -113,7 +113,7 @@ static struct wireaddr *make_onion(const tal_t *ctx, port, fmt_wireaddr(tmpctx, local))); while ((line = tor_response_line(rbuf)) != NULL) { - const char *name; + const char *name, *err; if (!strstarts(line, "ServiceID=")) continue; @@ -124,9 +124,10 @@ static struct wireaddr *make_onion(const tal_t *ctx, name = tal_fmt(tmpctx, "%s.onion", line); onion = tal(ctx, struct wireaddr); - if (!parse_wireaddr(name, onion, local->port, false, NULL)) + err = parse_wireaddr(tmpctx, name, local->port, NULL, onion); + if (err) status_failed(STATUS_FAIL_INTERNAL_ERROR, - "Tor gave bad onion name '%s'", name); + "Tor gave bad onion name '%s': %s", name, err); status_info("New autotor service onion address: \"%s:%d\" bound from extern port:%d", name, local->port, port); discard_remaining_response(rbuf); return onion; @@ -150,7 +151,7 @@ static struct wireaddr *make_fixed_onion(const tal_t *ctx, blob64, port, fmt_wireaddr(tmpctx, local))); while ((line = tor_response_line_wfail(rbuf))) { - const char *name; + const char *name, *err; if (strstarts(line, "Onion address collision")) status_failed(STATUS_FAIL_INTERNAL_ERROR, "Tor address in use"); @@ -164,9 +165,10 @@ static struct wireaddr *make_fixed_onion(const tal_t *ctx, name = tal_fmt(tmpctx, "%s.onion", line); onion = tal(ctx, struct wireaddr); - if (!parse_wireaddr(name, onion, port, false, NULL)) + err = parse_wireaddr(tmpctx, name, port, false, onion); + if (err) status_failed(STATUS_FAIL_INTERNAL_ERROR, - "Tor gave bad onion name '%s'", name); + "Tor gave bad onion name '%s': %s", name, err); #ifdef SUPERVERBOSE status_info("Static Tor service onion address: \"%s:%d,%s\"from blob %s base64 %s ", name, port ,fmt_wireaddr(tmpctx, local), blob ,blob64); diff --git a/devtools/gossipwith.c b/devtools/gossipwith.c index 617856331..70a399468 100644 --- a/devtools/gossipwith.c +++ b/devtools/gossipwith.c @@ -326,8 +326,8 @@ int main(int argc, char *argv[]) opt_usage_exit_fail("Invalid id %.*s", (int)(at - argv[1]), argv[1]); - if (!parse_wireaddr_internal(at+1, &addr, chainparams_get_ln_port(chainparams), NULL, - true, false, &err_msg)) + err_msg = parse_wireaddr_internal(tmpctx, at+1, chainparams_get_ln_port(chainparams), true, &addr); + if (err_msg) opt_usage_exit_fail("%s '%s'", err_msg, argv[1]); switch (addr.itype) { diff --git a/lightningd/connect_control.c b/lightningd/connect_control.c index 3b16c7548..3901826bb 100644 --- a/lightningd/connect_control.c +++ b/lightningd/connect_control.c @@ -199,15 +199,28 @@ static struct command_result *json_connect(struct command *cmd, if (id_addr.host) { u16 port = id_addr.port ? *id_addr.port : chainparams_get_ln_port(chainparams); addr = tal(cmd, struct wireaddr_internal); - if (!parse_wireaddr_internal(id_addr.host, addr, port, false, - !cmd->ld->always_use_proxy - && !cmd->ld->pure_tor_setup, - true, - &err_msg)) { + err_msg = parse_wireaddr_internal(tmpctx, id_addr.host, port, + !cmd->ld->always_use_proxy + && !cmd->ld->pure_tor_setup, addr); + if (err_msg) { return command_fail(cmd, LIGHTNINGD, "Host %s:%u not valid: %s", id_addr.host, port, err_msg); } + /* Check they didn't specify some weird type! */ + switch (addr->itype) { + case ADDR_INTERNAL_SOCKNAME: + case ADDR_INTERNAL_WIREADDR: + /* Can happen if we're disable DNS */ + case ADDR_INTERNAL_FORPROXY: + break; + case ADDR_INTERNAL_ALLPROTO: + case ADDR_INTERNAL_AUTOTOR: + case ADDR_INTERNAL_STATICTOR: + return command_fail(cmd, LIGHTNINGD, + "Host %s:%u not a simple type", + id_addr.host, port); + } } else { addr = NULL; /* Port without host name? */ diff --git a/lightningd/options.c b/lightningd/options.c index bedc4eea9..0aac0c238 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -1,5 +1,6 @@ #include "config.h" #include +#include #include #include #include @@ -267,11 +268,25 @@ static char *opt_add_addr_withtype(const char *arg, || is_wildcardaddr(address) || (is_dnsaddr(address) && !ld->announce_dns) || ala != ADDR_ANNOUNCE) { - if (!parse_wireaddr_internal(arg, &wi, ld->portnum, - wildcard_ok, dns_ok, false, - &err_msg)) { + err_msg = parse_wireaddr_internal(tmpctx, arg, ld->portnum, + dns_ok, &wi); + if (err_msg) { return tal_fmt(NULL, "Unable to parse address '%s': %s", arg, err_msg); } + /* Check they didn't specify some weird type! */ + switch (wi.itype) { + case ADDR_INTERNAL_SOCKNAME: + case ADDR_INTERNAL_WIREADDR: + case ADDR_INTERNAL_AUTOTOR: + case ADDR_INTERNAL_STATICTOR: + break; + case ADDR_INTERNAL_ALLPROTO: + if (!wildcard_ok) + return tal_fmt(NULL, "Cannot use wildcard address '%s'", arg); + break; + case ADDR_INTERNAL_FORPROXY: + return tal_fmt(NULL, "Cannot resolve address '%s' (not using DNS!)", arg); + } /* Sanity check for exact duplicates. */ for (size_t i = 0; i < tal_count(ld->proposed_wireaddr); i++) { @@ -348,14 +363,14 @@ static char *opt_add_addr(const char *arg, struct lightningd *ld) struct wireaddr_internal addr; /* handle in case you used the addr option with an .onion */ - if (parse_wireaddr_internal(arg, &addr, 0, true, false, true, NULL)) { + if (parse_wireaddr_internal(tmpctx, arg, 0, false, &addr) == NULL) { if (addr.itype == ADDR_INTERNAL_WIREADDR && addr.u.wireaddr.type == ADDR_TYPE_TOR_V3) { - log_unusual(ld->log, "You used `--addr=%s` option with an .onion address, please use" - " `--announce-addr` ! You are lucky in this node live some wizards and" - " fairies, we have done this for you and announce, Be as hidden as wished", - arg); - return opt_add_announce_addr(arg, ld); + log_unusual(ld->log, "You used `--addr=%s` option with an .onion address, please use" + " `--announce-addr` ! You are lucky in this node live some wizards and" + " fairies, we have done this for you and announce, Be as hidden as wished", + arg); + return opt_add_announce_addr(arg, ld); } } /* the intended call */ @@ -394,7 +409,7 @@ static char *opt_add_bind_addr(const char *arg, struct lightningd *ld) struct wireaddr_internal addr; /* handle in case you used the bind option with an .onion */ - if (parse_wireaddr_internal(arg, &addr, 0, true, false, true, NULL)) { + if (parse_wireaddr_internal(tmpctx, arg, 0, false, &addr) == NULL) { if (addr.itype == ADDR_INTERNAL_WIREADDR && addr.u.wireaddr.type == ADDR_TYPE_TOR_V3) { log_unusual(ld->log, "You used `--bind-addr=%s` option with an .onion address," @@ -473,18 +488,17 @@ static char *opt_set_offline(struct lightningd *ld) static char *opt_add_proxy_addr(const char *arg, struct lightningd *ld) { bool needed_dns = false; + const char *err; + tal_free(ld->proxyaddr); /* We use a tal_arr here, so we can marshal it to gossipd */ ld->proxyaddr = tal_arr(ld, struct wireaddr, 1); - if (!parse_wireaddr(arg, ld->proxyaddr, 9050, - ld->always_use_proxy ? &needed_dns : NULL, - NULL)) { - return tal_fmt(NULL, "Unable to parse Tor proxy address '%s' %s", - arg, needed_dns ? " (needed dns)" : ""); - } - return NULL; + err = parse_wireaddr(tmpctx, arg, 9050, + ld->always_use_proxy ? &needed_dns : NULL, + ld->proxyaddr); + return cast_const(char *, err); } static char *opt_add_plugin(const char *arg, struct lightningd *ld) diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index c66eb4f1e..09ee1d3c1 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -1171,8 +1171,7 @@ static bool test_wallet_outputs(struct lightningd *ld, const tal_t *ctx) CHECK_MSG(!wallet_err, wallet_err); /* Add another utxo that's CSV-locked for 5 blocks */ - parse_wireaddr_internal("localhost:1234", &addr, 0, false, false, false, - NULL); + assert(parse_wireaddr_internal(tmpctx, "localhost:1234", 0, false, &addr) == NULL); channel.peer = new_peer(ld, 0, &id, &addr, false); channel.dbid = 1; channel.type = channel_type_anchor_outputs(tmpctx); @@ -1501,8 +1500,7 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) c1.blockheight_states = new_height_states(w, c1.opener, &blockheight); mempat(scriptpubkey, tal_count(scriptpubkey)); c1.first_blocknum = 1; - parse_wireaddr_internal("localhost:1234", &addr, 0, false, false, false, - NULL); + assert(parse_wireaddr_internal(tmpctx, "localhost:1234", 0, false, &addr) == NULL); c1.final_key_idx = 1337; p = new_peer(ld, 0, &id, &addr, false); c1.peer = p; @@ -1664,8 +1662,7 @@ static bool test_channel_inflight_crud(struct lightningd *ld, const tal_t *ctx) pubkey_from_der(tal_hexdata(w, "02a1633cafcc01ebfb6d78e39f687a1f0995c62fc95f51ead10a02ee0be551b5dc", 66), 33, &pk); node_id_from_pubkey(&id, &pk); - parse_wireaddr_internal("localhost:1234", &addr, 0, false, false, false, - NULL); + assert(parse_wireaddr_internal(tmpctx, "localhost:1234", 0, false, &addr) == NULL); /* new channel! */ p = new_peer(ld, 0, &id, &addr, false); diff --git a/wallet/wallet.c b/wallet/wallet.c index da0faf676..1939b67e6 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -852,7 +852,7 @@ static bool wallet_shachain_load(struct wallet *wallet, u64 id, static struct peer *wallet_peer_load(struct wallet *w, const u64 dbid) { - const char *addrstr; + const char *addrstr, *err; struct peer *peer = NULL; struct node_id id; struct wireaddr_internal addr; @@ -876,12 +876,13 @@ static struct peer *wallet_peer_load(struct wallet *w, const u64 dbid) /* This can happen for peers last seen on Torv2! */ addrstr = db_col_strdup(tmpctx, stmt, "address"); - if (!parse_wireaddr_internal(addrstr, &addr, chainparams_get_ln_port(chainparams), - false, false, true, NULL)) { - log_unusual(w->log, "Unparsable peer address %s: replacing", - addrstr); - parse_wireaddr_internal("127.0.0.1:1", &addr, chainparams_get_ln_port(chainparams), - false, false, true, NULL); + err = parse_wireaddr_internal(tmpctx, addrstr, chainparams_get_ln_port(chainparams), true, &addr); + if (err) { + log_unusual(w->log, "Unparsable peer address %s (%s): replacing", + addrstr, err); + err = parse_wireaddr_internal(tmpctx, "127.0.0.1:1", chainparams_get_ln_port(chainparams), + false, &addr); + assert(!err); } /* FIXME: save incoming in db! */