mirror/bitcoin
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#31130: Drop miniupnp dependency

Browse Source 
40e5f26a3f mapport: remove dead code in DispatchMapPort (Antoine Poinsot)
38fdf7c1fb mapport: drop outdated comments (Antoine Poinsot)
b7b2435290 doc: add release note for #31130 (Antoine Poinsot)
1b6dec98da depends: drop miniupnpc (Antoine Poinsot)
953533d021 doc: remove mentions of UPnP (Antoine Poinsot)
94ad614482 ci: remove UPnP options (Antoine Poinsot)
a9598e5eaa build: drop miniupnpc dependency (Antoine Poinsot)
a5fcfb7385 interfaces: remove now unused 'use_upnp' arg from 'mapPort' (Antoine Poinsot)
038bbe7b20 daemon: remove UPnP support (Antoine Poinsot)
844770b05e qt: remove UPnP settings (Antoine Poinsot)

Pull request description:

  This PR removes UPnP IGD support and drops our [miniupnp](https://github.com/miniupnp/miniupnp) dependency.

  Miniupnpc is a C library (somewhat) maintained by a single person which had several vulnerabilities in the past (a couple dozens are listed [here](https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=miniupnp)), some of which directly affected our software ([RCE in 2015](https://bitcoincore.org/en/2024/07/03/disclose_upnp_rce/), [OOM in 2020](https://bitcoincore.org/en/2024/07/31/disclose-upnp-oom/)).

  The main purpose of this functionality is to have more (non-data-center) reachable nodes on the network. For a non-technical user running Bitcoin Core at home, the software would automatically open a port on their router to receive incoming connections. This way, users not able to manually open a port on their router would still provide the network with more resources and enhance its diversity.

  However, due to past vulnerabilities (and a worry about unknown future ones) in miniupnpc this feature was disabled by default in https://github.com/bitcoin/bitcoin/pull/6795. Having it disabled by default kills (most of?) the purpose of having this functionality in the first place: someone technical enough to understand the `-upnp` startup option or the "enable UPnP" setting is most likely able to open a port on his box in the first place.

  In addition, laanwj implemented PCP with a NAT-PMP fallback directly in Bitcoin Core in https://github.com/bitcoin/bitcoin/pull/30043. If we ever want to re-enable automatic NAT traversal by default in Bitcoin Core, this is the best option (and in my opinion the only sane one). The NAT-PMP fallback makes it so compatibility shouldn't be (much of) an issue.

  On balance, i believe that keeping this functionality and this barely maintained C dependency has higher costs than benefits. Therefore i propose that we get rid of it.

ACKs for top commit:
  jarolrod:
    ACK https://github.com/bitcoin/bitcoin/commit/40e5f26a3ff77e50df808f6f850c617aec2df203
  1440000bytes:
    Code Review ACK https://github.com/bitcoin/bitcoin/pull/31130/commits/40e5f26a3ff77e50df808f6f850c617aec2df203
  laanwj:
    Code review ACK 40e5f26a3f
  i-am-yuvi:
    Tested ACK 40e5f26a3f

Tree-SHA512: 9ea48662775510f5ec6de7af65790f7c8d211603398e9d8c634a86387be81b28081419a95b4d6680d3d7fe6a9f16cec99f16516548201dc7e49781909899a657
This commit is contained in:
merge-script committed 2024-10-28 10:47:34 +00:00
2 parents 2a52718d73 40e5f26a3f
commit 6e21dedbf2
40 files changed
+45 -428
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

depends/Makefile
+1 -5
View File
@@ -40,7 +40,6 @@ NO_BDB ?=
NO_SQLITE ?=
NO_WALLET ?=
NO_ZMQ ?=
NO_UPNP ?=
NO_USDT ?=
MULTIPROCESS ?=
LTO ?=
@@ -157,13 +156,11 @@ bdb_packages_$(NO_BDB) = $(bdb_packages)
sqlite_packages_$(NO_SQLITE) = $(sqlite_packages)
wallet_packages_$(NO_WALLET) = $(bdb_packages_) $(sqlite_packages_)
upnp_packages_$(NO_UPNP) = $(upnp_packages)
zmq_packages_$(NO_ZMQ) = $(zmq_packages)
multiprocess_packages_$(MULTIPROCESS) = $(multiprocess_packages)
usdt_packages_$(NO_USDT) = $(usdt_$(host_os)_packages)
packages += $($(host_arch)_$(host_os)_packages) $($(host_os)_packages) $(boost_packages_) $(libevent_packages_) $(qt_packages_) $(wallet_packages_) $(upnp_packages_) $(usdt_packages_)
packages += $($(host_arch)_$(host_os)_packages) $($(host_os)_packages) $(boost_packages_) $(libevent_packages_) $(qt_packages_) $(wallet_packages_) $(usdt_packages_)
native_packages += $($(host_arch)_$(host_os)_native_packages) $($(host_os)_native_packages)
ifneq ($(zmq_packages_),)
@@ -231,7 +228,6 @@ $(host_prefix)/toolchain.cmake : toolchain.cmake.in $(host_prefix)/.stamp_$(fina
-e 's|@wallet_packages@|$(wallet_packages_)|' \
-e 's|@bdb_packages@|$(bdb_packages_)|' \
-e 's|@sqlite_packages@|$(sqlite_packages_)|' \
-e 's|@upnp_packages@|$(upnp_packages_)|' \
-e 's|@usdt_packages@|$(usdt_packages_)|' \
-e 's|@no_harden@|$(NO_HARDEN)|' \
-e 's|@multiprocess@|$(MULTIPROCESS)|' \
depends/README.md
-1
View File
@@ -112,7 +112,6 @@ The following can be set when running make: `make FOO=bar`
- `NO_WALLET`: Don't download/build/cache libs needed to enable the wallet
- `NO_BDB`: Don't download/build/cache BerkeleyDB
- `NO_SQLITE`: Don't download/build/cache SQLite
- `NO_UPNP`: Don't download/build/cache packages needed for enabling UPnP
- `NO_USDT`: Don't download/build/cache packages needed for enabling USDT tracepoints
- `MULTIPROCESS`: Build libmultiprocess (experimental)
- `DEBUG`: Disable some optimizations and enable more runtime checking
depends/packages/miniupnpc.mk
-36
View File
@@ -1,36 +0,0 @@
package=miniupnpc
$(package)_version=2.2.7
$(package)_download_path=https://miniupnp.tuxfamily.org/files/
$(package)_file_name=$(package)-$($(package)_version).tar.gz
$(package)_sha256_hash=b0c3a27056840fd0ec9328a5a9bac3dc5e0ec6d2e8733349cf577b0aa1e70ac1
$(package)_patches=dont_leak_info.patch cmake_get_src_addr.patch fix_windows_snprintf.patch
$(package)_build_subdir=build
define $(package)_set_vars
$(package)_config_opts = -DUPNPC_BUILD_SAMPLE=OFF -DUPNPC_BUILD_SHARED=OFF
$(package)_config_opts += -DUPNPC_BUILD_STATIC=ON -DUPNPC_BUILD_TESTS=OFF
$(package)_config_opts_mingw32 += -DMINIUPNPC_TARGET_WINDOWS_VERSION=0x0601
endef
define $(package)_preprocess_cmds
patch -p1 < $($(package)_patch_dir)/dont_leak_info.patch && \
patch -p1 < $($(package)_patch_dir)/cmake_get_src_addr.patch && \
patch -p1 < $($(package)_patch_dir)/fix_windows_snprintf.patch
endef
define $(package)_config_cmds
$($(package)_cmake) -S .. -B .
endef
define $(package)_build_cmds
$(MAKE)
endef
define $(package)_stage_cmds
cmake --install . --prefix $($(package)_staging_prefix_dir)
endef
define $(package)_postprocess_cmds
rm -rf bin && \
rm -rf share
endef
depends/packages/packages.mk
-2
View File
@@ -17,8 +17,6 @@ sqlite_packages=sqlite
zmq_packages=zeromq
upnp_packages=miniupnpc
multiprocess_packages = libmultiprocess capnp
multiprocess_native_packages = native_libmultiprocess native_capnp
depends/patches/miniupnpc/cmake_get_src_addr.patch
-22
View File
@@ -1,22 +0,0 @@
commit cb2026239c2a3aff393952ccb0ee1c448189402d
Author: fanquake <fanquake@gmail.com>
Date: Fri Mar 22 14:03:54 2024 +0000
build: add MINIUPNPC_GET_SRC_ADDR to CMake build
This mirrors the autotools build.
See https://github.com/miniupnp/miniupnp/pull/721.
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1aa95a8..0cacf3e 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -31,6 +31,7 @@ endif ()
if (NOT WIN32)
target_compile_definitions(miniupnpc-private INTERFACE
MINIUPNPC_SET_SOCKET_TIMEOUT
+ MINIUPNPC_GET_SRC_ADDR
_BSD_SOURCE _DEFAULT_SOURCE)
if (NOT APPLE AND NOT CMAKE_SYSTEM_NAME MATCHES ".*BSD" AND NOT CMAKE_SYSTEM_NAME STREQUAL "SunOS")
# add_definitions (-D_POSIX_C_SOURCE=200112L)
depends/patches/miniupnpc/dont_leak_info.patch
-32
View File
@@ -1,32 +0,0 @@
commit 51f6dd991c29af66fb4f64c6feb2787cce23a1a7
Author: fanquake <fanquake@gmail.com>
Date: Mon Jan 8 11:21:40 2024 +0000
Don't leak OS and miniupnpc version info in User-Agent
diff --git a/src/minisoap.c b/src/minisoap.c
index 903ac5f..046e0ea 100644
--- a/src/minisoap.c
+++ b/src/minisoap.c
@@ -90,7 +90,7 @@ int soapPostSubmit(SOCKET fd,
headerssize = snprintf(headerbuf, sizeof(headerbuf),
"POST %s HTTP/%s\r\n"
"Host: %s%s\r\n"
- "User-Agent: " OS_STRING " " UPNP_VERSION_STRING " MiniUPnPc/" MINIUPNPC_VERSION_STRING "\r\n"
+ "User-Agent: " UPNP_VERSION_STRING "\r\n"
"Content-Length: %d\r\n"
#if (UPNP_VERSION_MAJOR == 1) && (UPNP_VERSION_MINOR == 0)
"Content-Type: text/xml\r\n"
diff --git a/src/miniwget.c b/src/miniwget.c
index e76a5e5..0cc36fe 100644
--- a/src/miniwget.c
+++ b/src/miniwget.c
@@ -444,7 +444,7 @@ miniwget3(const char * host,
"GET %s HTTP/%s\r\n"
"Host: %s:%d\r\n"
"Connection: Close\r\n"
- "User-Agent: " OS_STRING " " UPNP_VERSION_STRING " MiniUPnPc/" MINIUPNPC_VERSION_STRING "\r\n"
+ "User-Agent: " UPNP_VERSION_STRING "\r\n"
"\r\n",
path, httpversion, host, port);
depends/patches/miniupnpc/fix_windows_snprintf.patch
-25
View File
@@ -1,25 +0,0 @@
commit a1e9de80ab99b4c956a6a4e21d3e0de6f7a1014d
Author: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Sat Apr 20 15:14:47 2024 +0100
Fix macro expression that guards `snprintf` for Windows
Otherwise, the `snprintf` is still wrongly emulated for the following
cases:
- mingw-w64 6.0.0 or new with ucrt
- mingw-w64 8.0.0 or new with iso c ext
--- a/src/win32_snprintf.h
+++ b/src/win32_snprintf.h
@@ -23,9 +23,9 @@
(defined(_MSC_VER) && _MSC_VER < 1900) /* Visual Studio older than 2015 */ || \
(defined(__MINGW32__) && !defined(__MINGW64_VERSION_MAJOR) && defined(__NO_ISOCEXT)) /* mingw32 without iso c ext */ || \
(defined(__MINGW64_VERSION_MAJOR) && /* mingw-w64 not ... */ !( \
- (defined (__USE_MINGW_ANSI_STDIO) && __USE_MINGW_ANSI_STDIO != 0)) /* ... with ansi stdio */ || \
+ (defined (__USE_MINGW_ANSI_STDIO) && __USE_MINGW_ANSI_STDIO != 0) /* ... with ansi stdio */ || \
(__MINGW64_VERSION_MAJOR >= 6 && defined(_UCRT)) /* ... at least 6.0.0 with ucrt */ || \
- (__MINGW64_VERSION_MAJOR >= 8 && !defined(__NO_ISOCEXT)) /* ... at least 8.0.0 with iso c ext */ || \
+ (__MINGW64_VERSION_MAJOR >= 8 && !defined(__NO_ISOCEXT))) /* ... at least 8.0.0 with iso c ext */ || \
0) || \
0)
depends/toolchain.cmake.in
-7
View File
@@ -139,13 +139,6 @@ else()
set(WITH_SQLITE ON CACHE BOOL "")
endif()
set(upnp_packages @upnp_packages@)
if("${upnp_packages}" STREQUAL "")
set(WITH_MINIUPNPC OFF CACHE BOOL "")
else()
set(WITH_MINIUPNPC ON CACHE BOOL "")
endif()
set(usdt_packages @usdt_packages@)
if("${usdt_packages}" STREQUAL "")
set(WITH_USDT OFF CACHE BOOL "")