408903 - CERT_PKIXVerifyCert should do revocation checking of the leaf cert

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Kai Engert (:KaiE:)]

From bug 375019 comment 10:

"Looking at function CERT_VerifyCert, we see that revocation checking on 
the leaf cert is done in that function, after the call to CERT_VerifyCertChain
has returned successfully.  From that fact, we can infer that 
CERT_PKIXVerifyCert should likewise do the revocation checking on the leaf 
cert after libPKIX has been called to do the equivalent of
CERT_VerifyCertChain."

(you might want to read comment 8 and 9 in that bug, too)

Comment 1

[Kai Engert (:KaiE:)]

I propose this bug should target 3.12.

I'm tentatively adding NSS312B2 to the whiteboard to make it show up on the radar.

Comment 2

[Alexei Volkov]

I've verified, that the leaf cert is get checked for revocation by crl and ocsp checkers. Kai, could you please repeat the experiment. Make sure that ocspTurnedOnForChain is set to true. For that you need to allow ocsp test on the chain. It will enable ocsp checking for the whole chain and a leaf certificate. This is currently the only way to check the leaf cert with ocsp(until revocation controller is implemented to properly parse and interpreter all revocation flags).

Comment 3

[Kai Engert (:KaiE:)]

I think I can confirm what Alexei said.

PSM asks NSS to attempt OCSP on leaf and cert.

I tested using 
  https://www.digicert.com/images/footer-1-left-digicert-ssl.gif
which will load a single image and enabled OCSP tracing.

I used the debugger and set a breakpoint on CERT_PKIXVerifyCert

The image gets loaded, and I see NSS creates an OCSP request for the server cert (only).

Then the breakpoint at PKIXVerifyCert.

I stepped over the function, which results in two more attempts to do OCSP:
- for the intermediate cert 
- again for the server cert (which has the OCSP response already cached).


I think this is acceptable for PSM's current needs, because of the fact that PSM asks NSS to attempt OCSP on the chain.



However, I think NSS' behavior is quite surprising.
As an experiment, I changed PSM to not ask for OCSP on the chain.
As Alexei said, in that combination libpkix will not call into the OCSP code to check status for the leaf.

So, as Alexei said, if you want checking on the leaf, you must ask for checking the chain.

Comment 4

[Kai Engert (:KaiE:)]

I did another experiment, which is not strictly related to this bug.
I was curious to see

Comment 5

[Kai Engert (:KaiE:)]

Please see comment 3 if you were recently added to this bug.
To summarize, I'd declare this bug as WORKSFORME as far as PSM is concerned (but we must work to make the behavior match the parameters).

Please ignore comment 4.

Not strictly related to this bug, I did another another experiment.

I was interested what happens if the first OCSP request for the leaf cert succeeds (the one initiated by classic NSS as part of SSL),
while the second OCSP request for the leaf cert (the one initiated by CERT_PKIXVerifyCert) fails.
I made a simple experiment and changed the OCSP cache to return "failure" for each request where a cache entry is found.
Luckily, I get the expected result. CERT_PKIXVerifyCert returns the desired failure in this experiment (and PSM does not show EV).

Comment 6

[Alexei Volkov]

Setting the bug status as WorkForMe.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

PSM may not need CERT_PKIXVerifyCert to be able to send OCSP requests to 
validate leaf certs, because it is using the old CERT_VerifyCert* functions
first, which DO send the OCSP request.  

But this is still a deficiency of the CERT_PKIXVerifyCert function, and 
getting this bug fixed is still a prerequisite to being able to switch 
over entirely from the old to the new cert verification code.  

So, I'm reopening this bug and changing the target

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Also must do validity period checking

Comment 9

[Alexei Volkov]

Attachment: Patch v1 - make libpkix validate leaf cert

Comment 10

[Alexei Volkov]

Comment on attachment 371729 [details] [diff] [review]
Patch v1 - make libpkix validate leaf cert

Patch need to have some function name change and add comments to the code.

Comment 11

[Alexei Volkov]

Attachment: Patch v2 - make libpkix validate leaf cert(committed)

Address review comments.

Comment 12

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 371800 [details] [diff] [review]
Patch v2 - make libpkix validate leaf cert(committed)

r=nelson