Last Comment Bug 880269 - Consider to remove the isBuiltinToken() check
: Consider to remove the isBuiltinToken() check
Status: NEW
:
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: Trunk
: All All
: -- normal with 3 votes (vote)
: ---
Assigned To: Kai Engert (:kaie)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-06 08:20 PDT by Kai Engert (:kaie)
Modified: 2015-04-21 06:49 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v1 (22.60 KB, patch)
2013-06-25 11:15 PDT, Kai Engert (:kaie)
no flags Details | Diff | Review

Description Kai Engert (:kaie) 2013-06-06 08:20:30 PDT
mozilla/toolkit/modules/CertUtils.jsm contains:

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

This check has been originally added in bug 340198. It got moved around several times, and has last been touched in bug 401292.

It should be triaged if that check is still desired/useful.

The current motivation for filing this bug: Fedora 19 will store the the systemwide root CA list in a token with a different name. We'll have to patch Firefox to allow the name of the root module being used in Fedora.

The current check requires that software (e.g. addons) are installed from a site using a TLS server certificate from a commercial CA.

On the other hand, users who decide that they trust cacert.org and install that CA will still be blocked from installing sites using a cert from cacert.org

A server certificate can be obtained using simple domain validation, without identity, without corporate checks. Mozilla shouldn't assume that's higher security than a server certificate issued by a CA that the user/administrator has deliberately added and marked trusted to the software token.

I propose to remove the check for the root CA being stored on a token named "builtin object token", and rely on the use of a verified server certificate.

I assume this isn't used as a protection for Mozilla software updates, but hope Mozilla already requires a match with a pinned signing key etc.

If you want to raise the bar for installing addons, you could use a different approach, e.g. requiring that a non-revoked server certificate is being used (require OCSP check), or even require that providers of addons start actually signing addons with a code signing cert, which would give real confidence about the origin of the software.

In any way, let's remove this existing check.
Comment 1 Stef Walter 2013-06-06 08:26:38 PDT
FWIW, as I noted elsewhere, if this is being done for security reasons, then this is a broken check. PKCS#11 token labels come from the tokens themselves, and therefore it would be simple to have that exact label on a completely different token.
Comment 2 Kai Engert (:kaie) 2013-06-06 12:42:41 PDT
I you don't like the idea to remove the check completely, but if you'd rather like to keep the check, despite above arguments, here is a replacement approach, which would work with the original and the replacement root modules:

- use C API PK11_HasRootCerts()
- expose that API to a JS function
- change isBuiltinToken() to query that C function, not the token name

(Suggestion from Bob Relyea.)
Comment 3 Robert Strong [:rstrong] (use needinfo to contact me) 2013-06-06 13:05:15 PDT
I would prefer removing the check as well as some of the other checks but let's ask dveditz.
Comment 4 Stef Walter 2013-06-25 07:58:00 PDT
Work around for p11-kit available. The work around uses "Builtin Object token" as a label for the token that contains installed certificates (rather than configured ones).

See: https://bugs.freedesktop.org/show_bug.cgi?id=66161
Comment 5 Daniel Veditz [:dveditz] 2013-06-25 09:03:56 PDT
The attack this was initially protecting against is no longer possible as far as I know. It went as follows (bug 340198)
 * user visits site with self-signed cert
 * it's "random blog" so the user doesn't care about secure transport
 * user "clicks through"
 * this installs random blog's invalid cert as "trusted"
   (for the session only iirc)
 * one of the SAN in random blog was AMO
 * now the evil owner of random blog can send
   spoofed updates because we trust that cert

At some point (Firefox 3.0) we stopped adding "trusted" override certs and instead create "exceptions" that tie a specific cert to a specific hostname. This is all the user thought they were doing in the first place and protects them from adding unexpected hidden trust.

The certCheck() routine that looks at this can also do a weak form of pinning. We use that for updates to protect against a Comodogate/Diginotar situation where an attacker gets a fraudulent cert. We pin by issuer name (currently) which helps when the attacker can only get end-entity certs like Comodogate/Diginotar, but can be easily worked around if the attacker has obtained an issuing/intermediate certificate. The replacement feature would be true CA pinning in the browser: once we have that and do a real pin for AMO then we don't need any of this hacky substitute.
Comment 6 Kai Engert (:kaie) 2013-06-25 11:10:32 PDT
Dan, thanks a lot for looking into this proposal and your comments.

I understand you'd agree to a patch that removes the built-in check, but we must keep the other checks, such as checking for cert attributes.

I'll attempt to make that patch - which will require updating the callers and tests.
Comment 7 Kai Engert (:kaie) 2013-06-25 11:15:54 PDT
Created attachment 767339 [details] [diff] [review]
patch v1

initial attempt. submitted as a try build, with all tests on one platform.

https://tbpl.mozilla.org/?tree=Try&rev=08d2b3ae60af
Comment 8 nearwood 2014-02-07 12:45:55 PST
(In reply to Kai Engert (:kaie) from comment #0)
> The current check requires that software (e.g. addons) are installed from a
> site using a TLS server certificate from a commercial CA.
> 
> On the other hand, users who decide that they trust cacert.org and install
> that CA will still be blocked from installing sites using a cert from
> cacert.org
> 
> A server certificate can be obtained using simple domain validation, without
> identity, without corporate checks. Mozilla shouldn't assume that's higher
> security than a server certificate issued by a CA that the
> user/administrator has deliberately added and marked trusted to the software
> token.
> 
> I propose to remove the check for the root CA being stored on a token named
> "builtin object token", and rely on the use of a verified server certificate.
> 
> I assume this isn't used as a protection for Mozilla software updates, but
> hope Mozilla already requires a match with a pinned signing key etc.
> 
> If you want to raise the bar for installing addons, you could use a
> different approach, e.g. requiring that a non-revoked server certificate is
> being used (require OCSP check), or even require that providers of addons
> start actually signing addons with a code signing cert, which would give
> real confidence about the origin of the software.
> 
> In any way, let's remove this existing check.

I agree. We are using non-root, private CA issued certs and need to be able to install add-ons.
Comment 9 David Keeler [:keeler] (use needinfo?) 2014-02-07 13:55:06 PST
Looks like you can set the following prefs to false to install/update addons from non-builtin roots:
"extensions.install.requireBuiltInCerts"
"extensions.update.requireBuiltInCerts"

But I agree, this does seem unnecessary now that each certificate override is tied to a specific domain name.
Comment 10 Robert Strong [:rstrong] (use needinfo to contact me) 2014-02-07 13:57:41 PST
App update is removing the checks in other bugs so moving to add-ons manager.
Comment 11 nearwood 2014-11-05 08:48:51 PST
Is this dead? The check is obsolete and prevents any intranets with private CAs from easily getting Addons.

(In reply to Kai Engert (on vacation) (:kaie) from comment #0)
> Mozilla shouldn't assume that's higher
> security than a server certificate issued by a CA that the
> user/administrator has deliberately added and marked trusted to the software
> token.
Comment 12 Kai Engert (:kaie) 2014-12-04 10:50:10 PST
I haven't tested this in a while.

If you've removed the check, then this bug is obsolete.

If Mozilla still requires to set a preference for installing an addon from a site that uses a cert from a CA that isn't stored in a token with the hardcoded token name, ...

... then we'd have to update the patch used in Fedora Linux to set the preference.
Comment 13 Denis Kasak 2015-04-21 06:49:25 PDT
Ping! What's the status on this?

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