Last Comment Bug 776490 - variable lock (lockPref) does not work
: variable lock (lockPref) does not work
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Preferences: Backend (show other bugs)
: 15 Branch
: All All
: -- critical with 3 votes (vote)
: mozilla17
Assigned To: Martin Stránský
: Ioana (away)
: Benjamin Smedberg [:bsmedberg]
Mentors:
Depends on: 790905
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-23 04:52 PDT by Martin Stránský
Modified: 2016-08-10 20:56 PDT (History)
20 users (show)
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified
+
verified
verified
-


Attachments
patch without autoconfig_cx clean up (1.65 KB, patch)
2012-07-25 05:31 PDT, Martin Stránský
bzbarsky: review+
Details | Diff | Splinter Review
patch for check-in (1.92 KB, patch)
2012-07-26 03:20 PDT, Martin Stránský
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
bzbarsky: checkin+
Details | Diff | Splinter Review

Description Martin Stránský 2012-07-23 04:52:21 PDT
I followed the instructions from http://kb.mozillazine.org/Locking_preferences

created the mozilla.cfg file:
//
lockPref("xpinstall.enabled", false);

and add it to config files:

// my lock file
pref("general.config.obscure_value", 0);
pref("general.config.filename", "mozilla.cfg");

But the xpinstall.enabled is not locked on latest trunk. The same config works as expected on Firefox 14.
Comment 1 Mike Kaply [:mkaply] 2012-07-23 05:58:32 PDT
This is broken in Firefox 15 as well.
Comment 2 Mike Kaply [:mkaply] 2012-07-23 06:23:57 PDT
The regression appears to have happened between 

2012-04-29-03-05-04-mozilla-central
and
2012-04-30-03-05-04-mozilla-central

http://hg.mozilla.org/mozilla-central/rev/8f377102841b
and
http://hg.mozilla.org/mozilla-central/rev/cfaf90b22fc3
Comment 3 Martin Stránský 2012-07-23 07:12:39 PDT
The LockPref() from mozilla.cfg is not executed (at least not the nsPrefBranch::LockPref()) on Trunk, but the file is processed. Investigating.
Comment 4 Martin Stránský 2012-07-23 07:33:52 PDT
Evaluation of mozilla.cfg produces such errors:

-134424768[7ffff6b65150]: general.config.filename = mozilla.cfg
-134424768[7ffff6b65150]: evaluating .cfg file mozilla.cfg with obscureValue 0
-134424768[7ffff6b65150]: ###!!! ASSERTION: About to do a meaningless security check!: '!AccessCheck::callerIsChrome()', file /home/komat/tmp965/src/js/xpconnect/wrappers/AccessCheck.cpp, line 373
-134424768[7ffff6b65150]: ###!!! ASSERTION: Script compiled without principals!: 'Error', file /home/komat/tmp965/src/caps/src/nsScriptSecurityManager.cpp, line 2121
-134424768[7ffff6b65150]: ###!!! ASSERTION: About to do a meaningless security check!: '!AccessCheck::callerIsChrome()', file /home/komat/tmp965/src/js/xpconnect/wrappers/AccessCheck.cpp, line 373
-134424768[7ffff6b65150]: ###!!! ASSERTION: Script compiled without principals!: 'Error', file /home/komat/tmp965/src/caps/src/nsScriptSecurityManager.cpp, line 2121
Comment 5 Mike Kaply [:mkaply] 2012-07-23 07:48:39 PDT
I'm wondering if bug 735280 is the cause.
Comment 6 Bobby Holley (:bholley) (busy with Stylo) 2012-07-23 07:52:40 PDT
(In reply to Michael Kaply (mkaply) from comment #5)
> I'm wondering if bug 735280 is the cause.

I very much doubt it. bug 754202 might be a better bet.

> -134424768[7ffff6b65150]: ###!!! ASSERTION: About to do a meaningless
> security check!: '!AccessCheck::callerIsChrome()', file
> /home/komat/tmp965/src/js/xpconnect/wrappers/AccessCheck.cpp, line 373
> -134424768[7ffff6b65150]: ###!!! ASSERTION: Script compiled without
> principals!: 'Error', file
> /home/komat/tmp965/src/caps/src/nsScriptSecurityManager.cpp, line 2121
> -134424768[7ffff6b65150]: ###!!! ASSERTION: About to do a meaningless
> security check!: '!AccessCheck::callerIsChrome()', file
> /home/komat/tmp965/src/js/xpconnect/wrappers/AccessCheck.cpp, line 373
> -134424768[7ffff6b65150]: ###!!! ASSERTION: Script compiled without
> principals!: 'Error', file
> /home/komat/tmp965/src/caps/src/nsScriptSecurityManager.cpp, line 2121

Sounds like a script is being compiled without principals, which is bad.
Comment 7 Martin Stránský 2012-07-23 07:56:51 PDT
It's compiled without principals, on both Firefox 14 and Trunk.
Comment 8 Martin Stránský 2012-07-24 05:07:47 PDT
It's called from EvaluateAdminConfigScript(). Which principal should be used for the autoconfig file evaluation?
Comment 9 Bobby Holley (:bholley) (busy with Stylo) 2012-07-24 05:13:33 PDT
(In reply to Martin Stránský from comment #8)
> It's called from EvaluateAdminConfigScript(). Which principal should be used
> for the autoconfig file evaluation?

It currently seems to use NULL, so nsNullPrincipal, probably. 

I'd suggest just replacing all the autoconfig_cx junk with the safe JS context (nsContentUtils::GetSafeJSContext()).
Comment 10 Martin Stránský 2012-07-24 08:09:15 PDT
nsNullPrincipal does not have permissions to read the preferences, I have to use the system principal. I'll post a patch with system principal and autoconfig_cx clean up.
Comment 11 Mike Kaply [:mkaply] 2012-07-24 08:58:43 PDT
FYI, autoconfig files do a lot more than set and read preferences.

They need full access to Components.

http://mike.kaply.com/2012/03/22/customizing-firefox-advanced-autoconfig-files/
Comment 12 Alex Keybl [:akeybl] 2012-07-24 16:50:03 PDT
bholley - please re-assign if you're not the right person to drive this investigation. Thanks!
Comment 13 Bobby Holley (:bholley) (busy with Stylo) 2012-07-25 02:23:49 PDT
(In reply to Alex Keybl [:akeybl] from comment #12)
> bholley - please re-assign if you're not the right person to drive this
> investigation. Thanks!

I am not, but do not know who is.
Comment 14 Martin Stránský 2012-07-25 05:31:46 PDT
Created attachment 645729 [details] [diff] [review]
patch without autoconfig_cx clean up

Minimal patch wihich fixes the broken locking.

The JS engine crashes (compartment() == (*dictp)->compartment() in Shape::insertIntoDictionary()) when the script is launched with SafeJSContext. 

The autoconfig_cx uses  different error handler and has attached its own security manager, not sure how it matters here.

Anyway, I think it's better to fix the property locking and then do any clean-up.
Comment 15 Martin Stránský 2012-07-25 05:34:20 PDT
Ahh, the crashes may be happen because I used the global object attached to autoconfig_cx. But let's split it anyway.
Comment 16 Martin Stránský 2012-07-25 05:57:42 PDT
Comment on attachment 645729 [details] [diff] [review]
patch without autoconfig_cx clean up

Boris, can you check it please?
Comment 17 Boris Zbarsky [:bz] (still a bit busy) 2012-07-25 21:43:05 PDT
Comment on attachment 645729 [details] [diff] [review]
patch without autoconfig_cx clean up

r=me if you fix the indent.
Comment 18 Martin Stránský 2012-07-26 03:20:06 PDT
Created attachment 646076 [details] [diff] [review]
patch for check-in

Fixed the indentation + check-in format
Comment 19 Boris Zbarsky [:bz] (still a bit busy) 2012-07-26 09:40:13 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/f4796a2c5af3
Comment 21 Dimas 2012-07-27 09:37:22 PDT
Sorry for my ignorance. This bug will be fixed in Firefox 15? Mozilla17 milestone is Firefox 17?
Comment 22 Boris Zbarsky [:bz] (still a bit busy) 2012-07-27 09:39:15 PDT
It's fixed for Firefox 17.

Whether it gets fixed in 16 and 15, both of which are in their stabilization, not active development, phases depends on whether it's nominated and approved for those.
Comment 23 Mike Kaply [:mkaply] 2012-07-28 07:51:42 PDT
Comment on attachment 646076 [details] [diff] [review]
patch for check-in

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 754202
User impact if declined: Enterprise users lose the primary enterprise feature in Firefox.
Testing completed (on m-c, etc.): Tested and verified patch works.
Risk to taking this patch (and alternatives if risky): Little to none. Only affects autoconfig, and autoconfig was not working anyway.
String or UUID changes made by this patch: 

Without this patch, autoconfig is completely broke in Firefox 15/16.

Although this is an "enterprise" feature, we are trying to encourage enterprises to use rapid release, and this would mean that the core Firefox enterprise feature would be broke in the rapid release.
Comment 24 Alex Keybl [:akeybl] 2012-07-30 08:23:30 PDT
Comment on attachment 646076 [details] [diff] [review]
patch for check-in

[Triage Comment]
Given the risk calculus, and our focus on supporting enterprises on mainline, approving for branches. Let's get this landed before tomorrow's beta build.
Comment 25 Andrew McCreight [:mccr8] 2012-07-30 08:36:09 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/8ca36187554a

I'll check in to beta in a little bit here, I want to make sure it builds (the includes are a little different on beta).
Comment 26 Andrew McCreight [:mccr8] 2012-07-30 09:21:40 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/9d0de687aea2
Comment 27 Ioana (away) 2012-08-13 06:47:50 PDT
Verified as fixed with the steps from comment 0 on:
Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20100101 Firefox/15.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:15.0) Gecko/20100101 Firefox/15.0
Mozilla/5.0 (Windows NT 6.1; rv:15.0) Gecko/20100101 Firefox/15.0
BuildID: 20120808131812
Comment 28 Ioana (away) 2012-09-28 05:43:35 PDT
Verified as fixed on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/20100101 Firefox/16.0
Mozilla/5.0 (X11; Linux i686; rv:16.0) Gecko/20100101 Firefox/16.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:16.0) Gecko/20100101 Firefox/16.0
BuildID: 20120925201946
Comment 29 Ioana (away) 2012-10-11 08:43:49 PDT
Verified as fixed on Firefox 17.0 beta 1 (20121010150351) on Windows 7, Ubuntu 12.04, and Mac OS X 10.7.5.
Comment 30 Cristiano Guadagnino 2012-12-05 13:59:31 PST
Tried on Firefox 17.0.1, it does not work.

I can see that general.config.filename correctly points to "mozilla.cfg" in the about:config dialog, but the settings are not locked.
Comment 31 Alex Keybl [:akeybl] 2012-12-10 06:48:59 PST
(In reply to Ioana Budnar [QA] from comment #29)
> Verified as fixed on Firefox 17.0 beta 1 (20121010150351) on Windows 7,
> Ubuntu 12.04, and Mac OS X 10.7.5.

Ioana - can you retest based upon comment 30?
Comment 32 Ioana (away) 2012-12-11 04:10:09 PST
(In reply to Alex Keybl [:akeybl] from comment #31)
> (In reply to Ioana Budnar [QA] from comment #29)
> > Verified as fixed on Firefox 17.0 beta 1 (20121010150351) on Windows 7,
> > Ubuntu 12.04, and Mac OS X 10.7.5.
> 
> Ioana - can you retest based upon comment 30?

Verified on Firefox 17.0.1:
Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/20100101 Firefox/17.0 (20121128204232).

I used the steps in comment 0 and the xpinstall.enabled pref was locked as false.
Comment 33 Ioana (away) 2012-12-11 04:11:09 PST
(In reply to criguada from comment #30)
> Tried on Firefox 17.0.1, it does not work.
> 
> I can see that general.config.filename correctly points to "mozilla.cfg" in
> the about:config dialog, but the settings are not locked.

What steps did you use to locked the prefs? And what OS were you on?
Comment 34 Cristiano Guadagnino 2012-12-13 06:14:58 PST
I am on Windows XP Professional S.P.3, Firefox 17.0.1.
I created the file c:\programmi\Mozilla Firefox\defaults\pref\local-settings.js, which contains only the following line:

pref("general.config.filename","mozilla.cfg");

Then I created the file c:\programmi\Mozilla Firefox\mozilla.cfg, which contains the following lines:

//
lockPref("privacy.cdp.cookies", true);
lockPref("privacy.item.cookies", true);
lockPref("privacy.donottrackheader.enabled", true);
lockPref("network.proxy.type", 0);
lockPref("general.warnOnAboutConfig", true);

When I start Firefox, none of the mentioned preferences is locked.

As an example, in the following screenshot you can see that the first checkbox at the top is not locked (it is in italian, it corresponds to the "privacy.donottrackheader.enabled" preference):

https://dl.dropbox.com/u/1200235/Cattura-1.png
Comment 35 James Pearson 2012-12-13 06:22:23 PST
I believe you need to add the line:

pref('general.config.obscure_value', 0);

to your local-settings.js file

See: http://mike.kaply.com/2012/03/16/customizing-firefox-autoconfig-files/
Comment 36 Mike Kaply [:mkaply] 2012-12-13 06:35:03 PST
Also, the directory is defaults/preferences now.
Comment 37 Cristiano Guadagnino 2012-12-13 08:14:56 PST
I did both (adding the obscure_value line AND copying the file to the new "defaults/preferences" dir), and I also left the modified file in the "default/pref" dir.
It worked, although I don't know which mod did the trick.
Comment 38 nessento 2016-08-09 18:48:22 PDT
[Tracking Requested - why for this release]: Doesn't work anymore.
Comment 39 Mike Kaply [:mkaply] 2016-08-09 19:01:11 PDT
If lockpref has stopped working please open a new bug.

Note You need to log in before you can comment on or make changes to this bug.