termux / termux-packages

A package build system for Termux.
https://termux.dev
Other
13.33k stars 3.06k forks source link

[Bug]: `getaddrinfo()` should read `$PREFIX/etc/hosts` instead of `/etc/hosts` #10277

Open thunder-coding opened 2 years ago

thunder-coding commented 2 years ago

Problem description

Currently we rely on getaddrinfo() implementation provided by Bionic. It will read /etc/hosts instead of $PREFIX/etc/hosts. Although this shouldn't cause any problems in most cases, but default /etc/hosts on AOSP (atleast till Android 11) does not map the IPv6 of ::1 to localhost, instead it does map it to ip6-localhost. Here's the default /etc/hosts on my device:

127.0.0.1       localhost
::1             ip6-localhost

We can make necessary changes in $PREFIX/etc/hosts in order to overcome this issue.

What steps will reproduce the bug?

This problem was found while trying to find the cause behind a regression in nodejs when trying to resolve IPv6 addresses of localhost

What is the expected behavior?

getaddrinfo and friends should read $PREFIX/etc/hosts instead of /etc/hosts. Maybe we can provide an inline implementation of the same so that newly built packages work as expected

System information

Irrelevant here

xtkoba commented 2 years ago

Or maybe we can provide a shared library libandroid-dns-net.so.

Reference to Bionic source: https://android.googlesource.com/platform/bionic/+/refs/heads/master/libc/dns/net/.

thunder-coding commented 2 years ago

Or maybe we can provide a shared library libandroid-dns-net.so.

That's a much better approach

xtkoba commented 2 years ago

This does not seem to reproduce on my environment (Android 8.1.0 aarch64, asus/WW_X01AD). Here is the test case:

#undef NDEBUG

#include <stdlib.h>
#include <linux/in6.h>
#include <netdb.h>
#include <assert.h>

#ifndef GAI_HOST
#define GAI_HOST "localhost"
#endif
#define GAI_PORT "80"

int main(void)
{
  struct addrinfo hints = { .ai_family = AF_INET6, .ai_socktype = SOCK_STREAM };
  struct addrinfo *res;

  if (getaddrinfo(GAI_HOST, GAI_PORT, &hints, &res) != 0) abort();
  assert(res != NULL);
  for (struct addrinfo *r = res; r != NULL; r = r->ai_next) {
    uint8_t *a = ((struct sockaddr_in6 *)r->ai_addr)->sin6_addr.s6_addr;
    for (size_t i = 0; i < 15; i++) {
      assert(a[i] == 0);
    }
    assert(a[15] == 1);
  }
  freeaddrinfo(res);

  return 0;
}

This means getaddrinfo() on my environment resolves "localhost" for IPv6 as ::1, which is the expected behavior.

It is true that some routines in Bionic libc are affected by issue caused by /etc/hosts vs. $PREFIX/etc/hosts. But we need a test case to trigger that.

xtkoba commented 2 years ago

Also note that $PREFIX/etc/hosts that comes from resolv-conf package defines the hosts in the same way as /etc/hosts:

https://github.com/termux/termux-packages/blob/e606f919cc9ecc890e405c845a78b923a66b477e/packages/resolv-conf/build.sh#L14

thunder-coding commented 2 years ago

This means getaddrinfo() on my environment resolves "localhost" for IPv6 as ::1, which is the expected behavior.

I ran the test case which you sent in your comment and on my device (with modifications to print the return code of getaddrinfo) it fails with EAI_NODATA.

I tried to build the source files inside dns/net from Bionic a week ago, got no luck (seems like GCC is needed to build them).

Maybe we can fix this the same way as we are doing for execve calls using termux-exec. In this case adding libandroid-dns-net.so to ${LD_PRELOAD} would make sense (since some devices seem to need this fix including some Android One devices (and probably AOSP too)). Maybe we should keep a unified libtermux-fixes.so or something like that to keep these kind of stuff at the same place.

xtkoba commented 2 years ago

I ran the test case which you sent in your comment and on my device (with modifications to print the return code of getaddrinfo) it fails with EAI_NODATA.

Curious. Could you share the strace output of the test case? FWIW, on my environment it never reads /etc/hosts.

xtkoba commented 2 years ago

As for LD_PRELOAD hack, it could be possible to "redirect" the open(2) for /etc/hosts to $PREFIX/etc/hosts. The same could go for /etc/resolv.conf.

thunder-coding commented 2 years ago

Curious. Could you share the strace output of the test case? FWIW, on my environment it never reads /etc/hosts.

Same here, it actually connects to the Unix socket at /dev/socket/dnsproxyd. Yet another Android 10 restriction imposed by Google. See https://source.android.com/devices/architecture/modular-system/dns-resolver. Seems like from there /etc/hosts is read by netd, this means the solution proposed by you to redirect open(2) from /etc/hosts to $PREFIX/etc/hosts isn't possible!

Don't know why it works on your device though, can you confirm whether /etc/hosts doesn't include localhost binded to the IPv6. As for other standard Linux distributions, reading /etc/hosts is done by getaddrinfo according to https://jameshfisher.com/2018/02/03/what-does-getaddrinfo-do/

setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
connect(3, {sa_family=AF_UNIX, sun_path="/dev/socket/dnsproxyd"}, 110) = 0
fcntl(3, F_GETFL)                       = 0x2 (flags O_RDWR)                                                          mmap(0x74a2cca000, 262144, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x74a2cca000
prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, 0x74a2cca000, 262144, "scudo:primary") = 0                                    fstat(3, {st_mode=S_IFSOCK|0777, st_size=0, ...}) = 0
mmap(0x74b2cc9000, 262144, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x74b2cc9000
prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, 0x74b2cc9000, 262144, "scudo:primary") = 0
write(3, "getaddrinfo localhost 80 0 10 1 "..., 36) = 36
read(3, "401\0\0\0\0\4\7\0\0\0", 4096)  = 12
close(3)                                = 0
xtkoba commented 2 years ago

So probably borrowing and patching the code from Bionic libc does not solve the issue. We need to borrow the code of getaddrinfo() from e.g. musl.

The /etc/hosts on my environment:

$ cat /etc/hosts
127.0.0.1       localhost
::1             ip6-localhost
thunder-coding commented 2 years ago

Assigning myself, will also run tests for Node.js, libuv, c-ares to make sure nothing else using getaddrinfo and other stuff relying on /etc/hosts breaks.

As proposed by @xtkoba, I'll try to get a workaround done by using musl libc's getaddrinfo and other stuff. The sources which we are looking for is available in https://git.musl-libc.org/cgit/musl/tree/src/network.

Will also look into adding a patch in our build scripts so that any shared library or executable using getaddrinfo if not linked against libandroid-dns-net will raise an error. Lots of package will need to be rebuilt, particularly those which make use of networking calls including libcurl. Does not seem to be a breaking change in any way, but will test it just to be sure

xtkoba commented 2 years ago

I suggest the same way as libandroid-shmem or libandroid-posix-semaphore. The public header should declare libandroid_getaddrinfo and define getaddrinfo as an alias for it. For the check just add libandroid_getaddrinfo to termux_step_massage.sh:

https://github.com/termux/termux-packages/blob/493b04705e0f08a78d52e9a84dbbac432fd73d73/scripts/build/termux_step_massage.sh#L85

stale[bot] commented 2 years ago

This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.