From ec025344cc03140f18161cf3ce38e0c9e18c7695 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 8 Dec 2022 15:02:35 +1030 Subject: [PATCH] lightningd: don't announce names as DNS by default. This broke BTCPayServer, so revert. I originally (accidentally!) implemented this such that it broadcast both DNS and IP entries, but Michael reported earlier that they still don't propagage well, so simply suppress them. Signed-off-by: Rusty Russell Fixes: #5795 Changelog-Changeed: Config: `announce-addr-dns` needs to be set to *true* to put DNS names into node announcements, otherwise they are suppressed. Changelog-Deprecated: Config: `announce-addr-dns` (currently defaults to `false`). This will default to `true` once enough of the network has upgraded to understand DNS entries. --- doc/lightning-listconfigs.7.md | 3 ++- doc/lightningd-config.5.md | 11 ++++++---- doc/schemas/listconfigs.schema.json | 4 ++++ lightningd/lightningd.c | 4 ++++ lightningd/lightningd.h | 3 +++ lightningd/options.c | 7 +++++- tests/test_gossip.py | 34 ++++++++++++++++++++++++++--- 7 files changed, 57 insertions(+), 9 deletions(-) diff --git a/doc/lightning-listconfigs.7.md b/doc/lightning-listconfigs.7.md index d6f65b128..cc4b79ee0 100644 --- a/doc/lightning-listconfigs.7.md +++ b/doc/lightning-listconfigs.7.md @@ -101,6 +101,7 @@ On success, an object is returned, containing: - **accept-htlc-tlv-types** (string, optional): `accept-extra-tlvs-type` fields from config or cmdline, or not present - **tor-service-password** (string, optional): `tor-service-password` field from config or cmdline, if any - **dev-allowdustreserve** (boolean, optional): Whether we allow setting dust reserves +- **announce-addr-dns** (boolean, optional): Whether we put DNS entries into node_announcement [comment]: # (GENERATE-FROM-SCHEMA-END) @@ -218,4 +219,4 @@ RESOURCES --------- Main web site: -[comment]: # ( SHA256STAMP:5871ac751654339ed65ab905d61f0bc3afbb8576a33a5c4e9a73d2084f438582) +[comment]: # ( SHA256STAMP:745268f7f4e4eb19d04ec1a221fbb734d89b4a266049cde3adc3131d86423294) diff --git a/doc/lightningd-config.5.md b/doc/lightningd-config.5.md index 45f8933f5..c8c39d925 100644 --- a/doc/lightningd-config.5.md +++ b/doc/lightningd-config.5.md @@ -533,7 +533,7 @@ its use disables autolisten. If necessary, and 'always-use-proxy' is not specified, a DNS lookup may be done to resolve 'DNS' or 'TORIPADDRESS'. If a 'DNS' hostname was given that resolves to a local interface, the daemon -will bind to that interface and also announce that as type 'DNS'. +will bind to that interface: if **announce-addr-dns** is true then it will also announce that as type 'DNS' (rather than announcing the IP address). * **bind-addr**=*\[IPADDRESS\[:PORT\]\]|SOCKETPATH|DNS\[:PORT\]|DNS\[:PORT\]* @@ -565,9 +565,12 @@ announced addresses are public (e.g. not localhost). This option can be used multiple times to add more addresses, and its use disables autolisten. - Since v22.11 'DNS' hostnames can be used for announcement. -Please note that a lot of mainnet nodes do not yet use, read or propagate this -information correctly. + Since v22.11 'DNS' hostnames can be used for announcement: see **announce-addr-dns**. + +* **announce-addr-dns**=*BOOL* + + Set to *true* (default is *false), this so that names given as arguments to **addr** and **announce-addr** are published in node announcement messages as names, rather than IP addresses. Please note that most mainnet nodes do not yet use, read or propagate this information correctly. + * **offline** diff --git a/doc/schemas/listconfigs.schema.json b/doc/schemas/listconfigs.schema.json index 4377c4cea..0eea482da 100644 --- a/doc/schemas/listconfigs.schema.json +++ b/doc/schemas/listconfigs.schema.json @@ -294,6 +294,10 @@ "dev-allowdustreserve": { "type": "boolean", "description": "Whether we allow setting dust reserves" + }, + "announce-addr-dns": { + "type": "boolean", + "description": "Whether we put DNS entries into node_announcement" } } } diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 5dfe18194..58729f3c5 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -206,6 +206,10 @@ static struct lightningd *new_lightningd(const tal_t *ctx) ld->proposed_wireaddr = tal_arr(ld, struct wireaddr_internal, 0); ld->proposed_listen_announce = tal_arr(ld, enum addr_listen_announce, 0); + /*~ The network is not yet ready for DNS names inside node_announcements, + * so we disable this by default for now. */ + ld->announce_dns = false; + ld->remote_addr_v4 = NULL; ld->remote_addr_v6 = NULL; ld->discovered_ip_v4 = NULL; diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index 029b9038f..f587fafc7 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -234,6 +234,9 @@ struct lightningd { /* If they force db upgrade on or off this is set. */ bool *db_upgrade_ok; + /* Announce names in config as DNS records (recently BOLT 7 addition) */ + bool announce_dns; + #if DEVELOPER /* If we want to debug a subdaemon/plugin. */ const char *dev_debug_subprocess; diff --git a/lightningd/options.c b/lightningd/options.c index ff8747733..968f0ae82 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -230,6 +230,7 @@ static char *opt_add_addr_withtype(const char *arg, if (is_ipaddr(address) || is_toraddr(address) || 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, @@ -254,7 +255,7 @@ static char *opt_add_addr_withtype(const char *arg, } /* Add ADDR_TYPE_DNS to announce DNS hostnames */ - if (is_dnsaddr(address) && ala & ADDR_ANNOUNCE) { + if (is_dnsaddr(address) && ld->announce_dns && (ala & ADDR_ANNOUNCE)) { /* BOLT-hostnames #7: * The origin node: * ... @@ -1103,6 +1104,10 @@ static void register_opts(struct lightningd *ld) opt_register_early_noarg("--experimental-shutdown-wrong-funding", opt_set_shutdown_wrong_funding, ld, "EXPERIMENTAL: allow shutdown with alternate txids"); + opt_register_early_arg("--announce-addr-dns", + opt_set_bool_arg, opt_show_bool, + &ld->announce_dns, + "Use DNS entries in --announce-addr and --addr (not widely supported!)"); opt_register_noarg("--help|-h", opt_lightningd_usage, ld, "Print this message."); diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 0e07183b6..d12351c14 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -117,7 +117,8 @@ def test_announce_address(node_factory, bitcoind): """Make sure our announcements are well formed.""" # We do not allow announcement of duplicates. - opts = {'disable-dns': None, 'announce-addr': + opts = {'announce-addr-dns': True, + 'announce-addr': ['4acth47i6kxnvkewtm6q7ib2s3ufpo5sqbsnzjpbi7utijcltosqemad.onion', '1.2.3.4:1234', 'example.com:1236', @@ -158,6 +159,31 @@ def test_announce_address(node_factory, bitcoind): assert addresses_dns[0]['port'] == 1236 +def test_announce_dns_suppressed(node_factory, bitcoind): + """By default announce DNS names as IPs""" + opts = {'announce-addr': 'example.com:1236', + 'start': False} + l1, l2 = node_factory.get_nodes(2, opts=[opts, {}]) + # Remove unwanted disable-dns option! + del l1.daemon.opts['disable-dns'] + l1.start() + + # Need a channel so l1 will announce itself. + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + scid, _ = l1.fundchannel(l2, 10**6) + bitcoind.generate_block(5) + + # Wait for l2 to see l1, with addresses. + wait_for(lambda: l2.rpc.listnodes(l1.info['id'])['nodes'] != []) + wait_for(lambda: 'addresses' in only_one(l2.rpc.listnodes(l1.info['id'])['nodes'])) + + addresses = only_one(l2.rpc.listnodes(l1.info['id'])['nodes'])['addresses'] + assert len(addresses) == 1 + assert addresses[0]['type'] == 'ipv4' + assert addresses[0]['address'] != 'example.com' + assert addresses[0]['port'] == 1236 + + @pytest.mark.developer("gossip without DEVELOPER=1 is slow") def test_announce_and_connect_via_dns(node_factory, bitcoind): """ Test that DNS annoucements propagate and can be used when connecting. @@ -176,6 +202,7 @@ def test_announce_and_connect_via_dns(node_factory, bitcoind): - 'dev-allow-localhost' must not be set, so it does not resolve localhost anyway. """ opts1 = {'disable-dns': None, + 'announce-addr-dns': True, 'announce-addr': ['localhost.localdomain:12345'], # announce dns 'bind-addr': ['127.0.0.1:12345', '[::1]:12345']} # and bind local IPs opts3 = {'may_reconnect': True} @@ -225,7 +252,8 @@ def test_announce_and_connect_via_dns(node_factory, bitcoind): def test_only_announce_one_dns(node_factory, bitcoind): # and test that we can't announce more than one DNS address l1 = node_factory.get_node(expect_fail=True, start=False, - options={'announce-addr': ['localhost.localdomain:12345', 'example.com:12345']}) + options={'announce-addr-dns': True, + 'announce-addr': ['localhost.localdomain:12345', 'example.com:12345']}) l1.daemon.start(wait_for_initialized=False, stderr_redir=True) wait_for(lambda: l1.daemon.is_in_stderr("Only one DNS can be announced")) @@ -234,7 +262,7 @@ def test_announce_dns_without_port(node_factory, bitcoind): """ Checks that the port of a DNS announcement is set to the corresponding network port. In this case regtest 19846 """ - opts = {'announce-addr': ['example.com']} + opts = {'announce-addr-dns': True, 'announce-addr': ['example.com']} l1 = node_factory.get_node(options=opts) # 'address': [{'type': 'dns', 'address': 'example.com', 'port': 0}]