Closed Bug 791743 Opened 12 years ago Closed 12 years ago

Sign packaged apps for reviewers

Categories

(Marketplace Graveyard :: General, defect, P2)

x86
macOS
defect

Tracking

(Not tracked)

RESOLVED FIXED
2012-12-20

People

(Reporter: andy+bugzilla, Assigned: robhudson)

References

Details

In bug 791738 we allow packaged apps to be signed for reviewers. This bug covers how we actually sign the apps. Note this is all about verification, not encryption. How the client will be checking the apps is covered in bug 772365.
Summary: Sign packaged apps for users → Sign packaged apps for reviewers
These bug titles are confusing
Blocks: 786867
For reviewers there was discussion around:

* Using a different set of keys to sign packaged apps for reviewers only, but it is still unknown how the reviewers get the key for verification on their device. So this may be a v2 feature.

* Using short-lived signatures. When this came up bsmith mentioned it wasn't on his radar and would probably have to happen for v2.

Given that, I don't know exactly what this bug entails but we do need a solution so reviewers can install and test apps.

One fallback solution would be to sign the apps just like we'd sign them when the reviewer approves the app, but put the file in a location that only reviewers have access to. This will require a solution to bug 795181.

CC'ing some folks to help chime in on this.
I really don't like the idea of signing apps with the "official" key for review purposes.  A "reviewer mode" with a review-specific cert would be nice, or the ability to whitelist a cert via ADB.

In the short run we may have to rely on a simple boolean flag to turn off signature verification but that's obviously not awesome.
A way of installing a review specific cert would be great. Should we file a seperate bug for that?
Given that our current solution for developers is to have a switch which basically says "allow installing unsigned privileged/certified apps from anywhere", I think it would be good enough here to have a switch that says "allow installing privileged/certified apps signed with the review cert". And then ship the review cert with the device just like we'll ship the normal marketplace cert with the device.
Blocks: 814236
From an app review perspective, my concern with relying on turning off the security model is that we'll be installing apps that are we don't trust to test them.  So, (if I understand https://bugzilla.mozilla.org/show_bug.cgi?id=813797#c0 correctly) any hosted app could declare themselves as certified and take control of the phone to an extent that the only safe solution would be to reflash the device after every installation.
comment 5 says to add a switch which enables a review cert.  Jonas - are you saying we can build that into the device in v1?
Priority: -- → P2
This is more a question for Brian. But yes, I believe that the intent is that we'd build a review cert into the device for v1.

And we should only allow installing privileged apps, not certified.
Now that we've decided reviewers have their own cert, we need to finish the implementation here:
https://github.com/mozilla/zamboni/blob/master/lib/crypto/packaged.py#L135
to pass reviewer=True in so it knows to use the reviewer cert and not the public cert.
(In reply to Jonas Sicking (:sicking) from comment #8)
> This is more a question for Brian. But yes, I believe that the intent is
> that we'd build a review cert into the device for v1.

I'd rather we didn't ship the review cert on the production device. Instead, we should have the reviewers update their device to add the review cert to their certificate database. Doing things this way would reduce the amount of crypto-related code I need to write for v1. We can do it that way for v2, if it is beneficial.

I assume we have some way of pushing/pulling files to devices using USB. I propose we use such mechanism on the reviewer's phones to push the review cert into their cert database.

(In reply to Rob Hudson [:robhudson] from comment #9)
> Now that we've decided reviewers have their own cert, we need to finish the
> implementation here:
> https://github.com/mozilla/zamboni/blob/master/lib/crypto/packaged.py#L135
> to pass reviewer=True in so it knows to use the reviewer cert and not the
> public cert.

We basically can't do things that way and I don't think we need to. Instead, if the reviewer has a reviewer cert in their device, then the device will trust that cert AND the production marketplace cert.
(In reply to Brian Smith (:bsmith) from comment #10)
> > Now that we've decided reviewers have their own cert, we need to finish the
> > implementation here:
> > https://github.com/mozilla/zamboni/blob/master/lib/crypto/packaged.py#L135
> > to pass reviewer=True in so it knows to use the reviewer cert and not the
> > public cert.
> 
> We basically can't do things that way and I don't think we need to. Instead,
> if the reviewer has a reviewer cert in their device, then the device will
> trust that cert AND the production marketplace cert.

Actually, I think I misread your comment. Now I think your comment means "we need to change the signing server to sign the app with the reviewer cert instead of the production cert" and I agree that is reasonable.
(In reply to Brian Smith (:bsmith) from comment #11)
> Actually, I think I misread your comment. Now I think your comment means "we
> need to change the signing server to sign the app with the reviewer cert
> instead of the production cert" and I agree that is reasonable.

Yes, thanks for making that clear. That was my intention.
Depends on: 819054
Depends on: 820445
Depends on: 820451
Assignee: nobody → robhudson.mozbugs
Target Milestone: --- → 2012-12-20
Flags: mkt-blocker+
https://github.com/mozilla/zamboni/commit/9fb161c
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
No longer blocks: packaged-apps
You need to log in before you can comment on or make changes to this bug.