Opened 7 weeks ago

Closed 3 weeks ago

Last modified 2 weeks ago

#28144 closed defect (fixed)

Update projects/tor-browser for Android

Reported by: gk Owned by: tbb-team
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-rbm, tbb-mobile, TorBrowserTeam201811R, TBA-a2
Cc: sisbell, sysrqb, boklm Actual Points:
Parent ID: #26693 Points:
Reviewer: Sponsor: Sponsor8

Description

We ship Torbutton and NoScript in Tor Browser for Android albeit at a different location. We need to update projects/tor-browser for that.

Child Tickets

Attachments (2)

0001-hacked-up-extension-bundling.patch (5.3 KB) - added by gk 5 weeks ago.
crash.txt (44.1 KB) - added by sisbell 5 weeks ago.
Crash log of debug apk

Download all attachments as: .zip

Change History (42)

comment:1 Changed 6 weeks ago by boklm

I don't know how much of the current projects/tor-browser/build file can be shared between the desktop and android builds. If nothing, or almost nothing is common to both, then it might make sense to use a completely separate build file for the android build. This can be done by adding this to projects/tor-browser/config:

diff --git a/projects/tor-browser/config b/projects/tor-browser/config
index bb1c259..8edd1e2 100644
--- a/projects/tor-browser/config
+++ b/projects/tor-browser/config
@@ -41,6 +41,8 @@ targets:
   windows-x86_64:
     var:
       mar_osname: win64
+  android-armv7:
+    build: '[% INCLUDE build.android %]'
 
 input_files:
   - project: container-image

Then the build instructions for the android build can be written to projects/tor-browser/build.android.

comment:2 Changed 5 weeks ago by gk

Keywords: TorBrowserTeam201811 added; TorBrowserTeam201810 removed

Moving our tickets to November.

comment:3 Changed 5 weeks ago by gk

Cc: sysrqb added
Status: newneeds_information

Turns out I did all the things needed in the firefox part before packaging things up. See my attached patch I used a while back.

sysrqb: does that somewhat match what you do?

comment:4 Changed 5 weeks ago by sysrqb

That looks pretty good. I'm not familiar with abicheck.cc, and it looks like the patch is missing https-everywhere. I don't immediately see anything else that's different from what I'm doing. For reference, the script I use is here:
https://github.com/sysrqb/build_tba_alpha/blob/master/build_tba_alpha

One difference is that I was downloading https-everywhere from the EFF website (for simplicity), instead of building it locally. Of course, with tor-browser-build we should use the locally built xpi.

comment:5 Changed 5 weeks ago by gk

So, from a high-level point we'd like to get a properly built and named Tor Browser for Android by executing commands like make alpha or make nightly. That means after e.g. entering make alpha and after the build job is done the result should be a tor-browser-android-arm-8.5a4.apk in alpha/unsigned/8.5a4-buildN (yes, I think once we have the mobile build integrated it could be a good time to switch to our "regular versioning" scheme).

From a less high-level viewpoint this requires (at least) a) adapting the Makefile to add the new targets and b) getting the extensions we need funneled into the .apk.

For b) we usually have the bundling step (i.e. the tor-browser project). If that's not going to work (Is that the case? If so, why?) for Android (which would mean reopening the .apk we got in the firefox step and putting the extensions into place after building them if needed) we need to find a way doing that in the firefox build step and could do just the renaming in the tor-browser step.

comment:6 Changed 5 weeks ago by gk

Cc: boklm added

comment:7 in reply to:  5 ; Changed 5 weeks ago by boklm

Replying to gk:

For b) we usually have the bundling step (i.e. the tor-browser project). If that's not going to work (Is that the case? If so, why?) for Android (which would mean reopening the .apk we got in the firefox step and putting the extensions into place after building them if needed) we need to find a way doing that in the firefox build step and could do just the renaming in the tor-browser step.

However doing that in the firefox build means we need to rebuild firefox to update the extensions, so if that's possible, doing it in the tor-browser step would be better.

It looks like .apk files are zip files, so I'm wondering if extracting them and adding the extensions in the right directory before rezipping the .apk file would work, or if there is something more needed.

comment:8 Changed 5 weeks ago by gk

Keywords: TBA-a2 added

Moving on TBA-a2 radar

comment:9 in reply to:  7 ; Changed 5 weeks ago by sisbell

Replying to boklm:

Replying to gk:

