Tor Build for Android

added TorBrowserTeam202004R component::applications/tor browser owner::sisbell parent::28704 points::1 priority::medium resolution::fixed reviewer::boklm severity::normal status::closed tbb-mobile tbb-parity tbb-rbm type::defect labels

Tor needs rust 1.28. However for android we are using 1.26.1. How should we handle this?

For the linux nightly build, we build rust 1.28.0. Maybe we can do the same.

We can also start by building Tor without rust (like on Windows and macOS).

Replying to boklm:

For the linux nightly build, we build rust 1.28.0. Maybe we can do the same.

We can also start by building Tor without rust (like on Windows and macOS).

Yes, let's start without building Tor on Android with the newer Rust.

tbb-parity items.

Trac:
Keywords: N/A deleted, tbb-parity added

Trac:
Cc: sisbell, gk to sisbell, gk, hans@guardianproject.info

Trac:
0001-move-Android-build-setup-into-enable-android-flag.patch

I just added a patch to move key Android configuration to ./configure --enable-android so that it is shared across all the builds. There is more stuff that can be moved there, that is not yet:

Android NDK version detection and enforcement (e.g. >= r19 or only exactly r20)
CFLAGS and LDFLAGS to find parity with Android NDK builds done with ndk-build, which automatically sets things there.
Perhaps the state of --disable-linker-hardening and --disable-gcc-hardening could also be maintained there, since we're all building with the Android NDK's clang.

The patch is attached, here's the diff:

diff --git a/configure.ac b/configure.ac
index a639ffaf3..afee6940c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -88,6 +88,27 @@ else
             [Defined if we're building with OpenSSL or LibreSSL])
 fi
 
+dnl Enable Android only features.
+AC_ARG_ENABLE(android,
+     AS_HELP_STRING(--enable-android, [build with Android features enabled]))
+AM_CONDITIONAL([USE_ANDROID], [test "x$enable_android" = "xyes"])
+
+if test "x$enable_android" = "xyes"; then
+  AC_DEFINE([USE_ANDROID], [1], [Compile with Android specific features enabled])
+
+  dnl Check if the Android log library is available.
+  AC_CHECK_HEADERS([android/log.h])
+  AC_SEARCH_LIBS(__android_log_write, [log])
+
+  asciidoc=false
+  enable_html_manual=no
+  enable_manpage=no
+  enable_pic=yes
+  enable_system_torrc=no
+  enable_tool_name_check=no
+  have_systemd=no
+fi
+
 if test "$enable_static_tor" = "yes"; then
   enable_static_libevent="yes";
   enable_static_openssl="yes";
@@ -117,7 +138,7 @@ AC_ARG_ENABLE(asciidoc,
         "yes") asciidoc=true ;;
         "no")  asciidoc=false ;;
         *) AC_MSG_ERROR(bad value for --disable-asciidoc) ;;
