Bug 966424 - Extension installation in Firefox from anywhere but mozilla.org fails
Summary: Extension installation in Firefox from anywhere but mozilla.org fails
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Fedora
Classification: Fedora
Component: xulrunner
Version: 27
Hardware: All
OS: All
high
high
Target Milestone: ---
Assignee: Jan Horak
QA Contact: Fedora Extras Quality Assurance
Jan Horak
URL:
Whiteboard:
Depends On:
Blocks: 466626
TreeView+ depends on / blocked
 
Reported: 2013-05-23 09:14 UTC by Adam Williamson
Modified: 2018-11-27 16:30 UTC (History)
10 users (show)

Fixed In Version: thunderbird-17.0.6-2.fc19
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-11-27 16:30:40 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
patch v1 (936 bytes, patch)
2013-06-04 00:30 UTC, Kai Engert (:kaie) (inactive account)
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
FreeDesktop.org 66161 0 None None None Never

Description Adam Williamson 2013-05-23 09:14:24 UTC
To reproduce: just try to install an extension from any third-party site, e.g. http://adblockplus.org/ . Firefox will pop up the usual bar about "denied extension installation, click here to allow" or whatever. But if you click it, you get "The add-on could not be downloaded because of a connection failure on adblockplus.org".

I think this broke around FF 19 or 20. It used to work.

Installing extensions from https://addons.mozilla.org/en-US/firefox/ still works, but some popular extensions are not available there, notably HTTPS Everywhere.

Comment 1 Jan Horak 2013-05-23 12:52:59 UTC
Hm, works for me. Please check Tools/Web Developer/Error Console for error message and check xpinstall.enabled in about:config.

You may also check this hints:
http://support.mozilla.org/en-US/kb/unable-install-add-ons-or-extensions

If it doesn't help first try -safe-mode and then new profile (ie. run by firefox -ProfileManager).

