Closed
Bug 1166515
Opened 9 years ago
Closed 9 years ago
Ensure that the two slices in the libnssckbi.dylib binary have the same "type"
Categories
(NSS :: Libraries, defect, P1)
Tracking
(firefox38 wontfix, firefox38.0.5 wontfix, firefox39+ fixed, firefox40+ fixed, firefox41+ fixed, firefox-esr31 wontfix, firefox-esr3839+ fixed)
People
(Reporter: smichaud, Assigned: smichaud)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
482 bytes,
patch
|
wtc
:
review+
|
Details | Diff | Splinter Review |
Someone from Apple (Michael Wong <wong1@apple.com>) recently sent us (Mozilla) an email warning us that "heterogenous fat binaries will not be supported in the very near future". The example he gave of a "heterogenous fat binary" is libnssckbi.dylib, which has "two differently typed slices": Firefox.app/Contents/MacOS/libnssckbi.dylib (for architecture x86_64): Mach-O 64-bit dynamically linked shared library x86_64 Firefox.app/Contents/MacOS/libnssckbi.dylib (for architecture i386): Mach-O bundle i386 By this I think he means that the x86_64 bit architecture slice is a "Mach-O 64-bit dynamically linked shared library", while the i386 architecture slice is a "Mach-O bundle". I've found where in the tree this distinction is implemented. I'm going to post a patch that gets rid of it (that makes both slices "Mac-O bundles"). Then I'm going to do a tryserver build for Michael Wong to test, and tell us whether or not it fits Apple's new requirements.
Assignee | ||
Comment 1•9 years ago
|
||
I should mention that, if I'm right about what a "heterogenous fat binary" is, libnssckbi.dylib is the only one we have in any of our current distros (including Firefox, Firefox ESR and Thunderbird).
Assignee | ||
Comment 2•9 years ago
|
||
Here's a possible fix. And here's the tryserver build I just started: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f543e0fc04d3
Assignee: nobody → smichaud
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•9 years ago
|
||
Here's a link to the tryserver build made from my patch from comment #2: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-f543e0fc04d3/try-macosx64/firefox-41.0a1.en-US.mac.dmg
Comment 4•9 years ago
|
||
Do you want me to get clarification on exactly what is going to be desupported?
Assignee | ||
Comment 5•9 years ago
|
||
> Do you want me to get clarification on exactly what is going to be desupported?
I'm not sure I understand your question.
If all we need do is fix libnssckbi.dylib, I'm happy to let that be the end of the matter.
Comment 6•9 years ago
|
||
(In reply to Steven Michaud from comment #5) > > Do you want me to get clarification on exactly what is going to be desupported? > > I'm not sure I understand your question. > > If all we need do is fix libnssckbi.dylib, I'm happy to let that be the end > of the matter. Sorry, I didn't finish reading mail before this comment - I noticed afterwards that you already got clarification.
Assignee | ||
Comment 7•9 years ago
|
||
The code my patch removes was first added to NSS code in bug 469738, as part of a patch to "provide 64 bit MAC OS X support to nss". But as best I can tell, this code is not at all necessary: Without it (in other words with my patch), Firefox has no trouble accessing root certs in 64-bit mode. (Root certs is just what libnssckbi.dylib contains.) Here's an example (using my tryserver build from comment #3): 1) Start it with a clean profile 2) Choose Preferences : Advanced : Certificates 3) Click View Certificates You should have no trouble viewing all the "Builtin Object Token"s, which are stored in libnssckbi.dylib. None of the comments at bug 469738 says why the code was added which my patch removes. But it seems to have been just an afterthought, which was incorrect even then. Here's bug 469738's patch as landed: https://bug469738.bugzilla.mozilla.org/attachment.cgi?id=368482
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8607810 [details] [diff] [review] Possible fix I really don't know who I should ask for review. Please pass the review request to someone else if you think that's appropriate. Note that it's quite urgent we get this bug fixed. So it may be necessary to change this code in the Mozilla tree before it's formally included in an NSS release. Also note that (as you can see from the original patch for bug 469738), libnsscapi.dylib is also effected. But current Mozilla distros (of Firefox, Firefox ESR and Thunderbird) no longer contain that file.
Attachment #8607810 -
Flags: review?(wtc)
Comment 9•9 years ago
|
||
This affects all Firefox OSX binaries. Tracking across the board. I'm not yet sure we're going to be able to or will need to fix this in 39 but best to track in case we do.
status-firefox38:
--- → wontfix
status-firefox38.0.5:
--- → wontfix
status-firefox39:
--- → affected
status-firefox40:
--- → affected
status-firefox41:
--- → affected
status-firefox-esr31:
--- → wontfix
status-firefox-esr38:
--- → affected
tracking-firefox39:
--- → +
tracking-firefox40:
--- → +
tracking-firefox41:
--- → +
tracking-firefox-esr38:
--- → ?
Comment 10•9 years ago
|
||
Comment on attachment 8607810 [details] [diff] [review] Possible fix r=wtc. The original change for 64-bit Mac OS X build was made in this changeset: https://hg.mozilla.org/projects/nss/rev/b2bee49982f2 Bug 469738 didn't mention why we didn't want to use -bundle for 64-bit. It may have been part of the original workaround for bug 480730. Patch checked in: https://hg.mozilla.org/projects/nss/rev/17b065430727 Note that I also reverted a similar change in nss/lib/ckfw/capi/config.mk.
Attachment #8607810 -
Flags: review?(wtc)
Attachment #8607810 -
Flags: review+
Attachment #8607810 -
Flags: checkin+
Updated•9 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Component: Security: PSM → Libraries
Flags: checkin+
Product: Core → NSS
Resolution: --- → FIXED
Target Milestone: --- → 3.19.1
Version: unspecified → 3.12.3
Updated•9 years ago
|
Priority: -- → P1
Assignee | ||
Comment 11•9 years ago
|
||
Thanks, Wan-Teh! I'm not entirely sure where your checkin leaves us. I presumably still need to land my patch on mozilla-central, but I don't know what to call it -- an import of NSS 3.19.1?
Flags: needinfo?(wtc)
Assignee | ||
Comment 12•9 years ago
|
||
And I assume I should also change security/nss/lib/ckfw/capi/config.mk (on mozilla-central).
Comment 13•9 years ago
|
||
Tracking for esr: if we can avoid having to deal with Apple for each upload, this would be better for us.
Assignee | ||
Comment 14•9 years ago
|
||
Comment on attachment 8607810 [details] [diff] [review] Possible fix I'm just going to land this patch (my original patch) on mozilla-inbound, on the strength of wtc's r+. Unfortunately, I didn't get up early enough to catch it awake ... so I'm going to need to wait until it comes around again.
Flags: needinfo?(wtc)
Comment 15•9 years ago
|
||
Hi Steven, Your patch is already in mozilla-central: https://hg.mozilla.org/mozilla-central/rev/38ff380719e4 In general security/nss needs to be updated by pushing a new NSS hg tag. The procedure is documented here: https://developer.mozilla.org/en-US/docs/Updating_NSPR_or_NSS_in_mozilla-central Only an NSS team member can create a new hg tag, which you can then push to mozilla-inbound.
Assignee | ||
Comment 16•9 years ago
|
||
Thanks again, Wan-Teh! Are there any special procedures for uplifting your changes to aurora (40) and beta (39)? We're going to need to do that, and also (I think) uplift to esr38.
Flags: needinfo?(wtc)
Assignee | ||
Comment 17•9 years ago
|
||
Another thing, Wan-Teh: Would it be feasible to only uplift this bug's changes to those older branches? Your other NSS changes my be considered too risky to uplift.
Comment 18•9 years ago
|
||
Hi Steven, I am sorry that I am not familiar with the various Firefox branches. Please coordinate with Martin Thomson (:mt), who needs to uplift NSS 3.19.1 to some Firefox branches. He may have gotten the approval to uplift NSS 3.19.1 to some of the branches you want to uplift your patch to.
Flags: needinfo?(wtc)
Assignee | ||
Comment 19•9 years ago
|
||
Thanks, Wan-Teh. I'm going to be away for the Memorial Day weekend, starting tomorrow. When I get back I'll get in touch with Martin Thomson. If any work needs to be done here before that, someone else will need to do it.
Comment 20•9 years ago
|
||
Martin can you fill out the approval request?
Flags: needinfo?(martin.thomson)
Comment 21•9 years ago
|
||
This change is under the tag NSS_3_19_1_BETA1 currently. We are tracking that in bug 1166031. I don't think that we need specific approval for this fix.
Flags: needinfo?(martin.thomson)
Assignee | ||
Comment 22•9 years ago
|
||
> I don't think that we need specific approval for this fix. I agree. The fixes for bug 1166031 now include Wan-Teh's patch from comment #10. Those fixes are also quite urgent, and are going to land everywhere this bug's fix needs to land. So this bug can depend on bug 1166031 for all the uplifts. As of right now, bug 1166031 has been uplifted to aurora, but not yet to the other branches.
Comment 23•9 years ago
|
||
OK. Thanks for the clarification Steven! I'll mark this wontfix for 39, since we're not expecting more activity on this bug and are tracking the other bug where the uplifts are planned to happen. If that seems confusing since we do expect this to be fixed for 39 release, then we can come back and switch the flag to "fixed" once the patch in bug 1166031 lands.
Updated•9 years ago
|
Assignee | ||
Comment 24•9 years ago
|
||
Fixed in beta by the uplifts in bug 1166031 comment #31.
Assignee | ||
Comment 25•9 years ago
|
||
Fixed in esr38 by uplifts in bug 1166031 comment #46.
Assignee | ||
Comment 27•9 years ago
|
||
Apple has informed us that El Capitan (OS X 10.11) breaks on FF distros where this bug isn't fixed -- so, for example, on FF 38.0.5.
Assignee | ||
Comment 29•9 years ago
|
||
(Following up comment #27) For the record: When (on OS X 10.11) you double-click on FF 38.0.5 or below, Firefox won't run and the OS says "Firefox is damaged and can't be opened." There is no crash. The work around is to run FF 39 or above. The current FF 39 beta should do.
Comment 30•9 years ago
|
||
(In reply to Steven Michaud [:smichaud] from comment #29) > The work around is to run FF 39 or above. The current FF 39 beta should do. A supplemental work around for FF 38 or below is to run firefox-bin from the App path in terminal.
Assignee | ||
Updated•9 years ago
|
Blocks: el-capitan
You need to log in
before you can comment on or make changes to this bug.
Description
•