Bug 966424

Summary: Extension installation in Firefox from anywhere but mozilla.org fails
Product: [Fedora] Fedora Reporter: Adam Williamson <awilliam>
Component: xulrunnerAssignee: Jan Horak <jhorak>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: high Docs Contact: Jan Horak <jhorak>
Priority: high    
Version: 19CC: caillon+fedoraproject, emaldona, eparis, gecko-bugs-nobody, jhorak, john.j5live, kdudka, kengert, rrelyea, stefw, stransky, walters
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: All   
Whiteboard:
Fixed In Version: thunderbird-17.0.6-2.fc19 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-06-16 02:08:57 EDT Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 466626    
Attachments:
Description Flags
patch v1 none

Description Adam Williamson 2013-05-23 05:14:24 EDT
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 08:52:59 EDT
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 08:59:34 EDT
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 09:20:52 EDT
Sorry, I've missed that you're using F19. I'll look into it.
Comment 4 Jan Horak 2013-05-30 07:10:17 EDT
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 04:18:38 EDT
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) 2013-06-03 14:03:49 EDT
(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) 2013-06-03 14:23:26 EDT
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) 2013-06-03 20:30:30 EDT
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 02:35:50 EDT
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) 2013-06-04 10:19:13 EDT
> 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 10:46:14 EDT
(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) 2013-06-06 11:22:32 EDT
(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 11:36:20 EDT
(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) 2013-06-06 11:43:02 EDT
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 11:48:36 EDT
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) 2013-06-06 12:07:19 EDT
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 12:16:28 EDT
(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) 2013-06-06 12:34:06 EDT
(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 13:23:27 EDT
> 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 13:34:59 EDT
(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) 2013-06-11 15:22:46 EDT
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) 2013-06-11 15:23:44 EDT
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) 2013-06-11 15:26:46 EDT
I've filed bug 973371 for Thunderbird.
Comment 24 Jan Horak 2013-06-12 05:00:02 EDT
Sure, no problem.
Comment 25 Fedora Update System 2013-06-13 08:27:02 EDT
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 14:03:58 EDT
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 02:08:57 EDT
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.