arvidn / libtorrent

an efficient feature complete C++ bittorrent implementation
http://libtorrent.org
Other
5.26k stars 997 forks source link

`NETLINK_ROUTE` socket binding not available on Android 11+, can we live without it for Android? #6251

Closed gubatron closed 3 years ago

gubatron commented 3 years ago

libtorrent version (or branch): 1.2.x

platform/architecture: android

Once again Android is about to introduce mandatory breaking changes, all apps must be fixed before November 2021.

image

Some users already running Android 11 are reporting crashes with jlibtorrent

02-24 16:24:34.925 E/mac-restrictions(1901): idm.internet.download.manager.plus:DownloadService tried to call bind() on a NETLINK_ROUTE socket, which is not allowed. Please follow instructions at go/netlink-bug if this app behaves incorrectly.
02-24 16:24:35.030 E/IorapForwardingService(1456): No service published for: iorapd
02-24 16:24:35.030 E/IorapForwardingService(1456): android.os.ServiceManager$ServiceNotFoundException: No service published for: iorapd

Luckily I see that in: include/libtorrent/netlink.hpp src/enum_net.cpp src/ip_notifier.cpp

all the code with NETLINK_ROUTE exists within a compile time defined (I think) TORRENT_USE_NETLINK flag.

I have 2 questions:

ssiloti commented 3 years ago

Netlink is enabled on all Linux systems, including Android. Netlink is used to enumerate network interfaces and routes as well as get notification when the system's network interfaces change. Unfortunately it looks like Google intends to remove the ability of native code to do any of those things.

Without the ability to enumerate network interfaces libtorrent will have to fall back to listening on the unspecified address. This will produce incorrect behavior if the system is multi-homed (e.g. has both Wi-Fi and LTE connections). NAT-PMP support will not work without the ability to enumerate routes. Setting outgoing_interfaces won't work. There may be other issues as well, I don't think the cannot enumerate case is well tested.

wrtorrent commented 3 years ago

api 30 is android 11 https://stackoverflow.com/questions/64884994/cannot-bind-netlink-socket-when-targeting-android-api-30 https://android.googlesource.com/platform/libcore/+/refs/heads/gingerbread-release/luni/src/main/native/ifaddrs-android.h

gubatron commented 3 years ago

Netlink is used to enumerate network interfaces and routes as well as get notification when the system's network interfaces change.

As of API 24, according to the stackoverflow thread shared by @wrtorrent, the solution is to enumerate interfaces using ifaddrs. Not sure if it ifaddrs also covers routes and network interface change callbacks.

Nonetheless, I see the libtorrent code uses ifaddr when not using netlink

gubatron commented 3 years ago

@ssiloti, @arvidn Sorry to bother you, but I'm stuck now for 2 days How can I force the compilation to not use NETLINK and instead use IFADDRS sockets?

I'm trying this on my config.jam (perhaps redundantly) -DTORRENT_USE_IFADDRS=1 -DTORRENT_USE_NETLINK=0

import os ;

ANDROID_TOOLCHAIN = [ os.environ ANDROID_TOOLCHAIN ] ;
ANDROID_API = [ os.environ android_api ] ;

using clang : x86 : $(ANDROID_TOOLCHAIN)/bin/i686-linux-android$(ANDROID_API)-clang++ :
    <cxxflags>-fPIC
    <cxxflags>-std=c++14
    <cxxflags>-DANDROID
    <cxxflags>-DTORRENT_USE_IFADDRS=1
    <cxxflags>-DTORRENT_USE_NETLINK=0
    <cxxflags>-D__STDC_FORMAT_MACROS
    <cxxflags>-D__USE_FILE_OFFSET64
    <cxxflags>-D_FILE_OFFSET_BITS=64
    <cxxflags>-DWITH_IPP=OFF
    <cxxflags>-frtti
    <cxxflags>-fno-strict-aliasing
    <cxxflags>-fvisibility=hidden
    # x86 devices have stack alignment issues, http://b.android.com/222239
    <cxxflags>-mstackrealign
    # debug information
    <cxxflags>-g
    <cxxflags>-gdwarf-4
    <cxxflags>-fdebug-macro
    <cxxflags>-ggdb
    <linkflags>-static-libstdc++
    ;

However when it's building I see a bunch of pre-processors warnings that these macros keep getting redefined, so I think it's not having the effect I wish, which is to disable NETLINK sockets and use instead IFADDR (given the SELinux limitations imposed on Android 11+)

