427081 - Allow to override SEC_ERROR_INADEQUATE_KEY_USAGE

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Kai Engert (:KaiE:)]

It appears that many people run into websites with certificates that yield error code SEC_ERROR_INADEQUATE_KEY_USAGE.

As discussed in some other bugs, the error code is seen as "not worse" than any of the other error codes that we allow to override.

It was unclear, whether at the same time we should allow to override even more errors, but so far we have not researched what other codes are good candidates.

It had also been mentioned that in a perfect world we'd use new error strings to describe the new scenarios we allow to override.

However, given that
- SEC_ERROR_INADEQUATE_KEY_USAGE is the only code I see people complaining about
- it happens a lot, there are discussion threads in forums
- we are too late for adding strings

I propose we go ahead, allow this error code to be overridden, group it into category "missing trust of some kind", and should this error happen to be the only error, we use the same wording as for SEC_ERROR_UNTRUSTED_CERT.


See also:
http://groups.google.com/group/mozilla.feedback.firefox.prerelease/browse_thread/thread/885b8914a0cc9e80
http://forums.mozillazine.org/viewtopic.php?p=3214810&sid=83ec36b154173a769dcf000463a6b153
bug 412277
bug 421248
bug 424077
bug 407523

Comment 1

[Kai Engert (:KaiE:)]

Attachment: Patch v1

Bob or Nelson, could you please review?

Comment 2

[Johnathan Nightingale [:johnath]]

Sounds good to me!

Comment 3

[Damon Sicore (:damons)]

So, this isn't something that we'd necessarily block the release for, but after
discussing this a bit with johnath, this seems to be a pretty good idea. 
Please request approval once reviewed and we'll accept the patch.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

The issue occurs with all self-signed non-CA server certs that are not marked
trusted.  Because they are not marked trusted, NSS naturally attempts to 
validate the certificate by finding its issuer certificate, and verifying 
the cert's signature against that issuer certificate.  

Because the cert is self signed, it is its own issuer.  However because it is 
not a CA cert, it is invalid as a cert issuer on several grounds, including
a) it is not a CA cert, and 
b) its issuer has given it key usages that are only valid for servers, not 
for certificate issuers.  

So, there are two reasons why it is not a valid issuer, and the complaint is
ultimately that the cert has an invalid issuer.  It happens that we check
the key usage first before checking that the issuer is a CA cert.  If we did
it in the other order, the error code would be a different one, but the effect
would be the same.

I have looked to find any standard from any standards body that defines 
self-signed EE (server-only, not CA) certs, and I have not found any such 
standard.  I had wondered (hoped) that there would be some provision that
says "if a self signed cert is not a CA cert, then it does not need the 
usual key usages for cert signing."  But I did not find any such provision.
The only provisions I can find for self-signed certs in the standards are 
for self-signed CA certs.  So, it seems that to be standards conformant, 
a self-signed cert must be a CA cert.  But if anyone knows of a standard
that provides for self-signed EE certs, please advise.  Note that OpenSSL
documentation does not satisfy this request.

Comment 5

[Kai Engert (:KaiE:)]

Nelson, my intention of this bug is not to analyze any certificates.

My intention was to suggest that we add this error code to the list of errors we allow to override, without further thinking.

I had assumed we are ready to add it.
I would like to point you to bug 424077 comment 5 where you said:

"Considering that we allow overrides for FAR more 
egregious security errors, I think it's reasonable to allow overrides for
SEC_ERROR_INADEQUATE_KEY_USAGE."

Comment 6

[Kai Engert (:KaiE:)]

Attachment: screen shot, new message for https://sage.math.washington.edu:8103/

In order to illustrate the effect of the patch, and to show you what UI we get for various certs, I'm attaching 3 screen shots.

Comment 7

[Kai Engert (:KaiE:)]

Attachment: screen shot, new message for https://sc.snu.ac.kr/manhat-bin/doorstep

Comment 8

[Kai Engert (:KaiE:)]

Attachment: screen shot, new message for an intranet site I have access to

Comment 9

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 313653 [details] [diff] [review]
Patch v1

Although I don't usually do reviews for PSM, I'll make an exception
in this case.

r=nelson

I would suggest one change.  In function AppendErrorTextUntrusted,
I would suggest making case SEC_ERROR_INADEQUATE_KEY_USAGE: fall 
through into case SEC_ERROR_CA_CERT_INVALID.  But this is not mandatory.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

Kai, in comment 4, I was merely attempting to 
a) explain why self signed EE certs get this error code, and
b) discuss other potential ways to mitigate it.

Comment 11

[Kai Engert (:KaiE:)]

Attachment: Patch v1 + Nelson's proposed change

Thanks Nelson

Comment 12

[Damon Sicore (:damons)]

Sorry, but I have to ask, is there a way we can write a test for this change?

Comment 13

[Kai Engert (:KaiE:)]

(In reply to comment #12)
> Sorry, but I have to ask, is there a way we can write a test for this change?

I'd recommend to manually verify this bug after it got fixed.

As of today, if you connect to a https site with such a broken cert, you'll get a dead-end error page. For example, go to https://sage.math.washington.edu:8103/ or https://sc.snu.ac.kr/

With the patch applied, you'll get the page that allows you to override.


The second step in testing would require you to go through the necessary steps to add an override and confirm that you're finally able to connect to the target site.


Setting up a full test would involve multiple steps, I'm not sure if you want to go that path.

An automated test would consist of three separate steps.

First, as you can't rely on the mentioned servers to be online or broken forever, you will need a local test https server which uses such an invalid certificate. We'd have to generate one, and potentially use a test server executable like the one currently being worked on in bug 426867.

Second, you'll connect to the server and verify you get the page that allows to add on override.

The third step would be whether adding an override and connecting afterwards (without error) is possible. This third step could be reused for other broken sites, like those with domain mismatches, expired certs or self signed certs etc.

Comment 14

[Damon Sicore (:damons)]

Comment on attachment 313719 [details] [diff] [review]
Patch v1 + Nelson's proposed change

Alright.  We really need a way to automate these types of things.  

Thanks for the detailed answer.  

a1.9+=damons

Comment 15

[Kai Engert (:KaiE:)]

Checked in.
Thanks to everybody for the immediate attention.

Comment 16

[(not currently active) Ted Mielczarek]

Damon: yeah, we're working on integrating SSL support into mochitest in bug 426867.

Comment 18

[cecil_t]

OK there is a work-around to this as well for those on the betas:

1) go to an affected page, get the error.
2) right-click, view page info, security tab, view certificate
3) details tab, export, save as somefile.cer, close, close
4) go to preferences, advanced, encryption tab
5) view certificates, servers tab, import, select the saved .cer file
6) hit ok, close preferences
7) refresh the page with the cert error, now should be a different error
8) click "Or you can add an exception", "Add exception..."
9) get certificate, confirm security exception - now you're in

Comment 19

[Breno Leitao]

(In reply to comment #18)
> OK there is a work-around to this as well for those on the betas:
> 5) view certificates, servers tab, import, select the saved .cer file

The problem is that this workaround doesnt work all the time. Here. for example, after step 5 I was asked for a password that was used to encrypt the certificate backup, which I don't make any idea what it is.

I am using the Firefox version 3 Beta 5, which came in Ubuntu 8.04.

Comment 20

[Kai Engert (:KaiE:)]

The fact that we allowed to override sec_error_inadequate_key_usage introduced bug 520830. If you're interested, please add yourself to that bug.