Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib/hostip.c: make NAT64 address synthesis on macOS working #7121

Closed
wants to merge 8 commits into from
Closed

lib/hostip.c: make NAT64 address synthesis on macOS working #7121

wants to merge 8 commits into from

Conversation

zajdee
Copy link

@zajdee zajdee commented May 24, 2021

On macOS running in an IPv6-only access network where DNS64 and NAT64 functions are also deployed, the operating system is able to synthesize the NAT64 IPv6 address when an IPv4-only literal is used. This is described in man 3 getaddrinfo as follows:

     The current implementation supports synthesis of NAT64 mapped IPv6 addresses.  If hostname
     is a numeric string defining an IPv4 address (for example, ``192.0.2.1'' ) and ai_family is
     set to PF_UNSPEC or PF_INET6, getaddrinfo() will synthesize the appropriate IPv6
     address(es) (for example, ``64:ff9b::192.0.2.1'' ) if the current interface supports IPv6,
     NAT64 and DNS64 and does not support IPv4. If the AI_ADDRCONFIG flag is set, the IPv4
     address will be suppressed on those interfaces.  On non-qualifying interfaces,
     getaddrinfo() is guaranteed to return immediately without attempting any resolution, and
     will return the IPv4 address if ai_family is PF_UNSPEC or PF_INET. NAT64 address synthesis
     can be disabled by setting the AI_NUMERICHOST flag. To best support NAT64 networks, it is
     recommended to resolve all IP address literals with ai_family set to PF_UNSPEC and ai_flags
     set to AI_DEFAULT.

     Note that NAT64 address synthesis is always disabled for IPv4 addresses in the following
     ranges: 0.0.0.0/8, 127.0.0.0/8, 169.254.0.0/16, 192.0.0.0/29, 192.88.99.0/24, 224.0.0.0/4,
     and 255.255.255.255/32.  Additionally, NAT64 address synthesis is disabled when the network
     uses the well-known prefix (64:ff9b::/96) for IPv4 addresses in the following ranges:
     10.0.0.0/8, 100.64.0.0/10, 172.16.0.0/12, and 192.168.0.0/16.

     Historically, passing a host's own hostname to getaddrinfo() has been a popular technique
     for determining that host's IP address(es), but this is fragile, and doesn't work reliably
     in all cases. The appropriate way for software to discover the IP address(es) of the host
     it is running on is to use getifaddrs(3).

The operating system however does not perform this synthesis unless the SCDynamicStoreCopyProxies function from the SystemConfiguration framework is called.

This commit therefore attempts to call the function in lib/hostip.c if USE_RESOLVE_ON_IPS is set (which is currently only set on macOS) before any hostname resolution is performed. As a result of this function call, curl is able to perform requests such as curl http://1.1.1.1/ even on networks where there is no IPv4 connectivity in the access network.

@zajdee
Copy link
Author

zajdee commented May 24, 2021

A working demonstration of the code:

The curl binary I regularly use:

$ curl --version
curl 7.64.1 (x86_64-apple-darwin20.0) libcurl/7.64.1 (SecureTransport) LibreSSL/2.8.3 zlib/1.2.11 nghttp2/1.41.0
Release-Date: 2019-03-27
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: AsynchDNS GSS-API HTTP2 HTTPS-proxy IPv6 Kerberos Largefile libz MultiSSL NTLM NTLM_WB SPNEGO SSL UnixSockets

$ curl -v http://1.1.1.1/
*   Trying 1.1.1.1...
* TCP_NODELAY set
* Immediate connect fail for 1.1.1.1: Network is unreachable
* Closing connection 0
curl: (7) Couldn't connect to server

curl built with the patch:

$ ./src/curl --version
curl 7.77.0-DEV (x86_64-apple-darwin20.4.0) libcurl/7.77.0-DEV SecureTransport zlib/1.2.11 libidn2/2.3.0
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS HSTS IDN IPv6 Largefile libz NTLM NTLM_WB SSL UnixSockets

