Closed Bug 1091109 Opened 5 years ago Closed 5 years ago

web app is "damaged and can't be opened" after install via Firefox Beta

Categories

(Firefox Graveyard :: Webapp Runtime, defect)

34 Branch
x86
macOS
defect
Not set

Tracking

(firefox33 unaffected, firefox34+ fixed, firefox35+ fixed, firefox36+ fixed, firefox-esr31 unaffected)

RESOLVED FIXED
Firefox 36
Tracking Status
firefox33 --- unaffected
firefox34 + fixed
firefox35 + fixed
firefox36 + fixed
firefox-esr31 --- unaffected

People

(Reporter: myk, Assigned: spohl)

References

Details

Attachments

(6 files, 1 obsolete file)

On the latest Firefox Beta (34.0b4), starting a web app by double-clicking on its Applications icon (or otherwise activating its application bundle, such as via Spotlight), results in an error dialog like the attached screenshot that says the application is "damaged and can't be opened."

For example, I saw that screenshot after installing the Scary Sounds HD app <https://marketplace.firefox.com/app/scary-sounds-hd> and then trying to start it.

Starting an app from the command line by directly invoking its webapprt stub executable, however, works correctly.  For example, I was able to start Scary Sounds HD by invoking:

  /Applications/Scary\ Sounds\ HD.app/Contents/MacOS/webapprt

