1034124 - mozilla::pkix: the error encountered when a CA certificate is used as an end-entity is not overridable

Status: VERIFIED FIXED

(Core :: Security: PSM, defect)

Description

[suivios]

Attachment: self-signed certificate 127.0.0.1.crt

User Agent: Mozilla/5.0 (compatible; MSIE 10.0; Windows NT 6.1; WOW64; Trident/6.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; InfoPath.2; .NET4.0C)

Steps to reproduce:

We used a self-signed certificate to established secure connexion with a local server application: https://127.0.0.1:1943.

This certificate was installed with the Authority certificates.


Actual results:

When we update Firefox to the version 31 BETA, this self-signed certificate is  now considered as "invalid". We have an error sec_error_ca_cert_invalid




Expected results:

We expect to have the same behaviour as previous Firefox version. 
The self-signed certificate should be used to establish secure connexion with our local server application.

Comment 1

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

Looks like that certificate has a basicConstraints extension with the value cA: TRUE. We stopped allowing CA certificates to act as end-entity certificates. That certificate should be regenerated without the basicConstraints extension.

Comment 3

[cleatusmcfarlan]

(In reply to David Keeler (:keeler) [use needinfo?] from comment #1)
> Looks like that certificate has a basicConstraints extension with the value
> cA: TRUE. We stopped allowing CA certificates to act as end-entity
> certificates. That certificate should be regenerated without the
> basicConstraints extension.

is this also the likely reason that ocsp stapling just stopped working(at least on the site i'm working on) with the upgrade to ff31 but works just fine with downgrade to ff30? or should i submit a separate bug? thanks.

Comment 4

[chesnok]

(In reply to David Keeler (:keeler) [use needinfo?] from comment #1)
> We stopped allowing CA certificates to act as end-entity
> certificates. That certificate should be regenerated without the
> basicConstraints extension.

Are you joking? In case you didn't know, the vast majority of web interfaces for various applications, from WiFi controllers to toasters use self signed ssl certs, and most don't even have a way to change them!
I can understand hiding the big "Skip warning and shut up" button for the end-user security benefit, but power users, developers and systems administrators are pretty much boned by this change. Now I have to either browse development mailing lists for about:config options or screw around with my firefox profile, just to administer my goddamn internal network controller! In fact, it is easier to just use a different browser, which I'm sure was your original goal with this brain damaged decision.

Comment 5

[Matthew Musgrove]

I agree with [e-mail] and [e-mail]. This is a bug. 

The thousands of entities using self-signed certs should not have to regenerate their certs.

It's definitely time to move on to a different browser. Chrome is nice and it does not have this issue.

Comment 6

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

Updating summary and dependency for better tracking.
Comment 4 hidden for profanity.

(In reply to cleatusmcfarlan from comment #3)
> is this also the likely reason that ocsp stapling just stopped working(at
> least on the site i'm working on) with the upgrade to ff31 but works just
> fine with downgrade to ff30? or should i submit a separate bug? thanks.

I don't think that would be the case - please file a new bug.

Comment 7

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

I think it is probably a good idea to treat this problem as overridable in Firefox, at least for self-signed certificates.

When we did compatibility testing for the new certificate verification code, we did not see (m)any instances of this problem. That may be because the problem is very rare, and/or it may be because there are not "enough" self-signed certificates in the set of certificates tested. Consequently, we didn't (and don't) know the compatibility impact of the change for self-signed certificates.

Note that this change was made in response to the TURKTrust incident (see bug 825022). With this change, a CA cannot "accidentally" issue a sub-CA certificate that the customer is unaware of. Additionally, Certificate Transparency and possibly changes to revocation checking mean makes it important to know whether a certificate is an end-entity or not. So, I don't think we should completely remove the restriction. But, these factors aren't really relevant for self-signed certificates.

(I unhid comment #4 after discussion with David. Let's all try harder to be polite and avoid profanity in Bugzilla.)

Comment 8

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

Attachment: patch

It actually already is the case that self-signed certificates with CA:True are overridable (see the test for self-signed-end-entity-with-cA-true.example.com in test_cert_overrides.js - it results in "SEC_ERROR_UNKNOWN_ISSUER"). This patch changes PSM so it allows overrides where a certificate has a trusted issuer but looks like a CA when it should be an end-entity.

Comment 9

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 8475423 [details] [diff] [review]
patch

Review of attachment 8475423 [details] [diff] [review]:
-----------------------------------------------------------------

I'm guessing that the error text is non-localizable currently. That is something that should also be fixed.

::: dom/browser-element/BrowserElementChildPreload.js
@@ +99,4 @@
>        return Ci.nsINSSErrorsService.ERROR_CLASS_BAD_CERT;
>      default:
>        return Ci.nsINSSErrorsService.ERROR_CLASS_SSL_PROTOCOL;
>    }

Please file a follow-up bug to redo this error classification using the PSM function provided for this (NSSErrorsService::GetErrorClass). It is bad to have this code duplicated, especially when this code lives in DOM.

::: security/manager/boot/src/StaticHPKPins.h
@@ +88,5 @@
>    "WoiWRyIOVNa9ihaBciRSC7XHjliYS9VwUGOIud4PB18=";
>  
>  /* End Entity Test Cert */
>  static const char kEnd_Entity_Test_CertFingerprint[] =
> +  "C5Z5x0yOcsEGmsyZM8uyAJV8u+laWKP5S6tGZWhEJ/Y=";

Please file a bug to make it so that this kind of unrelated change isn't necessary for this type of bug. We shouldn't have to modify this file every time we add a test.

@@ +1013,5 @@
>    { "history.google.com", true, false, false, -1, &kPinset_google_root_pems },
>    { "hostedtalkgadget.google.com", true, false, false, -1, &kPinset_google_root_pems },
>    { "include-subdomains.pinning.example.com", true, false, false, -1, &kPinset_mozilla_test },
>    { "liberty.lavabit.com", true, true, false, -1, &kPinset_lavabit },
> +  { "login.corp.google.com", true, false, false, -1, &kPinset_google_root_pems },

Ditto. This type of change doesn't belong in this type of patch.

@@ +1066,5 @@
>    { "youtube.com", true, false, false, -1, &kPinset_google_root_pems },
>    { "ytimg.com", true, false, false, -1, &kPinset_google_root_pems },
>  };
>  
> +// Pinning Preload List Length = 329;

Ditto.

@@ +1071,4 @@
>  
>  static const int32_t kUnknownId = -1;
>  
> +static const PRTime kPreloadPKPinsExpirationTime = INT64_C(1416947234807000);

Ditto.

::: security/manager/ssl/src/SSLServerCertVerification.cpp
@@ +305,3 @@
>    }
>    NS_WARNING("Unknown certificate error code. Does MapCertErrorToProbeValue "
>               "handle everything in PRErrorCodeToOverrideType?");

NIT (another follow-up): It seems like this string is out of date. In particular, do we still have PRErrorCodeToOverrideTyype?

::: security/manager/ssl/tests/unit/test_cert_overrides.js
@@ +124,5 @@
>                           getXPCOMStatusFromNSS(SEC_ERROR_UNKNOWN_ISSUER));
> +
> +  add_cert_override_test("ca-used-as-end-entity.example.com",
> +                         Ci.nsICertOverrideService.ERROR_UNTRUSTED,
> +                         getXPCOMStatusFromNSS(MOZILLA_PKIX_ERROR_CA_CERT_USED_AS_END_ENTITY));

Please add a couple more test cases:

1. When such a certificate is issued from an ERROR_UNTRUSTED_ISSUER (actively distrusted issuer), the error returned must be ERROR_UNTRUSTED_ISSUER, and an override should not be allowed.

2. When this certificate is revoked, the error code must be ERROR_REVOKED_CERTIFICATE, and an override should not be allowed.

3. When there is a hostname mismatch (from CERT_VerifyCertName) with such a certificate, what should the result be?

In other words, more thoroughly test the non-obvious error ranking logic.

Comment 10

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

Thanks for the review.

(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #9)
> Comment on attachment 8475423 [details] [diff] [review]
> I'm guessing that the error text is non-localizable currently. That is
> something that should also be fixed.

Yeah :/ See bug 1052529.

> ::: dom/browser-element/BrowserElementChildPreload.js
> Please file a follow-up bug to redo this error classification using the PSM
> function provided for this (NSSErrorsService::GetErrorClass). It is bad to
> have this code duplicated, especially when this code lives in DOM.

Filed bug 1057497.

> ::: security/manager/ssl/src/SSLServerCertVerification.cpp
> @@ +305,3 @@
> >    }
> >    NS_WARNING("Unknown certificate error code. Does MapCertErrorToProbeValue "
> >               "handle everything in PRErrorCodeToOverrideType?");
> 
> NIT (another follow-up): It seems like this string is out of date. In
> particular, do we still have PRErrorCodeToOverrideTyype?

Looks like DetermineCertOverrideErrors is what we mean, now.

> ::: security/manager/ssl/tests/unit/test_cert_overrides.js
> Please add a couple more test cases:
> 
> 1. When such a certificate is issued from an ERROR_UNTRUSTED_ISSUER
> (actively distrusted issuer), the error returned must be
> ERROR_UNTRUSTED_ISSUER, and an override should not be allowed.

Added.

> 2. When this certificate is revoked, the error code must be
> ERROR_REVOKED_CERTIFICATE, and an override should not be allowed.

Added.

> 3. When there is a hostname mismatch (from CERT_VerifyCertName) with such a
> certificate, what should the result be?

I think CA_CERT_USED_AS_END_ENTITY, which is what it turned out to be when I added a testcase for it.

Comment 11

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

https://tbpl.mozilla.org/?tree=Try&rev=35ab98d3bd5d
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2a0935b1b76

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 14

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

Attachment: patch for beta

Approval Request Comment
[Feature/regressing bug #]: mozilla::pkix
[User impact if declined]: Some corporate certificate hierarchies and embedded network devices have improperly generated certificates. mozilla::pkix started out enforcing some properties more strictly than other certificate verification libraries, and as a result many users could not visit sites using these certificates. This patch (and the ones it depends on) allows exceptions to be added for some improperly generated certificates.
[Describe test coverage new/current, TBPL]: there are xpcshell tests for this
[Risks and why]: low
[String/UUID change made/needed]: none

Comment 15

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

[Tracking Requested - why for this release]: see comment 14

Comment 16

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to David Keeler (:keeler) [use needinfo?] from comment #14)
> Approval Request Comment
> [Feature/regressing bug #]: mozilla::pkix

In what release was the feature shipped that added this constraint?

> [User impact if declined]: Some corporate certificate hierarchies and
> embedded network devices have improperly generated certificates.

How prevalent are these scenarios? What percentage of the Firefox user base due you expect to be impacted by shipping Firefox 33 without this fix?

> mozilla::pkix started out enforcing some properties more strictly than other
> certificate verification libraries, and as a result many users could not
> visit sites using these certificates. This patch (and the ones it depends
> on) allows exceptions to be added for some improperly generated certificates.

By patches that this one depends on, are you referring to bug 1040446? Are there any other bugs?

Comment 17

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to Lawrence Mandel [:lmandel] from comment #16)
> By patches that this one depends on, are you referring to bug 1040446? Are
> there any other bugs?

I take it bug 1039064 should be included in the list as well.

Comment 18

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to Lawrence Mandel [:lmandel] from comment #16)
> (In reply to David Keeler (:keeler) [use needinfo?] from comment #14)
> > Approval Request Comment
> > [Feature/regressing bug #]: mozilla::pkix
> 
> In what release was the feature shipped that added this constraint?

Looks like Firefox 31. (bug 915930)

Comment 19

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

(In reply to Lawrence Mandel [:lmandel] from comment #16)
> (In reply to David Keeler (:keeler) [use needinfo?] from comment #14)
> > Approval Request Comment
> > [Feature/regressing bug #]: mozilla::pkix
> 
> In what release was the feature shipped that added this constraint?

Right - mozilla::pkix was enabled by default in 31. In 31 and 32 there is still the option of using "classic" (i.e. NSS-based) verification if mozilla::pkix causes compatibility issues. In 33 we removed that option, so 33 is really the version at which this becomes a problem.

> > [User impact if declined]: Some corporate certificate hierarchies and
> > embedded network devices have improperly generated certificates.
> 
> How prevalent are these scenarios? What percentage of the Firefox user base
> due you expect to be impacted by shipping Firefox 33 without this fix?

These scenarios don't really happen on the web at large at all (we're fairly confident about this). However, it seems not to be uncommon on intranet sites at various companies that users need for work. The overall percentage is probably quite small, but if a user encounters this issue, they have no recourse but to use another browser (or an older version of Firefox).

> > mozilla::pkix started out enforcing some properties more strictly than other
> > certificate verification libraries, and as a result many users could not
> > visit sites using these certificates. This patch (and the ones it depends
> > on) allows exceptions to be added for some improperly generated certificates.
> 
> By patches that this one depends on, are you referring to bug 1040446? Are
> there any other bugs?

The patches in bug 1040446 and bug 1039064 are the prerequisites for this patch landing.

Comment 20

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to David Keeler (:keeler) [use needinfo?] from comment #19)
> Right - mozilla::pkix was enabled by default in 31. In 31 and 32 there is
> still the option of using "classic" (i.e. NSS-based) verification if
> mozilla::pkix causes compatibility issues. In 33 we removed that option, so
> 33 is really the version at which this becomes a problem.

Can we restore classic verification in 33? Is that safer than taking these patches?

> These scenarios don't really happen on the web at large at all (we're fairly
> confident about this). However, it seems not to be uncommon on intranet
> sites at various companies that users need for work. The overall percentage
> is probably quite small, but if a user encounters this issue, they have no
> recourse but to use another browser (or an older version of Firefox).

Or ESR. In any case, that's not good. Is there really no way to work around this issue? Can the user manually add certificates to a directory? Can they modify a config file?

Comment 21

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

(In reply to Lawrence Mandel [:lmandel] from comment #20)
> (In reply to David Keeler (:keeler) [use needinfo?] from comment #19)
> > Right - mozilla::pkix was enabled by default in 31. In 31 and 32 there is
> > still the option of using "classic" (i.e. NSS-based) verification if
> > mozilla::pkix causes compatibility issues. In 33 we removed that option, so
> > 33 is really the version at which this becomes a problem.
> 
> Can we restore classic verification in 33? Is that safer than taking these
> patches?

That would involve reverting the patch in bug 975229 and updating some tests, I believe. It's a similar amount of code to the patch in bug 1039064. It would be doable, but I'm reluctant to recommend that because we basically wontfix'd a couple of NSS bugs because using the new library meant they didn't affect Firefox any longer (see for example bug 978354).

> > These scenarios don't really happen on the web at large at all (we're fairly
> > confident about this). However, it seems not to be uncommon on intranet
> > sites at various companies that users need for work. The overall percentage
> > is probably quite small, but if a user encounters this issue, they have no
> > recourse but to use another browser (or an older version of Firefox).
> 
> Or ESR. In any case, that's not good. Is there really no way to work around
> this issue? Can the user manually add certificates to a directory? Can they
> modify a config file?

Unfortunately, the only other option would be to get their respective IT departments to fix and regenerate the improper certificates. Some of the certificates are generated automatically by buggy software - that software would need to be fixed and updated.

Comment 22

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

Attachment: patch for beta rebased

Approval Request Comment
(see previous approval request)
Rebased patch due to changes in the patch for bug 1039064 so it wouldn't require idl changes.

Comment 23

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

FWIW, here's a try run: https://tbpl.mozilla.org/?tree=Try&rev=4bec67d8ad8a

Comment 24

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8488912 [details] [diff] [review]
patch for beta rebased

I reviewed this feature and the corresponding prereq bugs with keeler and dveditz. Although the issue described in this bug only impacts a small number of users, for those users there is no workaround and they will be forced to use another browser because Firefox won't be able to support a specific site because of its certificate. We don't want to ship in this state. We're taking more code change than usual because we're picking up some prereq work for this patch. keeler is on call for any breakage as a result of this change.

keeler - Please add instructions on how to test this feature so that QA can verify as I don't know that our beta audience to hit this feature.

beta+

Comment 25

[Ryan VanderMeulen [:RyanVM]]

(In reply to David Keeler (:keeler) [use needinfo?] from comment #23)
> FWIW, here's a try run: https://tbpl.mozilla.org/?tree=Try&rev=4bec67d8ad8a

Is there a particular reason we're ignoring the mass-fail in this push?

Comment 26

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

I initially thought that was because of infrastructure issues on Friday, but it's actually an error in the patch. I'm pretty sure I figured out what the problem was, but I'm rebuilding now just to be sure.

Comment 27

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

Attachment: patch for beta fixed

Comment 28

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

https://hg.mozilla.org/releases/mozilla-beta/rev/198d06258284

Comment 29

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

For QA, here's steps to test this:

1. Generate or obtain a certificate with a basic constraints extension with the value cA: TRUE (e.g. `openssl req -new -x509 -nodes -out server.crt -keyout server.key' will probably work by default on most systems (in particular, Fedora Linux)). To verify, examine the certificate with `openssl x509 -in server.crt -text -noout'. It should contain the following:

            X509v3 Basic Constraints: 
                CA:TRUE

2. Start a simple https server using that certificate/key pair.
3. Import and trust that certificate using the certificate manager in Firefox (Preferences -> Advanced -> Certificates -> View Certificates -> Authorities -> Import..., find the file, click ok, check the box marked "Trust this CA to identify websites", click ok)
4. In versions before this bug is fixed, navigating to the server will result in a grey error page with the error code "sec_error_ca_cert_invalid". This cannot be overridden. After this bug is fixed, navigating will result in the yellow certificate override error page. This can be overridden.

Comment 30

[Ryan VanderMeulen [:RyanVM]]

Backed out from beta per bug 1039064 comment 43.

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a6856f90ce36

Comment 32

[Bogdan Maris, Desktop Test Engineering [OUT until 12 Jan]]

Reproduced the initial issue after setting up the HTTPS server on a build before the fix I am prompted with Secure Connection Failed page (Error code: sec_error_ca_cert_invalid). Verified that using Firefox 33 beta 4 and latest Aurora on Windows 7 64bit, Ubuntu 14.04 32bit and Mac OS X 10.7 I am prompted with This Connection is Untrusted page and I`m able to Add Exception and confirm security exception.

Comment 33

[lsof]

Could the invalid part of the certificate for sec_error_ca_cert_invalid be highlighted? It can be lots of different things. I can't fix it without knowing.

Comment 34

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

There are a lot of improvements we can make to the certificate error page. See bug 1059185 for progress on that.

Comment 35

[Bernd Nies]

We'd love to see the certificate security override button coming back. We have hundreds of devices that use web interfaces with self-signed certificates and that sometimes don't allow certificate management. These are for intranet management purposes where a valid certificate chain is not necessary.

To name a few:

* APC SmartUPS and PDU
* Cisco UCS blade system management console
* Cisco call manager
* Cisco ASA firewalls
* IBM hardware management console 

Workaround is to use another browser to manage those devices.

Comment 36

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

Bernd, what version of Firefox are you using? This should be fixed in 33.

Comment 37

[Bernd Nies]

On 16 September Red Hat updated Firefox ESR 31.1.0 for RHEL 5/6/7.

Comment 39

[vey.quentin]

(In reply to Bernd Nies from comment #35)
> We'd love to see the certificate security override button coming back. We
> have hundreds of devices that use web interfaces with self-signed
> certificates and that sometimes don't allow certificate management. These
> are for intranet management purposes where a valid certificate chain is not
> necessary.
> 
> To name a few:
> 
> * APC SmartUPS and PDU
> * Cisco UCS blade system management console
> * Cisco call manager
> * Cisco ASA firewalls
> * IBM hardware management console 
> 
> Workaround is to use another browser to manage those devices.

Same problem here, using firefox (iceweasel) 35.0a2 (2014-11-11) on some intranet website of my company.

Comment 40

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

(In reply to vey.quentin from comment #39)
> Same problem here, using firefox (iceweasel) 35.0a2 (2014-11-11) on some
> intranet website of my company.

This should already be fixed in 35. It would be helpful if you would file a new bug with steps to reproduce and what error you're encountering.

Comment 42

[vey.quentin]

In the meantime the certificate on my corporate network has been updated and generates now a ssl_error_bad_cert_domain, which is easily overridable, so I can't reproduce this anymore.