Opened 5 months ago

Closed 5 months ago

Last modified 5 months ago

#28697 closed defect (fixed)

Our QA and testing .apks are signed with a key per build

Reported by: gk Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-mobile, TorBrowserTeam201812R
Cc: boklm, sisbell, igt0, sysrqb Actual Points:
Parent ID: #25164 Points:
Reviewer: Sponsor:

Description

For every .apk build we do a

keytool -genkey -v -keystore qa.keystore -storepass android -alias androidqakey -keypass android -keyalg RSA -keysize 2048 -validity 10000 -dname "CN=Android Tor QA,O=Tor,C=US"

which

a) results in differences between the resulting .apk files defeating our reproducible builds goal and

b) results in a hassle testing those .apk files by trying to overwrite an older installation: the keys must be the same, otherwise the app would not get installed over the already available one.

Child Tickets

Change History (18)

comment:1 Changed 5 months ago by gk

I guess the solution here would be to create a key for those QA .apks, upload that one to one of our people directories and add it as a dependency of the tor-browser project for Android.

comment:2 Changed 5 months ago by sisbell

When we sign with the apk, the generated META-INF directory contains a timestamp on the files. This causes the apk checksums to be different. I'm looking into how fix it.

comment:3 Changed 5 months ago by gk

I guess the second-best (or third-best :) ) solution would be using tools like faketime (which we do at some places to deal with timestamp issues).

comment:4 in reply to:  3 Changed 5 months ago by sisbell

Replying to gk:

I guess the second-best (or third-best :) ) solution would be using tools like faketime (which we do at some places to deal with timestamp issues).

Yes this works. Thanks.

comment:5 Changed 5 months ago by sisbell

Status: newneeds_review

Fixed in android-1203

  • Use pre-generated keystore
  • touch extension files so that timestamps remain same across builds
  • Use faketime with jarsigner to fix the timestamp of signatures

Verified checksums match across builds

comment:6 Changed 5 months ago by gk

Status: needs_reviewneeds_revision
-
+mv $rootdir/[% c('input_files_by_name/keystore') %] .
+ 

contains a trailing whitespace at the end of the last line. Please remove that one.

Why do we need to touch the extensions here? There were no differences in that regard when looking at the 8.5a5 diff of boklm's and my .apk.

comment:7 in reply to:  6 Changed 5 months ago by boklm

Replying to gk:

-
+mv $rootdir/[% c('input_files_by_name/keystore') %] .
+ 

contains a trailing whitespace at the end of the last line. Please remove that one.

Do we really need to move android-qa.keystore to the current directory, or could we just use -keystore $rootdir/android-qa.keystore instead of -keystore android-qa.keystore in the jarsigner command?

For the how-to-generate-keystore file:

  • maybe this could be renamed to how-to-generate-keystore.txt
  • we can add a : after type the following command
  • we can add a comment pointing to this file in projects/tor-browser/config near android-qa.keystore

comment:8 in reply to:  6 Changed 5 months ago by sisbell

Replying to gk:

-
+mv $rootdir/[% c('input_files_by_name/keystore') %] .
+ 

contains a trailing whitespace at the end of the last line. Please remove that one.

Why do we need to touch the extensions here? There were no differences in that regard when looking at the 8.5a5 diff of boklm's and my .apk.

When I ran diffoscope on the apks, it was flagging the timestamp as different. I'll do some more testing on this

│ --rw----     1.0 fat        0 bx stor 18-Dec-04 06:21 assets/distribution/extensions/
│ --rw----     2.0 fat   486514 bX defN 18-Dec-04 06:20 assets/distribution/extensions/{73a6fe31-595d-460b-a920-fcc0f8843232}.xpi
│ --rw----     2.0 fat  1766322 bX defN 18-Dec-04 06:20 assets/distribution/extensions/https-everywhere-eff@eff.org.xpi
│ +-rw----     1.0 fat        0 bx stor 18-Dec-04 06:49 assets/distribution/extensions/
│ +-rw----     2.0 fat   486514 bX defN 18-Dec-04 06:48 assets/distribution/extensions/{73a6fe31-595d-460b-a920-fcc0f8843232}.xpi
│ +-rw----     2.0 fat  1766322 bX defN 18-Dec-04 06:48 assets/distribution/extensions/https-everywhere-eff@eff.org.xpi

