719697 - robocop isn't signed properly from buildbot builds

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

while trying to get robocop to run from tinderbox builds, I see this in logcat:
W/ActivityManager(  109): Permission Denial: starting instrumentation ComponentInfo{org.mozilla.roboexample.test/android.test.InstrumentationTestRunner} from pid=6046, uid=6046 not allowed because package org.mozilla.roboexample.test does not have a signature matching the target org.mozilla.fennec

Here is where we do the signing for robocop.apk:
http://mxr.mozilla.org/mozilla-central/source/build/mobile/robocop/Makefile.in#132

Here is where we do the signing for fennec.apk:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#357

Is it possible that we are not defining JARSIGNER for the mobile/build/robocop builds, and we are for the toolkit/mozapps/installer?

Comment 1

[Aki Sasaki (not active)]

Is robocop packaged during make package?
That will fix it.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

I won't have time to look into this until Monday.  Right now robocop is build during the 'make' process and not the 'make package'.  Maybe it is an easy change to do, if anybody knows some magic.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: experiment to use robocop during make package

I created this patch last night and it generated a robocop.apk in the build directory and at make package time.  Unfortunately when I ran robocop tests, it failed with the same permission denied error.

Is there something I am doing wrong in the patch?

Here is the try server run if logs will help:
https://tbpl.mozilla.org/?tree=Try&rev=f42a113d739f

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

ok, after looking into this further, we use mozpass.py to wrap jarsigner on the production machines.  I set this up locally using my debug cert, but inside the mozpass.py toolchain.  I was able to build, install and test just fine.

Next I pushed my patch to try and it generated a fennec/robocop set of apks and those failed to run with the same error.  After talking with bear, I ran some verification scripts:
jmaher@jmaher-MacBookPro:~/mozilla/tools/release/signing$ ./verify-android-signature.sh --tools-dir=/home/jmaher/mozilla/tools/ --apk=/home/jmaher/Downloads/fennec-12.0a1.en-US.android-arm.apk -n
TOOLS DIR: /home/jmaher/mozilla/tools/
Archive:  /home/jmaher/Downloads/fennec-12.0a1.en-US.android-arm.apk
  inflating: META-INF/NIGHTLY.DSA    
Valid
jmaher@jmaher-MacBookPro:~/mozilla/tools/release/signing$ ./verify-android-signature.sh --tools-dir=/home/jmaher/mozilla/tools/ --apk=/home/jmaher/Downloads/robocop.apk -nTOOLS DIR: /home/jmaher/mozilla/tools/
Archive:  /home/jmaher/Downloads/robocop.apk
  inflating: META-INF/NIGHTLY.DSA    
Valid
jmaher@jmaher-MacBookPro:~/mozilla/tools/release/signing$


it appears that both are signed the same.  I am not sure why this is failing, but I need somebody else who understands the certificates to look into this.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

blassey, can you or somebody from mobile development look into this?

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

aki mentioned that I don't have the sharedId in my .xml file.  Pushing to try and seeing what happens.

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: sign robocop with the same key as fennec and only 1 key (1.0)

this is pretty simple once you realize that you can have >1 key in a .apk.  When testing locally, I found that I didn't always have a key in robocop-raw.apk, so I added a " & echo ''" so we wouldn't error out during make.  Hacky, so suggest something better please!  Otherwise this final patch is running on try while a similar patch build and tested fine last night.

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 591476 [details] [diff] [review]
sign robocop with the same key as fennec and only 1 key (1.0)

Review of attachment 591476 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/testsuite-targets.mk
@@ +297,2 @@
>  	$(NSINSTALL) $(DEPTH)/build/mobile/robocop/fennec_ids.txt $(PKG_STAGE)/mochitest
> +	zip -q -d 'META-INF/*' $(DIST)/robocop-raw.apk & echo ''

what's the & echo '' for?

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: sign robocop with the same key as fennec and only 1 key (2.0)

sorry for the re-review, but this is much different than the last patch.  All the logic is in toolkit/mozapps/installer/packager.mk and will be built during 'make package' instead of 'make package-tests'.  

Tested on try and ran locally, all looks good.

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

note to hwine (and other releng folks): this creates a file robocop.apk inside the upload folder (same locations as fennec.apk and fennec.tests.zip).  There will NOT be a tests.zip:/bin/robocop.apk anymore.  We will need to fix our existing code to handle this new location.  Here is an example:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-d7dc53a1af3f/try-android/

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 591764 [details] [diff] [review]
sign robocop with the same key as fennec and only 1 key (2.0)

Review of attachment 591764 [details] [diff] [review]:
-----------------------------------------------------------------

I feel much better about this patch

Comment 12

[Aki Sasaki (not active)]

Comment on attachment 591764 [details] [diff] [review]
sign robocop with the same key as fennec and only 1 key (2.0)

Nit: I think this would read better for me if it said

+MAKE_PACKAGE    = $(PREPARE_PACKAGE) && $(INNER_MAKE_PACKAGE) && $(INNER_ROBOCOP_PACKAGE)

and removed the first && from INNER_ROBOCOP_PACKAGE.  But I think moving to make package is the right move here; we may want to do the same with sutagent.

Comment 13

[Matt Brubeck (:mbrubeck)]

Sorry, I had to back this out on inbound along with bug 713970:
https://hg.mozilla.org/integration/mozilla-inbound/rev/547cea3b54fd

...because one of them caused some build bustage:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=f151cccff911

Comment 14

[Chris Cooper [:coop] (he/him)]

Joel: do you still need me to pull a try slave for you to diagnose this error/iterate more quickly?

