build: target Windows 7 when building libevent and fix ipv6 usage by fanquake · Pull Request #19375 · bitcoin/bitcoin

fanquake · 2020-06-25T03:21:11Z

TLDR: This poaches a commit from #18287 and adds one more to adjust the Windows version targeted when building libevent. These changes combined should fully fix ipv6 usage with the RPC server on Windows.

Binding the RPC server to a ipv6 address does not currently work on Windows.
We currently try and bind to 127.0.0.1 and ::1 by default.

On Windows you'll see lines like this in debug.log:

2020-06-24T01:49:04Z libevent: getaddrinfo: nodename nor servname provided, or not known
2020-06-24T01:49:04Z Binding RPC on address ::1 port 8332 failed

This issue was bought up in, and supposedly fixed by #18287, however the two people that tested it, both said that it didn't fix the problem. I think I now understand why that change alone is incomplete.

Our call into libevent starts with evhttp_bind_socket_with_handle():

evhttp_bind_socket_with_handle()
	bind_socket()
		make_addrinfo()
			evutil_getaddrinfo()
				if #USE_NATIVE_GETADDRINFO
					#ifndef AI_ADDRCONFIG
						evutil_adjust_hints_for_addrconfig_()
							evutil_check_interfaces()
								evutil_check_ifaddrs()
									evutil_found_ifaddr()
										// miss identifies ipv6 as ipv4?
					#endif
					evutil_getaddrinfo_common_()

The problem is falling into "#ifndef AI_ADDRCONFIG":

#ifndef AI_ADDRCONFIG
	/* Not every system has AI_ADDRCONFIG, so fake it. */
	if (hints.ai_family == PF_UNSPEC &&
	    (hints.ai_flags & EVUTIL_AI_ADDRCONFIG)) {
		evutil_adjust_hints_for_addrconfig_(&hints);
	}
#endif

When this occurs, hints end up being adjusted, and it seems that ipv6 addresses end up being mis-identified as ipv4?

However this shouldn't happen, as these AI_ definitions are available on Windows.
The issue is that in evutil.c, _WIN32_WINNT is set to 0x501 (XP).

This obviously predates Vista (0x0600), which is when the AI_ADDRCONFIG definition (and others) became available.

The change here will override libevents internal D_WIN32_WINNT defines. This should be ok, because it's only making "more" of the Windows API available. It's also aligned with what we do in our own configure, we pass D_WIN32_WINNT=0x0601. We also now use linker flags to restrict our binary from running on a Windows version earlier than Windows 7.

The combined fixes can be tested by running:

bitcoind -rpcbind=::1 rpcallowip='0.0.0.0/0' -debug=http

and then querying it using:

bitcoin-cli -rpcconnect=::1 getblockchaininfo

TODO:

Open an issue upstream. windows: issue with ipv6 address detection libevent/libevent#1041

libevent uses getaddrinfo when available, and falls back to gethostbyname Windows has both, but gethostbyname only supports IPv4 libevent fails to detect Windows's getaddrinfo due to not including the right headers This patches libevent's configure script to check it correctly

luke-jr · 2020-06-25T03:47:42Z

depends/packages/libevent.mk

I don't see how this can work... evutil.c #undef _WIN32_WINNT

It's only undef'd after the inclusion of ws2tcpip.h which is where the AI_* definitions are.

In the ws2tcpip.h header on my machine I've got:

#define AI_PASSIVE 0x00000001 #define AI_CANONNAME 0x00000002 #define AI_NUMERICHOST 0x00000004 #if (_WIN32_WINNT >= 0x0600) #define AI_NUMERICSERV 0x00000008 #define AI_ALL 0x00000100 #define AI_ADDRCONFIG 0x00000400 #define AI_V4MAPPED 0x00000800 #define AI_NON_AUTHORITATIVE 0x00004000 #define AI_SECURE 0x00008000 #define AI_RETURN_PREFERRED_NAMES 0x00010000 #endif

Oh, weird. That seems like it's probably a bug, but not our problem.

You might want to add a comment here in the makefile about why this is defined, and why it works.