The problem doesn't occur on the latest Nightly build (36.0a1).  I haven't tested Aurora yet.
looking into it
Assignee: nobody → spohl.mozilla.bugs
Status: NEW → ASSIGNED
The latest aurora does not appear to be affected. Will build and start debugging beta tomorrow.
So, this is bizarre. When I download the webapp via aurora or nightly, none of the files in the .app bundle are signed and no quarantine bit is set on the webapp, which allows us to start the app without a problem. In beta however, the webapprt binary is signed and the bundle has quarantine bits set on itself and the files contained in it. It also appears that the webapprt binary was signed individually, which causes the Gatekeeper to reject the .app bundle, i.e. when the bundle's signature is checked, it realizes that signed items are inside the bundle, but the bundle wasn't signed as a whole, so something appears to have modified the bundle and Gatekeeper will reject the app.
(In reply to Stephen Pohl [:spohl] from comment #4)
> When I download the webapp via aurora or nightly, none of the files in
> the .app bundle are signed and no quarantine bit is set on
> the webapp, which allows us to start the app without a problem.

Spoke too soon. The webapprt binary is signed no matter if it was downloaded by nightly, aurora or beta, but no quarantine bit is set in nightly or aurora.

This is a slightly different problem than I thought. The reason why nightly and aurora aren't affected is because they have a different bundle identifier than beta or release, i.e. org.mozilla.nightly and org.mozilla.aurora instead of org.mozilla.firefox. This is important because Gatekeeper uses bundle identifiers to decide if it should set quarantine bits on files downloaded by these apps. Gatekeeper sets quarantine bits if files are being downloaded by org.mozilla.firefox, but not org.mozilla.aurora or org.mozilla.nightly. Since we're now signing all binaries individually before signing the firefox.app bundle to conform with the v2 signing requirements, we end up with a signed webapprt-stub. This results in the following:

1. Firefox downloads webapp.
2. Gatekeeper sets quarantine bit on webapp because it was downloaded by org.mozilla.firefox.
3. On launch of the webapp, the quarantine bit prompts Gatekeeper to verify the webapp .app bundle.
4. Since a signed webapprt binary is contained in the bundle, but the bundle wasn't signed itself, Gatekeeper treats the webapp as damaged/modified/corrupted and recommends to trash it.

I'll see if there's a way to work around this next...
Attached patch PatchSplinter Review
This works around the issue by placing the webapprt-stub in Firefox.app/Contents/Resources, where it will be treated as a non-binary file and signed as part of the .app bundle (as opposed to Contents/MacOS where it would get an individual signature embedded into the binary file). This allows us to extract it later and use it inside of webapps without a signature, which is the existing behavior.

To fix this properly, with webapprt inside Contents/MacOS etc, we will need some type of webapp installer that manages this correctly. This is beyond the scope of any v2 signing work however and rstrong, smichaud and I agreed on IRC that this is the proper workaround for now.

I've verified locally that this works now with beta builds.
Attachment #8515278 - Flags: review?(smichaud)
Attachment #8515278 - Flags: review?(myk)
Comment on attachment 8515278 [details] [diff] [review]
Patch

Looks fine to me.
Attachment #8515278 - Flags: review?(smichaud) → review+
Comment on attachment 8515278 [details] [diff] [review]
Patch

That makes sense.  Thanks for digging into it!
Attachment #8515278 - Flags: review?(myk) → review+
[Tracking Requested - why for this release]:
We're planning to ship v2 signed Firefox.app bundles in Firefox 34. Without this bug here fixed, webapps will basically be completely broken on MacOS, starting with Firefox 34.
I'm planning to request uplift approval on the patch after a few days on m-c, but please let me know if there is no time for that and I should request approval now. Thanks!
This looks like we got bit by the fact that try doesn't run tests on OSX 10.8 even though all platforms were requested here:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8196898ebc2a

Ryan, to confirm: I see webapp failures on OSX 10.8 only, but you're referring to "multiple platforms". Am I missing something? Thanks!
Flags: needinfo?(ryanvm)
You're right, they were only on 10.8 (sorry, it was midnight :P).

Well coop, that sure didn't take long.
Flags: needinfo?(ryanvm)
So, this may be a fundamental issue with the way we're looking for an updated webapprt. Here's what I see when running ./mach webapprt-test-chrome:

2014-11-01 11:12:35.358 webapprt-stub[1889:124690] ### This Application has an old webrt. Updating it.
2014-11-01 11:12:35.358 webapprt-stub[1889:124690] ### My webapprt path: /Users/Stephen/Documents/objdir-ff-debug/dist/NightlyDebug.app/Contents/Resources/webapprt
2014-11-01 11:12:35.358 webapprt-stub[1889:124690] ### Firefox webapprt path: /Applications/FirefoxNightly.app/Contents/Resources/webapprt-stub
2014-11-01 11:12:35.358 webapprt-stub[1889:124690] got exception: This version of Firefox (20140930040205) cannot run web applications, because it is not recent enough or damaged

I don't think we should ever have been looking for a webapprt-stub in the Firefox install under /Applications. I'm concerned that webapps don't support multiple Firefox installs on the same system. Myk, is that true?
Flags: needinfo?(myk)
(In reply to Stephen Pohl [:spohl] from comment #16)

> I don't think we should ever have been looking for a webapprt-stub in the
> Firefox install under /Applications. I'm concerned that webapps don't
> support multiple Firefox installs on the same system. Myk, is that true?

Stephen, my recollection is that Dan Walkowski wrote this code assuming there would be multiple Firefoxes, see

http://dxr.mozilla.org/mozilla-central/source/webapprt/mac/webapprt.mm
Yes, it is supposed to work even if there are multiple Firefox installations.

I guess to fix the failing test you just need to update it, since it was using the directory format that you've just changed: http://mxr.mozilla.org/mozilla-central/source/toolkit/webapps/tests/test_webapp_runtime_executable_update.xul
(In reply to Marco Castelluccio [:marco] from comment #18)
> I guess to fix the failing test you just need to update it, since it was
> using the directory format that you've just changed:

I suppose we would have caught this on 10.6 except the tests are all disabled there IIRC? This is somewhat worrying to me...
Ah, this makes me feel a lot better. I'll get this fixed on Monday. Thanks!
Flags: needinfo?(myk)
(In reply to Bill Walker [:bwalker] [@wfwalker] from comment #17)
> Stephen, my recollection is that Dan Walkowski wrote this code assuming
> there would be multiple Firefoxes

I'm assuming that you're referring to |PathToWebRT| [1]. I do see that we're looping through the possible list of identifiers, i.e. org.mozilla.nightly, org.mozilla.aurora and org.mozilla.firefox. The problem is that we're using the native API [NSWorkspace absolutePathForAppBundleWithIdentifier], which will return one result even if multiple bundles with the same identifier are installed on the system. This may not be a problem for most users, but developers and test systems might pick a bundle at random.

[1] http://dxr.mozilla.org/mozilla-central/source/webapprt/mac/webapprt.mm#341
(In reply to Stephen Pohl [:spohl] from comment #12)
> I'm planning to request uplift approval on the patch after a few days on
> m-c, but please let me know if there is no time for that and I should
> request approval now. Thanks!

Let's let this bake on m-c for a few days. If this fix is shown to be stable before Thu we can get this into beta7.
Attached patch Fix test failures (obsolete) — Splinter Review
So, this *should* fix the test failures, but I'm unable to verify this at the moment. try doesn't seem to run tests on 10.8 and I have too many local nightly builds for tests to pick up the correct webapprt-stub. I don't quite understand why we're not doing more of a version check before we copy webapprt-stubs into webapps (currently, we seem to happily downgrade webapprt-stubs with little to no downgrade protection), but I don't know enough about webapp history to tell for sure that this is a bug. Might be worth looking into by Myk or Marco.

Ryan, would you know how I could force try to run tests on 10.8? This would avoid me having to set up a dedicated test VM...
Attachment #8515713 - Flags: feedback?(ryanvm)
Flags: needinfo?(ryanvm)
Attachment #8515713 - Flags: feedback?(ryanvm)
Oh, I missed the end of coop's blog post[1] where it explains how to run tests on 10.8 explicitly using try syntax alone.

[1] http://coop.deadsquid.com/2014/10/10-8-testing-disabled-by-default-on-try/
This was all that was needed. Try run is green:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=03d85b014afe
Attachment #8515713 - Attachment is obsolete: true
Attachment #8516091 - Flags: review?(myk)
Attachment #8516091 - Flags: review?(myk)
Better patch. Try run should be available soon. I'll request review once it comes back green:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=fe9f862f98bd
Attachment #8516091 - Attachment is obsolete: true
Comment on attachment 8516127 [details] [diff] [review]
Fix test failures

Let's get this reviewed now so that I can land this as soon as try comes back green. The test slaves seem backed up again...
Attachment #8516127 - Flags: review?(myk)
Try is green. Will land this as soon as it's reviewed to give it maximum bake time on m-c before beta 7:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=fe9f862f98bd
Comment on attachment 8516127 [details] [diff] [review]
Fix test failures

I don't have a 10.8 VM, and my 10.7 VM is having unrelated build trouble, but this looks correct, and try is happy, so I am too.
Attachment #8516127 - Flags: review?(myk) → review+
https://hg.mozilla.org/mozilla-central/rev/7ddbbb8fcec7
https://hg.mozilla.org/mozilla-central/rev/f07f3b28d761
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Approval Request Comment
[Feature/regressing bug #]: Apple v2 signing (bug 1047584)
[User impact if declined]: Webapps would be broken on OSX per Firefox 34
[Describe test coverage new/current, TBPL]: Manual testing & mochitest-oth
[Risks and why]: Minimal and restricted to webapps only.
[String/UUID change made/needed]: n/a
Attachment #8517588 - Flags: approval-mozilla-beta?
Attachment #8517588 - Flags: approval-mozilla-aurora?
(In reply to Stephen Pohl [:spohl] from comment #33)
> [Describe test coverage new/current, TBPL]: Manual testing & mochitest-oth

This has also been on m-c since Monday, no issues reported.
Comment on attachment 8517588 [details] [diff] [review]
Combined patch for aurora

Beta+
Aurora+
Attachment #8517588 - Flags: approval-mozilla-beta?
Attachment #8517588 - Flags: approval-mozilla-beta+
Attachment #8517588 - Flags: approval-mozilla-aurora?
Attachment #8517588 - Flags: approval-mozilla-aurora+
Target Milestone: Firefox 36 → Firefox 34
Target milestone tracks trunk landing. Please stick to status flags for branch uplifts.
Target Milestone: Firefox 34 → Firefox 36
Seeing frequent failures like these on beta since this landed. Please fix or backout ASAP.
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=130232&repo=mozilla-beta
Flags: needinfo?(spohl.mozilla.bugs)
Looking into it
Flags: needinfo?(spohl.mozilla.bugs)
Backed out from beta in https://hg.mozilla.org/releases/mozilla-beta/rev/81cf187bba10.

Looks like this is causing the intermittent bug 1059238 to occur more frequently. I have no idea why this might be. Myk and Marco, any ideas?
Flags: needinfo?(myk)
Flags: needinfo?(mar.castelluccio)
It's not really bug 1059238 (though it's been getting starred as such). Note that the initial failure line is:
 2252 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/webapps/tests/test_custom_origin.xul | App was launched and is still running 

Which then leads into timeouts and errors in the subsequent tests.
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #41)
> It's not really bug 1059238 (though it's been getting starred as such). Note
> that the initial failure line is:
>  2252 INFO TEST-UNEXPECTED-FAIL |
> chrome://mochitests/content/chrome/toolkit/webapps/tests/test_custom_origin.
> xul | App was launched and is still running 
> 
> Which then leads into timeouts and errors in the subsequent tests.

Thanks, Ryan! So this looks like the very first test under toolkit/webapps/tests is failing, which could mean that webapps don't run at all. I've built the beta branch locally and ran the tests over 20 times, but have yet to see any failures. Still have no idea why we might run into this intermittent issue here.

I've also confirmed that aurora seems to be doing just fine with the patch here.
Ah, figured it out. We're running into an issue where a different Firefox bundle (with the same bundle identifier) is left on the test slave. This is the issue described in comment 23: We're happily using whatever webapprt-stub that we can find, as long as the bundle identifier matches. This may not have been noticed before because there weren't significant differences between the webapprt-stubs. However with this patch, we're changing the location where the webapprt-stub is to be found inside of the Firefox bundle. Since there isn't one under Contents/Resources in the old bundle left on the test slave, the test will fail. The old bundle here seems to be from something called 'addonsdk-poller'.

Here is a run that fails:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-beta-macosx64/1415237926/mozilla-beta_mountainlion_test-mochitest-other-bm107-tests1-macosx-build157.txt.gz

Here is a run that succeeds:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-beta-macosx64/1415247590/mozilla-beta_mountainlion_test-mochitest-other-bm108-tests1-macosx-build29.txt.gz

(Search for 'Custom Origin Test.app' to quickly jump to the relevant place in the log).

Revamping the way we detect the proper webapprt-stub to be used is way beyond the scope of this bug. But maybe there is a way to ensure that test slaves don't have old Firefox bundles sitting around? Ryan, would you know?
Flags: needinfo?(ryanvm)
Nope, sounds like a question for RelEng.
Flags: needinfo?(ryanvm)
Comment on attachment 8516091 [details] [diff] [review]
Workaround for webapprt update issues

I hate this because it's gross, but I think we may have to add this to look for webapprt-stubs in both Contents/Resources as well as Contents/MacOS to work around the flawed webapprt update mechanism. I doubt that it will be easy to guarantee that there will be no other Firefox bundles on our test slaves. Even if it was easy, our end-users may have multiple Firefox bundles installed and risk running into this same issue.

I'm planning to land this on m-c, aurora and beta to avoid any further problems like we were seeing on beta.
Attachment #8516091 - Attachment is obsolete: false
Flags: needinfo?(myk)
Flags: needinfo?(mar.castelluccio)
Attachment #8516091 - Flags: review?(myk)
Attachment #8516091 - Attachment description: Fix test failures → Workaround for webapprt update issues
Comment on attachment 8516091 [details] [diff] [review]
Workaround for webapprt update issues

It's unfortunate, but if this addresses the issue on the test boxes, then it's worth taking, while we tackle the larger issue of determining the right runtime separately.
Attachment #8516091 - Flags: review?(myk) → review+
Attachment #8517588 - Attachment description: Combined patch for aurora and beta → Combined patch for aurora
Comment on attachment 8516091 [details] [diff] [review]
Workaround for webapprt update issues

Landed on mozilla-inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/2033324d4571
Landed on aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/819a7b517057
Landed on beta as part of the combined patch in comment 47.

These landings were on CLOSED TREEs with RyanVM's approval.
Attachment #8518227 - Attachment description: Patch for beta → Combined patch for beta
Depends on: 1125394
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.