gossipd: don't try to connect to non-routable addresses.

Someone could try to announce an internal address, and we might probe
it.

This breaks tests, so we add '--dev-allow-localhost' for our tests, so
we don't eliminate that one.  Of course, now we need to skip some more
tests in non-developer mode.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

common/test/run-ip_port_parsing.c

@@ -8,9 +8,6 @@
 /* Generated stub for fromwire */
 const u8 *fromwire(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, void *copy UNNEEDED, size_t n UNNEEDED)
 { fprintf(stderr, "fromwire called!\n"); abort(); }
-/* Generated stub for fromwire_bool */
-bool fromwire_bool(const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
-{ fprintf(stderr, "fromwire_bool called!\n"); abort(); }
 /* Generated stub for fromwire_fail */
 const void *fromwire_fail(const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
 { fprintf(stderr, "fromwire_fail called!\n"); abort(); }
@@ -26,9 +23,6 @@ void fromwire_u8_array(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, u8 *arr
 /* Generated stub for towire */
 void towire(u8 **pptr UNNEEDED, const void *data UNNEEDED, size_t len UNNEEDED)
 { fprintf(stderr, "towire called!\n"); abort(); }
-/* Generated stub for towire_bool */
-void towire_bool(u8 **pptr UNNEEDED, bool v UNNEEDED)
-{ fprintf(stderr, "towire_bool called!\n"); abort(); }
 /* Generated stub for towire_u16 */
 void towire_u16(u8 **pptr UNNEEDED, u16 v UNNEEDED)
 { fprintf(stderr, "towire_u16 called!\n"); abort(); }

gossipd/gossip.c

@@ -1606,14 +1606,14 @@ static bool handle_wireaddr_listen(struct daemon *daemon,
 }
 
 /* If it's a wildcard, turns it into a real address pointing to internet */
-static bool public_address(struct wireaddr *wireaddr)
+static bool public_address(struct daemon *daemon, struct wireaddr *wireaddr)
 {
 	if (wireaddr_is_wildcard(wireaddr)) {
 		if (!guess_address(wireaddr))
 			return false;
 	}
 
-	return address_routable(wireaddr);
+	return address_routable(wireaddr, daemon->rstate->dev_allow_localhost);
 }
 
 static void add_announcable(struct daemon *daemon, const struct wireaddr *addr)
@@ -1684,7 +1684,7 @@ static struct wireaddr_internal *setup_listeners(const tal_t *ctx,
 							 true);
 			if (ipv6_ok) {
 				add_binding(&binding, &wa);
-				if (public_address(&wa.u.wireaddr))
+				if (public_address(daemon, &wa.u.wireaddr))
 					add_announcable(daemon, &wa.u.wireaddr);
 			}
 			wa.u.wireaddr.type = ADDR_TYPE_IPV4;
@@ -1693,7 +1693,7 @@ static struct wireaddr_internal *setup_listeners(const tal_t *ctx,
 			if (handle_wireaddr_listen(daemon, &wa.u.wireaddr,
 						   ipv6_ok)) {
 				add_binding(&binding, &wa);
-				if (public_address(&wa.u.wireaddr))
+				if (public_address(daemon, &wa.u.wireaddr))
 					add_announcable(daemon, &wa.u.wireaddr);
 			}
 			continue;
@@ -1701,7 +1701,7 @@ static struct wireaddr_internal *setup_listeners(const tal_t *ctx,
 		case ADDR_INTERNAL_WIREADDR:
 			handle_wireaddr_listen(daemon, &wa.u.wireaddr, false);
 			add_binding(&binding, &wa);
-			if (public_address(&wa.u.wireaddr))
+			if (public_address(daemon, &wa.u.wireaddr))
 				add_announcable(daemon, &wa.u.wireaddr);
 			continue;
 		}
@@ -1723,18 +1723,21 @@ static struct io_plan *gossip_init(struct daemon_conn *master,
 {
 	struct bitcoin_blkid chain_hash;
 	u32 update_channel_interval;
+	bool dev_allow_localhost;
 
 	if (!fromwire_gossipctl_init(
 		daemon, msg, &daemon->broadcast_interval, &chain_hash,
 		&daemon->id, &daemon->globalfeatures,
 		&daemon->localfeatures, &daemon->proposed_wireaddr,
 		&daemon->proposed_listen_announce, daemon->rgb,
-		daemon->alias, &update_channel_interval, &daemon->reconnect)) {
+		daemon->alias, &update_channel_interval, &daemon->reconnect,
+		&dev_allow_localhost)) {
 		master_badmsg(WIRE_GOSSIPCTL_INIT, msg);
 	}
 	/* Prune time is twice update time */
 	daemon->rstate = new_routing_state(daemon, &chain_hash, &daemon->id,
-					   update_channel_interval * 2);
+					   update_channel_interval * 2,
+					   dev_allow_localhost);
 
 	/* Load stored gossip messages */
 	gossip_store_load(daemon->rstate, daemon->rstate->store);
@@ -1944,7 +1947,6 @@ gossip_resolve_addr(const tal_t *ctx,
 		    const struct pubkey *id)
 {
 	struct node *node;
-	struct addrhint *a;
 
 	/* Get from routing state. */
 	node = get_node(rstate, id);
@@ -1952,18 +1954,23 @@ gossip_resolve_addr(const tal_t *ctx,
 	/* No matching node? */
 	if (!node)
 		return NULL;
-	/* Node has no addresses? */
-	if (tal_count(node->addresses) == 0)
-		return NULL;
 
 	/* FIXME: When struct addrhint can contain more than one address,
-	 * we should copy all addresses.
-	 * For now getting first address should be fine. */
-	a = tal(ctx, struct addrhint);
-	a->addr.itype = ADDR_INTERNAL_WIREADDR;
-	a->addr.u.wireaddr = node->addresses[0];
+	 * we should copy all routable addresses. */
+	for (size_t i = 0; i < tal_count(node->addresses); i++) {
+		struct addrhint *a;
+
+		if (!address_routable(&node->addresses[i],
+				      rstate->dev_allow_localhost))
+			continue;
 
-	return a;
+		a = tal(ctx, struct addrhint);
+		a->addr.itype = ADDR_INTERNAL_WIREADDR;
+		a->addr.u.wireaddr = node->addresses[i];
+		return a;
+	}
+
+	return NULL;
 }
 
 static void try_reach_peer(struct daemon *daemon, const struct pubkey *id,

gossipd/gossip_wire.csv

@@ -18,6 +18,7 @@ gossipctl_init,,rgb,3*u8
 gossipctl_init,,alias,32*u8
 gossipctl_init,,update_channel_interval,u32
 gossipctl_init,,reconnect,bool
+gossipctl_init,,dev_allow_localhost,bool
 
 # Activate the gossip daemon, so others can connect.
 gossipctl_activate,3025

gossipd/netaddress.c

@@ -265,7 +265,9 @@ bool guess_address(struct wireaddr *addr)
     abort();
 }
 
-bool address_routable(const struct wireaddr *wireaddr)
+bool address_routable(const struct wireaddr *wireaddr, bool allow_localhost)
 {
+    if (allow_localhost && IsLocal(wireaddr))
+	return true;
     return IsRoutable(wireaddr);
 }

gossipd/netaddress.h

@@ -8,6 +8,7 @@
 bool guess_address(struct wireaddr *wireaddr);
 
 /* Is this address public? */
-bool address_routable(const struct wireaddr *wireaddr);
+bool address_routable(const struct wireaddr *wireaddr,
+		      bool allow_localhost);
 
 #endif /* LIGHTNING_GOSSIPD_NETADDRESS_H */

gossipd/routing.c

@@ -86,7 +86,8 @@ static struct node_map *empty_node_map(const tal_t *ctx)
 struct routing_state *new_routing_state(const tal_t *ctx,
 					const struct bitcoin_blkid *chain_hash,
 					const struct pubkey *local_id,
-					u32 prune_timeout)
+					u32 prune_timeout,
+					bool dev_allow_localhost)
 {
 	struct routing_state *rstate = tal(ctx, struct routing_state);
 	rstate->nodes = empty_node_map(rstate);
@@ -95,6 +96,7 @@ struct routing_state *new_routing_state(const tal_t *ctx,
 	rstate->local_id = *local_id;
 	rstate->prune_timeout = prune_timeout;
 	rstate->store = gossip_store_new(rstate);
+	rstate->dev_allow_localhost = dev_allow_localhost;
 	list_head_init(&rstate->pending_cannouncement);
 	uintmap_init(&rstate->chanmap);
 

gossipd/routing.h

@@ -162,6 +162,9 @@ struct routing_state {
 	 * restarts */
 	struct gossip_store *store;
 
+	/* For testing, we announce and accept localhost */
+	bool dev_allow_localhost;
+
         /* A map of channels indexed by short_channel_ids */
 	UINTMAP(struct chan *) chanmap;
 };
@@ -183,7 +186,8 @@ struct route_hop {
 struct routing_state *new_routing_state(const tal_t *ctx,
 					const struct bitcoin_blkid *chain_hash,
 					const struct pubkey *local_id,
-					u32 prune_timeout);
+					u32 prune_timeout,
+					bool dev_allow_localhost);
 
 struct chan *new_chan(struct routing_state *rstate,
 		      const struct short_channel_id *scid,

gossipd/test/run-bench-find_route.c

@@ -219,7 +219,7 @@ int main(int argc, char *argv[])
 						 | SECP256K1_CONTEXT_SIGN);
 	setup_tmpctx();
 
-	rstate = new_routing_state(tmpctx, &zerohash, &me, 0);
+	rstate = new_routing_state(tmpctx, &zerohash, &me, 0, false);
 	opt_register_noarg("--perfme", opt_set_bool, &perfme,
 			   "Run perfme-start and perfme-stop around benchmark");
 

gossipd/test/run-find_route-specific.c

@@ -152,7 +152,7 @@ int main(void)
 			   strlen("02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06"),
 			   &c);
 
-	rstate = new_routing_state(tmpctx, &zerohash, &a, 0);
+	rstate = new_routing_state(tmpctx, &zerohash, &a, 0, false);
 
 	/* [{'active': True, 'short_id': '6990:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'last_update': 1504064344}, */
 	nc = get_or_make_connection(rstate, &c, &b, "6990:2:1");

gossipd/test/run-find_route.c

@@ -188,7 +188,7 @@ int main(void)
 
 	memset(&tmp, 'a', sizeof(tmp));
 	pubkey_from_privkey(&tmp, &a);
-	rstate = new_routing_state(tmpctx, &zerohash, &a, 0);
+	rstate = new_routing_state(tmpctx, &zerohash, &a, 0, false);
 
 	new_node(rstate, &a);
 

lightningd/gossip_control.c

@@ -185,6 +185,11 @@ void gossip_init(struct lightningd *ld)
 	u64 capabilities = HSM_CAP_ECDH | HSM_CAP_SIGN_GOSSIP;
 	struct wireaddr_internal *wireaddrs = ld->proposed_wireaddr;
 	enum addr_listen_announce *listen_announce = ld->proposed_listen_announce;
+	bool allow_localhost = false;
+#if DEVELOPER
+	if (ld->dev_allow_localhost)
+		allow_localhost = true;
+#endif
 
 	msg = towire_hsm_client_hsmfd(tmpctx, &ld->id, capabilities);
 	if (!wire_sync_write(ld->hsm_fd, msg))
@@ -219,7 +224,8 @@ void gossip_init(struct lightningd *ld)
 	    get_offered_global_features(tmpctx),
 	    get_offered_local_features(tmpctx), wireaddrs,
 	    listen_announce, ld->rgb,
-	    ld->alias, ld->config.channel_update_interval, ld->reconnect);
+	    ld->alias, ld->config.channel_update_interval, ld->reconnect,
+	    allow_localhost);
 	subd_send_msg(ld->gossip, msg);
 }
 

lightningd/lightningd.c

@@ -51,6 +51,7 @@ static struct lightningd *new_lightningd(const tal_t *ctx)
 	ld->dev_debug_subdaemon = NULL;
 	ld->dev_disconnect_fd = -1;
 	ld->dev_subdaemon_fail = false;
+	ld->dev_allow_localhost = false;
 
 	if (getenv("LIGHTNINGD_DEV_MEMLEAK"))
 		memleak_init(ld, backtrace_state);

lightningd/lightningd.h

@@ -189,6 +189,9 @@ struct lightningd {
 	/* If we have --dev-fail-on-subdaemon-fail */
 	bool dev_subdaemon_fail;
 
+	/* Allow and accept localhost node_announcement addresses */
+	bool dev_allow_localhost;
+
 	/* Things we've marked as not leaking. */
 	const void **notleaks;
 #endif /* DEVELOPER */

lightningd/options.c

@@ -415,6 +415,9 @@ static void dev_register_opts(struct lightningd *ld)
 			 "Time between gossip broadcasts in milliseconds");
 	opt_register_arg("--dev-disconnect=<filename>", opt_subd_dev_disconnect,
 			 NULL, ld, "File containing disconnection points");
+	opt_register_noarg("--dev-allow-localhost", opt_set_bool,
+			   &ld->dev_allow_localhost,
+			   "Announce and allow announcments for localhost address");
 }
 #endif
 

tests/test_lightningd.py

@@ -664,6 +664,7 @@ class LightningDTests(BaseLightningDTests):
                                "Cryptographic handshake: ",
                                l1.rpc.connect, '032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e', 'localhost', l2.port)
 
