353530 - CERT_VerifyCertificate MUST report revoked regardless of usage, when cert is revoked

Status: UNCONFIRMED

(NSS :: Libraries, defect, P5)

Description

[Kai Engert (:KaiE:)]

This is actually a complaint about a wrong behaviour in PSM, but the cause is completely within NSS, so I'm filing this as an NSS bug.


With OCSP enabled, an attempt to use PSM's cert viewer to display a revoked cert results in the display text:
  could not verify for unknown reasons

(You must get the fix for bug 351897 / bug 351873 before you can reproduce)

This is unfortunate, the UI should say "revoked" and would do so, if NSS gave the correct error code.

But our current implementation in PSM and NSS asks to verify the cert for multiple usages at once. NSS does not abort the testing on an OCSP failure. Instead it goes on, and the final test happens to be a test for some uncertified usage. This causes the NSS function to end with the error code set to SEC_ERROR_INADEQUATE_CERT_TYPE.

I propose the NSS function CERT_VerifyCertificate exits its testing loop, as soon as the revocation is detected.

Does that make sense?
I assume a revocation has a higher priority than any other errors, and cert revocation is always global to all usages?

Comment 1

[Kai Engert (:KaiE:)]

Attachment: Patch v1

I am proposing this patch.

(I'm ok to use the indendation you suggest, for now I used one that is easy to read).

Comment 2

[Kai Engert (:KaiE:)]

FYI, I have tested this patch and PSM's UI does correctly display "cert revoked".

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Kai, I'll make you a deal.  

You fix PSM so that it doesn't show "unknown reasons" for SEC_ERROR_INADEQUATE_CERT_TYPE, or any other known NSS error code,
and we'll consider this suggested change.

I think CERT_VerifyCertificate should have a way to return a separate
error code for each requested usage.  The error code returned for any
given usage should not vary based on what other usages were also the 
subject of the inquiry.

Comment 4

[Kai Engert (:KaiE:)]

(In reply to comment #3)
> 
> You fix PSM so that it doesn't show "unknown reasons" for
> SEC_ERROR_INADEQUATE_CERT_TYPE, or any other known NSS error code,
> and we'll consider this suggested change.

:-)
Yes, you still have my promise that I work on better error messages for PSM this year. Now that 1.8 branch work comes to an end, I want to get this done very soon.


> I think CERT_VerifyCertificate should have a way to return a separate
> error code for each requested usage.  The error code returned for any
> given usage should not vary based on what other usages were also the 
> subject of the inquiry.

Maybe PSM should use the CERTVerifyLog approach that Bob explained to me recently. If PSM wants to decide that in a given situation, an OCSP failure has a higher priority than other verification failures, it could use the CERTVerifyLog, scan the list of failures, and detect the presence of any OCSP failure.

Comment 5

[Julien Pierre]

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

Kai, Nelson,

The CERTVerifyLog is supposed to be the way that would look at individual errors for different usages. I suggest that you explore that option. It is true that the revocation error will make all usages fail; however if we short-circuit them within CERT_VerifyCertificate because of this error, you wouldn't be able to get the detail of errors in the CERTVerifyLog for each usage. Please try it and see if CERTVerifyLog has what you need. If not, we can look at how to proceed next.

Comment 6

[Kai Engert (:KaiE:)]

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

I take it your comments mean r- on this patch. Obsoleting.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

I agree that, when a cert is revoked, no other problems with it matter. 
When CERT_VerifyCertificate has been asked to verify a cert for multiple
usages, and any one of them finds the cert to be revoked, the whole process
should stop there, and report that the cert is revoked.  Nothing else matters.
Reporting a problem with cert usage, when the cert is revoked, is potentially
seriously misleading!

Comment 8

[Aristotelis]

Attachment: My version of a patch

Hello,
I had the same problem. I believe that if the cert is revoked then it should stop checking for further usages. Just saying the cert cannot be verified for unknown reasons can be misleading, and perhaps someone just decides to ignore this error. 
I have attached my version of patch that at least what i tried in my OpenBSD system works ok! (The solution I took was that if the cert is revoked, then just stop any further processing and return the revocation error).

Comment 9

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

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.