-      esac], [asciidoc=true])
+      esac], [test "x$asciidoc" = "xtrue" && asciidoc=true])
 
 # systemd notify support
 AC_ARG_ENABLE(systemd,
@@ -227,20 +248,6 @@ if test x$enable_event_tracing_debug = xyes; then
   AC_DEFINE([TOR_EVENT_TRACING_ENABLED], [1], [Compile the event tracing instrumentation])
 fi
 
-dnl Enable Android only features.
-AC_ARG_ENABLE(android,
-     AS_HELP_STRING(--enable-android, [build with Android features enabled]))
-AM_CONDITIONAL([USE_ANDROID], [test "x$enable_android" = "xyes"])
-
-if test "x$enable_android" = "xyes"; then
-  AC_DEFINE([USE_ANDROID], [1], [Compile with Android specific features enabled])
-
-  dnl Check if the Android log library is available.
-  AC_CHECK_HEADERS([android/log.h])
-  AC_SEARCH_LIBS(__android_log_write, [log])
-
-fi
-
 dnl ---
 dnl Tor modules options. These options are namespaced with --disable-module-XXX
 dnl ---
-- 
2.20.1

Trac:
Points: N/A to 0.25

Trac:
Points: 0.25 to 1

Do we know if this patch works with Android NDK 17b?

That configure.ac patch is now here for #31882 (moved):

https://github.com/torproject/tor/pull/1408

I just got the gitlab-ci job for r17b working, so I can confirm that tor with my changes builds with NDK r17b, but this job does not include lzma or zstd.

Replying to eighthave:

That configure.ac patch is now here for #31882 (moved):

https://github.com/torproject/tor/pull/1408

I just got the gitlab-ci job for r17b working, so I can confirm that tor with my changes builds with NDK r17b, but this job does not include lzma or zstd.

I'm assuming we need to open new tickets for building these libraries? Or is this something we can live without for now?

Trac:
Keywords: TorBrowserTeam201812 deleted, TorBrowserTeam201912 added
Owner: tbb-team to sisbell
Status: new to assigned

Trac:
Cc: sisbell, gk, hans@guardianproject.info to sisbell, gk, hans@guardianproject.info, tbb-team

Trac:
Keywords: TorBrowserTeam201912 deleted, TorBrowserTeam202001 added

Moving tickets to February

Trac:
Keywords: TorBrowserTeam202001 deleted, TorBrowserTeam202002 added

-cp $distdir/share/tor/geoip $TORCONFIGDIR
-cp $distdir/share/tor/geoip6 $TORCONFIGDIR
-
+[% IF !c("var/android") -%]
+  cp $distdir/share/tor/geoip $TORCONFIGDIR
+  cp $distdir/share/tor/geoip6 $TORCONFIGDIR
+[% END -%]

Can you set TORCONFIGDIR=assets/common and use the original code instead?

In the build script, please use install instead of cp (see how files are copied within the other platform blocks).

+[% IF c("var/android") %]
+  libsdir=jniLibs/[% c("var/abi") %]
+  mkdir -p $libsdir assets/common
+  # Copy tor using naming convention that Android will recognize 
+  cp bin/tor $libsdir/libTor.so

There's a trailing space after 'recognize'

Can you move the tar command into the general platform-specific tar/zip block that immediately follows the android block?

Trac:
Status: assigned to needs_revision

I should mention this is based on commit b00b0d40320431abd0d13a4fa959ce53407b35e9 from ticket:28704#comment:23.

Made the following changes:

Set TORCONFIGDIR using original code
Use install command
Use general tar command
Various cleanup

https://github.com/sisbell/tor-browser-build/commits/bug-28766-1

Trac:
Status: needs_revision to needs_review

When looking at commit 4f636796fb728ce614e3223780dbdab86892eabf, I see that var/configure_opt containing options for android is defined in 3 places: tor, xz, libevent.

Instead I think we should define it only one time, in rbm.conf, as I said in ticket:28704#comment:14:

it seems we could have a var/configure_opt for android in rbm.conf containing something like CC=clang --host=[% c("var/host") %] [% c("var/configure_opt_project") %], where var/configure_opt_project is defined in each project to define options specific to this project

Trac:
Status: needs_review to needs_revision

I added the configure_opt_project var to the build

https://github.com/sisbell/tor-browser-build/commits/bug-28766-2

Trac:
Status: needs_revision to needs_review
Keywords: TorBrowserTeam202002 deleted, TorBrowserTeam202003R added

Trac:
Cc: sisbell, gk, hans@guardianproject.info, tbb-team to sisbell, sysrqb, boklm, gk, tbb-team

In ticket:28763#comment:2 gk asked a question about zlib support:

What about zlib support which we currently have on desktop?

Trac:
Status: needs_review to needs_information

I added zlib support

https://github.com/sisbell/tor-browser-build/commits/bug-28766-3

Trac:
Status: needs_information to needs_review

We are no longer in March

Trac:
Keywords: TorBrowserTeam202003R deleted, TorBrowserTeam202004R added

Trac:
Reviewer: N/A to boklm

Fixed copying of libevent for linux

https://github.com/sisbell/tor-browser-build/commits/bug-28766-5

Replying to sisbell:

Fixed copying of libevent for linux

https://github.com/sisbell/tor-browser-build/commits/bug-28766-5

The file change from libevent-2.1.so.6 to libevent-2.1.so.7 is caused by the libevent update to version 2.1.11. So this should be done in the commit for #31499 (moved). There is also no reason to change libstdc++.so.6 to libstdc++.so.7.

Trac:
Keywords: TorBrowserTeam202004R deleted, TorBrowserTeam202004 added
Status: needs_review to needs_revision

I added the libevent version change for tor in #31499 (moved).

This new commit uses that change, otherwise is the same as before

https://github.com/sisbell/tor-browser-build/commits/bug-28766-6

Trac:
Keywords: TorBrowserTeam202004 deleted, TorBrowserTeam202004R added
Status: needs_revision to needs_review

Replying to sisbell:

I added the libevent version change for tor in #31499 (moved).

This new commit uses that change, otherwise is the same as before

https://github.com/sisbell/tor-browser-build/commits/bug-28766-6

It seems we duplicate the lines for extracting zlib and setting zlibdir. I think we can them just one time behind a [% IF c("var/windows") || c("var/android") %]
The pkg-config symlink can be avoided by setting the --disable-tool-name-check configure flag
Looking at https://github.com/torproject/tor/pull/1408 other flags we can set are --disable-system-torrc and --disable-systemd.

Trac:
Keywords: TorBrowserTeam202004R deleted, TorBrowserTeam202004 added
Status: needs_review to needs_revision

+  - name: xz
+    project: xz
+    enable: '[% c("var/android") %]'
+  - name: zstd
+    project: zstd
+    enable: '[% c("var/android") %]'

I chatted with gk and ahf about enabling xz and zstd. gk pointed me at #22341 (moved), as well, where they previously decided only starting with zstd seemed smart. Let's stick with that idea and only enable zstd (and zlib) now.

Therefore, please delete the xz parts from this patch.

Made the following changes:

removed xz/lzma support for android
removed pkg-config symlink and added flag to disable check
added '--disable-system-torrc' and '--disable-systemd' for android builds
removed duplicate of zlib setup
moved extracting of openssl and libevent projects to after the platform conditional extracting blocks (small readability cleanup)

https://github.com/sisbell/tor-browser-build/commits/bug-28766-7

Trac:
Status: needs_revision to needs_review

I opened #33915 (moved) for enabling rust. Overall, this looks okay to me.

boklm, do you see anything that should be changed?

Replying to sysrqb:

I opened #33915 (moved) for enabling rust. Overall, this looks okay to me.

boklm, do you see anything that should be changed?

This looks good to me too. This is now commit 7894190b275b412c8d4f9cf7cad281601594da24 on master.

Trac:
Keywords: TorBrowserTeam202004 deleted, TorBrowserTeam202004R added
Resolution: N/A to fixed
Status: needs_review to closed

Replying to boklm:

Replying to sysrqb:

I opened #33915 (moved) for enabling rust. Overall, this looks okay to me.

boklm, do you see anything that should be changed?

This looks good to me too. This is now commit 7894190b275b412c8d4f9cf7cad281601594da24 on master.

I don't think ZSTD is enabled for Android. I've filed #34219 (moved) to fix that.