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

Status: RESOLVED FIXED

(Firefox Graveyard :: Webapp Runtime, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

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.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Attachment: Screen Shot 2014-10-29 at 10.58.20.png

Comment 2

[Stephen A Pohl [:spohl]]

looking into it

Comment 3

[Stephen A Pohl [:spohl]]

The latest aurora does not appear to be affected. Will build and start debugging beta tomorrow.

Comment 4

[Stephen A Pohl [:spohl]]

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.

Comment 5

[Stephen A Pohl [:spohl]]

(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...

Comment 6

[Stephen A Pohl [:spohl]]

Attachment: Patch

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.

Comment 7

[Stephen A Pohl [:spohl]]

Sent to try to make sure that nothing else breaks:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8196898ebc2a

Comment 8

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8515278 [details] [diff] [review]
Patch

Looks fine to me.

Comment 9

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 8515278 [details] [diff] [review]
Patch

That makes sense.  Thanks for digging into it!

Comment 10

[Stephen A Pohl [:spohl]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5eebce6c8845

Comment 11

[Stephen A Pohl [:spohl]]

[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.

Comment 12

[Stephen A Pohl [:spohl]]

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!

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Backed out for test webapp test failures across multiple platforms.
https://hg.mozilla.org/integration/mozilla-inbound/rev/889b0085249d

https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3500214&repo=mozilla-inbound

Comment 14

[Stephen A Pohl [:spohl]]

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!

Comment 15

[Ryan VanderMeulen [:RyanVM]]

You're right, they were only on 10.8 (sorry, it was midnight :P).

Well coop, that sure didn't take long.

Comment 16

[Stephen A Pohl [:spohl]]

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?

Comment 17

[Bill Walker [:bwalker] [@wfwalker]]

(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

Comment 18

[Marco Castelluccio [:marco]]

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

Comment 19

[Ryan VanderMeulen [:RyanVM]]

(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...

Comment 20

[Stephen A Pohl [:spohl]]

Ah, this makes me feel a lot better. I'll get this fixed on Monday. Thanks!

Comment 21

[Stephen A Pohl [:spohl]]

(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

Comment 22

[Lawrence Mandel [:lmandel] (use needinfo)]

(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.

Comment 23

[Stephen A Pohl [:spohl]]

Attachment: Fix test failures

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...

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://wiki.mozilla.org/ReleaseEngineering/How_To/Trigger_arbitrary_jobs

Comment 25

[Stephen A Pohl [:spohl]]

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/

Comment 26

[Stephen A Pohl [:spohl]]

Attachment: Workaround for webapprt update issues

This was all that was needed. Try run is green:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=03d85b014afe

Comment 27

[Stephen A Pohl [:spohl]]

Attachment: Fix test failures

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

Comment 28

[Stephen A Pohl [:spohl]]

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...

Comment 29

[Stephen A Pohl [:spohl]]

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 30

[Myk Melez [:myk] [@mykmelez]]

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.

Comment 31

[Stephen A Pohl [:spohl]]

Thanks, Myk.

https://hg.mozilla.org/integration/fx-team/rev/7ddbbb8fcec7
https://hg.mozilla.org/integration/fx-team/rev/f07f3b28d761

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7ddbbb8fcec7
https://hg.mozilla.org/mozilla-central/rev/f07f3b28d761

Comment 33

[Stephen A Pohl [:spohl]]

Attachment: Combined patch for aurora

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

Comment 34

[Stephen A Pohl [:spohl]]

(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 35

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8517588 [details] [diff] [review]
Combined patch for aurora

Beta+
Aurora+

Comment 36

[Stephen A Pohl [:spohl]]

Thanks, Lawrence!

https://hg.mozilla.org/releases/mozilla-aurora/rev/17e3fe48e08a
https://hg.mozilla.org/releases/mozilla-beta/rev/a8edc81c39d5

Comment 37

[Ryan VanderMeulen [:RyanVM]]

Target milestone tracks trunk landing. Please stick to status flags for branch uplifts.

Comment 38

[Ryan VanderMeulen [:RyanVM]]

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

Comment 39

[Stephen A Pohl [:spohl]]

Looking into it

Comment 40

[Stephen A Pohl [:spohl]]

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?

Comment 41

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 42

[Stephen A Pohl [:spohl]]

(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.

Comment 43

[Stephen A Pohl [:spohl]]

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?

Comment 44

[Ryan VanderMeulen [:RyanVM]]

Nope, sounds like a question for RelEng.

Comment 45

[Stephen A Pohl [:spohl]]

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.

Comment 46

[Myk Melez [:myk] [@mykmelez]]

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.

Comment 47

[Stephen A Pohl [:spohl]]

Attachment: Combined patch for beta

Landed on CLOSED TREE with RyanVM's approval:
https://hg.mozilla.org/releases/mozilla-beta/rev/557655b23004

Comment 48

[Stephen A Pohl [:spohl]]

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.

Comment 49

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2033324d4571