$ ./src/curl -v http://1.1.1.1/
*   Trying 64:ff9b::101:101:80...
* Connected to 1.1.1.1 (64:ff9b::101:101) port 80 (#0)
> GET / HTTP/1.1
> Host: 1.1.1.1
> User-Agent: curl/7.77.0-DEV
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 301 Moved Permanently
< Date: Mon, 24 May 2021 14:47:24 GMT
< Content-Type: text/html
< Content-Length: 167
< Connection: keep-alive
< Location: https://1.1.1.1/
< cf-request-id: 0a40715cde0000410dc9bea000000001
< Server: cloudflare
< CF-RAY: 654751a7c950410d-PRG
<
<html>
<head><title>301 Moved Permanently</title></head>
<body>
<center><h1>301 Moved Permanently</h1></center>
<hr><center>cloudflare</center>
</body>
</html>
* Connection #0 to host 1.1.1.1 left intact

@bagder bagder added the name lookup DNS and related tech label May 24, 2021
@bagder
Copy link
Member

bagder commented May 24, 2021

You modified two m4 files to add build flags. Those places only run if configure finds rustls or secure-transport to get used, not other TLS backends, but this name resolver thing is independent of TLS right? It seems better to have it done in a more central place for all macOS (and more?) builds, doesn't it?

I assume the test build you did and proved working was done with cmake? Did you verify a configure build too?

(my mac doesn't have a working IPv6 connection, I can't easily test this)

@zajdee
Copy link
Author

zajdee commented May 24, 2021

Actually I've built it on macOS Big Sur using ./configure && make. I need to figure out how to build it using cmake.

And yes, you're right - this is independent on the TLS backend, so it'd better to be move out. I need to figure out how to include the framework correctly for all cases (even when no/other TLS backend is used).

Let's wait for the CI jobs to finish to eventually fix what the jobs find.

@bagder
Copy link
Member

bagder commented May 24, 2021

The github action ones have been horribly slow the last few days though so there's a significant risk they will take 10+ hours to complete.

@zajdee
Copy link
Author

zajdee commented May 24, 2021

I must be doing something wrong, but can't really build curl using cmake. :(
cmake runs fine, but then make fails claiming it can't link against libidn2. The library and includes exist on my system (installed from homebrew), but I don't know how to tell why the link is failing. Have tried lots of hacks, no luck so far. :(

$ cmake  -DCMAKE_BUILD_TYPE=Release  ../curl -DCMAKE_USE_SECTRANSP=1   -DLIBIDN2_PATH=/usr/local/Cellar/libidn2/2.3.0/
-- curl version=[7.77.0-DEV]
-- Enabled features: SSL IPv6 unixsockets libz AsynchDNS Largefile alt-svc HSTS NTLM MultiSSL HTTPS-proxy
-- Enabled protocols: DICT FILE FTP FTPS GOPHER GOPHERS HTTP HTTPS IMAP IMAPS LDAP MQTT POP3 POP3S RTSP SCP SFTP SMB SMBS SMTP SMTPS TELNET TFTP
-- Enabled SSL backends: OpenSSL Secure Transport
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/rzajic/Work/git-priv/curl-cmake
$ make
Consolidate compiler generated dependencies of target libcurl
[  0%] Linking C shared library libcurl.dylib
Undefined symbols for architecture x86_64:
  "_idn2_check_version", referenced from:
      _Curl_idnconvert_hostname in url.c.o
      _curl_version in version.c.o
      _curl_version_info in version.c.o
  "_idn2_free", referenced from:
      _conn_free in url.c.o
      _Curl_free_idnconverted_hostname in url.c.o
      _create_conn in url.c.o
  "_idn2_lookup_ul", referenced from:
      _Curl_idnconvert_hostname in url.c.o
  "_idn2_strerror", referenced from:
      _Curl_idnconvert_hostname in url.c.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [lib/libcurl.dylib] Error 1
make[1]: *** [lib/CMakeFiles/libcurl.dir/all] Error 2
make: *** [all] Error 2

@zajdee
Copy link
Author

zajdee commented May 24, 2021

Apparently I needed a clean build environment for cmake. Moved out of the way, now working on fixing up the m4 files.

@zajdee
Copy link
Author

zajdee commented May 24, 2021

I've moved the framework addition to a separate if in CMakeLists.txt for cmake and created a new file, m4/curl-sysconfig.m4, and hooked that m4 function (CURL_DARWIN_SYSTEMCONFIGURATION) in configure.ac (it's hard for me to say if it's a good place for the hook or not - please shout if there's a better one).

The curl_winssl_* failing workflows are causing some sweat to me, however the failures do not seem to be directly related to my changes - again, feedback is welcome, if it's me breaking the jobs. :)

I've tested builds on macOS with cmake and with the autoreconf -i && ./configure && make toolset, in both cases it does seem to work just fine. (For the ./configure && make, I have tried options --without-ssl and --with-secure-transport, to check if the LDFLAGS variable in Makefile gets populated correctly. It does.)

Thanks.

@zajdee zajdee changed the title lib/hostip6.c: make NAT64 address synthesis on macOS working lib/hostip.c: make NAT64 address synthesis on macOS working May 24, 2021
lib/hostip.c Outdated
@@ -529,6 +533,21 @@ enum resolve_t Curl_resolv(struct Curl_easy *data,
return CURLRESOLV_ERROR;
}

#ifdef ENABLE_IPV6
#if defined(USE_RESOLVE_ON_IPS) && TARGET_OS_OSX
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this perhaps rather be ...&& defined(TARGET_OS_OSX) ? It risks causing a warning otherwise when not defined.

Also: where is TARGET_OS_OSX defined? Is that from the macOS SDK?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TARGET_OS_OSX is defined in TargetConditionals.h from the macOS SDK, yes.
I can include it explicitly in the file, currently it only seems to be included implicitly.

Copy link
Author

@zajdee zajdee May 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more minutes of research:
TARGET_OS_OSX is defined with value 0 for non-macOS targets (such as iOS), therefore a simple #ifdef TARGET_OS_OSX is not the best way forward.
#if defined(TARGET_OS_OSX) && TARGET_OS_OSX == 1 on the other hand seems to do the job.

@@ -68,6 +68,10 @@
#include "curl_memory.h"
#include "memdebug.h"

#ifdef USE_RESOLVE_ON_IPS
#include <SystemConfiguration/SystemConfiguration.h>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure here if to wrap it with more conditions like the ones below - I need to check if it exists on targets like iOS. If not, the extra #if wrapping will be needed (or another, shared #define created elsewhere).

@bagder
Copy link
Member

bagder commented May 25, 2021

Thanks!

@bagder bagder closed this in 31f631a May 25, 2021
zajdee pushed a commit to zajdee/curl that referenced this pull request May 25, 2021
SpaceIm added a commit to SpaceIm/conan-center-index that referenced this pull request May 28, 2021
conan-center-bot pushed a commit to conan-io/conan-center-index that referenced this pull request May 29, 2021
* libcurl: Bump to 7.77.0

Signed-off-by: Alejandro Colomar <alejandro.colomar@exfo.com>

* add SystemConfiguration frameworks since 7.77.0

see curl/curl#7121

* propagate frameworks for all Apple OS, not only macOS

also replace Cocoa by CoreFoundation

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
madebr pushed a commit to madebr/conan-center-index that referenced this pull request Jun 21, 2021
* libcurl: Bump to 7.77.0

Signed-off-by: Alejandro Colomar <alejandro.colomar@exfo.com>

* add SystemConfiguration frameworks since 7.77.0

see curl/curl#7121

* propagate frameworks for all Apple OS, not only macOS

also replace Cocoa by CoreFoundation

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
stanhu added a commit to stanhu/curl that referenced this pull request Jun 5, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

Closes curl#11252
stanhu added a commit to stanhu/curl that referenced this pull request Jun 5, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

Closes curl#11252
stanhu added a commit to stanhu/curl that referenced this pull request Jun 5, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

Closes curl#11252
stanhu added a commit to stanhu/curl that referenced this pull request Jun 5, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

Closes curl#11252
stanhu added a commit to stanhu/curl that referenced this pull request Jun 5, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

Closes curl#11252
stanhu added a commit to stanhu/curl that referenced this pull request Jun 9, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

In addition, this change is beneficial because it:

1. Avoids extra macOS system calls for every IP lookup.
2. Consolidates macOS-specific initialization in a separate file.

Closes curl#11252
bagder pushed a commit that referenced this pull request Jul 9, 2023
#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

In addition, this change is beneficial because it:

1. Avoids extra macOS system calls for every IP lookup.
2. Consolidates macOS-specific initialization in a separate file.

Fixes #11252
Closes #11254
bch pushed a commit to bch/curl that referenced this pull request Jul 19, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

In addition, this change is beneficial because it:

1. Avoids extra macOS system calls for every IP lookup.
2. Consolidates macOS-specific initialization in a separate file.

Fixes curl#11252
Closes curl#11254
@badger200
Copy link

badger200 commented Jul 22, 2023

This breaks curl on macOS 10.9 MacPorts due to failing to link SystemConfiguration Framework, causing missing SCDynamicStoreCopyProxies called from libcurl.4.dylib. The configure line checks whether we need to link macOS frameworks SystemConfiguration but for some reason answers: no.

@ryandesign
Copy link
Contributor

TARGET_OS_OSX did not exist until the macOS 10.12 SDK so a different check should be used to identify macOS. See #11502.

ptitSeb pushed a commit to wasix-org/curl that referenced this pull request Sep 25, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

In addition, this change is beneficial because it:

1. Avoids extra macOS system calls for every IP lookup.
2. Consolidates macOS-specific initialization in a separate file.

Fixes curl#11252
Closes curl#11254
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
name lookup DNS and related tech
Development

Successfully merging this pull request may close these issues.

None yet

4 participants