Comment 15

[Joel Maher ( :jmaher ) (UTC -8)]

:coop, not anymore, I think I got it to stick this time around.

inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c593ef0f4353

Comment 16

[Matt Brubeck (:mbrubeck)]

This was backed out again:
https://hg.mozilla.org/mozilla-central/rev/5221f1397829

Comment 17

[Aki Sasaki (not active)]

Attachment: detect robocop.apk

Comment 18

[Hal Wine [:hwine] use NI!]

Comment on attachment 592168 [details] [diff] [review]
detect robocop.apk

Thanks for the quick turn, Aki!

Comment 19

[Aki Sasaki (not active)]

Comment on attachment 592168 [details] [diff] [review]
detect robocop.apk

http://hg.mozilla.org/build/buildbotcustom/rev/64570a8b2530

Comment 20

[Joel Maher ( :jmaher ) (UTC -8)]

landed on m-c:
https://hg.mozilla.org/mozilla-central/rev/80cc0dcc7fec

Comment 21

[Aki Sasaki (not active)]

This breaks android single locale repacks:

java.io.FileNotFoundException: /builds/slave/m-cen-andrd-l10n-2/mozilla-central/obj-l10n/mobile/android/locales/../../../dist/../build/mobile/robocop/robocop.ap_ does not exist
make[1]: *** [repackage-zip] Error 1
make[1]: Leaving directory `/builds/slave/m-cen-andrd-l10n-2/mozilla-central/obj-l10n/mobile/android/locales'
make: *** [repackage-zip-gl] Error 2
command: END (17.74s elapsed)

Error creating locale 'gl': Command '['make', 'installers-gl']' returned non-zero exit status 2
Traceback (most recent call last):
  File "/builds/slave/m-cen-andrd-l10n-2/scripts/scripts/l10n/nightly-mobile-repacks.py", line 192, in <module>
    options.platform, stage_platform, mobileDirName, en_us_binary_url)
  File "/builds/slave/m-cen-andrd-l10n-2/scripts/scripts/l10n/nightly-mobile-repacks.py", line 74, in createRepacks
    raise RepackError("At least one repack failed, see above")
__main__.RepackError: At least one repack failed, see above

Comment 22

[Chris Cooper [:coop] (he/him)]

(In reply to Aki Sasaki [:aki] from comment #21)
> This breaks android single locale repacks:

blassey is going to help joel look at the l10n repacks.

Comment 23

[Joel Maher ( :jmaher ) (UTC -8)]

I have been able to repack locally and I don't see the problem.  It could be I am using the wrong mozharness config file.  I noticed that there was a problem when building without --enable-tests which has the same error.  This was fixed already:
https://bugzilla.mozilla.org/show_bug.cgi?id=722118#c0

Is there a chance that these are working now?

Comment 24

[Aki Sasaki (not active)]

No, you're testing the wrong thing.
Single locale is broken.
Multilocale works.
You're testing multilocale which works.
You can't test single locale until I'm done with my script.
My script may or may not be slowed by the fact that robocop will break it.

Comment 25

[Aki Sasaki (not active)]

Gave Joel instructions on how to repack single locale builds.

I think the proper fix may be:

1. back out robocop packaging
2. go back to robocop in |make build|
3. land JARSIGNER fix in mozilla-{aurora,beta,release} - bug 611648
4. releng enables JARSIGNER in |make build|
5. verify that robocop is signed properly in |make build|

Comment 26

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: an option to fix the makefile to solve the repack issue

In looking at the single locale repack issues with fennec and robocop, it appears that we don't do a 'make -f client.mk build', but instead try to do the 'make package' steps.  While this is fine, we never end up with our robocop stuff.

There are 2 solutions I have found:
1) add a 'ac_add_options --disable-tests' to ALL the mozconfigs used for the repacks.  This is about 12.
2) add a check for existence of the ROBOCOP_PATH in the block where we define INNER_ROBOCOP_PACKAGE.  This is the patch I have uploaded and does a simple check for the directory to exist.

Both of these solutions solve the problem, we could take one of these or maybe something else.

Comment 27

[Aki Sasaki (not active)]

Attachment: --disable-tests on buildbot-configs android* l10n-mozconfigs

Comment 28

[Aki Sasaki (not active)]

Comment on attachment 593976 [details] [diff] [review]
--disable-tests on buildbot-configs android* l10n-mozconfigs

http://hg.mozilla.org/build/buildbot-configs/rev/fea1378ff972

Comment 29

[Armen [:armenzg]]

(In reply to Joel Maher (:jmaher) from comment #26)
> There are 2 solutions I have found:
> 1) add a 'ac_add_options --disable-tests' to ALL the mozconfigs used for the
> repacks.  This is about 12.
I see that aki landed the fixed and it is live now according to the maintenance page [1].

What is next? Is this closed and we move now to bug 719443?

[1] https://wiki.mozilla.org/ReleaseEngineering:Maintenance

Comment 30

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 593972 [details] [diff] [review]
an option to fix the makefile to solve the repack issue

Review of attachment 593972 [details] [diff] [review]:
-----------------------------------------------------------------

not sure whether to f+ or f- this, so just going to clear it with the comment

::: toolkit/mozapps/installer/packager.mk
@@ +343,1 @@
>  UPLOAD_EXTRA_FILES += robocop.apk

why wouldn't it exist if test are enabled? This looks like a recipe for a hard to find failure

Comment 31

[Joel Maher ( :jmaher ) (UTC -8)]

can we close this bug?

Comment 32

[Aki Sasaki (not active)]

Go for it.