mozilla::pkix rejects certificates with a critical Netscape Cert Type extension

RESOLVED FIXED in Firefox 33

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: seocam, Assigned: keeler)

Tracking

({regression})

31 Branch
mozilla34
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox31 wontfix, firefox32 wontfix, firefox33 fixed, firefox34 fixed, firefox-esr31 wontfix)

Details

Attachments

(4 attachments, 2 obsolete attachments)

When accessing https://colab.interlegis.leg.br on Firefox 29 and 30 I get sec_error_untrusted_issuer (with an option to add a security exception) but when I access the same URL from 31 and 32 I get sec_error_unknown_critical_extension (which does not allow me to add the exception).
Keywords: regression
Posted file interlegis.der
The certificate I'm seeing on that site has the extension "Netscape Cert Type" marked as critical. Since mozilla::pkix doesn't handle that extension and since it's marked critical, the library bails with the error SEC_ERROR_UNKNOWN_CRITICAL_EXTENSION.
I'm sorry but I'm not sure I got it.

Firefox versions before 30 were able to handle this extension but newer versions won't be able? Or previous versions were just ignoring this extension?

Is this a bug or just a behavior change that will break some websites?

Thanks!
We're working on a new certificate verification library[0] that was turned on by default in 31. Its behavior doesn't completely match the old library, and we've seen some incompatibilities. It's not clear to me whether this case is something the new library should handle more gracefully or if we should ask CAs not to issue certificates like this.

[0] https://blog.mozilla.org/security/2014/04/24/exciting-updates-to-certificate-verification-in-gecko/
Summary: Exception for sec_error_untrusted_issuer is now sec_error_unknown_critical_extension → mozilla::pkix doesn't handle the Netscape Cert Type extension (which is problematic when it's marked critical)
AFAICT, the root certificate that this certificate chains to is not in Microsoft's nor Mozilla's root program. When I added the root certificate to Windows's trust database as a trusted CA, then MSIE will successfully connect to the site without any cert error warning. Thus, they either implement the Netscape extension or they ignore it even when critical==True.

I suggest that we don't make any change to mozilla::pkix for this bug. One sub-goal of mozilla::pkix is to get rid of cruft we don't need and this obsolete Netscape extension is said cruft.

Instead, I suggest that we contact the CA that issued this certificate and ask them to stop using the Netscape cert type extension and/or at least stop marking it critical==true. Note that they already include the standard EKU and KU extensions that we support and which make the Netscape extension obsolete. This should improve the interoperability of their certificates with other products too, since I doubt every other product implements the Netscape extension.

Kathleen, do you think this is reasonable? If so, let's move this bug to "NSS: CA Certificates" and let's update the wiki to document the behavior change and to request CAs to fix this issue by never including the Netscape cert type extension in certificates (or at least never marking it critical).
Flags: needinfo?(kwilson)
Added as 4th bullet point under item 1 in the Behavior Changes list...
https://wiki.mozilla.org/SecurityEngineering/mozpkix-testing#Behavior_Changes
"1. End-entity certificates used in SSL/TLS servers:
    - Are not allowed to have basic constraints asserting isCA=TRUE.
    - When the EKU extension is specified, must assert the serverAuth bit.
    - Are no longer allowed to include the OCSPSigning EKU.
    - Cannot have the Netscape Cert Type extension marked as critical. "
Flags: needinfo?(kwilson)
I sent email to the CA.

I think we should close this bug as WONTFIX. OK?
I've also contacted the CA. They are aware of the issue and will re-issue their cert soon.

In my perspective WONTFIX is ok.

Thanks!
Thanks!
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
Changing resolution to INVALID since this is working as intended.

Sergio, thank you for testing out stuff, for reporting this bug, and for contacting the CA that issued the problematic certificate(s). Please let us know if you notice anything else that seems wack.
Resolution: WONTFIX → INVALID
In a short the responsible person at Interlegis told me their CA is not able to issue Certificates without this extension and they don't have the time to address this issue.

They are suggesting to their users to down grade to Firefox 29 or witch over to other browsers since Firefox is the only browser affected. Here is their answer to my appeal (in pt-br): https://colab.interlegis.leg.br/archives/thread/gitec/colab-down#msg-146415

