Repository: bitcoin/bitcoin (language: cpp)

Issue:
Setting `onlynet=onion` still makes some IPv4 connections.
Using Bitcoin Core v0.15.1

I have just set `onlynet=onion` in my `bitcoin.conf` file and have the following in my logs after node restart:

    2018-02-03 12:40:52 receive version message: /Satoshi:0.15.1/: version 70015, blocks=507429, us={IPv4}:{port), peer=0
    2018-02-03 12:41:15 receive version message: /Satoshi:0.14.2/: version 70015, blocks=507429, us={IPv4}:{port), peer=2
    2018-02-03 12:41:22 receive version message: /Satoshi:0.14.2/: version 70015, blocks=507429, us={IPv4}:{port), peer=3
    2018-02-03 12:41:28 receive version message: /Satoshi:0.15.0/: version 70015, blocks=507429, us={IPv4}:{port), peer=4
    2018-02-03 12:41:57 receive version message: /Satoshi:0.15.99/: version 70015, blocks=526679, us={IPv4}:{port), peer=5
    ...
    2018-02-03 12:44:11 receive version message: /Satoshi:0.15.1/: version 70015, blocks=507429, us=[::]:0, peer=6

The peer 6 was an onion.

My `bitcoin.conf` file is as follows:

    uacomment={redacted}
    prune=10239
    banscore=10
    bantime=31536000
    maxmempool=900
    mempoolexpiry=2160
    proxy=127.0.0.1:9050
    listen=1
    bind=127.0.0.1
    onlynet=onion

All the `Used for reaching peers via` tick boxes are still ticked but disabled when I look in Options>Newtork.

Bitcoin Core should not initiate IPv4 (or IPv6) connections when `onlynet=onion`.  These connections should not be seed nodes due to low addresses:

    2018-02-03 12:40:30 Loaded 66198 addresses from peers.dat  254ms

I get some IPv4 version messages each time I restart the node.

Note: added both `dns=0` and `dnsseed=0` at the same time and this *seems* to resolve the issue with limited testing. These entries should not be necessary to limit IPv4 outgoing connect.

If people need to setup Tor for testing, the setup I have used is here on [Bitcoin.SE](https://bitcoin.stackexchange.com/a/70070/75001)


Discussion / hints:
onlynet=onion tells your node to only attempt outbound connections to onion peers.  It doesn't have anything to do with inbound, and in fact it can't actually tell which inbound are onion or not (the logs say that say they are are peer self reports).  To only get onion inbounds, bind your p2p port only to 127.0.0.1 or firewall off the port. 
I agree that setting `onlynet=onion` should tell your node to only attempt outbound connections to onion peers but, v0.15.1 still makes some IPv4 connections (I presume they are made using the proxy settings - I have no easy way to check at hand - my network log box is away in storage!). Onion peers always report as `us=[::]:0` in the log but these were IPv4 addresses as noted. As you can see from the `bitcoin.conf` file I have posted I am using `bind=127.0.0.1` already and do not expect any incoming connects on IPv4 either. All of the connections from the log snippet I posted are outbound, I have added peer 6 to that now.
Using Bitcoin Core v0.17.0rc2

I've noticed an exclusive `onlynet=onion` is ignored in a couple cases as a result of DNS seeding:

1.  If `proxy=127.0.0.1:9050` is set, I get log entries like 
```
YYYY-MM-DDTHH:MM:SSZ SOCKS5 connected dnsseed.bluematt.me
...
YYYY-MM-DDTHH:MM:SSZ New outbound peer connected: version: xxxxx, blocks=xxxxxx, peer=1, peeraddr=[::]:0
```

2. If `proxy` is unset, and a service bit DNS seed lookup fails 
	
```
$ dig +short x9.seed.bitcoin.jonasschnelli.ch | wc -l
0
```

debug.log:
```
YYYY-MM-DDTHH:MM:SSZ Added connection to seed.bitcoin.jonasschnelli.ch peer=0
...
YYYY-MM-DDTHH:MM:SSZ New outbound peer connected: version: xxxxx, blocks=xxxxxx, peer=0, peeraddr=xx.xx.xx.xx:8333
```
Even with Tor proxy set the client is capable of resolving and making a connection to DNS resolved entries via the proxy (I presume that it correctly uses Tor to resolve the DNS?). If you don't even want that then you need to look at `-dns` and `-dnsseed` settings. 


Is this still an issue with a recent version of Bitcoin Core? If yes, what are the steps to reproduce?

Fix patch:
diff --git a/src/init.cpp b/src/init.cpp
index a94bbe6460e5b..7bf60d3b76b9c 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -719,6 +719,16 @@ void InitParameterInteraction(ArgsManager& args)
         if (args.SoftSetBoolArg("-whitelistrelay", true))
             LogPrintf("%s: parameter interaction: -whitelistforcerelay=1 -> setting -whitelistrelay=1\n", __func__);
     }
