Closed
Bug 287850
Opened 19 years ago
Closed 18 years ago
chain validation returns ambiguous error codes when OCSP enabled
Categories
(NSS :: Libraries, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
3.11.3
People
(Reporter: nelson, Assigned: nelson)
Details
(Whiteboard: Sun CR 6420859)
Attachments
(1 file, 1 obsolete file)
3.86 KB,
patch
|
julien.pierre
:
review+
|
Details | Diff | Splinter Review |
The error code SEC_ERROR_BAD_SIGNATURE is ambiguous when OCSP is enabled. The caller of CERT_VerifyCertificate cannot tell whether that error indicates that the cert's own signature was invalid, or the OCSP response's signature was invalid. That's because CERT_VerifyCertificate returns CERT_CheckOCSPStatus's error code as its own error code, and CERT_CheckOCSPStatus returns SEC_ERROR_BAD_SIGNATURE for an invalid signature on the OCSP response. This problem affects CERT_VerifyCert and CERT_VerifyCertificate. SEC_ERROR_BAD_SIGNATURE is one of SEVERAL AMBIGUOUS error codes, where the OCSP client uses the same error codes for OCSP failures as the cert chain validation code uses for cert failures. Other ambiguous errors include SEC_ERROR_BAD_DER, SEC_ERROR_INVALID_TIME, SEC_ERROR_EXTENSION_NOT_FOUND (aargh!). There are several ways this could be fixed. I suggest that the OCSP code never return an error code that is one of the ones also returned by the cert chain validity code when OCSP is not in use. That may mean the creation of some new error codes, or may mean overloading some existing OCSP specific error codes. I might suggest using SEC_ERROR_OCSP_MALFORMED_RESPONSE instead of SEC_ERROR_BAD_DER SEC_ERROR_CRL_EXPIRED instead of SEC_ERROR_INVALID_TIME SEC_ERROR_CRL_BAD_SIGNATURE instead of SEC_ERROR_BAD_SIGNATURE SEC_ERROR_EXTENSION_NOT_FOUND should never be returned by CERT_VerifyCert, and should be replaced with SEC_ERROR_CERT_BAD_ACCESS_LOCATION or, in the case where the EKU extension is not found, it should return SEC_ERROR_INADEQUATE_KEY_USAGE (however, see bug 58507 about this last point).
Comment 1•19 years ago
|
||
The approach that is currently in the tree for CRLs is that most errors that relate to the validity of the CRLs end up being considered as grounds for considering the certificate itself revoked. The logic is that if the user/administrator installed a CRL, he intended for a revocation check to be done, and if it couldn't be, then the cert is considered revoked . Thus, the CRLs error you list aren't actually returned by CERT_VerifyCert*. The story for OCSP is different, as it is possible for an application to call an OCSP checking API. CRL checking functions are not currently exposed from libnss. I believe there is an existing bug about this ambiguity for OCSP, but I can't find out at the moment, or I would mark it as duplicate.
Assignee | ||
Updated•19 years ago
|
QA Contact: bishakhabanerjee → jason.m.reid
Updated•19 years ago
|
Assignee: julien.pierre.bugs → wtchang
OS: Windows XP → All
Hardware: PC → All
Updated•19 years ago
|
Status: NEW → ASSIGNED
Comment 2•19 years ago
|
||
Reassigning back to me. Sorry, I was click-happy.
Assignee: wtchang → julien.pierre.bugs
Status: ASSIGNED → NEW
Updated•19 years ago
|
Priority: -- → P3
Target Milestone: --- → 3.12
Assignee | ||
Updated•18 years ago
|
QA Contact: jason.m.reid → libraries
Assignee | ||
Comment 3•18 years ago
|
||
Taking. The issue of Cert chain validation returning SEC_ERROR_EXTENSION_NOT_FOUND is becoming a hot issue at Sun. I examined the OCSP code today and found 6 places where it happens, representing 6 ways it can happen in http://lxr.mozilla.org/security/source/security/nss/lib/certhigh/ocsp.c Here are the 6 ways, and what I propose to do about them. 1. static Function ocsp_AddServiceLocatorExtension fails to find an AIA extension in the cert. (line 848) It then goes on to take remedial action and this is not considered a failure. It sets RV to SECSUccess, but it leaves this error code set. IT SHOULD NOT LEAVE THIS ERROR SET!! Called from ocsp_CreateSingleRequestList() at line 938, which is called from CERT_CreateOCSPRequest at line 1035 called from CERT_GetEncodedOCSPResponse at line 2354 called from CERT_CheckOCSPStatus at line 3525 ---> change to error 0 2. static Function ocsp_CertIsOCSPSigner fails to find EKU extension. (line 2420) called from ocsp_AuthorizedResponderForCertID at line 3015, called from ocsp_VerifySingleResponse at line 3152 called from CERT_GetOCSPStatusForCertID at line 3637 called from CERT_CheckOCSPStatus at line 3579 I propose to change this to set one of these other two error codes: ---> SEC_ERROR_OCSP_INVALID_SIGNING_CERT ---> SEC_ERROR_INADEQUATE_KEY_USAGE (but this seems ambiguous, too). 3. Function CERT_GetOCSPAuthorityInfoAccessLocation fails to find the AIA extension in the cert (line 3227) called from ocsp_GetResponderLocation at line 3341 called from CERT_CheckOCSPStatus at line 3481 ---> SEC_ERROR_CERT_BAD_ACCESS_LOCATION 4. Function CERT_GetOCSPAuthorityInfoAccessLocation finds the AIA extension but it did not include an OCSP method, (line 3253) called from ocsp_GetResponderLocation at line 3341 called from CERT_CheckOCSPStatus at line 3481 ---> SEC_ERROR_CERT_BAD_ACCESS_LOCATION 5. Function CERT_GetOCSPAuthorityInfoAccessLocation finds the AIA extension and it includes an OCSP method, but that method contains no URI. (line 3271) called from ocsp_GetResponderLocation at line 3341 called from CERT_CheckOCSPStatus at line 3481 ---> SEC_ERROR_CERT_BAD_ACCESS_LOCATION 6. When ocsp_GetResponderLocation() returns SEC_ERROR_EXTENSION_NOT_FOUND, function CERT_CheckOCSPStatus returns SECSuccess, but leaves the error code set. (line 3471) ---> change to error 0
Assignee: julien.pierre.bugs → nelson
Priority: P3 → P2
Whiteboard: Sun CR 6420859
Target Milestone: 3.12 → 3.11.3
Assignee | ||
Comment 4•18 years ago
|
||
Attachment #229616 -
Flags: review?(julien.pierre.bugs)
Assignee | ||
Comment 5•18 years ago
|
||
In writing the above patch, I found a path that returns SECFailure without setting any error code. It occured in ocsp_CertIsOCSPSigner() if the cert contains an EKU extension, but the EKU does not include the special OCSP_RESPONDER OID. My patch now sets SEC_ERROR_OCSP_INVALID_SIGNING_CERT in that case, also.
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•18 years ago
|
||
The previous patch turned all errors (including NO_MEMORY) in one functin into BAD_ACCESS_LOCATION. This patch is more exacting.
Attachment #229616 -
Attachment is obsolete: true
Attachment #229618 -
Flags: review?(julien.pierre.bugs)
Attachment #229616 -
Flags: review?(julien.pierre.bugs)
Updated•18 years ago
|
Attachment #229618 -
Flags: review?(julien.pierre.bugs) → review+
Assignee | ||
Comment 7•18 years ago
|
||
Checking in ocsp.c; new revision: 1.21.2.7; previous revision: 1.21.2.6 Checking in ocsp.c; new revision: 1.26; previous revision: 1.25 Marking Resolved/Fixed. Thanks for the quick review, Julien.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•