For OSX v2 signatures, we need to modify the structure of our .app bundle. In particular, we need to move all non-code files out of Contents/MacOS and into Contents/Resources. This applies recursively to any .app bundles packaged by the top-level .app bundle as well (crashreporter.app, updater.app etc.). Also, any directory under Contents/MacOS will be interpreted as bundle. Since we have many directories there that aren't bundles (like browser, defaults etc.) we need to move those to Resources as well. Note: although we could sign individual non-code files under Contents/MacOS, these would create xattrs that are not straightforward to package in MARs and deploy during an update. There is no alternative to moving any non-bundle directory out of Contents/MacOS however.

This simply moves the file into the right place before packaging. We'll have to figure out the proper way of doing this before we can land, but it should unblock the work that's needed in 'make package'.

This moves the files into their new place and updates package-manifest for the new structure: 1. Code files (executable code, dylibs, .app bundles) remain in *.app/Contents/MacOS 2. Data files that don't change at runtime are moved to *.app/Contents/Resources 3. Data files that could change at runtime now live under *.app/MozResources I will request feedback/reviews once we've fixed up all the dependencies that broke due to this change.

Let's make sure to delete the in-tree CodeResources file in the final patch, since it's no longer used for signing.

These are the current proposed changes: New/added directories: > .app/MozResources/ Moved from .app/Contents/MacOS to .app/Contents/Resources: > application.ini > browser/ > dependentlibs.list > dictionaries/ > libfreebl3.chk > libnssdbm3.chk > libsoftokn3.chk > omni.ja > platform.ini > removed-files > update-settings.ini > update.ini > webapprt/ Moved from .app/Contents/MacOS/crashreporter.app/Contents/MacOS to .app/Contents/MacOS/crashreporter.app/Contents/Resources: > crashreporter.ini Moved from .app/Contents/MacOS/res to .app/Contents/Resources: > MainMenu.nib/ > cursors/ Moved from .app/Contents/MacOS to .app/MozResources: > defaults/

