lockpref not honored in /etc/thunderbird/pref/thunderbird.js

Bug #623844 reported by Christoph Erdle on 2010-08-25
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Mozilla Thunderbird
Fix Released
Medium
thunderbird (Ubuntu)
Wishlist
Unassigned

Bug Description

Binary package hint: thunderbird

appended /etc/thunderbird/pref/thunderbird.js with the following line:
lockpref("app.update.enabled", false);

When starting Thunderbird and checking the config app.update.enabled is set to false, but can be edited by the user.

ProblemType: Bug
DistroRelease: Ubuntu 10.04
Package: thunderbird 3.0.6+build2+nobinonly-0ubuntu0.10.04.1
ProcVersionSignature: Ubuntu 2.6.32-24.39-generic 2.6.32.15+drm33.5
Uname: Linux 2.6.32-24-generic x86_64
Architecture: amd64
Date: Wed Aug 25 09:58:43 2010
InstallationMedia: Ubuntu 10.04 LTS "Lucid Lynx" - Release amd64 (20100429)
ProcEnviron:
 PATH=(custom, no user)
 LANG=de_DE.utf8
 SHELL=/bin/bash
SourcePackage: thunderbird

Created attachment 326060
patch

The attached patch allows to use lockPref() in any .js preference file (prefs.js, files in default/preferences...)

Created attachment 326061
patch

Obviously, neither the debian changelog nor the fprintf should be in the patch. The first patch doesn't exist. You haven't seen it. The only patch existing is this one.

Because I apparently inherited ownership of this code by default, I'm going to need some more summary here:

* what is the motivation for this change?
* where did lockPref use to work? In what new cases are we making it work?
* I'm going to need tests. I think writing an xpcshell test for this shouldn't be too hard.

Comment on attachment 326061
patch

canceling review request until questions are answered

(In reply to comment #2)
> Because I apparently inherited ownership of this code by default, I'm going to
> need some more summary here:
>
> * what is the motivation for this change?
> * where did lockPref use to work? In what new cases are we making it work?

lockPref is an exclusive feature of the autoconfig stuff right now. Autoconfig uses a single file, located in the application directory, and which name is given by the general.config.filename preference, with pseudo security featured by a general.config.obscure_value preference.

Autoconfig also allows to use javascript constructs, contrary to standard pref files that are limited to pref() and user_pref().

The problem we had in debian is that we need to lock some preferences, to avoid users to harm themselves with bad preferences. app.update.enable, for instance. The first solution was to use the general.config.filename/obscure_value pair to just do that. The problem is that this file being provided by the package, it is also overwritten on any package upgrade, which means admins needing to use autoconfig just can't.

I found it a convenient way to solve this by allowing standard preferences to lock preferences.

I'm pretty sure you can find some other uses to this feature.

> * I'm going to need tests. I think writing an xpcshell test for this shouldn't
> be too hard.

Can you still review the patch without the tests ?

Frankly, to solve the specific case of Debian users trying to enable updates, I think the best course of action is to get rid of the app.update.url pref from the Debian builds: then even if the user were to turn updates on, nothing bad could happen.

I'm trying to find somebody to help walk through the twisted jungle here; this is not likely to make it for FF3.1 because it touches something close to PAC, which is some of our most fragile code.

This has nothing to do with app.update.url, but with admins wanting to lock some prefs to their users without fidling with general.config.filename.

Which may be a worthwhile use case, but I'm pretty sure we don't want extensions to be able to lock preferences (for example), and it's not clear to me that we distinguish between admins and extensions very well.

Actually, AFAIK, an extension can already lock preferences...

Comment on attachment 326061
patch

At least extensions should not be allowed to set locked prefs, so r-. I'm not sure what the right design is.

As I said, extensions can already lock preferences...

Comment on attachment 326061
patch

Re-requesting review due to glandium's overlooked comment.

How can an extension lock preferences?

Created attachment 347810
extension locking a pref

This is one way i was thinking of for an extension to lock a preference. There are, I guess, many others.

*** Bug 467738 has been marked as a duplicate of this bug. ***

Comment on attachment 326061
patch

ok, yes, extensions can do anything. I don't think we want them doing this, and this will be an attractive nuisance.

The point is, extensions already can lock prefs, so why not allow this patch, which doesn't change that, and file another bug to prevent extensions from locking prefs ?

Comment on attachment 326061
patch

Re-requesting review per comment #16

Christoph Erdle (erdle) wrote :
Christoph Erdle (erdle) wrote :

also the following does not work:

var userName = getenv("USER");
pref("mail.server.server2.hostname", "mailserver.mydomain.mytld");
pref("mail.server.server2.name", userName + "@mydomain.mytld");

this seems to stop processing when encountering the "var" line. When commenting out this line, "mail.server.server2.hostname" gets set, but for obvious reasons "mail.server.server2.name" does not get set, as the variable userName is not defined.

Changed in thunderbird (Ubuntu):
importance: Undecided → Wishlist
Jan-Marek Glogowski (jmglogow) wrote :
Changed in thunderbird:
importance: Unknown → Medium
status: Unknown → In Progress

Comment on attachment 326061
patch

I cannot review this without spending boatloads of time understanding the use cases and code, which I don't think is worth my time at this point. Sorry :-(

Just voted for this bug. We are locking certain prefs in the default installation (for example browser and extension updates are done via packages, so we diable eg "app.update.enabled").

Debian is shipping a patch in their packages, Ubuntu isn't (see https://bugs.launchpad.net/ubuntu/+source/thunderbird/+bug/623844).

Created attachment 572068
Allow .js preference files to set locked prefs with lockPref()

Refreshed after the PRBool/bool change.

Now, as bsmedberg won't review it, and dwitte is essentially not around anymore, who can review this?

Created attachment 576542
Allow .js preference files to set locked prefs with lockPref()

Sorry for the noise, just testing "hg bzexport". I'm still interested to know who could review this, though.

This is a fly by night review.

I don't think it should be "lockPref" like autoconfig. With autoconfig, it's a function call.

With the default pref files, you're specifying a preference.

so:

locked_pref("foo", "bar")

makes more sense it that context (the same as user_pref)

Looking through the code, it looks like you've assumed that if a pref is locked, it becomes a default as well, is that correct?

The code looks good to me, but I'm not the right person to do this.

I think this is a good solution to the problem you are seeing and makes it a lot easier for enterprises to lock prefs, so this would be a good thing to have.

*** Bug 788481 has been marked as a duplicate of this bug. ***

(In reply to Benjamin Smedberg [:bsmedberg] (Away 24-25 October) from comment #18)
> Comment on attachment 326061
> patch
>
> I cannot review this without spending boatloads of time understanding the
> use cases and code, which I don't think is worth my time at this point.
> Sorry :-(

In that case, can you suggest someone that could review it?

No, at this point I don't think there is anyone who can own and review this.

seems like there is some interest in this bug over in

 https://bugzilla.mozilla.org/show_bug.cgi?id=984012#c15
 https://bugzilla.mozilla.org/show_bug.cgi?id=984012#c17

to help protect international users from getting phished into changing the pref named security.turn_off_all_security_so_that_viruses_can_take_over_this_computer

and not necessarily understanding the ramifications of that action if they don't understand English.

Changed in thunderbird:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.