For b) we usually have the bundling step (i.e. the tor-browser project). If that's not going to work (Is that the case? If so, why?) for Android (which would mean reopening the .apk we got in the firefox step and putting the extensions into place after building them if needed) we need to find a way doing that in the firefox build step and could do just the renaming in the tor-browser step.

However doing that in the firefox build means we need to rebuild firefox to update the extensions, so if that's possible, doing it in the tor-browser step would be better.

It looks like .apk files are zip files, so I'm wondering if extracting them and adding the extensions in the right directory before rezipping the .apk file would work, or if there is something more needed.

To add assets, we can just use 'aapt add' command from the android SDK. We also need to make sure we resign the apk as part of a tor-browser step. We would do this with 'apksigner'.

comment:10 in reply to:  9 ; Changed 5 weeks ago by sysrqb

Replying to sisbell:

Replying to boklm:

Replying to gk:

For b) we usually have the bundling step (i.e. the tor-browser project). If that's not going to work (Is that the case? If so, why?) for Android (which would mean reopening the .apk we got in the firefox step and putting the extensions into place after building them if needed) we need to find a way doing that in the firefox build step and could do just the renaming in the tor-browser step.

However doing that in the firefox build means we need to rebuild firefox to update the extensions, so if that's possible, doing it in the tor-browser step would be better.

It looks like .apk files are zip files, so I'm wondering if extracting them and adding the extensions in the right directory before rezipping the .apk file would work, or if there is something more needed.

To add assets, we can just use 'aapt add' command from the android SDK. We also need to make sure we resign the apk as part of a tor-browser step. We would do this with 'apksigner'.

The app won't be signed within tor-browser-build, so that isn't a problem. We only must produce a valid unsigned-unaligned apk as the result.

I agree adding the extensions in the tor-browser project (like we do for desktop) makes the most sense. We'll need to investigate what Mozilla do when a distribution is defined and replicate that in build - I haven't looked closely enough at this, so I don't know what is needed.

comment:11 Changed 5 weeks ago by sisbell

Status: needs_informationneeds_revision

Changes (android-1106)

  • Added var/desktop field. Used to disable the projects that android doesn't need (if there is better way open to suggestions)
  • Created build.android file
  • Build unzips apk and copies xpi files into resources and then rezips apk

comment:12 Changed 5 weeks ago by sisbell

Status: needs_revisionneeds_review

comment:13 in reply to:  11 Changed 5 weeks ago by boklm

Status: needs_reviewneeds_revision

Replying to sisbell:

Changes (android-1106)

  • Added var/desktop field. Used to disable the projects that android doesn't need (if there is better way open to suggestions)

I think you could use ! c("var/android").

For a review of 2614830bb270321eaca0796587d1bd1a13295997:

  • the build.android script is missing a [% c("var/set_default_env") -%] line at the begining
  • maybe projects/firefox/build could rename the apk files to a more predictible filename (for the desktop builds we use tor-browser.tar.gz) so that there is no need to use find in projects/tor-browser/build.android to find the filename.
  • I think that:
    apkname='tor-browser-[% c("version") %]-[% c("var/osname") %].apk'
    [...]
    zip -r $apkname .
    cp $apkname [% dest_dir _ '/' _ c('filename') %]
    

could be replaced by:

zip -r '[% dest_dir %]/[% c('filename') %]/tor-browser-[% c("version") %]-[% c("var/osname") %].apk' .

comment:14 Changed 5 weeks ago by gk

I think we still lack sufficient integration into our Makefile so that we get e.g. a .apk in tor-browser-build/tetsbuild after running make testbuild.

comment:15 in reply to:  10 ; Changed 5 weeks ago by gk

Replying to sysrqb:

Replying to sisbell:

Replying to boklm:

Replying to gk:

For b) we usually have the bundling step (i.e. the tor-browser project). If that's not going to work (Is that the case? If so, why?) for Android (which would mean reopening the .apk we got in the firefox step and putting the extensions into place after building them if needed) we need to find a way doing that in the firefox build step and could do just the renaming in the tor-browser step.

However doing that in the firefox build means we need to rebuild firefox to update the extensions, so if that's possible, doing it in the tor-browser step would be better.

It looks like .apk files are zip files, so I'm wondering if extracting them and adding the extensions in the right directory before rezipping the .apk file would work, or if there is something more needed.

