Closed Bug 1047738 Opened 5 years ago Closed 5 years ago

Make distribution code look for the distribution directory under Contents/Resources due to v2 signing requirements

Categories

(Firefox :: General, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 35
Tracking Status
firefox34 --- fixed
firefox35 --- fixed

People

(Reporter: spohl, Assigned: smichaud)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-needed, relnote)

Attachments

(1 file, 2 obsolete files)

Due to the new v2 signing requirements on OSX, the 'distribution' directory can no longer live under Contents/MacOS. Also, its contents can no longer be modified after the top-level .app has been signed, essentially breaking our way of doing partner builds at the moment. We will need to find a new way to allow for partner builds with these new signing requirements.
Blocks: 1047740
Blocks: 1047742
Flagging Connor with needinfo just to be absolutely sure he sees this.
Flags: needinfo?(mconnor)
Same for bsmedberg

Benjamin, the distribution directory will no longer work on Mac OS X 10.9.5 due to new signing requirements. This will also affect adding default preference files or for that matter any files to the app bundle that weren't present when the bundle was signed. See bug 1047584 and associated bugs for further info.

I don't see a clean way to deal with adding a distribution directory or adding default preference files. :(

Please note: this is not currently public knowledge and Apple is holding us to our NDA with them regarding this issue. As such, all the bugs are currently Mozilla confidential.
Flags: needinfo?(benjamin)
Summary: Find new way to allow for partner builds on OSX due to v2 signing requirements → Find new way to allow for partner builds on OSX to use the distribution directory due to v2 signing requirements
Component: Application Update → General
Product: Toolkit → Firefox
Can we put it anywhere in the app bundle? I don't think I have a useful opinion here.
Flags: needinfo?(benjamin)
I should have updated this bug too. It looks like we might be able to put arbitrary files in the root of the .app bundle and change them even after it was signed. I'm working on updating our Makefiles etc. to create bundles with the new structure to confirm.
Flags: needinfo?(mconnor)
The solution suggested in bug 1047584 is to move the 'distribution' directory to *.app/MozResources. Let's try and work with that for partner builds.
If we could support both locations (for a fairly long period of time), that would be good. Otherwise we're going to have to figure out how to re-arrange partner bits in vanilla mars fairly quickly.
(In reply to Ben Hearsum [:bhearsum] from comment #6)
> If we could support both locations (for a fairly long period of time), that
> would be good. Otherwise we're going to have to figure out how to re-arrange
> partner bits in vanilla mars fairly quickly.

Actually, this probably doesn't matter...we _have_ to move distribution, there's no point in supporting the old location. I filed bug 1048921 to figure out how to deal with this.
Blocks: 1046906
No longer blocks: 1047584
Group: mozilla-employee-confidential
Robert has asked me to work on this, so here I go.
Assignee: nobody → smichaud
Status: NEW → ASSIGNED
On Robert's advice, I'm going to try to move "Contents/Mac OS/distribution" to "Contents/Resources/distribution", plus whatever changes are needed to make that work properly.
If you need it, you can find an example partner repack (which uses the distribution mechanism) at http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/32.0-candidates/build1/partner-repacks/bing/mac/en-US/Firefox%2032.0.dmg
Attached patch Proof of concept patch (obsolete) — Splinter Review
Here's what I call a proof of concept patch for this bug.  It's not in its final form.  But it can be tested independently of the other v2 signature patches (the ones Stephen Pohl and Robert Strong are working on).

I've started a full set of tryserver builds:
https://tbpl.mozilla.org/?tree=Try&rev=eb2fb9be28a4

If they work out, we should be able to transform this patch into the final version by getting rid of the added #ifdef XP_MACOSX section in nsXREDirProvider.cpp.  In its final form this patch will need to be landed with the other v2 signature patches (which change the location of the GREDir on OS X).
Attached patch Proof of concept patch rev1 (obsolete) — Splinter Review
My previous patch had a bunch of errors in the xpcshell tests.  Let's see if this revision does better:

https://tbpl.mozilla.org/?tree=Try&rev=55ee126087e0
Attachment #8481688 - Attachment is obsolete: true
Comment on attachment 8483069 [details] [diff] [review]
Proof of concept patch rev1

> https://tbpl.mozilla.org/?tree=Try&rev=55ee126087e0

There were no non-spurious failures, and no xpcshell failures at all.  So it looks like this patch can be folded in with the other v2 signature patches (with the appropriate change), when it's time for that.
Attached patch PatchSplinter Review
Steven's patch, updated to make use of all the other v2 signature patches.
Attachment #8483069 - Attachment is obsolete: true
Comment on attachment 8488109 [details] [diff] [review]
Patch

Benjamin, please feel free to forward this review request, just like any others that I've sent your way today. Thanks!
Attachment #8488109 - Flags: review?(benjamin)
Comment on attachment 8488109 [details] [diff] [review]
Patch

What does this patch actually accomplish? Are we putting this into Firefox.app/Contents/Resources/distribution and signing distributions separately?
Attachment #8488109 - Flags: review?(benjamin) → review+
Flags: needinfo?(spohl.mozilla.bugs)
Hmm, you bring up a very good point. Initially, we thought that we could move the distribution directory to .app/MozResources and avoid any signing issues. Once we learned that this wouldn't be possible, the decision was made in comment 9 to move it to .app/Contents/Resources instead. However, I'm not sure this solves any of our problems regarding v2 signatures.

Robert and Steven: Could you remind us why we thought this change was necessary? Were we planning on signing partner builds individually until we have installers that install the distribution directory in a user-specific location? If we don't, it seems like partner builds would have invalid signatures regardless of where the distribution directory is located inside of the .app bundle...
Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(smichaud)
Flags: needinfo?(robert.strong.bugs)
My understanding of the v2 signature business is rather limited.  But no, I don't know of any particular *functional* reason to move the distribution directory from Contents/MacOS/ to Contents/Resources/ -- since it (and its contents) would have to be signed in either location.  But maybe there's a valid *conceptual* reason:  Contents/MacOS/ is generally meant for executables, and Contents/Resources is generally meant for "resources".

In any case, I think the simplicity of my patch shows that it'd be easy to move the distribution directory wherever we want without causing trouble.
Flags: needinfo?(smichaud)
Actually, the conceptual reason is a pretty good one, especially since it's also reflected in the way we will build the .app bundle. We used to rsync all files to Contents/MacOS. However, now we rsync them to Contents/Resources and cherry-pick the binaries and libraries that we want to have moved to Contents/MacOS (see bug 1047584). It would seem ugly to explicitly move a potential 'distribution' directory to Contents/MacOS going forward.

Benjamin, is this good enough to wave this patch through?
Flags: needinfo?(robert.strong.bugs) → needinfo?(benjamin)
I already marked r+ ;-) The question was merely about how we plan on managing the fact that anything in Resources/distribution will change the signature. Or if we're going to punt on that question for FF34...
Flags: needinfo?(benjamin)
Oh, I meant: do we still want to make this change due to conceptual reasons alone since we don't actually address the signing problem with partner builds. :-) I'm hearing yes.

It looks like we're going to punt on the signing issue for partner builds and try to fix it by using an installer in the future. This would allow us to store the distribution directory outside of the .app bundle and not risk breaking the signature. Short of asking releng to sign every single partner build, there doesn't seem a way to support v2 signatures with a distribution directory inside of the .app bundle like we have today.
Having it under Contents/Resources instead of Contents/MacOS will allow us to sign each partner builds if that ends up being the path we have to take. Also, see bug 1058119 for the releng bug to sign each partner build.
Summary: Find new way to allow for partner builds on OSX to use the distribution directory due to v2 signing requirements → Make distribution code look for the distribution directory under Contents/Resources due to v2 signing requirements
https://hg.mozilla.org/mozilla-central/rev/c6e905b1e209
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Landed on aurora in the Mac V2 signing combined patch in bug 1047584
The location change of the distribution directory on Mac OS X should be documented and possibly be included in the release notes.

Landed on aurora in the Mac V2 signing combined patch in bug 1047584
Depends on: 1105189
You need to log in before you can comment on or make changes to this bug.