(Just a reminder, the link doesn't open in Firefox).

Not sure I can help any longer here.
[Tracking Requested - why for this release]:

Sergio, thanks for the update.

Kathleen, can you please work with the CA to get them to adjust their software? I am sure this is just one line in a configuration file or similar for them. I think there is probably a misunderstanding about how difficult this change is.

See also bug 395224 regarding libpkix. It seems like libpkix DOES process the Netscape cert type extension. I don't think it is worthwhile for us to implement complete support for the extension but at the same time I don't think it is worthwhile for us to break compatibility with websites over the issue either.
Status: RESOLVED → REOPENED
Flags: needinfo?(kwilson)
Resolution: INVALID → ---
See Also: → 395224
Summary: mozilla::pkix doesn't handle the Netscape Cert Type extension (which is problematic when it's marked critical) → mozilla::pkix rejects certificates with a critical Netscape Cert Type extension
(In reply to Brian Smith from comment #11)
> Kathleen, can you please work with the CA to get them to adjust their
> software? I am sure this is just one line in a configuration file or similar
> for them. I think there is probably a misunderstanding about how difficult
> this change is.

As far as I can tell the SSL cert at https://colab.interlegis.leg.br/ is signed by a "Autoridade Certificadora do Interlegis" cert not chaining up to a root in Mozilla's program. 

Am I missing something?

If not, then perhaps the main issue is that users used to get the over-ride-able Untrusted Connection Error with sec_error_untrusted_issuer, so they could add a permanent exception. But, now users get the Secure Connection Failed error with no way to view the cert, and no way to add an exception -- currently in FF32 the error is sec_error_unknown_critical_extension
Flags: needinfo?(kwilson)
Here's what I think we should do here:
* add an error code to mozilla::pkix like ERROR_DEPRECATED_CRITICAL_EXTENSION
* return this error when extensions like this are encountered
* in PSM, we can make ERROR_DEPRECATED_CRITICAL_EXTENSION overridable but keep ERROR_UNKNOWN_CRITICAL_EXTENSION non-overridable
That sounds reasonable to me.
(In reply to David Keeler (:keeler) [use needinfo?] from comment #13)
> Here's what I think we should do here:
> * add an error code to mozilla::pkix like ERROR_DEPRECATED_CRITICAL_EXTENSION
> * return this error when extensions like this are encountered
> * in PSM, we can make ERROR_DEPRECATED_CRITICAL_EXTENSION overridable but
> keep ERROR_UNKNOWN_CRITICAL_EXTENSION non-overridable

If it's worth doing anything for this bug, then we should solve the problem in a transparent (to the user) way. In particular, if the user is trusting the root CA from which these certificates were issued, then the user shouldn't get a cert error page just because this extension is present and critical.

Counterproposal #1:

* If the Netscape cert type extension is present and NOT marked critical, then just ignore it.
* If the Netscape cert type extension is present and marked critical AND the cert has EKU and basicConstraints extensions present, then just ignore the Netscape cert type extension, since the other two extensions (which are standardized) cover the same stuff.
* If the Netscape cert type extension is present and marked critical AND the cert is missing EKU or it is missing basicConstraints, then return ERROR_UNKNOWN_CRITICAL_EXTENSION.

Counterproposal #2:

* Do nothing. The test site's certificate isn't trusted by MSIE or Chrome on Windows, so it can't be that high of a priority.
Posted patch patch (obsolete) — Splinter Review
I think option 1 is reasonable.
Assignee: nobody → dkeeler
Status: REOPENED → ASSIGNED
Attachment #8473800 - Flags: review?(brian)
(In reply to David Keeler (:keeler) [use needinfo?] from comment #16)
> 
> I think option 1 is reasonable.

I agree.
Comment on attachment 8473800 [details] [diff] [review]
patch

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

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

Again, I will complain that such changes don't belong in this kind of patch., and we should rework things so that such changes are not necessary (in a separate bug).

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

Ditto for everything in this file.

::: security/pkix/lib/pkixcert.cpp
@@ +156,5 @@
> +    // It is acceptable to accept this certificate if the Netscape certificate
> +    // type extension is present and marked critical if both the basic
> +    // constraints and extended key usages extensions are present, because they
> +    // are standardized and cover the same information. Otherwise, we must
> +    // reject the certificate.

I suggest a different comment:

The Netscape Certificate Type extension is an obsolete Netscape-proprietary mechanism that we ignore in favor of the standard extensions. However, some CAs have issued certificates with the Netscape Cert Type extension marked critical. Thus, for compatibility reasons, we "understand" this extension by ignoring it when it is not critical, and by ensuring that the equivalent standardized extensions are present when it is marked critical, based on the assumption that the information in the Netscape Cert Type extension is consistent with the information in the standard extensions.

Here is a mapping between the Netscape Cert Type extension and the standard extensions:

Netscape Cert Type  |  BasicConstraints.cA  |  Extended Key Usage  
--------------------+-----------------------+--------------------
SSL Server          |  false                | id_kp_serverAuth
SSL Client          |  false                | id_kp_clientAuth
[...]
SSL Server CA       |  true                 | id_pk_serverAuth
[...]

@@ +157,5 @@
> +    // type extension is present and marked critical if both the basic
> +    // constraints and extended key usages extensions are present, because they
> +    // are standardized and cover the same information. Otherwise, we must
> +    // reject the certificate.
> +    if (netscapeCertificateType.GetLength() > 0 &&

I suggest you rename this to be "criticalNetscapeCertificateType" since it is only set when the extension is marked critical.

::: security/pkix/lib/pkixutil.h
@@ +138,5 @@
> +  // The extension Netscape certificate type is not supported. If it is
> +  // present but not marked critical, we ignore it. If it is present and
> +  // marked critical, if the basic constraints and extended key usage
> +  // extensions are present, it is ignored. Otherwise, Init will return
> +  // an error indicating an unknown critical extension is present.

I suggest that you put all this comment text in the place where this is enforced and rename this to "criticalNetscapeCertificateType" or "netscapeCertificateTypeIfCritical".
Attachment #8473800 - Flags: review?(brian) → review+
Thanks for the review.

(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #18)
> Comment on attachment 8473800 [details] [diff] [review]
> ::: security/manager/boot/src/StaticHPKPins.h
> Again, I will complain that such changes don't belong in this kind of
> patch., and we should rework things so that such changes are not necessary
> (in a separate bug).

Good call. Bug 1057128 fixes this, is inbound now.

> ::: security/pkix/lib/pkixcert.cpp
> @@ +156,5 @@
> I suggest a different comment:

Done.

> @@ +157,5 @@
> I suggest you rename this to be "criticalNetscapeCertificateType" since it
> is only set when the extension is marked critical.

Changed.

> ::: security/pkix/lib/pkixutil.h
> @@ +138,5 @@
> I suggest that you put all this comment text in the place where this is
> enforced and rename this to "criticalNetscapeCertificateType" or
> "netscapeCertificateTypeIfCritical".

The other comment supersedes this one, so I just took it out.

Try: https://tbpl.mozilla.org/?tree=Try&rev=8bfc0d139fd6
Posted patch patch v2 (obsolete) — Splinter Review
Addressed review comments, carrying over r+.
Attachment #8473800 - Attachment is obsolete: true
Attachment #8477548 - Flags: review+
Well, glad I used try: windows complained about an unused variable. Fixed: https://tbpl.mozilla.org/?tree=Try&rev=35ab98d3bd5d
https://hg.mozilla.org/mozilla-central/rev/5abee1368f02
Status: ASSIGNED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Is this going to land only on 34? Any chance to land on 33 as well?
Wow - somehow the later versions of this patch neglected to actually add test_nsCertType.js to xpcshell.ini.
Attachment #8480773 - Flags: review?(mmc)
(In reply to Sergio Oliveira Campos from comment #24)
> Is this going to land only on 34? Any chance to land on 33 as well?

Once the test has actually been running on central for a bit, I'll request uplift.
Attachment #8480773 - Flags: review?(mmc) → review+
Approval Request Comment
[Feature/regressing bug #]: mozilla::pkix
[User impact if declined]: some users will experience non-overridable certificate errors on sites that use the nsCertType extension in their certificates (which is rare)
[Describe test coverage new/current, TBPL]: xpcshell tests, gtests
[Risks and why]: low - this is thoroughly tested and has been on nightly (recently merged to aurora) for about a week
[String/UUID change made/needed]: none

Note for if a sheriff checks this in: the patch in bug 1057128 (consists of test-only changes) should be checked in before this patch.
Attachment #8483772 - Flags: review+
Attachment #8483772 - Flags: approval-mozilla-beta?
Attachment #8483772 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: in-testsuite+
This actually isn't essential for esr31, since classic verification is still available and handles this type of extension in a different way. Also, it would be a non-trivial amount of work to prepare a patch for esr31, as we've made significant changes since it was released.
Flags: needinfo?(dkeeler)
Issue: Secure Connection Failed error.
Version: 34.0.5
OS: Windows XP

 We use firefox browser for developing javascript rich applications and it helps alot to debug and identify javascript issues. Offlate following security exception is coming while accessing our QA server. Can you please suggest how to go ahead and add an exception to navigate to this site. This is blocking our development using Firefox.

Thanks

Secure Connection Failed

An error occurred during a connection to tenantsvr2.vmwaredomain.local. Certificate contains unknown critical extension. (Error code: sec_error_unknown_critical_extension)

    The page you are trying to view cannot be shown because the authenticity of the received data could not be verified.
    Please contact the website owners to inform them of this problem.
Vani, please file a new bug here: https://bugzilla.mozilla.org/enter_bug.cgi?product=Core&component=Security%3A%20PSM and attach the output of `openssl s_client -connect {hostname:port} -showcerts < /dev/null`. Thanks!
You need to log in before you can comment on or make changes to this bug.