Closed Bug 562842 Opened 12 years ago Closed 12 years ago

Android nightly signing

Categories

(Release Engineering :: General, defect, P4)

ARM
Android
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aki, Assigned: bear)

References

Details

(Whiteboard: [android][q2goal])

Attachments

(4 files, 16 obsolete files)

16.94 KB, patch
aki
: review+
Details | Diff | Splinter Review
3.38 KB, text/plain
aki
: review+
Details
3.69 KB, patch
aki
: review+
Details | Diff | Splinter Review
1.59 KB, patch
bhearsum
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
We need to figure out nightly signing for Android builds.
I'm not terribly familiar with this, but from IRC I gathered:

1) auto-generate a [nightly] signing key on a slave
2) roll that key out to all the build slaves so they're consistent
Blocks: 562843
Why do we need to sign them at all?
Not sure. Certainly, if there isn't a requirement for nightly signing, we can close this as WONTFIX; but I got the impression this might be needed to avoid ugly uninstall-reinstall issues.
Okay, I just wanted to bring it up. We explicitly don't sign our nightly desktop builds to help avoid confusing them as "official" releases, we may want to take the same approach here.
Signing is part of the packaging process. Packages will not install without a signature. The key is autogenerated during build (if not present) and placed in the ~/.android directory.
(In reply to comment #4)
> Signing is part of the packaging process. Packages will not install without a
> signature. The key is autogenerated during build (if not present) and placed in
> the ~/.android directory.

Thanks for the explanation
More critically, trying to install a package with the same name but signed by a different key than the one that's already installed results in an installation error -- this is why it's important to have the same key being used for all nightly and for all release builds.

To switch between the two, the user would have to uninstall, though I'd actually suggest that we use different package names for nightly/beta/release versions.
We're discussing separate nightly keys+release keys, and differing package names.

If we had nightly+release packages having the same name and same key, we would need to know the potential impact... all nightlies would be signed with the release key.

http://developer.android.com/guide/publishing/app-signing.html
Assignee: nobody → bear
Priority: P3 → P4
Stuart is leaning towards a single key for nightlies+releases, that lives on the build slaves.

This would significantly simplify things technically; we need to figure out what risks that entails.
Status: NEW → ASSIGNED
create the .android directory and push the nightly+release keystore file to the cltbld home dir
Attachment #444032 - Flags: feedback?(bhearsum)
I have serious concerns when it comes to sharing the same key for releases and nightlies, especially if it lives on the build machines themselves. It would be fairly trivially for someone to land a change that would send the key off to where-ever they want. This is especially worrisome on try because there is little to no visibility on those changes and a wider group has access to it.

What if we had a small set of dedicated "signing" machines, with more limited access, no ability to execute arbitrary makefile code, etc.

Clyon, could you weigh in on a few things here?
* Risks of sharing a signing key for nightlies and releases
* Risks of that key being on our Linux build machines, which can execute arbitrary code from check-ins.
* How would those risks change if signing was done on dedicated machines, which couldn't execute arbitrary code

... and anything else that you think is relevant.


> To switch between the two, the user would have to uninstall, though I'd
> actually suggest that we use different package names for nightly/beta/release
> versions.

(This would match what we do for desktop Firefox -- unbranded for nightlies, codename for alphas, Firefox for beta/release)
If we did this, we could have separate signing keys for nightlies and release, right? Would there be any way to share the profiles between the installations, or eg, import a nightly profile to a release one?
(In reply to comment #10)
> > To switch between the two, the user would have to uninstall, though I'd
> > actually suggest that we use different package names for nightly/beta/release
> > versions.
> 
> (This would match what we do for desktop Firefox -- unbranded for nightlies,
> codename for alphas, Firefox for beta/release)
> If we did this, we could have separate signing keys for nightlies and release,
> right? Would there be any way to share the profiles between the installations,
> or eg, import a nightly profile to a release one?

They'd need to have different package names and could havedifferent keys, right.  The profiles would not be shared, but I don't think that's that big of a deal.  If we need to, we could probably do something like have a way to specify you want a shared profile on an SD card (or even just put it there by default, in which case it could be shared).

Easiest thing to me seems like we should have one key/package name for the nightlies, then figure out what to do with releases once we get to a beta, with the plan of record being that we'll use a separate release-only key there.
is org.mozilla.@MOZ_APP_NAME@ good for the nightlies (for now at least)?
Comment on attachment 444032 [details] [diff] [review]
[WIP] add nightly keystore for Android signing to puppet manifest

I'm fine with this approach for now. I'd like for us to look at moving the nightly signing to a small pool of dedicated machines, but that shouldn't block the initial rollout.
Attachment #444032 - Flags: feedback?(bhearsum) → feedback+
If we don't fix things before the initial rollout, everyone who has installed one of these builds will have to forcefully uninstall and reinstall the application.
(In reply to comment #14)
> If we don't fix things before the initial rollout, everyone who has installed
> one of these builds will have to forcefully uninstall and reinstall the
> application.

Per meeting thursday (6th), Stuart wants us to have signing in place before we enable build-on-checkin and build-nightly in production.
initial pass - needs some definite feedback from the mobile team as I'm sure there are a lot of mobile Makefile defines/macros I could be using
Attachment #444596 - Flags: feedback?(aki)
Comment on attachment 444596 [details] [diff] [review]
[WIP] Makefile change to allow for apk signing

(Caveat: I don't know this stuff very well.)

I still think we should be creating a gecko-unsigned.apk.  We don't necessarily have to call the same target as the developers.

>+ifdef MOZ_ANDROID_KEYSTORE
>+    ANDROID_KEYSTORE = $(MOZ_ANDROID_KEYSTORE)
>+else
>+    ANDROID_KEYSTORE = ~/.keystore/android-staging.keystore
>+endif
>+
>+ifdef MOZ_ANDROID_KEYALIAS
>+    ANDROID_KEYALIAS = $(MOZ_ANDROID_KEYALIAS)
>+else
>+    ANDROID_KEYALIAS = staging
>+endif

Not sure if this stuff belongs in here or in our mozconfig ?

> gecko-unaligned.apk: gecko.ap_ classes.dex dist $(FULL_LIBS)
>-	$(APKBUILDER) gecko-unaligned.apk -v -z gecko.ap_ -f classes.dex -nf `pwd`/libs -rf dist
>+	$(APKBUILDER) gecko-unaligned.apk -v -u -z gecko.ap_ -f classes.dex -nf `pwd`/libs -rf dist
>+	cp gecko-unaligned.apk gecko-unsigned.apk
>+	$(JARSIGNER) -keystore $(ANDROID_KEYSTORE) gecko-unaligned.apk $(ANDROID_KEYALIAS)
> 
> $(MOZ_APP_NAME).apk: gecko-unaligned.apk
> 	$(ZIPALIGN) -f -v 4 gecko-unaligned.apk $(MOZ_APP_NAME).apk

Maybe I'm overengineering, maybe not. I was thinking maybe something like:

ALL_APK_DEPS = gecko.ap_ classes.dex dist $(FULL_LIBS)

create-apk:
       $(APKBUILDER) $(APKNAME) -v $(UNSIGNED_OPT) -z gecko.ap_ -f classes.dex -nf `pwd`/libs -rf dist

# There's probably a macro I can use here
gecko-unaligned.apk: $(ALL_APK_DEPS)
       APKNAME="gecko-unaligned.apk" make create-apk

gecko-unsigned.apk: $(ALL_APK_DEPS)
       APKNAME="gecko-unsigned.apk" UNSIGNED_OPT="-u" make create-apk

sign-apk: gecko-unsigned.apk
ifndef ANDROID_KEYSTORE
        # error or noop
endif
	cp gecko-unsigned.apk gecko-unaligned.apk
	$(JARSIGNER) -keystore $(ANDROID_KEYSTORE) gecko-unaligned.apk $(ANDROID_KEYALIAS)

ifdef MOZILLA_OFFICIAL
$(MOZ_APP_NAME).apk: sign-apk
else
$(MOZ_APP_NAME).apk: gecko-unaligned.apk
endif
        $(ZIPALIGN) -f -v 4 gecko-unaligned.apk $(MOZ_APP_NAME).apk

needs testing and ironing out.  We might not even need the final ifdef if we |make sign-apk && make| ?

Anyway, we probably want more feedback from the developers, but in the meantime, if we're creating an unsigned apk and signing it with Jarsigner, that's awesome.
Attachment #444596 - Flags: feedback?(aki) → feedback+
I grok the direction your heading, I just wanted to use the current patch as a starting point tomorrow with the mobile devs.

But yea, adding a new target to create the various apk's that are then used by the packaging steps is the more robust Makefile'ish way.
Attachment #444596 - Attachment is obsolete: true
Attachment #444684 - Flags: feedback?(aki)
Attachment #444685 - Flags: feedback?(aki)
The biggest issue with this change to the Makefile is that, for right now, it would require any devs wanting to generate a signed apk to have a local keystore.  This isn't a huge issue, but I just wanted to point it out.
Comment on attachment 444684 [details] [diff] [review]
[WIP] Makefile change to allow for apk signing

If developers are ok with creating a keystore to build apks, this is much more elegant than my hacks :)
Attachment #444684 - Flags: feedback?(aki) → feedback+
Attachment #444684 - Flags: review?(mwu)
Attachment #444685 - Flags: review?(mwu)
Comment on attachment 444685 [details] [diff] [review]
[WIP] mozconfig changes to support apk signing via Makefile

Hmmmm.
I'm thinking this might belong in env, so we could potentially use the same mozconfig for staging and nightlies.  But this should be fine.

For releases, if we don't have the release key on the same slave, we can make fennec-unsigned-unaligned.apk and upload just that, and sign/align on the signing machine.
Attachment #444685 - Flags: feedback?(aki) → feedback+
Comment on attachment 444684 [details] [diff] [review]
[WIP] Makefile change to allow for apk signing

>-gecko-unaligned.apk: gecko.ap_ classes.dex dist $(FULL_LIBS)
>-	$(APKBUILDER) gecko-unaligned.apk -v -z gecko.ap_ -f classes.dex -nf `pwd`/libs -rf dist
>+gecko-unsigned-unaligned.apk: gecko.ap_ classes.dex dist $(FULL_LIBS)
>+	$(APKBUILDER) gecko-unsigned-unaligned.apk -v -u -z gecko.ap_ -f classes.dex -nf `pwd`/libs -rf dist
> 
>-$(MOZ_APP_NAME).apk: gecko-unaligned.apk
>-	$(ZIPALIGN) -f -v 4 gecko-unaligned.apk $(MOZ_APP_NAME).apk
>+gecko-unaligned.apk: gecko-unsigned-unaligned.apk
>+	cp  gecko-unsigned-unaligned.apk gecko-unaligned.apk
>+	$(JARSIGNER) -keystore $(ANDROID_KEYSTORE)  gecko-unaligned.apk: $(ANDROID_KEYALIAS)
> 
>+ $(MOZ_APP_NAME).apk: gecko-unaligned.apk
>+ 	$(ZIPALIGN) -f -v 4 gecko-unaligned.apk $(MOZ_APP_NAME).apk

I would prefer just not passing -u when there's no ANDROID_KEYSTORE or ANDROID_KEYALIAS passed in.

Actually we don't need ANDROID_KEYSTORE and ANDROID_KEYALIAS. Why not just have JARSIGNER_FLAGS? There's a large number of jarsigner flags that can be passed in, so this will be more future proof for you. Then apkbuilder -u + jarsigner can be done when JARSIGNER_FLAGS is set, and just plain apkbuilder when JARSIGNER_FLAGS isn't set.
Attachment #444684 - Flags: review?(mwu)
Whiteboard: [android][q2goal]
I could not use a single JARSIGNER_FLAGS because of the command line parameter requirements for jarsigner - the alias has to follow the apk filename and the apk filename has to follow the options
Attachment #444685 - Attachment is obsolete: true
Attachment #444832 - Flags: review?(mwu)
Attachment #444832 - Flags: feedback?(aki)
Attachment #444685 - Flags: review?(mwu)
Attachment #444684 - Attachment is obsolete: true
Attachment #444833 - Flags: review?(mwu)
Attachment #444833 - Flags: feedback?(aki)
The JARSIGNER_ALIAS and JARSIGNER_FLAGS could be set as environment variables and if they are not present then the apkbuilder will generate a keystore if the developer does not already have one, so nothing changes for dev builds
Comment on attachment 444833 [details] [diff] [review]
[WIP] Makefile change to allow for apk signing

Looks good! Only issue might be if you want to build on one set of machines and sign on another set of machines - then you'll have to pass in some dummy JARSIGNER_FLAGS to convince the Makefile to add -u. If you and aki are ok with that, I'll commit this patch.
Attachment #444833 - Flags: review?(mwu) → review+
Attachment #444832 - Flags: review?(mwu)
Attachment #444833 - Flags: feedback?(aki) → feedback+
For the moment we are going to "solve" this by uploading the unsigned and signed apk's
Attachment #444833 - Flags: checked-in+
a basic python helper program to allow the passwords to be added to a jarsigner call without having the password values appear in the output
Attachment #445374 - Flags: feedback?
Comment on attachment 445374 [details]
mozpass.py - call command with passwords but not have them appear in output

This seems cool.
Are you thinking about FileDownloading the .mozpass and then nuke?  

I would maybe lean towards searching for ./.mozpass.py before ~/.mozpass.py but probably not important.
(In reply to comment #32)
> (From update of attachment 445374 [details])
> This seems cool.
> Are you thinking about FileDownloading the .mozpass and then nuke?  

I wasn't but that is definitely an option.  To be honest I was going to go with something in cltbld's home dir so that the difference between staging and nightly can be independent of the mozpass.py call - i.e. just change the keystore file and the config.

I could make it fancier and use the keystore file but I was also trying to make this signing command independent so did not want to hard code anything that was android specific unless I really was forced

> 
> I would maybe lean towards searching for ./.mozpass.py before ~/.mozpass.py but
> probably not important.

I went with ~ for the reason above - it seems that other key related info is stored in the home dir.

This is moot if we use the download method IMO because then the config just becomes what url to pull
Attachment #444833 - Attachment is obsolete: true
Attachment #445798 - Flags: review?(mwu)
Comment on attachment 445798 [details] [diff] [review]
[WIP] Makefile change to allow for apk signing

ajusted version checked in after IRC discussion.

removed JARSIGNER_FLAGS and changed check to use JARSIGNER
Attachment #445798 - Flags: review?(mwu)
Attachment #444832 - Attachment is obsolete: true
Attachment #445881 - Flags: review?(aki)
Attachment #444832 - Flags: feedback?(aki)
Attachment #445882 - Flags: review?(aki)
Attachment #445374 - Attachment is obsolete: true
Attachment #445884 - Flags: feedback?(aki)
Attachment #445374 - Flags: feedback?
Attachment #445798 - Attachment is obsolete: true
Comment on attachment 445884 [details]
mozpass.py - call command with passwords but not have them appear in output

My only real comment here is if you're trying to make this generic, android signing is an interesting default.  However, we can address that later.
Attachment #445884 - Flags: feedback?(aki) → feedback+
Comment on attachment 445884 [details]
mozpass.py - call command with passwords but not have them appear in output

Oh, also, having $'s in the args will make for an interesting time running it in bash :)
Attachment #445882 - Attachment is obsolete: true
Attachment #445886 - Flags: review?(aki)
Attachment #445882 - Flags: review?(aki)
Attachment #445881 - Attachment is obsolete: true
Attachment #445887 - Flags: review?(aki)
Attachment #445881 - Flags: review?(aki)
(In reply to comment #41)
> (From update of attachment 445884 [details])
> Oh, also, having $'s in the args will make for an interesting time running it
> in bash :)

yea, I was going with known python items and I knew I was calling the binary direct and bypassing the shell completely so I could get away with $'s for now.

using the builtin template for replacements made it a safer choice for something that may turn out to be a one-off
Comment on attachment 445886 [details] [diff] [review]
buildbotcustom changes for Android

I think this looks good.
Attachment #445886 - Flags: review?(aki) → review+
Comment on attachment 445887 [details] [diff] [review]
buildbot-configs changes for Android

I think this looks good.
So what's left is a) seeing that your test pass succeeds, and b) rolling out staging and production .mozpass.cfg  and keystores via puppet.
Attachment #445887 - Flags: review?(aki) → review+
Attachment #445884 - Attachment is obsolete: true
Attachment #445899 - Flags: review?(aki)
Attachment #445886 - Attachment is obsolete: true
Attachment #445900 - Flags: review?(aki)
Comment on attachment 445899 [details]
mozpass.py - call command with passwords but not have them appear in output

http://hg.mozilla.org/build/tools/rev/12479a1f4e14

(landed with s,it's,its, spelling fix)
Attachment #445899 - Flags: review?(aki)
Attachment #445899 - Flags: review+
Attachment #445899 - Flags: checked-in+
Attachment #444032 - Attachment is obsolete: true
Attachment #445902 - Flags: review?(bhearsum)
Attachment #445902 - Attachment is obsolete: true
Attachment #445905 - Flags: review?(bhearsum)
Attachment #445902 - Flags: review?(bhearsum)
Attachment #445905 - Flags: review?(bhearsum) → review+
Attachment #445905 - Attachment is obsolete: true
Attachment #445906 - Flags: review?(bhearsum)
Comment on attachment 445906 [details] [diff] [review]
add Android keysigning config file and keystore to puppet manifest

Looks good.
Attachment #445906 - Flags: review?(bhearsum) → review+
Comment on attachment 445900 [details] [diff] [review]
buildbotcustom changes for Android

http://hg.mozilla.org/build/buildbotcustom/rev/6440cc540a94
Attachment #445900 - Flags: review?(aki)
Attachment #445900 - Flags: review+
Attachment #445900 - Flags: checked-in+
Comment on attachment 445887 [details] [diff] [review]
buildbot-configs changes for Android

Checked in staging portions only: http://hg.mozilla.org/build/buildbot-configs/rev/05a6ed9a4c92
staging tests pointed out a syntax error
Attachment #445912 - Flags: review?(bhearsum)
Attachment #445906 - Attachment is obsolete: true
Attachment #445912 - Attachment is obsolete: true
Attachment #445921 - Flags: review?(bhearsum)
Attachment #445912 - Flags: review?(bhearsum)
Attachment #445921 - Flags: review?(bhearsum) → review+
Comment on attachment 445921 [details] [diff] [review]
add Android keysigning config file and keystore to puppet manifest

changeset:   152:e03b742290aa
Attachment #445921 - Flags: checked-in+
puppet changes have landed in staging and production - currently using a self-signed key for nightly and staging.
ok, verified that the android.keystore and the signing config files have propagated to the production build slaves.  I've also double checked that the binary produced by the staging test can be installed (adb install'd it to my nexusone).

I think we are ready to push to production the buildbot changes for android nightly builds.
Stuart -- I'm ready to land the production side + kick off some builds when you're ready.
Blocks: 560911
No longer depends on: 560911
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.