560091 - "OCSP signer cert not found" is reported as sec_error_bad_database

Status: NEW

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

After downloading the FNMT root CA cert from bug 435736 and trusting it
https://bugzilla.mozilla.org/attachment.cgi?id=439758
and attempting to visit a site certified by one of its subordinate CAs
with OCSP enabled, e.g.   https://sedemeh.gob.es/ 
one gets a strange error page

> Secure Connection Failed
> An error occurred during a connection to sedemeh.gob.es.
> security library: bad database.
> (Error code: sec_error_bad_database)

The strange thing is the particular error code:  bad DATABASE

The error condition occurs when the OCSP response gives a cert subject 
name, and no cert by that name can be found (is not in the response, and
is not in the database, nor part of the https server's cert chain).

CERT_VerifyOCSPResponseSignature calls CERT_FindCertByName to find the
response signer.  If CERT_FindCertByName fails to find the signer cert,
it does NOT set the error code, leaving it set to whatever it was
previously set to (which most commonly is BAD_DATABASE).  
CERT_FindCertByName SHOULD (is expected to) set the error code to
SEC_ERROR_UNKNOWN_CERT.  CERT_VerifyOCSPResponseSignature specifically 
looks for that error code at line 3864, but does not find it. 

The first part of the fix is to make CERT_FindCertByName set that error code 
when it is about to return NULL.

However, when CERT_VerifyOCSPResponseSignature sees SEC_ERROR_UNKNOWN_CERT, 
it changes that to SEC_ERROR_OCSP_INVALID_SIGNING_CERT, which is reported as
"Invalid OCSP signing certificate in OCSP response."  A more accurate error
message would be "OCSP response signature certificate not found".  We have 
no error code that says that.  So, I propose to create one.  That is the 
second part of the fix.

Comment 1

[Konstantin Andreev]

SEC_ERROR_BAD_DATABASE is, in some sense, an legitimate error code for failed search, because it was a search in a particular DB.

There are other cases, where the choice of error code to return is ambiguous.

For consistency, NSS needs a document, which describe, what each error code means and establishes an eligibility and prioritization rules. Symbolic names of error codes are not self-explained, and descriptions in "security/nss/cmd/lib/SECerrs.h" are not sufficient.

The high quality example of documenting error codes is the "FUNCTION RETURN VALUES" chapter of PKCS#11 specification.

Comment 2

[Konstantin Andreev]

In the light of [bug 664666 comment 1] there is nothing wrong with that, that CERT_FindCertByName passes through the error code. The fix may be to make CERT_VerifyOCSPResponseSignature look for BAD_DATABASE, not for UNKNOWN_CERT.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, and this bug has priority 'P2'.
:beurdouche, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 4

[Marco Castelluccio [:marco]]

We have modified the bot to only consider P1 as high priority, so I'm cancelling the needinfo here.