414911 - CERT_PKIXVerifyCert says "good" with revoked intermediate cert

Status: RESOLVED INVALID

(NSS :: Libraries, defect)

Description

[Kai Engert (:KaiE:)]

I've changed the implementation of pkix_OcspChecker_Check to return SEC_ERROR_REVOKED_CERTIFICATE_OCSP if a certificate lacks an OCSP URL (for testing purposes).

With this change, CERT_PKIXVerifyCert still reports "success", although the parameters require OCSP.

I can see the revocation error arrives in pkix_CheckChain and 
pkixErrorCode gets set to PKIX_OCSPRESPONSESAYSCERTREVOKED.

However, later on this error gets ignored. 

In function pkix_BuildForwardDepthFirstSearch there is a call to PKIX_CHECK_ONLY_FATAL(pkix_Build_ValidateEntireChain...

This is where the error arrives. But apparently the revocation status is not being treated as fatal?

Comment 1

[Kai Engert (:KaiE:)]

Bug 375019 Patch v4 can be used to test this.
With the patch applied and used by Firefox, set a breakpoint on pkix_OcspChecker_Check after
  if (uriFound == PKIX_FALSE)

Try to visit an ev site like paypal.

Once you hit the breakpoint, follow the stack up and see how the failure gets ignored.

Instruct gdb to continue, and you'll get a valid EV UI.

Comment 2

[Alexei Volkov]

I could not reproduce the problem. Before the test, pkix_OcspChecker_Check was modified us suggested in comment #1. I've run tests validating paypal cert with an old CERT_VerifyCert function(with NSS_ENABLE_PKIX_VERIFY). Also used vfychain with applied patch from bug 412468. In both cases, the result was negative: cert was reported to be revoked.

Comment 3

[Kai Engert (:KaiE:)]

I apologize. This bug was my own fault, well, it was an unexpected side effect of the new OCSP caching for libpkix that I'm currently working on.

Alexei was unable to reproduce this code, thanks again for your help.

I confirm, when I run without the libpkix ocsp caching code, all works as expected.


For completeness, I will describe what happened my constructed test scenario:

- the intermediate lacking OCSP AIA gets tested for the first time
- libpkix checker detects the missing OCSP AIA
- the libpkix checker decides to remember a OCSP processing failure

I said earlier, I returned an error code of "revoked cert",
however, because I don't have a real ocsp response in my testing,
I've simply added a "processing failure" to the cache.

(If I had added a full revocation response to the cache, everything
would have been as expected, too.)

Now, the processing continues and eventually, libpkix requests to test the same intermediate using OCSP, again.

This time, we find a cache entry for the given cert.
We look at the status of the previous check.

Unfortunately, we're now given a "success" result.
The culprit is our global "OCSP soft failure mode".

When the cache is asked about the previous result, it finds we had a processing error. Because global OCSP mode is set to "soft failure", the OCSP cache returns a "success".


So, libpkix asks twice for OCSP checking of the same cert.
A single failure isn't sufficient to cause a failure in libpkix verification.
The libpkix processing code requires that both checks are consistent and both fail.
And I think we should indeed guarantee consistency here.


But how I can solve this problem?

Either we have to temporarily change the global failure mode "for the current thread only" (difficult) or we'd have to return more details about the exact status of the cache entry (no API yet).

As of today, when a fresh cache entry is found, the OCSP cache returns a single boolean flag only: success or failure.

(and "success" can mean, there was a failure, but we're in global soft failure mode).

Comment 4

[Kai Engert (:KaiE:)]

Are my findings relevant?

Yes, because the OCSP cache can hide an OCSP processing failure.

This is a problem for the strict revocation checking required for EV.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

EV's revocation checking requirements can be satisfied from the OCSP cache.
EV's requirements do NOT imply "Recheck the cert with the responder every 
time, and do not rely on any previously cached response."
If we are unable to connect to an OCSP responder, but we have a relevant 
unexpired OCSP response in the cache that says the cert is OK, then it's OK.
If we get back a new response that is negative (says cert is revoked), then
of course it should invalidate any previous cache entry.

Comment 6

[Kai Engert (:KaiE:)]

(In reply to comment #5)
> EV's revocation checking requirements can be satisfied from the OCSP cache.
> EV's requirements do NOT imply "Recheck the cert with the responder every 
> time, and do not rely on any previously cached response."
> If we are unable to connect to an OCSP responder, but we have a relevant 
> unexpired OCSP response in the cache that says the cert is OK, then it's OK.
> If we get back a new response that is negative (says cert is revoked), then
> of course it should invalidate any previous cache entry.

Sure, I absolutely agree.

My point was different.
The current OCSP cache might return "success", because the global mode is set to soft failure, circumventing the needs of the EV checking, which is interested in the "real" cached results (and not a "fake" positive result that is caused by the global soft failure setting).

During the last hours I've worked on improvements to the cache and I will attach an updated patch to bug 375019.


So, this bug has greatly helped me in improving the cache code.
Sorry for thinking out loud and for blaming libpkix.

This bug is invalid.