Comment 2 Adam Williamson 2013-05-23 12:59:34 UTC
I was testing with a brand new, dead stock F19 Beta RC4 install (I was doing https://fedoraproject.org/wiki/QA:Testcase_desktop_browser ). It's hit me every time I've tried that test case on an F19 build lately. I can investigate, but it'll have to be later, gotta finish validation at present.

Comment 3 Jan Horak 2013-05-23 13:20:52 UTC
Sorry, I've missed that you're using F19. I'll look into it.

Comment 4 Jan Horak 2013-05-30 11:10:17 UTC
This is affecting Fedora 19 only. I'm starting to slightly blame nss/nspr. Firefox share same srpm for all Fedoras. Upstream binaries are working on Fedora 19 (Mozilla bundle own nss/nspr).

Reproduction steps:
1. set location to: https://addons.mozilla.org/firefox/downloads/latest/1865/addon-1865-latest.xpi?src=search

Error console:
Warning: WARN addons.xpi: Download failed: [Exception... "Certificate issuer is not built-in."  nsresult: "0x80004004 (NS_ERROR_ABORT)"  location: "JS frame :: resource://gre/modules/CertUtils.jsm :: checkCert :: line 171"  data: no]
Source File: resource://gre/modules/CertUtils.jsm
Line: 171

There is addons.mozilla.org in Preferences/Advanced/Encryption/View Certificates/Servers/USERTHRUST Network.

Note: everything is fine when installing addon from: https://addons.mozilla.org/en-US/firefox/addon/adblock-plus/?src=search page.

Comment 5 Jan Horak 2013-06-03 08:18:38 UTC
I've checked nss-gui tool to compare F18 and F19 builtin root modules. 
There is missing Builtin Object Token (Manufacturer: Mozilla Foundation) in Fedora 19.

I got list of modules from nss-gui by Manage Secirity Devices (PKCS#11)/Buildin Roots Module.

Reassigning to nss, feel free to reassign to correct component, I'm not very good at nss issues.

Comment 6 Kai Engert (:kaie) (inactive account) 2013-06-03 18:03:49 UTC
(In reply to Adam Williamson from comment #0)
> you get "The add-on could not be downloaded because of a
> connection failure on adblockplus.org".

Thanks for your report, I'm able to reproduce and will investigate.


(In reply to Jan Horak from comment #5)
> I've checked nss-gui tool to compare F18 and F19 builtin root modules. 
> There is missing Builtin Object Token (Manufacturer: Mozilla Foundation) in
> Fedora 19.

This shouldn't be the problem. We have changed how NSS gets its list of root CA certificates in F19.
https://fedoraproject.org/wiki/Features/SharedSystemCertificates

The mentioned module is replaced by the other modules you see listed.

Comment 7 Kai Engert (:kaie) (inactive account) 2013-06-03 18:23:26 UTC
CC'ing FYI, only.

I identified the cause, which is inside the Mozilla application code.

File mozilla/toolkit/modules/CertUtils.jsm contains:

  function isBuiltinToken(tokenName) {
    return tokenName == "Builtin Object Token";
  }

The Mozilla code apparently wants to allow installation of an addon, if it's contained in the static set of root CA certificates as shipped with NSS. It doesn't allow installation, even if the user has decided to trust additional Certificate Authorities.

(It's debatable whether this is a reasonable policy in the first place, because that check ignores a user's or administrator's decision. But that's not the topic of this bug.)


In a default Fedora 19 install, we no longer use the NSS PKCS#11 module named "Builtin Object Token", but we rather use the new pkcs#11 modules provided by the p11-kit-trust.rpm package.

Because our new default pkcs#11 module use different names, the Mozilla application level check fails, and rejects installation.


The fix is to use a patch to the Mozilla toolkit code - and allow the additional module names.


Note that it's possible to configure a Fedora system to alternatively use the classic module - that's why our patch should allow both new and old module names.


I'll attach a patch - we should add it to all packages that build mozilla/toolkit.

Comment 8 Kai Engert (:kaie) (inactive account) 2013-06-04 00:30:30 UTC
Created attachment 756591 [details]
patch v1

Jan, I propose this Fedora specific patch for the xulrunner package.

I made a scratch build using the patch at
  http://koji.fedoraproject.org/koji/taskinfo?taskID=5463328
and it fixes the issue for me.

Comment 9 Jan Horak 2013-06-04 06:35:50 UTC
Thanks for patch and clarification.

Can we push this also to upstream or they are not interested in this fix?

Comment 10 Kai Engert (:kaie) (inactive account) 2013-06-04 14:19:13 UTC
> Can we push this also to upstream or they are not interested in this fix?

Unlikely, but I'll discuss it.

Comment 11 Stef Walter 2013-06-05 14:46:14 UTC
(In reply to Kai Engert (:kaie) from comment #7)
> CC'ing FYI, only.
> 
> I identified the cause, which is inside the Mozilla application code.
> 
> File mozilla/toolkit/modules/CertUtils.jsm contains:
> 
>   function isBuiltinToken(tokenName) {
>     return tokenName == "Builtin Object Token";
>   }

This is probably obvious, but if this code is meant to be some sort of security check, it's completely broken. 

The PKCS#11 token label is provided by the module itself. It's trivial for a PKCS#11 token to provide whatever tokenName here.

Comment 12 Kai Engert (:kaie) (inactive account) 2013-06-06 15:22:32 UTC
(In reply to Jan Horak from comment #9)
> Can we push this also to upstream or they are not interested in this fix?

I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=880269
for the upstream discussion.

However, I don't expect a quick decision or action. We should take the patch in the meantime.


(In reply to Stef Walter from comment #11)
> 
> The PKCS#11 token label is provided by the module itself. It's trivial for a
> PKCS#11 token to provide whatever tokenName here.

That's true, however, getting a pkcs#11 token installed into a user's system is a hurdle, too, and e.g. can't be done from inside the web sandbox.

Comment 13 Stef Walter 2013-06-06 15:36:20 UTC
(In reply to Kai Engert (:kaie) from comment #12)
> (In reply to Jan Horak from comment #9)
> > Can we push this also to upstream or they are not interested in this fix?
> 
> I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=880269
> for the upstream discussion.
> 
> However, I don't expect a quick decision or action. We should take the patch
> in the meantime.
> 
> 
> (In reply to Stef Walter from comment #11)
> > 
> > The PKCS#11 token label is provided by the module itself. It's trivial for a
> > PKCS#11 token to provide whatever tokenName here.
> 
> That's true, however, getting a pkcs#11 token installed into a user's system
> is a hurdle, too, and e.g. can't be done from inside the web sandbox.

Indeed. So I guess the check should be whether the token is not write protected. If you want to exclude tokens to which the browser can write to. There's a token flag for this: CKF_WRITE_PROTECTED

Comment 14 Kai Engert (:kaie) (inactive account) 2013-06-06 15:43:02 UTC
I don't see how it matters whether a token is write protected or not.

I didn't talk about installing cert into a token.

It's about getting a new token added in the first place.

If a system has only a fixed set of tokens, and controls the token configuration (that's the way it is in upstream Firefox), the installing the token is the hurdle.

Therefore I don't see how "token can use any name" can be used as a supporting argument to convince upstream to remove the check.

Comment 15 Stef Walter 2013-06-06 15:48:36 UTC
Breaking it down...

So in the default firefox configuration there are three tokens:

 * Builtin Object Token (write protected)
 * Generic Crypto Services (write protected)
 * Software Security Device (writable)

The code has very confusing intent, but it seems that the intent is to prevent CA anchors which are written to 'Software Security Device' from being used to install extensions.

CA anchors written to 'Software Security Device' could have been installed by the user during a browser session (following instructions printed on a web page) and are therefore subject to phishing.

Therefore a check to see whether the token is write protected prevents these CA anchors (which may have been installed via phishing) from being used to install extensions.

Comment 16 Kai Engert (:kaie) (inactive account) 2013-06-06 16:07:19 UTC
Thanks for explaining it in a way I was able to understand :)

You're saying: If upstream isn't willing to remove the existing check altogether, then you propose a different approach. Instead, you propose to check for the non-writable status of the token, which would work around your worry that token names can be faked.

That might be helpful, however, some systems could have read-only hardware token installed, and that token could contain some specific CA cert. As a consequence, that CA cert would be trusted for the specific situation, too, and that might be an undesired side effect.

Therefore I'm not yet convinced the non-writeable is a good alternative strategy. I'd rather see the check dropped altogether. In my opinion, any installed trust CA should be allowed, not just those from certain tokens.

Let's see what upstream decides, and I propose we continue to use the attached patch in the meantime.

(If you are proposing a different check, you'd have to implement it in Mozilla JS...)

Comment 17 Stef Walter 2013-06-06 16:16:28 UTC
(In reply to Kai Engert (:kaie) from comment #16)
> Thanks for explaining it in a way I was able to understand :)
> 
> You're saying: If upstream isn't willing to remove the existing check
> altogether, then you propose a different approach. Instead, you propose to
> check for the non-writable status of the token, which would work around your
> worry that token names can be faked.
> 
> That might be helpful, however, some systems could have read-only hardware
> token installed, and that token could contain some specific CA cert. As a
> consequence, that CA cert would be trusted for the specific situation, too,
> and that might be an undesired side effect.

Yes, I guess it depends on what the intent of this check is. 

> Therefore I'm not yet convinced the non-writeable is a good alternative
> strategy. I'd rather see the check dropped altogether. In my opinion, any
> installed trust CA should be allowed, not just those from certain tokens.

Agreed. In any case pinning should probably be used for *.mozilla.org.

> Let's see what upstream decides, and I propose we continue to use the
> attached patch in the meantime.

Fair enough. It's a shame that all downstreams will have to patch this.

> (If you are proposing a different check, you'd have to implement it in
> Mozilla JS...)

I think that's possible.

Comment 18 Kai Engert (:kaie) (inactive account) 2013-06-06 16:34:06 UTC
(In reply to Stef Walter from comment #17)
> 
> > Therefore I'm not yet convinced the non-writeable is a good alternative
> > strategy. I'd rather see the check dropped altogether. In my opinion, any
> > installed trust CA should be allowed, not just those from certain tokens.
> 
> Agreed. In any case pinning should probably be used for *.mozilla.org.

They don't even restrict it to the mozilla domain. They allow installation from anywhere, if it has a cert from any builtin CA :)

Comment 19 Bob Relyea 2013-06-06 17:23:27 UTC
> The Mozilla code apparently wants to allow installation of an addon, if it's
> contained in the static set of root CA certificates as shipped with NSS.

1) Sigh, that policy is lame.
2) The name is not the correct way to identify the builtins module. NSS provides and object which 'only' the builtins object should provide. The code in question should ask PSM to ask NSS if it's dealing with a builtin's object. (The relavent call is PK11_HasRootCerts(slot))

I think your patch is fine for fedora for now. We should get upstream mozilla to actually look at the using PK11_HasRootCerts(slot) to identify the builtins module.

bob

Comment 20 Stef Walter 2013-06-06 17:34:59 UTC
(In reply to Bob Relyea from comment #19)
> > The Mozilla code apparently wants to allow installation of an addon, if it's
> > contained in the static set of root CA certificates as shipped with NSS.
> 
> 1) Sigh, that policy is lame.
> 2) The name is not the correct way to identify the builtins module. NSS
> provides and object which 'only' the builtins object should provide. The
> code in question should ask PSM to ask NSS if it's dealing with a builtin's
> object. (The relavent call is PK11_HasRootCerts(slot))
> 
> I think your patch is fine for fedora for now. We should get upstream
> mozilla to actually look at the using PK11_HasRootCerts(slot) to identify
> the builtins module.

Confirming that this would also work with the p11-kit trust module, as it provides that same builtins object in order to provide a drop in replacement for libnssckbi.so.

Comment 21 Kai Engert (:kaie) (inactive account) 2013-06-11 19:22:46 UTC
Reassigning to the xulrunner package, because that's the package that needs patching.

A separate bug should be filed to patch other applications (e.g. Thunderbird) in the same way (those that don't use xulrunner).

The attached patch is the only solution that we have "in hand".

(The alternative solution discussed in comment 19 and 20 would require patching xulrunner, too. It would be a much more complicated patch, and we don't have that patch.)

Comment 22 Kai Engert (:kaie) (inactive account) 2013-06-11 19:23:44 UTC
Would you be able to get this built and included for Fedora 19 final?
The final change deadline is approaching.
Thanks for your help.

Comment 23 Kai Engert (:kaie) (inactive account) 2013-06-11 19:26:46 UTC
I've filed bug 973371 for Thunderbird.

Comment 24 Jan Horak 2013-06-12 09:00:02 UTC
Sure, no problem.

Comment 25 Fedora Update System 2013-06-13 12:27:02 UTC
thunderbird-17.0.6-2.fc19,xulrunner-21.0-8.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/thunderbird-17.0.6-2.fc19,xulrunner-21.0-8.fc19

Comment 26 Fedora Update System 2013-06-13 18:03:58 UTC
Package thunderbird-17.0.6-2.fc19, xulrunner-21.0-8.fc19:
* should fix your issue,
* was pushed to the Fedora 19 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing thunderbird-17.0.6-2.fc19 xulrunner-21.0-8.fc19'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2013-10755/thunderbird-17.0.6-2.fc19,xulrunner-21.0-8.fc19
then log in and leave karma (feedback).

Comment 27 Fedora Update System 2013-06-16 06:08:57 UTC
thunderbird-17.0.6-2.fc19, xulrunner-21.0-8.fc19 has been pushed to the Fedora 19 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 28 Martin Stransky 2017-08-02 08:57:25 UTC
Looks like we can't apply this patch to Firefox 55 due to changes at https://bugzilla.mozilla.org/show_bug.cgi?id=1319252 - Kai can you help here please? Thanks.

Comment 29 Kai Engert (:kaie) (inactive account) 2017-08-15 10:51:23 UTC
We don't need this patch any more.

Mozilla changed how they distinguish CAs that come with Firefox and CAs that are user installed.

The new approach that they are using works with both Firefox from Mozilla and Fedora.

Comment 30 Jan Kurik 2018-05-31 09:07:30 UTC
This bug is currently reported against a Fedora version which is already unsuported.
I am changing the version to '27', the latest supported release.

Please check whether this bug is still an issue on the '27' release.
If you find this bug not being applicable on this release, please close it.

Comment 31 Ben Cotton 2018-11-27 13:52:07 UTC
This message is a reminder that Fedora 27 is nearing its end of life.
On 2018-Nov-30  Fedora will stop maintaining and issuing updates for
Fedora 27. It is Fedora's policy to close all bug reports from releases
that are no longer maintained. At that time this bug will be closed as
EOL if it remains open with a Fedora  'version' of '27'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 27 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Comment 32 Adam Williamson 2018-11-27 16:30:40 UTC
Per #c29 it seems the bug is not still an issue.


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