In file included from /src/libtorrent/src/kademlia/sample_infohashes.cpp:33:
In file included from /src/libtorrent/include/libtorrent/kademlia/sample_infohashes.hpp:38:
In file included from /src/libtorrent/include/libtorrent/kademlia/traversal_algorithm.hpp:40:
In file included from /src/libtorrent/include/libtorrent/fwd.hpp:36:
/src/libtorrent/include/libtorrent/config.hpp:174:9: warning: 'TORRENT_USE_IFADDRS' macro redefined [-Wmacro-redefined]
#define TORRENT_USE_IFADDRS 0
        ^
<command line>:2:9: note: previous definition is here
#define TORRENT_USE_IFADDRS 1
        ^
2 warnings generated.
clang-linux.compile.c++.without-pch bin/release/android/x86/src/kademlia/traversal_algorithm.o
In file included from /src/libtorrent/src/kademlia/traversal_algorithm.cpp:33:
In file included from /src/libtorrent/include/libtorrent/kademlia/traversal_algorithm.hpp:40:
In file included from /src/libtorrent/include/libtorrent/fwd.hpp:36:
/src/libtorrent/include/libtorrent/config.hpp:173:9: warning: 'TORRENT_USE_NETLINK' macro redefined [-Wmacro-redefined]
#define TORRENT_USE_NETLINK 1
        ^
<command line>:3:9: note: previous definition is here
#define TORRENT_USE_NETLINK 0

Once the library is used on Android's SDK 30 (Android 11) it's forbidden by SELinux to bind netlink sockets. I keep getting this error onListenFailed(): error_message=listening on 0.0.0.0:0 (device: ) failed: [enum_if] [TCP] Permission denied

I wish I could build for Android 29 and be done, but as of November 2021 it will be mandatory making this upgrade and a bunch of torrent clients using jlibtorrent will break or won't be able to support android 11 devices, which we see are getting a very fast uptake the last few months.

aldenml commented 3 years ago

@gubatron , the ability to switch to ifaddrs for android requires a patch around the related logic in config.h, but that's not a big issue. The problem I see is that getifaddrs,freeifaddrs are only available for API >= 24. If you follow the path of just selecting the logic statically (at compile time) you will need to drop support for quite a few users. The other more complex solution is to detect and select at runtime.

You will definitely lose the ability to get notifications on network interfaces changes, but we were already prepared for this with session_handle::reopen_network_sockets :)

aldenml commented 3 years ago

@arvidn, I looked at this in detail, and there is no way enum_routes can handle android API >= 30 with the current code. Do you think reopen_network_sockets can receive a list of interfaces and routes? any idea of how to able to plug JNI android specific code without ugly hacks?

gubatron commented 3 years ago

@alden

The problem I see is that getifaddrs,freeifaddrs are only available for API >= 24.

This is correct, So far I was able to rebuild no problem with NDK API=24 (which I believe is Android 7.0 Nougat), I rather drop support for Android 6 (not that many users now, I believe it's from 2016) when I look at the existing ratio of Android 10, and increasingly fast growth of Android 11 users.

Screen Shot 2021-06-26 at 1 02 40 PM

But still, I don't think the way I've set the flags has worked. I've also tried setting, just -DTORRENT_USE_IFADDRS by itself. Need to try -DTORRENT_USE_NETLINK=0 by itself.

but we were already prepared for this with session_handle::reopen_network_sockets Good to know.

At this point these are decisions for the app to survive going forward, Google has made some very tough calls on Android 11.

gubatron commented 3 years ago

the ability to switch to ifaddrs for android requires a patch around the related logic in config.h

🙏 Thanks for this @aldenml .

I'll move away from defining those preprocessor constants in the cxxflags in config.jam and instead will experiment with a patch on include/libtorrent/config.hpp (if this is what you mean by "related logic in config.h")

Maybe something like this

diff --git a/include/libtorrent/config.hpp b/include/libtorrent/config.hpp
index 04c32ad68..600e4188d 100644
--- a/include/libtorrent/config.hpp
+++ b/include/libtorrent/config.hpp
@@ -159,6 +159,11 @@ see LICENSE file.
 #define TORRENT_HAS_FTELLO 0
 #endif // API < 24

+#if __ANDROID_API__ >= 24
+#define TORRENT_USE_IFADDRS 1
+#define TORRENT_USE_NETLINK 0
+#endif
+
 #else // ANDROID

 // posix_fallocate() is not available in glibc under these condition
gubatron commented 3 years ago

Managed to create a patch that hardcodes TORRENT_USE_IFADDRS 1 and TORRENT_USE_NETLINK 0 in config.hpp and enum_net.cpp, a libtorrent session can be started successfully from Android 7.0 (SDK 24) and all the way up to Android 11.0 (SDK 30), still have not been able to test on Android 11.

Here's the current patch in case anybody needs it. (to be applied on libtorrent's RC_1_2 branch)