+    @unittest.skipIf(not DEVELOPER, "needs --dev-allow-localhost")
     def test_connect_by_gossip(self):
         """Test connecting to an unknown peer using node gossip
         """
@@ -4284,10 +4285,13 @@ class LightningDTests(BaseLightningDTests):
     def test_address(self):
         l1 = self.node_factory.get_node()
         addr = l1.rpc.getinfo()['address']
-        assert len(addr) == 1
-        assert addr[0]['type'] == 'ipv4'
-        assert addr[0]['address'] == '127.0.0.1'
-        assert int(addr[0]['port']) == l1.port
+        if 'dev-allow-localhost' in l1.daemon.opts:
+            assert len(addr) == 1
+            assert addr[0]['type'] == 'ipv4'
+            assert addr[0]['address'] == '127.0.0.1'
+            assert int(addr[0]['port']) == l1.port
+        else:
+            assert len(addr) == 0
 
         bind = l1.rpc.getinfo()['binding']
         assert len(bind) == 1

tests/utils.py

@@ -261,9 +261,7 @@ class LightningD(TailableProc):
         opts = {
             'bitcoin-datadir': bitcoin_dir,
             'lightning-dir': lightning_dir,
-            'bind-addr': '127.0.0.1:{}'.format(port),
-            # lightningd won't announce non-routable addresses by default.
-            'announce-addr': '127.0.0.1:{}'.format(port),
+            'addr': '127.0.0.1:{}'.format(port),
             'allow-deprecated-apis': 'false',
             'override-fee-rates': '15000/7500/1000',
             'network': 'regtest',
@@ -283,6 +281,8 @@ class LightningD(TailableProc):
                 f.write(seed)
         if DEVELOPER:
             self.opts['dev-broadcast-interval'] = 1000
+            # lightningd won't announce non-routable addresses by default.
+            self.opts['dev-allow-localhost'] = None
         self.prefix = 'lightningd-%d' % (node_id)
 
     @property