+    if (args.IsArgSet("-onlynet")) {
+        const auto onlynets = args.GetArgs("-onlynet");
+        bool clearnet_reachable = std::any_of(onlynets.begin(), onlynets.end(), [](const auto& net) {
+            const auto n = ParseNetwork(net);
+            return n == NET_IPV4 || n == NET_IPV6;
+        });
+        if (!clearnet_reachable && args.SoftSetBoolArg("-dnsseed", false)) {
+            LogPrintf("%s: parameter interaction: -onlynet excludes IPv4 and IPv6 -> setting -dnsseed=0\n", __func__);
+        }
+    }
 }
 
 /**
@@ -1322,6 +1332,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     // 2.1. -onlynet is not given or
     // 2.2. -onlynet=cjdns is given
 
+    // Requesting DNS seeds entails connecting to IPv4/IPv6, which -onlynet options may prohibit:
+    // If -dnsseed=1 is explicitly specified, abort. If it's left unspecified by the user, we skip
+    // the DNS seeds by adjusting -dnsseed in InitParameterInteraction.
+    if (args.GetBoolArg("-dnsseed") == true && !IsReachable(NET_IPV4) && !IsReachable(NET_IPV6)) {
+        return InitError(strprintf(_("Incompatible options: -dnsseed=1 was explicitly specified, but -onlynet forbids connections to IPv4/IPv6")));
+    };
+
     // Check for host lookup allowed before parsing any network related parameters
     fNameLookup = args.GetBoolArg("-dns", DEFAULT_NAME_LOOKUP);
 
diff --git a/src/net.cpp b/src/net.cpp
index e9aa7ee43b30f..5db1b4e1a90ee 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1630,15 +1630,20 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
                 LOCK2(m_addr_fetches_mutex, m_added_nodes_mutex);
                 if (m_addr_fetches.empty() && m_added_nodes.empty()) {
                     add_fixed_seeds_now = true;
-                    LogPrintf("Adding fixed seeds as -dnsseed=0, -addnode is not provided and all -seednode(s) attempted\n");
+                    LogPrintf("Adding fixed seeds as -dnsseed=0 (or IPv4/IPv6 connections are disabled via -onlynet), -addnode is not provided and all -seednode(s) attempted\n");
                 }
             }
 
             if (add_fixed_seeds_now) {
+                std::vector<CAddress> seed_addrs{ConvertSeeds(Params().FixedSeeds())};
+                seed_addrs.erase(std::remove_if(seed_addrs.begin(), seed_addrs.end(),
+                                               [](const CAddress& addr) { return !IsReachable(addr); }),
+                                seed_addrs.end());
                 CNetAddr local;
                 local.SetInternal("fixedseeds");
-                addrman.Add(ConvertSeeds(Params().FixedSeeds()), local);
+                addrman.Add(seed_addrs, local);
                 add_fixed_seeds = false;
+                LogPrintf("Added %d fixed seeds from reachable networks.\n", seed_addrs.size());
             }
         }
 


Test patch:
diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py
index 6c51a5ac315bd..eb31bca29aeb7 100755
--- a/test/functional/feature_config_args.py
+++ b/test/functional/feature_config_args.py
@@ -186,11 +186,12 @@ def test_seed_peers(self):
         with self.nodes[0].assert_debug_log(expected_msgs=[
                 "Loaded 0 addresses from peers.dat",
                 "DNS seeding disabled",
-                "Adding fixed seeds as -dnsseed=0, -addnode is not provided and all -seednode(s) attempted\n",
+                "Adding fixed seeds as -dnsseed=0 (or IPv4/IPv6 connections are disabled via -onlynet), -addnode is not provided and all -seednode(s) attempted\n",
         ]):
             self.start_node(0, extra_args=['-dnsseed=0', '-fixedseeds=1'])
         assert time.time() - start < 60
         self.stop_node(0)
+        self.nodes[0].assert_start_raises_init_error(['-dnsseed=1', '-onlynet=i2p', '-i2psam=127.0.0.1:7656'], "Error: Incompatible options: -dnsseed=1 was explicitly specified, but -onlynet forbids connections to IPv4/IPv6")
 
         # No peers.dat exists and dns seeds are disabled.
         # We expect the node will not add fixed seeds when explicitly disabled.