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)

3.12.3
All
macOS

Tracking

(firefox38 wontfix, firefox38.0.5 wontfix, firefox39+ fixed, firefox40+ fixed, firefox41+ fixed, firefox-esr31 wontfix, firefox-esr3839+ fixed)

RESOLVED FIXED
3.19.1
Tracking Status
firefox38 --- wontfix
firefox38.0.5 --- wontfix
firefox39 + fixed
firefox40 + fixed
firefox41 + fixed
firefox-esr31 --- wontfix
firefox-esr38 39+ fixed

People

(Reporter: smichaud, Assigned: smichaud)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
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).
Attached patch Possible fixSplinter Review
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
Do you want me to get clarification on exactly what is going to be desupported?
> 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.
(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.
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
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)
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.
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+
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
Priority: -- → P1
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)
And I assume I should also change security/nss/lib/ckfw/capi/config.mk (on mozilla-central).
Tracking for esr: if we can avoid having to deal with Apple for each upload, this would be better for us.
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)
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.
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)
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.
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)
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.
Martin can you fill out the approval request?
Flags: needinfo?(martin.thomson)
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)
> 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.
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.
Blocks: 1166031
No longer depends on: 1166031
Fixed in beta by the uplifts in bug 1166031 comment #31.
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.
(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.
(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.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: