Skip to content

Commit

Permalink
Merge bitcoin#25678: p2p: skip querying dns seeds if -onlynet disab…
Browse files Browse the repository at this point in the history
…les IPv4 and IPv6

385f5a4 p2p: Don't query DNS seeds when both IPv4 and IPv6 are unreachable (Martin Zumsande)
91f0a7f p2p: add only reachable addresses to addrman (Martin Zumsande)

Pull request description:

  Currently, `-onlynet` does not work well in connection with initial peer discovery, because DNS seeds only resolve to IPv6 and IPv4 adresses:
  With `-onlynet=i2p`, we would load clearnet addresses from DNS seeds into addrman, be content our addrman isn't empty so we don't try to query hardcoded seeds (although these exist for i2p!), and never attempt to make an automatic outbound connection.
  With `-onlynet=onion` and `-proxy` set, we wouldn't load addresses via DNS, but will make AddrFetch connections (through a tor exit node) to a random clearnet peer the DNS seed resolves to (see bitcoin#6808 (comment)), thus breaching the `-onlynet` preference of the user - this has been reported in the two issues listed below.

  This PR proposes two changes:
  1.) Don't load addresses that are unreachable (so that we wouldn't connect to them) into addrman. This is already the case for addresses received via p2p addr messages, this PR implements the same for addresses received from DNS seeds and fixed seeds. This means that in the case of `-onlynet=onion`, we wouldn't load fixed seed IPv4 addresses into addrman, only the onion ones.
  2.) Skip trying the DNS seeds if neither IPv4 nor IPv6 are reachable and move directly to adding the hardcoded seeds from networks we can connect to. This is done by soft-setting `-dnsseed` to 0 in this case, unless `-dnsseed=1` was explicitly specified, in which case we abort with an `InitError`.

  Fixes bitcoin#6808
  Fixes bitcoin#12344

ACKs for top commit:
  naumenkogs:
    utACK 385f5a4
  vasild:
    ACK 385f5a4

Tree-SHA512: 33a8c29faccb2d9b937b017dba4ef72c10e05e458ccf258f1aed3893bcc37c2e984ec8de998d2ecfa54282abbf44a132e97d98bbcc24a0dcf1871566016a9b91
  • Loading branch information
fanquake committed Sep 7, 2022
2 parents fc44d17 + 385f5a4 commit 37095c7
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 3 deletions.
17 changes: 17 additions & 0 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,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__);
}
}
}

/**
Expand Down Expand Up @@ -1281,6 +1291,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);

Expand Down
9 changes: 7 additions & 2 deletions src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1642,15 +1642,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());
}
}

Expand Down
3 changes: 2 additions & 1 deletion test/functional/feature_config_args.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 37095c7

Please sign in to comment.