880269 - Consider to remove the isBuiltinToken() check

Status: RESOLVED WORKSFORME

(Toolkit :: Add-ons Manager, defect)

Description

[Kai Engert [:KaiE:]]

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]

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:]]

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 (they/them - no direct email)]

I would prefer removing the check as well as some of the other checks but let's ask dveditz.

Comment 4

[Stef Walter]

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]]

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:]]

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:]]

Attachment: 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

[Nick]

(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

[Dana Keeler (she/her) (use needinfo) [:keeler]]

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 (they/them - no direct email)]

App update is removing the checks in other bugs so moving to add-ons manager.

Comment 11

[Nick]

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:]]

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]

Ping! What's the status on this?

Comment 14

[Kai Engert [:KaiE:]]

This should have been fixed by bug 1324096.
We changed how Mozilla decides if a CA is a builtin root or an external root, independent of the token name, by looking at token attributes.

Comment 15

[Jim Mathies [:jimm]]

per comment 14