comment:9 Changed 5 months ago by sisbell

Status: needs_revisionneeds_review

Changes (android-1205)

  • No longer move keystore, use full location in jarsigner command
  • Changes to how-to-generate-keystore
  • Comment in tor-browser/config linking back to how-to-generate-keystore.txt

On my build machine (ubuntu 18.04), I still see the difference in timestamps in the assets folder. So I left the touch command in. I built the tor-browser project twice, using the same build of dependent projects.

comment:10 in reply to:  9 Changed 5 months ago by boklm

Status: needs_reviewneeds_revision

Replying to sisbell:

On my build machine (ubuntu 18.04), I still see the difference in timestamps in the assets folder. So I left the touch command in. I built the tor-browser project twice, using the same build of dependent projects.

You can replace touch -t '0101100000' by [% c("var/touch") %] (which will use the value from var/timestamp to set the timestamp).

You can also replace faketime '10-Jan-01 00:00' by [% c("var/faketime") %].

comment:11 Changed 5 months ago by sisbell

Status: needs_revisionneeds_review

Changes (android-1206)

  • Replaced touch and faketime variables

comment:12 Changed 5 months ago by gk

Keywords: TorBrowserTeam201812R added; TorBrowserTeam201812 removed

comment:13 Changed 5 months ago by gk

Blech. I have one machine where the build process is hanging due to the faketime call in the build.android file. I wonder why this is a non-issue for snowflake on macOS as it is using it as well. Maybe the different distro is the crucial difference here?

Last edited 5 months ago by gk (previous) (diff)

comment:14 Changed 5 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Okay, I tested a bunch of things. The problem goes away with Debian Jessie and openjdk-7 or Debian Buster. I don't know why (yet) but opted for the latter to get our build going and get the .apk reproducible.

I picked sisbell's patch and applied it to master (commit ea1d0b24448f0365b9c8ff60a6ab2c4a130ec279)and made an additional commit switching to Buster for the tor-browser step. This seems like low risk as I doubt the jarsigner tool is changing behavior here drastically in case newer packages arrive in Buster.

boklm: let me know whether you think we should change anything of my commit (commit 60163914e4f9418709bfe5b8fdc946fb56ca6e07). (I know I messed up the bug number which is highly annoying, but a thing we can't change anymore).

comment:15 in reply to:  14 ; Changed 5 months ago by boklm

Parent ID: #25164

Replying to gk:

boklm: let me know whether you think we should change anything of my commit (commit 60163914e4f9418709bfe5b8fdc946fb56ca6e07). (I know I messed up the bug number which is highly annoying, but a thing we can't change anymore).

I think the line arch: amd64 is not needed as we already set it to this value in rbm.conf. Otherwise this looks good.

comment:16 Changed 5 months ago by boklm

Parent ID: #25164

comment:17 in reply to:  15 Changed 5 months ago by gk

Parent ID: #25164

Replying to boklm:

Replying to gk:

boklm: let me know whether you think we should change anything of my commit (commit 60163914e4f9418709bfe5b8fdc946fb56ca6e07). (I know I messed up the bug number which is highly annoying, but a thing we can't change anymore).

I think the line arch: amd64 is not needed as we already set it to this value in rbm.conf. Otherwise this looks good.

Good point. Fixed in a follow-up commit (0a2cc62f08063cc0c91b1db57e87505c954349a9) on master (with a better bug number.

comment:18 Changed 5 months ago by gk

Parent ID: #25164
Note: See TracTickets for help on using tickets.