@laanwj I've added a comment here. These kinds of details probably deserve a separate document in depends (we could also add some of the macOS stuff there as well). I can follow up by writing up some more extensive docs.

luke-jr

utACK

sipsorcery · 2020-06-25T10:26:47Z

tACK b440aca96a338f16ca664065821b980c4b0537ae.

Adjusted test commands:

bitcoind -regtest -rpcbind=::1 -rpcallowip=0.0.0.0/0 -debug=http
bitcoin-cli -regtest -rpcconnect=::1 getblockchaininfo

Relevant parts of bitcoind console messages:

....
2020-06-25T10:21:59Z Command-line arg: debug="http"
2020-06-25T10:21:59Z Command-line arg: regtest=""
2020-06-25T10:21:59Z Command-line arg: rpcallowip="0.0.0.0/0"
2020-06-25T10:21:59Z Command-line arg: rpcbind=****
2020-06-25T10:21:59Z Using at most 125 automatic connections (2048 file descriptors available)
2020-06-25T10:21:59Z Using 16 MiB out of 32/2 requested for signature cache, able to store 524288 elements
2020-06-25T10:21:59Z Using 16 MiB out of 32/2 requested for script execution cache, able to store 524288 elements
2020-06-25T10:21:59Z Script verification uses 7 additional threads
2020-06-25T10:21:59Z Allowing HTTP connections from: 127.0.0.0/8 ::1/128 0.0.0.0/0
2020-06-25T10:21:59Z scheduler thread start
2020-06-25T10:21:59Z Binding RPC on address ::1 port 18443
2020-06-25T10:21:59Z Initialized HTTP server
....
2020-06-25T10:23:07Z Received a POST request for / from [::1]:52391

fanquake · 2020-06-25T11:26:52Z

Just a clarification to "how" this is working, and follow up to the comments above. By default, the mingw32 compiler, (on my machine, x86_64-w64-mingw32-g++ (GCC) 9.3.0) will set _WIN32_WINNT to 0x0502 (Windows Server 2003).

So the fix here isn't just that we are overriding libevents internal _WIN32_WINNT defines, it's because we are actually settings a value for _WIN32_WINNT, which overrides the default set by the compiler, and, is high enough that the definitions are available.

The seconds point is header include order. Because the ws2tcpip.h header, which contains the AI_* definitions:

#define AI_PASSIVE                  0x00000001
#define AI_CANONNAME                0x00000002
#define AI_NUMERICHOST              0x00000004
#if (_WIN32_WINNT >= 0x0600)
#define AI_NUMERICSERV              0x00000008
#define AI_ALL                      0x00000100
#define AI_ADDRCONFIG               0x00000400
#define AI_V4MAPPED                 0x00000800
#define AI_NON_AUTHORITATIVE        0x00004000
#define AI_SECURE                   0x00008000
#define AI_RETURN_PREFERRED_NAMES   0x00010000
#endif

is included before libevent #undefs _WIN32_WINNT and sets it to 0x0501, AI_ADDRCONFIG et al will be defined, because at that point, _WIN32_WINNT is set to the value we have passed (0x0601).

DrahtBot · 2020-06-25T19:16:27Z

Guix builds

File	commit `67881de` (master)	commit 63cf27e54c96aed84025b00932d9a316a01105b6 (master and this pull)
*-aarch64-linux-gnu-debug.tar.gz	`49113bc3ed7f1815...`	`318a738f9cdc2f48...`
*-aarch64-linux-gnu.tar.gz	`999ddbe4a7be7ba8...`	`573434b6feee7979...`
*-arm-linux-gnueabihf-debug.tar.gz	`bbac7bfb9860b5d6...`	`2c363eb44989f880...`
*-arm-linux-gnueabihf.tar.gz	`cf1a95295ee77021...`	`13ef283a7fb408e9...`
*-riscv64-linux-gnu-debug.tar.gz	`3fdf07215f80dfe3...`	`79c1a3e4e4a83698...`
*-riscv64-linux-gnu.tar.gz	`6224201d5937fcc4...`	`2aa3153560373b17...`
*-win-unsigned.tar.gz	`e0d22d0c535d9308...`	`cd45622e543e2ec3...`
*-win64-debug.zip	`a955e7f4d71b592e...`	`2831352a23310256...`
*-win64-setup-unsigned.exe	`4bf65e9ff53ccdf1...`	`b414e067c7035563...`
*-win64.zip	`156debb0703a96df...`	`0a6f4a96e9145d75...`
*-x86_64-linux-gnu-debug.tar.gz	`9f9195121960d3d3...`	`b1fc12491789faf9...`
*-x86_64-linux-gnu.tar.gz	`b8daebe585cc3061...`	`97fac4377623dafa...`
*.tar.gz	`a8caa4ce5c4f6179...`	`a0caef0b3a80fa5a...`
guix_build.log	`9e7212d4ac88ed34...`	`34f033d46ef9d45f...`
guix_build.log.diff		`464e4312d8f44626...`

