Skip to content

Commit

Permalink
daemon: remove UPnP support
Browse files Browse the repository at this point in the history 
Keep the "-upnp" option as a hidden arg for one major version in order
to show a more user friendly error to people who had this option set in
their config file.
  • Loading branch information
@darosior
darosior committed Oct 24, 2024
1 parent 844770b commit 038bbe7
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 110 deletions.
21 changes: 10 additions & 11 deletions src/init.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -561,11 +561,8 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
argsman.AddArg("-peertimeout=<n>", 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=<ip>:<port>", 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=<pass>", "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, ", ") + ". "
Expand Down Expand Up @@ -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");
}
Expand All @@ -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");
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down
95 changes: 1 addition & 94 deletions src/mapport.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <bitcoin-build-config.h> // IWYU pragma: keep

#include <mapport.h>

#include <clientversion.h>
Expand All @@ -18,15 +16,6 @@
#include <util/thread.h>
#include <util/threadinterrupt.h>

#ifdef USE_UPNP
#include <miniupnpc/miniupnpc.h>
#include <miniupnpc/upnpcommands.h>
#include <miniupnpc/upnperrors.h>
// 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 <atomic>
#include <cassert>
#include <chrono>
Expand Down Expand Up @@ -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<CNetAddr> 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;
Expand All @@ -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;
Expand Down Expand Up @@ -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();
}
Expand All @@ -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();
}
Expand Down
4 changes: 1 addition & 3 deletions src/mapport.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
};

Expand Down
2 changes: 1 addition & 1 deletion src/net.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion test/functional/test_framework/util.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 038bbe7

Please sign in to comment.