variable lock (lockPref) does not work

RESOLVED FIXED in Firefox 15

Status

()

Core
Preferences: Backend
--
critical
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: Martin Stránský, Assigned: Martin Stránský)

Tracking

({regression})

15 Branch
mozilla17
regression
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox15+ verified, firefox16+ verified, firefox17 verified, firefox-esr17-)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
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.
(Assignee)

Updated

5 years ago
Version: 10 Branch → Trunk

Comment 1

5 years ago
This is broken in Firefox 15 as well.
Severity: normal → critical
Keywords: regression
OS: Linux → All
Hardware: x86_64 → All
Version: Trunk → 15 Branch
(Assignee)

Updated

5 years ago
Version: 15 Branch → Trunk

Comment 2

5 years ago
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
(Assignee)

Comment 3

5 years ago
The LockPref() from mozilla.cfg is not executed (at least not the nsPrefBranch::LockPref()) on Trunk, but the file is processed. Investigating.
(Assignee)

Comment 4

5 years ago
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

5 years ago
I'm wondering if bug 735280 is the cause.
(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.
(Assignee)

Comment 7

5 years ago
It's compiled without principals, on both Firefox 14 and Trunk.
tracking-firefox15: --- → ?
tracking-firefox16: --- → ?
(Assignee)

Comment 8

5 years ago
It's called from EvaluateAdminConfigScript(). Which principal should be used for the autoconfig file evaluation?
(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()).
(Assignee)

Comment 10

5 years ago
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.
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/
bholley - please re-assign if you're not the right person to drive this investigation. Thanks!
Assignee: nobody → bobbyholley+bmo
tracking-firefox15: ? → +
tracking-firefox16: ? → +
(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.
Assignee: bobbyholley+bmo → nobody
(Assignee)

Comment 14

5 years ago
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.
(Assignee)

Comment 15

5 years ago
Ahh, the crashes may be happen because I used the global object attached to autoconfig_cx. But let's split it anyway.
(Assignee)

Comment 16

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

Boris, can you check it please?
Attachment #645729 - Flags: review?(bzbarsky)
Assignee: nobody → stransky
Comment on attachment 645729 [details] [diff] [review]
patch without autoconfig_cx clean up

r=me if you fix the indent.
Attachment #645729 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 18

5 years ago
Created attachment 646076 [details] [diff] [review]
patch for check-in

Fixed the indentation + check-in format
Attachment #646076 - Flags: checkin?
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
http://hg.mozilla.org/integration/mozilla-inbound/rev/f4796a2c5af3
Flags: in-testsuite?
Keywords: checkin-needed
Attachment #646076 - Flags: checkin? → checkin+
https://hg.mozilla.org/mozilla-central/rev/f4796a2c5af3
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17

Comment 21

5 years ago
Sorry for my ignorance. This bug will be fixed in Firefox 15? Mozilla17 milestone is Firefox 17?
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 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.
Attachment #646076 - Flags: approval-mozilla-beta?
Attachment #646076 - Flags: approval-mozilla-aurora?

Updated

5 years ago
Version: Trunk → 15 Branch
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.
Attachment #646076 - Flags: approval-mozilla-beta?
Attachment #646076 - Flags: approval-mozilla-beta+
Attachment #646076 - Flags: approval-mozilla-aurora?
Attachment #646076 - Flags: approval-mozilla-aurora+

Updated

5 years ago
Keywords: checkin-needed
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).
Keywords: checkin-needed
status-firefox15: --- → affected
status-firefox16: --- → fixed
status-firefox17: --- → fixed
https://hg.mozilla.org/releases/mozilla-beta/rev/9d0de687aea2
status-firefox15: affected → fixed

Comment 27

5 years ago
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
status-firefox15: fixed → verified
Depends on: 790905
Keywords: verifyme

Updated

5 years ago
QA Contact: ioana.budnar

Comment 28

5 years ago
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
status-firefox16: fixed → verified

Comment 29

5 years ago
Verified as fixed on Firefox 17.0 beta 1 (20121010150351) on Windows 7, Ubuntu 12.04, and Mac OS X 10.7.5.
status-firefox17: fixed → verified
Keywords: verifyme
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.
(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?
Keywords: qawanted

Updated

5 years ago
tracking-firefox-esr17: --- → ?

Comment 32

5 years ago
(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

5 years ago
(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?

Updated

5 years ago
Keywords: qawanted
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

5 years ago
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/
Also, the directory is defaults/preferences now.
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.
status-firefox-esr17: --- → affected
tracking-firefox-esr17: ? → 18+
status-firefox-esr17: affected → fixed
tracking-firefox-esr17: 18+ → -

Comment 38

a year ago
[Tracking Requested - why for this release]: Doesn't work anymore.
status-firefox49: --- → ?
status-firefox50: --- → ?
status-firefox51: --- → ?
status-firefox-esr17: fixed → ---
tracking-firefox49: --- → ?
tracking-firefox50: --- → ?
tracking-firefox51: --- → ?
If lockpref has stopped working please open a new bug.
status-firefox49: ? → ---
status-firefox50: ? → ---
status-firefox51: ? → ---
tracking-firefox49: ? → ---
tracking-firefox50: ? → ---
tracking-firefox51: ? → ---
You need to log in before you can comment on or make changes to this bug.