DrahtBot · 2020-06-26T09:38:12Z

Gitian builds

File	commit `f32f7e9` (master)	commit 7b0e5ab7cdd87dab8e8223929b3c748e23a95daf (master and this pull)
bitcoin-core-linux-0.21-res.yml	`0c3497a54c3bc7ba...`	`dffd10f16f22f162...`
bitcoin-core-win-0.21-res.yml	`0c96eabe3c4b7001...`	`bf5a8c1d6705aed5...`
*-aarch64-linux-gnu-debug.tar.gz	`e6e6eb3eb520abd8...`	`4f460ea874312118...`
*-aarch64-linux-gnu.tar.gz	`8e59a3b4af364dad...`	`114e69c66600410c...`
*-arm-linux-gnueabihf-debug.tar.gz	`2aed896e8c43a57d...`	`486d46b41f3a4765...`
*-arm-linux-gnueabihf.tar.gz	`63293d04b4ec7207...`	`0036110dd4373afb...`
*-riscv64-linux-gnu-debug.tar.gz	`1839d68fe79d7f13...`	`59e354dab7ae4e1e...`
*-riscv64-linux-gnu.tar.gz	`3cc74c73b47e7d01...`	`00718c78f1125dbf...`
*-win64-debug.zip	`57c9d758ab0df9b4...`	`56c12243d2341f26...`
*-win64-setup-unsigned.exe	`dc5ae19748a68fd9...`	`eb4c7f14642f5173...`
*-win64.zip	`65f0e902c07f6c21...`	`fe85fd08913f2199...`
*-x86_64-linux-gnu-debug.tar.gz	`10559cc3035c6d9d...`	`323088e38537ccc6...`
*-x86_64-linux-gnu.tar.gz	`c3566a5163bd0f91...`	`733f72a67a79c595...`
*.tar.gz	`1d0feaaf4c030a20...`	`5e1138a8e28fe411...`
linux-build.log	`7fd8dac8d8b12baa...`	`adf44e5c9b5151a0...`
osx-build.log	`985b22ed7d303197...`	`7804e935f79659a6...`
win-build.log	`cf06ae72cd7f8cdf...`	`08cd72c27b79b2af...`
bitcoin-core-linux-0.21-res.yml.diff		`1407cec4e401ae77...`
bitcoin-core-win-0.21-res.yml.diff		`ec8ff30e1a5276cf...`
linux-build.log.diff		`b9aff065c681ff8c...`
osx-build.log.diff		`3f019019e3bb75f9...`
win-build.log.diff		`7f5879c8074def9f...`

This enables of the use of AI_* definitions in the Windows headers, specifically AI_ADDRCONFIG, which fixes an issue with libevent and ipv6 on Windows. It also aligns with what we define in configure when building Core.

laanwj · 2020-07-01T12:56:36Z

ACK eb6b735

theuni · 2020-07-01T23:50:03Z

Post-merge ACK eb6b735 .

This enables of the use of AI_* definitions in the Windows headers, specifically AI_ADDRCONFIG, which fixes an issue with libevent and ipv6 on Windows. It also aligns with what we define in configure when building Core. Github-Pull: bitcoin#19375 Rebased-From: eb6b735

