diff --git a/src/init.cpp b/src/init.cpp index 058f88d46fb..be9b11c6ae3 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -561,11 +561,8 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc) argsman.AddArg("-peertimeout=", strprintf("Specify a p2p connection timeout delay in seconds. After connecting to a peer, wait this amount of time before considering disconnection based on inactivity (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-torcontrol=:", strprintf("Tor control host and port to use if onion listening enabled (default: %s). If no port is specified, the default port of %i will be used.", DEFAULT_TOR_CONTROL, DEFAULT_TOR_CONTROL_PORT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-torpassword=", "Tor control port password (default: empty)", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::CONNECTION); -#ifdef USE_UPNP - argsman.AddArg("-upnp", strprintf("Use UPnP to map the listening port (default: %u)", DEFAULT_UPNP), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); -#else - hidden_args.emplace_back("-upnp"); -#endif + // UPnP support was dropped. We keep `-upnp` as a hidden arg to display a more user friendly error when set. TODO: remove (here and below) for 30.0. + argsman.AddArg("-upnp", "", ArgsManager::ALLOW_ANY, OptionsCategory::HIDDEN); argsman.AddArg("-natpmp", strprintf("Use PCP or NAT-PMP to map the listening port (default: %u)", DEFAULT_NATPMP), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-whitebind=<[permissions@]addr>", "Bind to the given address and add permission flags to the peers connecting to it. " "Use [host]:port notation for IPv6. Allowed permissions: " + Join(NET_PERMISSIONS_DOC, ", ") + ". " @@ -740,8 +737,6 @@ void InitParameterInteraction(ArgsManager& args) LogInfo("parameter interaction: -proxy set -> setting -listen=0\n"); // to protect privacy, do not map ports when a proxy is set. The user may still specify -listen=1 // to listen locally, so don't rely on this happening through -listen below. - if (args.SoftSetBoolArg("-upnp", false)) - LogInfo("parameter interaction: -proxy set -> setting -upnp=0\n"); if (args.SoftSetBoolArg("-natpmp", false)) { LogInfo("parameter interaction: -proxy set -> setting -natpmp=0\n"); } @@ -752,8 +747,6 @@ void InitParameterInteraction(ArgsManager& args) if (!args.GetBoolArg("-listen", DEFAULT_LISTEN)) { // do not map ports or try to retrieve public IP when not listening (pointless) - if (args.SoftSetBoolArg("-upnp", false)) - LogInfo("parameter interaction: -listen=0 -> setting -upnp=0\n"); if (args.SoftSetBoolArg("-natpmp", false)) { LogInfo("parameter interaction: -listen=0 -> setting -natpmp=0\n"); } @@ -877,6 +870,12 @@ bool AppInitParameterInteraction(const ArgsManager& args) // also see: InitParameterInteraction() + // We drop UPnP support but kept the arg as hidden for now to display a friendlier error to user who have the + // option in their config. TODO: remove (here and above) for version 30.0. + if (args.IsArgSet("-upnp")) { + return InitError(_("UPnP support was dropped in version 29.0. Consider using '-natpmp' instead.")); + } + // Error if network-specific options (-addnode, -connect, etc) are // specified in default section of config file, but not overridden // on the command line or in this chain's section of the config file. @@ -1827,8 +1826,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) LogPrintf("nBestHeight = %d\n", chain_active_height); if (node.peerman) node.peerman->SetBestBlock(chain_active_height, std::chrono::seconds{best_block_time}); - // Map ports with UPnP or NAT-PMP - StartMapPort(args.GetBoolArg("-upnp", DEFAULT_UPNP), args.GetBoolArg("-natpmp", DEFAULT_NATPMP)); + // Map ports with NAT-PMP + StartMapPort(false, args.GetBoolArg("-natpmp", DEFAULT_NATPMP)); CConnman::Options connOptions; connOptions.m_local_services = g_local_services; diff --git a/src/mapport.cpp b/src/mapport.cpp index bdeda6da344..5d57f8dbb8f 100644 --- a/src/mapport.cpp +++ b/src/mapport.cpp @@ -2,8 +2,6 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include // IWYU pragma: keep - #include #include @@ -18,15 +16,6 @@ #include #include -#ifdef USE_UPNP -#include -#include -#include -// The minimum supported miniUPnPc API version is set to 17. This excludes -// versions with known vulnerabilities. -static_assert(MINIUPNPC_API_VERSION >= 17, "miniUPnPc API version >= 17 assumed"); -#endif // USE_UPNP - #include #include #include @@ -134,78 +123,6 @@ static bool ProcessPCP() return ret; } -#ifdef USE_UPNP -static bool ProcessUpnp() -{ - bool ret = false; - std::string port = strprintf("%u", GetListenPort()); - const char * multicastif = nullptr; - const char * minissdpdpath = nullptr; - struct UPNPDev * devlist = nullptr; - char lanaddr[64]; - - int error = 0; - devlist = upnpDiscover(2000, multicastif, minissdpdpath, 0, 0, 2, &error); - - struct UPNPUrls urls; - struct IGDdatas data; - int r; -#if MINIUPNPC_API_VERSION <= 17 - r = UPNP_GetValidIGD(devlist, &urls, &data, lanaddr, sizeof(lanaddr)); -#else - r = UPNP_GetValidIGD(devlist, &urls, &data, lanaddr, sizeof(lanaddr), nullptr, 0); -#endif - if (r == 1) - { - if (fDiscover) { - char externalIPAddress[40]; - r = UPNP_GetExternalIPAddress(urls.controlURL, data.first.servicetype, externalIPAddress); - if (r != UPNPCOMMAND_SUCCESS) { - LogPrintf("UPnP: GetExternalIPAddress() returned %d\n", r); - } else { - if (externalIPAddress[0]) { - std::optional resolved{LookupHost(externalIPAddress, false)}; - if (resolved.has_value()) { - LogPrintf("UPnP: ExternalIPAddress = %s\n", resolved->ToStringAddr()); - AddLocal(resolved.value(), LOCAL_MAPPED); - } - } else { - LogPrintf("UPnP: GetExternalIPAddress failed.\n"); - } - } - } - - std::string strDesc = PACKAGE_NAME " " + FormatFullVersion(); - - do { - r = UPNP_AddPortMapping(urls.controlURL, data.first.servicetype, port.c_str(), port.c_str(), lanaddr, strDesc.c_str(), "TCP", nullptr, "0"); - - if (r != UPNPCOMMAND_SUCCESS) { - ret = false; - LogPrintf("AddPortMapping(%s, %s, %s) failed with code %d (%s)\n", port, port, lanaddr, r, strupnperror(r)); - break; - } else { - ret = true; - LogPrintf("UPnP Port Mapping successful.\n"); - } - } while (g_mapport_interrupt.sleep_for(PORT_MAPPING_REANNOUNCE_PERIOD)); - g_mapport_interrupt.reset(); - - r = UPNP_DeletePortMapping(urls.controlURL, data.first.servicetype, port.c_str(), "TCP", nullptr); - LogPrintf("UPNP_DeletePortMapping() returned: %d\n", r); - freeUPNPDevlist(devlist); devlist = nullptr; - FreeUPNPUrls(&urls); - } else { - LogPrintf("No valid UPnP IGDs found\n"); - freeUPNPDevlist(devlist); devlist = nullptr; - if (r != 0) - FreeUPNPUrls(&urls); - } - - return ret; -} -#endif // USE_UPNP - static void ThreadMapPort() { bool ok; @@ -219,15 +136,6 @@ static void ThreadMapPort() if (ok) continue; } -#ifdef USE_UPNP - // Low priority protocol. - if (g_mapport_enabled_protos & MapPortProtoFlag::UPNP) { - g_mapport_current_proto = MapPortProtoFlag::UPNP; - ok = ProcessUpnp(); - if (ok) continue; - } -#endif // USE_UPNP - g_mapport_current_proto = MapPortProtoFlag::NONE; if (g_mapport_enabled_protos == MapPortProtoFlag::NONE) { return; @@ -268,7 +176,7 @@ static void DispatchMapPort() assert(g_mapport_thread.joinable()); assert(!g_mapport_interrupt); - // Interrupt a protocol-specific loop in the ThreadUpnp() or in the ThreadPCP() + // Interrupt a protocol-specific loop in the ThreadPCP() // to force trying the next protocol in the ThreadMapPort() loop. g_mapport_interrupt(); } @@ -284,7 +192,6 @@ static void MapPortProtoSetEnabled(MapPortProtoFlag proto, bool enabled) void StartMapPort(bool use_upnp, bool use_pcp) { - MapPortProtoSetEnabled(MapPortProtoFlag::UPNP, use_upnp); MapPortProtoSetEnabled(MapPortProtoFlag::PCP, use_pcp); DispatchMapPort(); } diff --git a/src/mapport.h b/src/mapport.h index 51202687f29..e6348e88dc8 100644 --- a/src/mapport.h +++ b/src/mapport.h @@ -5,13 +5,11 @@ #ifndef BITCOIN_MAPPORT_H #define BITCOIN_MAPPORT_H -static constexpr bool DEFAULT_UPNP = false; - static constexpr bool DEFAULT_NATPMP = false; enum MapPortProtoFlag : unsigned int { NONE = 0x00, - UPNP = 0x01, + // 0x01 was for UPnP, for which we dropped support. PCP = 0x02, // PCP with NAT-PMP fallback. }; diff --git a/src/net.h b/src/net.h index 6e8b91b5f91..6b63e2cc0d1 100644 --- a/src/net.h +++ b/src/net.h @@ -148,7 +148,7 @@ enum LOCAL_NONE, // unknown LOCAL_IF, // address a local interface listens on LOCAL_BIND, // address explicit bound to - LOCAL_MAPPED, // address reported by UPnP or PCP + LOCAL_MAPPED, // address reported by PCP LOCAL_MANUAL, // address explicitly specified (-externalip=) LOCAL_MAX diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index ce68de7eaaf..f9b19f218e3 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -433,7 +433,6 @@ def write_config(config_path, *, n, chain, extra_config="", disable_autoconnect= # in tests. f.write("peertimeout=999999999\n") f.write("printtoconsole=0\n") - f.write("upnp=0\n") f.write("natpmp=0\n") f.write("shrinkdebugfile=0\n") f.write("deprecatedrpc=create_bdb\n") # Required to run the tests