To add assets, we can just use 'aapt add' command from the android SDK. We also need to make sure we resign the apk as part of a tor-browser step. We would do this with 'apksigner'.

The app won't be signed within tor-browser-build, so that isn't a problem. We only must produce a valid unsigned-unaligned apk as the result.

I agree that we should produce a valid unsigned-unaligned .apk. sisbell: I think that should be reflected in the filename in the final output (as Mozilla does with Fennec). I am not so sure, though, that not signing it is not a problem. How are we testing our final result on Android devices without *any* signing? (We don't have that problem on desktop platforms as signing requirements can get disabled if they are existing at all)

comment:16 in reply to:  15 ; Changed 5 weeks ago by sysrqb

Replying to gk:

I am not so sure, though, that not signing it is not a problem. How are we testing our final result on Android devices without *any* signing? (We don't have that problem on desktop platforms as signing requirements can get disabled if they are existing at all)

Ah. Good point. The unsigned-unaligned apk should be (as the name implies) not signed. But when building Fennec with Mozilla's build system, they produce an additional apk that is signed with a debug signing key. It looks like that happens in config/android-common.mk, calling mobile/android/debug_sign_tool.py. I think we can use this, too.

comment:17 in reply to:  16 ; Changed 5 weeks ago by sisbell

Replying to sysrqb:

Replying to gk:

I am not so sure, though, that not signing it is not a problem. How are we testing our final result on Android devices without *any* signing? (We don't have that problem on desktop platforms as signing requirements can get disabled if they are existing at all)

Ah. Good point. The unsigned-unaligned apk should be (as the name implies) not signed. But when building Fennec with Mozilla's build system, they produce an additional apk that is signed with a debug signing key. It looks like that happens in config/android-common.mk, calling mobile/android/debug_sign_tool.py. I think we can use this, too.

We have different types of signing under consideration:

It looks like mozilla is using v1 for debug, this is the only case we need to consider for the debug build. For production level signing, we should consider looking into v3 (perhaps mozilla is already using v3 signing?)

comment:18 in reply to:  17 Changed 5 weeks ago by gk

Replying to sisbell:

Replying to sysrqb:

Replying to gk:

I am not so sure, though, that not signing it is not a problem. How are we testing our final result on Android devices without *any* signing? (We don't have that problem on desktop platforms as signing requirements can get disabled if they are existing at all)

Ah. Good point. The unsigned-unaligned apk should be (as the name implies) not signed. But when building Fennec with Mozilla's build system, they produce an additional apk that is signed with a debug signing key. It looks like that happens in config/android-common.mk, calling mobile/android/debug_sign_tool.py. I think we can use this, too.

We have different types of signing under consideration:

It looks like mozilla is using v1 for debug, this is the only case we need to consider for the debug build. For production level signing, we should consider looking into v3 (perhaps mozilla is already using v3 signing?)

Yes, but for the outcome in our tor-browser-build whatever Mozilla is doing is enough (e.g. v1 if we get that in our current firefox build). It's just for testing on devices that our code does what it should (and only that :) ). The real signing for release is done later, outside of our tor-browser-build environment.

comment:19 Changed 5 weeks ago by sisbell

I'm not seeing the debug flag on the debug signed build that we generate. From what I can tell in line 78 of ./mobile/android/app/build.gradle, it is saying if MOZILLA_OFFICIAL is set, then it flags the build as not debuggable. This has implications such as reduced logging and not being able to step through debug points with the IDE. If we also flag as nightly build AND debug, it will overwrite the MOZ_OFFICIAL flag.

I'm not sure if this is something we should be concerned about right now? Or can we just push it off for now?

comment:20 in reply to:  19 ; Changed 5 weeks ago by gk

Replying to sisbell:

I'm not seeing the debug flag on the debug signed build that we generate. From what I can tell in line 78 of ./mobile/android/app/build.gradle, it is saying if MOZILLA_OFFICIAL is set, then it flags the build as not debuggable. This has implications such as reduced logging and not being able to step through debug points with the IDE. If we also flag as nightly build AND debug, it will overwrite the MOZ_OFFICIAL flag.

I'm not sure if this is something we should be concerned about right now? Or can we just push it off for now?

I think that's fine for now.

comment:21 in reply to:  20 Changed 5 weeks ago by sysrqb

Replying to gk:

Replying to sisbell:

I'm not seeing the debug flag on the debug signed build that we generate. From what I can tell in line 78 of ./mobile/android/app/build.gradle, it is saying if MOZILLA_OFFICIAL is set, then it flags the build as not debuggable. This has implications such as reduced logging and not being able to step through debug points with the IDE. If we also flag as nightly build AND debug, it will overwrite the MOZ_OFFICIAL flag.

I'm not sure if this is something we should be concerned about right now? Or can we just push it off for now?

I think that's fine for now.

I agree. Currently, for releasing, we only build TBA with MOZILLA_OFFICIAL=1. If someone needs a debuggable build, then they can build that separately (we can document how a developer should accomplish this). In fact, the reason we added export MOZILLA_OFFICIAL=1 into .mozconfig-android (and subsequently other mozconfigs) was because when we first tried uploading the first TBA alpha version to Google Play, it still had debuggable=true, and Google Play rejected it. I believe tor-browser-build should output releasable bundles.

Changed 5 weeks ago by sisbell

Attachment: crash.txt added

Crash log of debug apk

comment:22 Changed 5 weeks ago by sisbell

I have something locally that is building a debug signed apk with the extensions in the apk. When I try to run the apk it crashes. I've attached the crash logs if anyone has any ideas.

After some research, I'm suspecting that a simple zip of the apk won't work. This file is what Mozilla is using
./python/mozbuild/mozbuild/action/package_fennec_apk.py

Last edited 5 weeks ago by sisbell (previous) (diff)

comment:23 in reply to:  22 ; Changed 5 weeks ago by sisbell

Replying to sisbell:

I have something locally that is building a debug signed apk with the extensions in the apk. When I try to run the apk it crashes. I've attached the crash logs if anyone has any ideas.

After some research, I'm suspecting that a simple zip of the apk won't work. This file is what Mozilla is using
./python/mozbuild/mozbuild/action/package_fennec_apk.py

I can get rid of the omni.ja errors by not compressing this specific file during the zip process but the sqlite libraries missing error is still showing up.

comment:24 in reply to:  23 Changed 5 weeks ago by sisbell

Replying to sisbell:

Replying to sisbell:

I have something locally that is building a debug signed apk with the extensions in the apk. When I try to run the apk it crashes. I've attached the crash logs if anyone has any ideas.

After some research, I'm suspecting that a simple zip of the apk won't work. This file is what Mozilla is using
./python/mozbuild/mozbuild/action/package_fennec_apk.py

I can get rid of the omni.ja errors by not compressing this specific file during the zip process but the sqlite libraries missing error is still showing up.

I have this solved. I'll check in changes shortly.

comment:25 Changed 5 weeks ago by sisbell

Status: needs_revisionneeds_review

Changes (android-1107)

  • build.android: added debug build signing
  • build.android: no longer unzip and then rezip. Now use the zip options for adding and deleting files. This prevents runtime problems where files are compressed incorrectly.
  • build.android: user fixed apk name from firefox project, rather than finding the name in script.
  • config: Add java JDK dependency. This is needed for the keytool and jarsigner executables
  • config: removed desktop var and use android var directly for enabling projects

Verified that the debug apk starts up. I'm not sure how to verify all the extensions are working correctly but they are included in the apk..

Last edited 5 weeks ago by sisbell (previous) (diff)

comment:26 Changed 5 weeks ago by gk

Status: needs_reviewneeds_revision

See my comment:14. We still need to adapt our Makefile to add missing Android related rules.

comment:27 Changed 4 weeks ago by gk

Note: #25013 landed and we build Torbutton for mobile now regularly during the Firefox part. So, we don't need it in the bundle step anymore.

comment:28 Changed 4 weeks ago by sisbell

Status: needs_revisionneeds_review

Changes (android-1115)

  • Removed the adding of torbutton to apk since its now part of Firefox build

Issues: Still seeing install Orbot prompt on start of application

comment:29 Changed 4 weeks ago by gk

Keywords: TorBrowserTeam201811R added; TorBrowserTeam201811 removed

comment:30 in reply to:  28 ; Changed 4 weeks ago by boklm

Keywords: TorBrowserTeam201811 added; TorBrowserTeam201811R removed
Status: needs_reviewneeds_revision

Replying to sisbell:

Changes (android-1115)

  • Removed the adding of torbutton to apk since its now part of Firefox build

I see commit b609be07156640d09182e96782614842e4358a4d is removing torbutton from input_files for all builds (not only android). However the torbutton lines in projects/tor-browser/build are still there, so it will probably break the desktop build.

If the torbutton change is for all platforms, maybe that should be done in a separate commit (as the title of this one only talks about android).

comment:31 in reply to:  30 Changed 4 weeks ago by sisbell

Replying to boklm:

Replying to sisbell:

Changes (android-1115)

  • Removed the adding of torbutton to apk since its now part of Firefox build

I see commit b609be07156640d09182e96782614842e4358a4d is removing torbutton from input_files for all builds (not only android). However the torbutton lines in projects/tor-browser/build are still there, so it will probably break the desktop build.

If the torbutton change is for all platforms, maybe that should be done in a separate commit (as the title of this one only talks about android).

Good point, I'll add this back in as to not break desktop

comment:32 Changed 4 weeks ago by sisbell

Changes (android-1116)

  • Added back in tor-button for non-android projects

comment:33 Changed 3 weeks ago by sisbell

Status: needs_revisionneeds_review

comment:34 Changed 3 weeks ago by gk

Status: needs_reviewneeds_revision
+cp $apk $base_apk-unsigned-unaligned.apk

Do we need the usigned-unaligned.apk here? What's its purpose in the final release directory? That said I think the .apk we produce before we do the final signing somewhere else (not just the dummy one in the tor-browser build script) should be something like tor-browser-tbb-nightly-android-armv7.apk (assuming we built a nightly version of it).

comment:35 in reply to:  34 Changed 3 weeks ago by sisbell

Replying to gk:

+cp $apk $base_apk-unsigned-unaligned.apk

Do we need the usigned-unaligned.apk here? What's its purpose in the final release directory? That said I think the .apk we produce before we do the final signing somewhere else (not just the dummy one in the tor-browser build script) should be something like tor-browser-tbb-nightly-android-armv7.apk (assuming we built a nightly version of it).

When you sign and apk (either with jarsigner or apksigner) there two ways to do it:

  1. Specify input apk and signed output apk
  2. Sign the apk directly, overwriting the input file apk

If we sign as (1), it would be clearer to leave the unsigned-unaligned in the file name. If we sign as (2), the apk of course will need to be renamed prior to signing. In case (2), It sound like we would want to redo the naming in the release project. I don't have any visibility as to how signing is done. Does a simple renaming of the unsigned apk in the release step work?

comment:36 Changed 3 weeks ago by gk

Okay, we might want to talk past each other here. So, what this step in the build process is doing is creating the actual bundles as we are shipping them code-wise. For the Android case we need to be a bit creative in order to allow e.g. folks on the tor-qa mailing list to test them as Android requires a signature to even run .apk files. But the code is and should be the same as in the final release bundle with our signature.

My first thinking was it is fine with just having tor-browser-tbb-nightly-android-armv7.apk or, say, for the alpha tor-browser-8.5a4-android-armv7.apk as a result of that. This is mainly used internally later on to check for matching builds in the release process (in the case of nightly builds, just having something that is running for testing) and available in an folder called alpha/unsigned (for alphas) to make it clear the contents there are unsigned (which means lack official signatures).

Thinking more about it we might want to make it clearer in the Android case that the bundle is not signed by an official key yet by adding -unsigned in the filename. I wonder what Mozilla is doing with their Fennec nightlies here as I can't imagine they use their "usual" signing key for that.

At any rate, as this is the step that produces the bundle as we ship it, we should copy only that one (however named and however signed). We can do the renaming later on during the signing if we think alpha/unsigned vs. alpha/signed directories for the bundles is not enough for that.

comment:37 Changed 3 weeks ago by sisbell

Status: needs_revisionneeds_review

Changes (android-1120)

  • Copy over $qa_apk which has *-qa.apk file name
  • Updated comments and signing signature to make it clear this is a QA build

comment:38 Changed 3 weeks ago by gk

Resolution: fixed
Status: needs_reviewclosed

Looks good and should be enough for now. Eventually, we'll want to think whether we want to drop the "-qa" part here as well, given that the output get into our sha256sums unsigned build file and all the other "QA" bundles don't have a "-qa" in their filename either.

This is commit 6028be7e4b62d6ab419258dc3237e395f1dc34e6 on master.

comment:39 Changed 3 weeks ago by gk

Keywords: TorBrowserTeam201811R added; TorBrowserTeam201811 removed

comment:40 Changed 2 weeks ago by pili

Sponsor: Sponsor8
Note: See TracTickets for help on using tickets.