Summary: Fix for ipv6 on Windows. Backport of core [[bitcoin/bitcoin#19375 | PR19375]]. Depends on D7742. Test Plan: make build-win64 cmake -GNinja .. \ -DCMAKE_TOOLCHAIN_FILE=../cmake/platforms/Win64.cmake \ -DBUILD_BITCOIN_SEEDER=OFF ninja Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D7743

fanquake added Build system Needs gitian build DrahtBot Guix build requested labels Jun 25, 2020

fanquake requested a review from sipsorcery June 25, 2020 03:21

luke-jr reviewed Jun 25, 2020

View reviewed changes

bitcoin deleted a comment Jun 25, 2020

luke-jr approved these changes Jun 25, 2020

View reviewed changes

fanquake mentioned this pull request Jun 25, 2020

windows: issue with ipv6 address detection libevent/libevent#1041

Open

fanquake added RPC/REST/ZMQ Windows labels Jun 25, 2020

fanquake mentioned this pull request Jun 25, 2020

depends: Patch libevent build to fix IPv6 -rpcbind on Windows #18287

Closed

sipsorcery approved these changes Jun 25, 2020

View reviewed changes

DrahtBot removed the DrahtBot Guix build requested label Jun 25, 2020

DrahtBot removed the Needs gitian build label Jun 26, 2020

build: pass _WIN32_WINNT=0x0601 when building libevent for Windows

eb6b735

This enables of the use of AI_* definitions in the Windows headers, specifically AI_ADDRCONFIG, which fixes an issue with libevent and ipv6 on Windows. It also aligns with what we define in configure when building Core.

fanquake force-pushed the libevent_target_0x0601 branch from b440aca to eb6b735 Compare June 30, 2020 05:29

fanquake mentioned this pull request Jun 30, 2020

doc: explain why passing -mlinker-version is required when cross-compiling #19407

Merged

laanwj merged commit e491e55 into bitcoin:master Jul 1, 2020

fanquake deleted the libevent_target_0x0601 branch July 1, 2020 13:08

PRCYDev mentioned this pull request Feb 1, 2021

build: target Windows 7 when building libevent and fix ipv6 usage PRCYCoin/PRCYCoin#13

Merged

str4d mentioned this pull request Jun 5, 2021

build: Set minimum required Windows version to Windows 7 zcash/zcash#5204

Merged

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: target Windows 7 when building libevent and fix ipv6 usage#19375

build: target Windows 7 when building libevent and fix ipv6 usage#19375
laanwj merged 2 commits intobitcoin:masterfrom
fanquake:libevent_target_0x0601

fanquake commented Jun 25, 2020 •

edited

Loading

Uh oh!

luke-jr Jun 25, 2020

Uh oh!

fanquake Jun 25, 2020

Uh oh!

luke-jr Jun 25, 2020

Uh oh!

laanwj Jun 25, 2020 •

edited

Loading

Uh oh!

fanquake Jun 30, 2020

Uh oh!

luke-jr left a comment

Uh oh!

sipsorcery commented Jun 25, 2020

Uh oh!

fanquake commented Jun 25, 2020

Uh oh!

DrahtBot commented Jun 25, 2020

Uh oh!

DrahtBot commented Jun 26, 2020

Uh oh!

laanwj commented Jul 1, 2020

Uh oh!

theuni commented Jul 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

fanquake commented Jun 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luke-jr Jun 25, 2020

Choose a reason for hiding this comment

Uh oh!

fanquake Jun 25, 2020

Choose a reason for hiding this comment

Uh oh!

luke-jr Jun 25, 2020

Choose a reason for hiding this comment

Uh oh!

laanwj Jun 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fanquake Jun 30, 2020

Choose a reason for hiding this comment

Uh oh!

luke-jr left a comment

Choose a reason for hiding this comment

Uh oh!

sipsorcery commented Jun 25, 2020

Uh oh!

fanquake commented Jun 25, 2020

Uh oh!

DrahtBot commented Jun 25, 2020

Guix builds

Uh oh!

DrahtBot commented Jun 26, 2020

Gitian builds

Uh oh!

laanwj commented Jul 1, 2020

Uh oh!

theuni commented Jul 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

fanquake commented Jun 25, 2020 •

edited

Loading

laanwj Jun 25, 2020 •

edited

Loading