284673 - about:config hides capability.policy preferences

Status: RESOLVED FIXED

(Toolkit :: Preferences, defect)

Description

[Thane Ross]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050223 Firefox/1.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050223 Firefox/1.0.1

When using about:config, string values in the capability.policy namespace are
hidden.  New entries created with new > string are saved to prefs.js after
closing, however the string remains hidden.

Reproducible: Always

Steps to Reproduce:
1. Type about:config in the URL bar
2. Right click and select new > string
3. Type capability.policy and click OK
4. Enter any value
Actual Results:  
The new string does not show on the list.

Expected Results:  
The string should be in the list in alphabetical order.

Comment 1

[OstGote!]

Same for Seamonkey (MAS). I did not find a existing bug report or a comment
which explains this behavior.

Perhaps the owner of Core/Security: CAPS could answer the question. I guess for
security reasons why don't show it and let edit it there.

http://www.mozilla.org/projects/security/components/ConfigPolicy.html

Comment 2

[Pappy Khaki]

Confirming with Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.5) Gecko/20041217:
none of the "capability.*" branch is visible in "about:config", nor does
entering "capability" or "policy" in "Filter:" reveal any of the branch
(including the several set in user.js).

Note that these prefs evidently override the "dom.disable*" UI prefs.  I was
scratching my head for longer than I care to admit about why several sites were
being hobbled by "Permission denied to set property HTMLImageElement.src", even
though "dom.disable_image_src_set" was defaulted to "false", until I finally
remembered that I had long ago set
"capability.policy.default.HTMLImageElement.src" to "NoAccess" in user.js.
I'd've found this much faster if the "capability.*" branch were visible in
"about:config".

I interpolate, uncertainly, from the comments in <span class="bz_closed"><a
href="show_bug.cgi?id=207840" title="VERIFIED FIXED - about:config enhancement -
filtering/searching preferences">bug 207840</a></span> and <span
class="bz_closed"><a href="show_bug.cgi?id=107418" title="VERIFIED FIXED -
about:config todo list">bug 107418</a></span> that this hiding of the
"capability.*" branch is intentional.  If so, perhaps this should be rethought?

Comment 3

[Darin Fisher]

not a firefox issue.  about:config is part of the toolkit.

Comment 4

[Darin Fisher]

cc'ing neil since he seems to be the owner for about:config (at least according
to folks on irc), and since seamonkey is on a migration patch to the new toolkit :-)

Comment 5

[[e-mail]]

CCing some people who might know whether we want to expose caps prefs.

Comment 6

[timeless]

personally i want to, but caps/prefs prevent this, so simply hacking the
about:config bits didn't work.

i believe there are reasons not to

Comment 7

[M.J.G.]