diff --git a/include/libtorrent/alert.hpp b/include/libtorrent/alert.hpp
index ab874a247..9ace38065 100644
--- a/include/libtorrent/alert.hpp
+++ b/include/libtorrent/alert.hpp
@@ -177,7 +177,7 @@ namespace alert_category {
    // interpreted as -1. For instance, boost.python
    // does that and fails when assigning it to an
    // unsigned parameter.
-   constexpr alert_category_t all = alert_category_t::all();
+    //deleted temporarily because it is defined twice

 } // namespace alert_category

diff --git a/include/libtorrent/config.hpp b/include/libtorrent/config.hpp
index bec1b4756..e11d7df9d 100644
--- a/include/libtorrent/config.hpp
+++ b/include/libtorrent/config.hpp
@@ -170,8 +170,8 @@ POSSIBILITY OF SUCH DAMAGE.

 #define TORRENT_HAS_SYMLINK 1
 #define TORRENT_HAVE_MMAP 1
-#define TORRENT_USE_NETLINK 1
-#define TORRENT_USE_IFADDRS 0
+#define TORRENT_USE_NETLINK 0
+#define TORRENT_USE_IFADDRS 1
 #define TORRENT_USE_IFCONF 1
 #define TORRENT_HAS_SALEN 0
 #define TORRENT_USE_FDATASYNC 1
@@ -185,6 +185,10 @@ POSSIBILITY OF SUCH DAMAGE.
 #define TORRENT_ANDROID
 #define TORRENT_HAS_FALLOCATE 0
 #define TORRENT_USE_ICONV 0
+#undef TORRENT_USE_NETLINK
+#undef TORRENT_USE_IFADDRS
+#define TORRENT_USE_NETLINK 0
+#define TORRENT_USE_IFADDRS 1
 #else // ANDROID

 // posix_fallocate() is not available in glibc under these condition
@@ -434,7 +438,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #endif

 #ifndef TORRENT_USE_IFADDRS
-#define TORRENT_USE_IFADDRS 0
+#define TORRENT_USE_IFADDRS 1
 #endif

 // if preadv() exists, we assume pwritev() does as well
diff --git a/src/enum_net.cpp b/src/enum_net.cpp
index 1ba578c6e..b6b685a5c 100644
--- a/src/enum_net.cpp
+++ b/src/enum_net.cpp
@@ -31,7 +31,10 @@ POSSIBILITY OF SUCH DAMAGE.
 */

 #include "libtorrent/config.hpp"
-
+#undef TORRENT_USE_NETLINK
+#undef TORRENT_USE_IFADDRS
+#define TORRENT_USE_IFADDRS 1
+#define TORRENT_USE_NETLINK 0
 #include "libtorrent/enum_net.hpp"
 #include "libtorrent/broadcast_socket.hpp"
 #include "libtorrent/assert.hpp"
@@ -1409,7 +1412,7 @@ int _System __libsocket_sysctl(int* mib, u_int namelen, void *oldp, size_t *oldl
        }

 #else
-#error "don't know how to enumerate network routes on this platform"
+       //#error "don't know how to enumerate network routes on this platform"
 #endif
        return ret;
    }

Android clients that use jlibtorrent will be able to get a hold of this fix on the 1.2.14.0 release coming out soon, hopefully keeping everything working for Android 11 and beyond.

arvidn commented 3 years ago

did a fix for this land in RC_1_2?

gubatron commented 3 years ago

Not sure I understand the question.

To build jlibtorrent for android, we currently check out RC_1_2 and apply the patch above.

We tried passing <cxxflags>-DTORRENT_USE_IFADDRS=1 or/and <cxxflags>-DTORRENT_USE_NETLINK=0 via config.jam for android builds and it didn't really work.

That patch is applied only right before building for android as a desperate measure in my build script.

We plan on migrating to RC_2_0 as soon as android lets us breath, the file systems changes Android made (being able to use File objects and regular file paths again) I believe will make it easier for us to migrate jlibtorrent to RC_2_0 without breaking android.