776490 - variable lock (lockPref) does not work

Status: RESOLVED FIXED

(Core :: Preferences: Backend, defect)

Description

[Martin Stránský [:stransky] (ni? me)]

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]]

This is broken in Firefox 15 as well.

Comment 2

[Mike Kaply [:mkaply]]

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ý [:stransky] (ni? me)]

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ý [:stransky] (ni? me)]

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]]

I'm wondering if bug 735280 is the cause.

Comment 6

[Bobby Holley (:bholley)]

(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ý [:stransky] (ni? me)]

It's compiled without principals, on both Firefox 14 and Trunk.

Comment 8

[Martin Stránský [:stransky] (ni? me)]

It's called from EvaluateAdminConfigScript(). Which principal should be used for the autoconfig file evaluation?

Comment 9

[Bobby Holley (:bholley)]

(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ý [:stransky] (ni? me)]

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]]

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]]

bholley - please re-assign if you're not the right person to drive this investigation. Thanks!

Comment 13

[Bobby Holley (:bholley)]

(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ý [:stransky] (ni? me)]

Attachment: 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ý [:stransky] (ni? me)]

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ý [:stransky] (ni? me)]

Comment on attachment 645729 [details] [diff] [review]
patch without autoconfig_cx clean up

Boris, can you check it please?

Comment 17

[Boris Zbarsky [:bzbarsky]]

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ý [:stransky] (ni? me)]

Attachment: patch for check-in

Fixed the indentation + check-in format

Comment 19

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/f4796a2c5af3

Comment 20

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/f4796a2c5af3

Comment 21

[Dimas]

Sorry for my ignorance. This bug will be fixed in Firefox 15? Mozilla17 milestone is Firefox 17?

Comment 22

[Boris Zbarsky [:bzbarsky]]

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]]

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]]

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]]

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]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9d0de687aea2

Comment 27

[Ioana (away)]

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)]

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)]

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]

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]]

(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)]

(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)]

(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]

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]

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]]

Also, the directory is defaults/preferences now.

Comment 37

[Cristiano Guadagnino]

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]

[Tracking Requested - why for this release]: Doesn't work anymore.

Comment 39

[Mike Kaply [:mkaply]]

If lockpref has stopped working please open a new bug.