(In reply to Stephen Pohl [:spohl] from comment #4) > Moved from .app/Contents/MacOS to .app/Contents/Resources: > > libfreebl3.chk > > libnssdbm3.chk > > libsoftokn3.chk Moving these away from the matching dylibs may break FIPS mode, I suggest talking to someone from the NSS team (eg :kaie).

We talked about this and I said that was acceptable to get this working.

1. Changed the name of the .lproj directory under Resources to en-US.lproj, from en.lproj previously. This avoids having to hardcode "en" in the packaging manifest, or trim "-US" from "en-US". I couldn't find a reason to justify the additional work to do so. 2. Removed CodeResources files since they are no longer used in v2 signing. 3. Fixed an issue where document.icns, firefox.icns and the .lproj directory didn't get packaged into Resources. 4. Changed CFBundleIconFile to "firefox.icns" instead of just "firefox" in the Info.plist. Without this, the bundle would show a default icon instead of the firefox icon. Although the Apple docs say that the file extension is optional, OSX seems to get confused by the other resources and can't find the proper icon when the extension isn't specified.

Forgot to remove CodeResources from package-manifest.

...and packager.mk. Will launch another try build once the trees reopen, but bhearsum was able to confirm that the build gets signed correctly.

Added minor tweak to createprecomplete.py to generate the precomplete file correctly. Benjamin, we still need to run this through all the tests and fix things up, but before I'm going further down this path I wanted to check with you if this approach is acceptable to you. To have a working .app bundle that launches and runs, the following patches need to be applied: 1. This patch (bug 1047584). 2. Patch from bug 1048687. 3. Patch from bug 1050944. I'll ask for your feedback in bug 1048687 and bug 1050944 as well. I'll also upload a patch shortly that enables signing of this new structure on our build machines.

Comment on attachment 8473006 [details] [diff] [review] Enable v2 signing Review of attachment 8473006 [details] [diff] [review]: ----------------------------------------------------------------- This is right, just make sure it lands at the same time as the other changes, of course.

Renaming the .lproj directory to en-US.lproj would break a bunch of l10n scripts, so un-renaming to en.lproj.

Comment on attachment 8473682 [details] [diff] [review] Patch In browser/app/Makefile.in the variables APPRESOURCES and MOZRESOURCES are unnecessary: they don't change based on configuration or make thing easier to read. Please remove them and just put "Resources" or "MozResources" directly in their place. The repackage step needs some work. rsync-then-move kinda sucks for this, resulting in lots of extra disk I/O in the common-rebuild case. I think it would be far better to do this: * have a list of files that go in MacOS instead of Resources * have a list of files that go in MozResources instead of Resources Do the initial rsync to Resources/ with --exclude-from to avoid syncing the excluded files. Do a separate rsync to MacOS and MozResources using rsync --include-from.

Addressed feedback. Still waiting for test harnesses to be updated and start passing before requesting review.

I hate scope creep as much as the next guy... but is there any chance this is an opportunity to stop duplicating all of omni.ja between the two sides of the universal build and so to reduce the size of our updates, dmgs, and .app bundles?

(In reply to :Gijs Kruitbosch from comment #16) > I hate scope creep as much as the next guy... but is there any chance this > is an opportunity to stop duplicating all of omni.ja between the two sides > of the universal build and so to reduce the size of our updates, dmgs, and > .app bundles? It would be great to have a bug on file to track this (please CC me). The more detail, the better. However, seeing how we're scrambling to get even the most basic things done in time for the v2 changes, I don't believe we'll be able to address this right now.

Fixed OSX debug build failure.

Comment on attachment 8487476 [details] [diff] [review] Patch Try push is almost completely green, so requesting review (updater xpcshell test failures and gaia python integration test suite failures are handled in separate bugs): https://tbpl.mozilla.org/?tree=Try&rev=7362867ad903

https://hg.mozilla.org/projects/oak/rev/6faa732a5a82 https://hg.mozilla.org/projects/oak/rev/012e80a2ad45

Pushed to fx-team https://hg.mozilla.org/integration/fx-team/rev/3ff20d4c7f39 https://hg.mozilla.org/integration/fx-team/rev/fbc1ceea4d6f

https://hg.mozilla.org/mozilla-central/rev/3ff20d4c7f39 https://hg.mozilla.org/mozilla-central/rev/fbc1ceea4d6f

Is OSX actually happy with Firefox loading the binary browsercomps component from the resources directory (which, even if OSX does it, feels very wrong)?

It is from a signing point of view. We might change its location to something more suitable after this initial work has been uplifted.

Approval Request Comment [Feature/regressing bug #]: OSX v2 bundle signing [User impact if declined]: Users would see a Gatekeeper warning saying that Firefox wasn't signed by a known developer and the user would be unable to start it without jumping through hoops. [Describe test coverage new/current, TBPL]: Most of these changes have been on m-c for over a week. Virtually all of our tests exercise this feature by verifying that Firefox can be launched and additional, dedicated v2 signature checks were added to mozmill. [Risks and why]: The bundle structure of our .app bundles has changed quite significantly. Although tested quite thoroughly, there may be edge-cases that were missed and that will need to be fixed before release. More exposure on aurora will help us find these cases. [String/UUID change made/needed]: none Aurora try run is in progress here: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=be4520a330b1 For reference, these are the patches that were folded into this combined patch: bug1047584-enableV2Signing bug1047584 bug1048687 bug1050944 bug1046924 bug1055774 bug1059504 bug1053838 bug1060652 bug1060562 bug1064952 bug1064910 bug1066123 bug1047738 bug1059467 bug1059567 bug1064523 bug1058182 bug1068439 bug1070661 bug1070148 bug1070149 bug1070863 bug1071134 bug1071465 bug1072554 bug1072716 bug1072722 bug1074000 bug1074044 bug1075169 bug1075492 bug1075691 bug1076370 bug1076977 bug1076941 bug1077099 bug1077099-followup bug1077282 bug1077282-cleanupCrashreporter bug1077282-workaround bug1077268 bug1075981 bug1078640 bug1079520 bug1079655-1 bug1079655-2

Updated patch with most recent patch from bug 1077268. New try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=2e00d0933001

Now also with patch from bug 1080395.

Comment on attachment 8503238 [details] [diff] [review] Patch for aurora Please see comment 25 for approval request comment.

Comment on attachment 8503238 [details] [diff] [review] Patch for aurora I believe that the patch from bug 1075169 isn't included. Still checking others

Specifically, diff --git a/toolkit/mozapps/update/updater/updater.cpp b/toolkit/mozapps/update/updater/updater.cpp --- a/toolkit/mozapps/update/updater/updater.cpp +++ b/toolkit/mozapps/update/updater/updater.cpp @@ -1986,16 +1986,24 @@ ProcessReplaceRequest() if (rv) { LOG(("Moving destDir to tmpDir failed, err: %d", rv)); return rv; } LOG(("Begin moving newDir (" LOG_S ") to destDir (" LOG_S ")", newDir, destDir)); rv = rename_file(newDir, destDir, true); +#ifdef XP_MACOSX + if (rv) { + LOG(("Moving failed. Begin copying newDir (" LOG_S ") to destDir (" LOG_S ")", + newDir, destDir)); + copy_recursive_skiplist<0> skiplist; + rv = ensure_copy_recursive(newDir, destDir, skiplist); + } +#endif if (rv) { LOG(("Moving newDir to destDir failed, err: %d", rv)); LOG(("Now, try to move tmpDir back to destDir")); ensure_remove_recursive(destDir); int rv2 = rename_file(tmpDir, destDir, true); if (rv2) { LOG(("Moving tmpDir back to destDir failed, err: %d", rv2)); }

This should fix it.

Comment on attachment 8503413 [details] [diff] [review] Patch for aurora From comment #25 - see comment #25 for more info Approval Request Comment [Feature/regressing bug #]: OSX v2 bundle signing [User impact if declined]: Users would see a Gatekeeper warning saying that Firefox wasn't signed by a known developer and the user would be unable to start it without jumping through hoops. [Describe test coverage new/current, TBPL]: Most of these changes have been on m-c for over a week. Virtually all of our tests exercise this feature by verifying that Firefox can be launched and additional, dedicated v2 signature checks were added to mozmill. [Risks and why]: The bundle structure of our .app bundles has changed quite significantly. Although tested quite thoroughly, there may be edge-cases that were missed and that will need to be fixed before release. More exposure on aurora will help us find these cases. [String/UUID change made/needed]: none

Comment on attachment 8503413 [details] [diff] [review] Patch for aurora This is a substantial patch and adds risk to the 34 release. This is manageable risk and rstrong, spohl and others have been very diligent with their work to limit the impact. Furthermore, this change is needed to support the new Gatekeeper requirements that we are currently supporting with Apple's help. cc lizzard and juanb to let them know that this change is about to hit Aurora. If possible, we should spend time before beta1 ships testing update and install on OSX 10.9.5 and older and any other platforms that rstrong and spohl think we should sanity check.

[Tracking Requested - why for this release]:

Comment on attachment 8503413 [details] [diff] [review] Patch for aurora Pushed to mozilla-aurora https://hg.mozilla.org/releases/mozilla-aurora/rev/42dba12fb601

(In reply to Stephen Pohl [:spohl] from comment #17) > (In reply to :Gijs Kruitbosch from comment #16) > > I hate scope creep as much as the next guy... but is there any chance this > > is an opportunity to stop duplicating all of omni.ja between the two sides > > of the universal build and so to reduce the size of our updates, dmgs, and > > .app bundles? > > It would be great to have a bug on file to track this (please CC me). The > more detail, the better. However, seeing how we're scrambling to get even > the most basic things done in time for the v2 changes, I don't believe we'll > be able to address this right now. Did this happen? What's the bug number?

(In reply to Florian Bender from comment #36) > (In reply to Stephen Pohl [:spohl] from comment #17) > > (In reply to :Gijs Kruitbosch from comment #16) > > > I hate scope creep as much as the next guy... but is there any chance this > > > is an opportunity to stop duplicating all of omni.ja between the two sides > > > of the universal build and so to reduce the size of our updates, dmgs, and > > > .app bundles? > > > > It would be great to have a bug on file to track this (please CC me). The > > more detail, the better. However, seeing how we're scrambling to get even > > the most basic things done in time for the v2 changes, I don't believe we'll > > be able to address this right now. > > Did this happen? What's the bug number? This got lost in my bugmail, and it did not happen. I don't know if it makes sense to pursue this; it's somewhat likely that it will be a difficult change to make, and I don't know if the investment would be worth it considering that 32-bit support is basically a dead end. OTOH, we sadly have quite a number of 10.6 users still, and AFAIK there are still no concrete plans as to when we'll be dropping support.

Filed Bug 1103566.

(In reply to Nick Thomas [:nthomas] from comment #5) > (In reply to Stephen Pohl [:spohl] from comment #4) > > Moved from .app/Contents/MacOS to .app/Contents/Resources: > > > libfreebl3.chk > > > libnssdbm3.chk > > > libsoftokn3.chk > > Moving these away from the matching dylibs may break FIPS mode, I suggest > talking to someone from the NSS team (eg :kaie). s/may break/breaks/ (In reply to Ted Mielczarek [:ted.mielczarek] from comment #6) > We talked about this and I said that was acceptable to get this working. Was a followup bug filed to make FIPS mode work with the chk files in the Resources directory?

(In reply to Mike Hommey [:glandium] from comment #39) > (In reply to Ted Mielczarek [:ted.mielczarek] from comment #6) > > We talked about this and I said that was acceptable to get this working. > > Was a followup bug filed to make FIPS mode work with the chk files in the > Resources directory? Bug 1100424. Also see bug 1096494 for more discussion.

Release noted as "Firefox signed by Apple OS X version 2 signature".

Clearing the needinfo. I'm not sure if we've done any particular followup, but I also haven't heard from support or user advocacy that Mac users have had problems downloading and installing Firefox. We have a knowledgebase article to catch this problem: https://support.mozilla.org/en-US/kb/firefox-cant-be-opened-after-you-install-it-on-mac Juan is there anything further you want to do to check this out?