(In reply to comment #6)
> personally i want to, but caps/prefs prevent this, so simply hacking the
> about:config bits didn't work.
> 
> i believe there are reasons not to

According to bug 207840 this is intentional: config.js specifically filters out all prefs starting with capability. So, severity for this bug should be enhancement, and resolution probably WONTFIX unless someone changes their mind.

Michael

Comment 8

[Justin Wood (:Callek)]

From Irc:

* Callek	wonders if darin or bz care to make any "call" on the suggestion in Bug 284673; or at least poke the "right people" to make the call
<Callek>	fwiw I'd like to have that bug "fixed"; not "wontfixed" :-)
<bz>	uh
<bz>	call on what?
<bz>	which suggestion?
<Callek>	bz: the exposing capability.* prefs in about:config
* Callek	isn't sure if you have any say in that security call
<bz>	Does it work?
<Callek>	bz: it doesn't show in about:config at all
<bz>	yes, I know that
<bz>	does trying to show them work?
<bz>	Last I checked, those prefs could only be accessed via a noscript API
<bz>	and THAT should not be changing
<Callek>	bz: the bug itself seems to claim that they are specifically not available even if you muck with the config.js stuff
<bz>	um
<bz>	meaning what?
<bz>	let me repeat
<bz>	about:config is implemented in JS
<Callek>	meaning, probably what you just said (unable to access via noscript api)
<bz>	JS should not have access to the capability.* prefs
<bz>	any more questions?
<Callek>	bz: so, the key here is 'as long as about:config is implemented using js, we will not [in the foreseeable future] be able to show/change them via it'?
<Callek>	or does the "implemented using js" portion of that, not really matter as far as security/design?
<bz>	as a minimum, yes
* Callek	admits to being clueless here.
<bz>	there is a separate question which is whether we want to be allowing easy editing ofthose prefs at all
<bz>	it's scary stuff
<Callek>	I'd be personally happy if it could at least *show* the current values of those prefs (enumerated) there
<bz>	esp. if someone adds stuff about them to one of these "Make Firefox faster" tutorials
<Callek>	[at least the ones that are set in prefs.js]
<bz>	I could accept showing the values, with editing disabled
<Callek>	even if they were "un-editable"
<bz>	In fact, I would support that

Comment 9

[bulk88]

I just ran into this bug, I added the capabilities preferences with about:config, and they disappeared, but were written into the .js. If for security reasons they don't show, they should show as read only. There is no warning anywhere either in about:config, mozillazine/MDC/mozilla wiki, that about:config hides certain settings from the user. It is assumed that about:config will always  reflect the .js by the user. This breaks that assumption and therefore is a bug.

If reading the settings for "read only" is a security problem. Put in a fake, locked/read only setting for "capabilities.*" that says to edit the .js directly.

The infrastructure for a read-only or locked setting seems to exist.

http://mxr.mozilla.org/firefox/source/toolkit/components/viewconfig/content/config.js#69

But I am no Mozilla expert.

I personally find this very inconvenient since I can't switch capability settings on the fly without restarting my browser.

Comment 10

[asmpgmr]

Attachment: comment out code to hide capability prefs

about:config should display all prefs and not hide anything. The CAPS prefs are documented and of course can be changed outside of the browser so there's no reason why they shouldn't be hidden in about:config

Comment 11

[asmpgmr]

Attachment: comment out code to hide capability prefs

Comment 12

[Jesse Ruderman]

asmpgmr, does changing them from about:config actually work?    Comment 8 claims it won't work.

Comment 13

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 342842 [details] [diff] [review]
comment out code to hide capability prefs

Please don't ask for approval on unreviewed patches. There's more information available about the patch submission process at http://developer.mozilla.org/en/Hacking_Mozilla and http://developer.mozilla.org/en/Getting_your_patch_in_the_tree .

If we're going to remove these lines, we need an answer to comment 12, and we would actually remove them rather than commenting them out.

Comment 14

[asmpgmr]

In response to comment #12, yes it does. In fact I have been making this change to config.js in toolkit.jar for myself ever since Mozilla Suite 1.7.x so I can change capability prefs from about:config since I have a JavaScript whitelist setup via CAPS. 

Note: Making the patch line numbers one less will make this work for the 1.8.1 branch (SeaMonkey 1.1.x and Firefox 2.x).

Comment 15

[asmpgmr]

I noticed the line numbers are different in Gecko 1.9.1b2 (SeaMonkey 2.0a2) but the actual lines are the same:

    if (/^capability\./.test(prefName)) // avoid displaying "private" preferences
      return;

    if (/^capability\./.test(prefName)) // avoid displaying "private" preferences
      continue;

I can say with 100% absolute certainty that removing these lines fixes the problem. This will work in Gecko 1.8 and above. To test it do the following:

- close the browser
- edit content\global\config.js in toolkit.jar and remove the above lines and re-add it to toolkit.jar
- open browser and goto about:config
- specify filter capa ; all of the capability prefs should appear
- specify filter java ; all of the javascript prefs should appear including "capability.policy.default.javascript.enabled" which is allAccess by default
- if it is allAccess then set it to noAccess
- load a page which uses JavaScript (may need to reload if cached), it should now be disabled. A good example is http://www.google.com/advanced_search - the box showing what your search parameters will go away without JavaScript enabled
- return to about:config filter java and set "capability.policy.default.javascript.enabled" back to allAccess
- reload the page which uses JavaScript and it should be enabled once again.

Comment 16

[asmpgmr]

Attachment: delete code in config.js which hides capability prefs

Comment 17

[asmpgmr]

Attachment: delete code in config.js which hides capability prefs

change patch format

Comment 18

[asmpgmr]

It has been well over a year, any news on this ? It is a fairly trivial fix - just removing a few lines from config.js to allow all preferences to be displayed.

Comment 19

[Justin Wood (:Callek)]

gavin, review ping for asmpgmr.

Comment 20

[asmpgmr]

Any news on this ? I know it's a minor thing but the fix is trivial (delete a few lines from config.js) and easily verified.

Comment 21

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 344751 [details] [diff] [review]
delete code in config.js which hides capability prefs

The lack of a prompt review response here has been unacceptable - I apologize for that.

I can't really think of any common use case for changing these prefs using the UI, and there are none listed here. That makes me think we should leave things as they are and not take this patch.

Comment 22

[nickers]

> I can't really think of any common use case for changing these prefs using the
> UI, and there are none listed here. That makes me think we should leave things
> as they are and not take this patch.

As it has been already written in the bug report, the preferences _can_ be changed using the about:config UI, but are not _displayed_. The typical user interaction looks like this:

* User sets a capability.policy preference.
* It appears not to work. Users sets more preferences, trying to figure out what is wrong.
* It appears not to work, again. User gives up, unaware (yet) that JavaScript is now partially broken in a weird and mysterious way.

(And even if the user figures where the breakage comes from, there is now no obvious way to reset the preferences to a usable state.)

Comment 23

[motohoss]

(In reply to comment #22)
> * User sets a capability.policy preference.
> * It appears not to work. Users sets more preferences, trying to figure out
> what is wrong.
> * It appears not to work, again. User gives up, unaware (yet) that
> JavaScript is now partially broken in a weird and mysterious way.
> 
> (And even if the user figures where the breakage comes from, there is now no
> obvious way to reset the preferences to a usable state.)

Embarrassedly having done this several times it took over a week for me to figure it out(the third time...). It is documented no argument there.. though difficult to find... (that's another bug report I won't get into). The fix is trivial. Please fix it for those of us (there are many) using this browser who find it difficult enough to use without 'hidden anything' and finding 'doublespeak' to explain it if the answer can be found anyway. Weird and mysterious is very accurate. The 'valid use case' exists.

Comment 24

[:Gavin Sharp [email: [e-mail]]]

AFAIK the plan is to remove these prefs entirely anyways (related to the work in bug 546848).

Comment 25

[asmpgmr]

So are the capability prefs staying or not ? We are up to Gecko 8 and they are still there. If they are going to stay then what reason is there not to accept the patch for display the capability prefs in config.js ?

If the capability prefs are being removed then when will this happen and does using permissions and the permissions.default.script pref provide equivalent functionality to enable/disable javascript on a site-by-site basis ?

Comment 26

[asmpgmr]

Why hasn't this issue been addressed in over 8 years ? What is this reasoning for hiding the capability.* prefs and only those prefs ? Also it seems clear that these preferences are not going away since it would be a significant code change and break things like the NoScript addon which relies on them so why not display them in about:config ?

Also a related question: which is the better method to use to whitelist/blacklist JavaScript access: Configurable Security Policies via capability.policy.* prefs or per-site permissions via permissions.sqlite and the permissions.default.script pref ?

Comment 27

[motohoss]

(In reply to :Gavin Sharp (email [e-mail]) from comment #24)
> AFAIK the plan is to remove these prefs entirely anyways (related to the
> work in bug 546848).

Seriously eight years? This code change is miniscule and would end the frustrations already described. As others have pointed out where are the docs as to best practice for setting these "preferences"? (Comments #25 & #26). Less I digress my comments in #22 would be redundant.

Comment 28

[:Gavin Sharp [email: [e-mail]]]

The capability prefs were removed in http://hg.mozilla.org/mozilla-central/rev/4df87ccf0f5d, so the inability to edit them using about:config won't be an issue in Firefox 29.

We should remove the code that special cases them from about:config. I noticed there was some copied code in Metro and Android as well:

http://mxr.mozilla.org/mozilla-central/search?string=^capability

Comment 29

[motohoss]

(In reply to :Gavin Sharp (email [e-mail]) from comment #28)
> The capability prefs were removed in
> http://hg.mozilla.org/mozilla-central/rev/4df87ccf0f5d, so the inability to
> edit them using about:config won't be an issue in Firefox 29.
> 
> We should remove the code that special cases them from about:config. I
> noticed there was some copied code in Metro and Android as well:
> 
> http://mxr.mozilla.org/mozilla-central/search?string=^capability

I am just an end user, I don't have any business complaining. The bug will be fixed. The frustration will still exist. There is still no easily found documentation on the 'best-practice' or even a 'how-to' for the javascript access stuff... 8 years... my how time flies. Thanks for your time and attention.

Comment 30

[asmpgmr]

(In reply to :Gavin Sharp (email [e-mail]) from comment #28)
> The capability prefs were removed in
> http://hg.mozilla.org/mozilla-central/rev/4df87ccf0f5d, so the inability to
> edit them using about:config won't be an issue in Firefox 29.
> 
> We should remove the code that special cases them from about:config. I
> noticed there was some copied code in Metro and Android as well:
> 
> http://mxr.mozilla.org/mozilla-central/search?string=^capability

I take it the permissions system should be used to whitelist/blacklist stuff on a site-by-site basis going forward and that this change will also affect SeaMonkey 2.26, correct ?

Comment 31

[[e-mail]]

Attachment: Proposed patch

Comment 32

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 8357987 [details] [diff] [review]
Proposed patch

>diff --git a/toolkit/components/viewconfig/content/config.js b/toolkit/components/viewconfig/content/config.js

> function ShowPrefs()

>-  prefArray.forEach(function (prefName) {
>-    fetchPref(prefName, gPrefArray.length);

>+  gPrefBranch.getChildList("").forEach(fetchPref);

This is a bit too magical for me - worth a comment clarifying that fetchPref's second argument is an index, or explicitly using .forEach((pref, index) => fetchPref(pref, index)).

(Or we could fix fetchPref to not take an index, but I haven't taken the time to understand why it's written that way.)

Other changes look fine to me too, but let's CC some product owners just as a heads up.

Comment 33

[:Gavin Sharp [email: [e-mail]]]

mfinkle, mbrubeck: android and metro forks being touched here, yell if you have any objections :)

Comment 34

[[e-mail]]

(In reply to Gavin Sharp from comment #32)
> (From update of attachment 8357987 [details] [diff] [review])
> >+  gPrefBranch.getChildList("").forEach(fetchPref);
> This is a bit too magical for me - worth a comment clarifying that
> fetchPref's second argument is an index
Fair enough.

> (Or we could fix fetchPref to not take an index, but I haven't taken the
> time to understand why it's written that way.)
Originally to save the callers from triplicating code to add to the array, although as it turns out bug 619356 might make a rewrite feasible.

Comment 35

[[e-mail]]

Attachment: Possible patch

This is the version with fetchPref rewritten to return the pref instead of just updating the array, and in turn the pref listener works out whether to remove the old entry, fetches the pref, and then inserts or replaces the new entry.

Comment 36

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to :Gavin Sharp (email [e-mail]) from comment #33)
> mfinkle, mbrubeck: android and metro forks being touched here, yell if you
> have any objections :)

No objections to the Android change

Comment 37

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 8359000 [details] [diff] [review]
Possible patch

(don't really have time to review this, probably should be new-bug with another toolkit reviewer flagged)

Comment 38

[[e-mail]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bf87bee88be9

Comment 39

[Carsten Book [:Tomcat]]

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