libpkix reports "unknown issuer" for nearly all certificate errors

RESOLVED FIXED in 3.12.3

Status

NSS
Libraries
P1
major
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: Nelson Bolyard (seldom reads bugmail), Assigned: Alexei Volkov)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: PKIX SUN_MUST_HAVE)

Attachments

(2 attachments, 1 obsolete attachment)

For most errors in certificates, libPKIX reports the error code that means
"unknown issuer" (a.k.a. "unrecognized issuer", "issuer not found").
This is very unhelpful, and we must fix it for 3.12.1, if possible.

NSS's old cert library reports error codes that are generally quite 
diagnostic, helpful to people to figure out what's wrong with their certs.  
That's an absolute requirement for NSS. NSS must continue to provide error
codes that tell the user what aspect of the cert chain to examine to find
the problem that needs correction.  

This bug blocks our ability to generally replace the old code in applications
that use the old cert path validation API (CERT_VerifyCert*), especially in
Firefox, but also in other server applications.  If those products would 
switch to using libPKIX while the problem remains, the NSS team would be 
overwhelmed with complaints, requesting help in finding problems in cert 
chains, because the real problems would no longer be apparent from the error
codes.

libPKIX's algorithm may be described as having an outer loop that is roughly:

   while (another cert path can be constructed) 
      test the new cert path

After every failure in the cert path test, it goes back and tries to 
construct another path.  So, the last failure it experiences before 
returning is nearly always a failure to find another issuer certificate.
This masks the potentially useful error code from the last path test 
by replacing it with the "issuer not found" error.

Somehow we must make libPKIX return the most relevant error code possible.
There are several ways we could use to attempt to do that.  Here are some:

1. Rank the error codes from most important to least important, and have
the rule that when going to set an error code, if the error code that is 
already set is more important than the new one, just leave it set as is,
and don't set the new error code.  The "unknown issuer" error would be 
last, or nearly last, in that ranking.

2. Change the code that attempts to set the "unknown issuer" error code, 
so that it checks for the existence of another error code, and only sets
the "unknown issuer" error code if no other error code has already been set.
This essentially changes the behavior of that single error code, and no 
others.

The above two ideas assume that error codes are set as (in the order) they
are experienced.  However, IINM, libPKIX builds a tree of information about
paths tested and errors experienced.  Perhaps an algorithm is needed that
searches that tree and decides what error is the most relevant from among
the errors in the tree.
I think this must be P1 for 3.12.1
Priority: -- → P1
Whiteboard: PKIX
(Assignee)

Comment 2

9 years ago
Need real chains(or ssl enabled servers) to make sure the fix works. Nelson, Christophe, please update the bug with chains that bogusly report "issue not found" error.   
(Assignee)

Comment 3

9 years ago
Created attachment 331217 [details] [diff] [review]
Patch v1 - not for review.

Tries to get info from verify node if error is not identified. But it actually does not improve error handling: if validation error was received it will be saved in finalError variable and in verify node tree as well. Does not make sense to search for errors in the verify node tree, unless we are looking for a highest priority error.
(Assignee)

Updated

9 years ago
Depends on: 448324
(Assignee)

Comment 4

9 years ago
Created attachment 331646 [details] [diff] [review]
Patch v1 - find occurred error (checked in)

patch tries to recover an error, that occurred earlier and was logged in to the verify node tree. The error search will be done only if final error(the pointer to error that happen during last stage of chain build)  still has NULL value.
Attachment #331217 - Attachment is obsolete: true
Attachment #331646 - Flags: review?(nelson)
Comment on attachment 331646 [details] [diff] [review]
Patch v1 - find occurred error (checked in)

This is probably a good first step in the right direction.
Attachment #331646 - Attachment description: Patch v1 - find occured error → Patch v1 - find occurred error
Attachment #331646 - Flags: review?(nelson) → review+
(Assignee)

Comment 6

9 years ago
(In reply to comment #5)
> (From update of attachment 331646 [details] [diff] [review])
commited
(Assignee)

Updated

9 years ago
Target Milestone: 3.12.1 → 3.12.2
(Assignee)

Updated

9 years ago
Whiteboard: PKIX → PKIX SUN_MUST_HAVE
(Reporter)

Updated

9 years ago
Attachment #331646 - Attachment description: Patch v1 - find occurred error → Patch v1 - find occurred error (checked in)
(Reporter)

Updated

9 years ago
Target Milestone: 3.12.2 → 3.12.3
(Assignee)

Comment 7

8 years ago
Created attachment 363348 [details] [diff] [review]
patch to pkix_build.c - update log with reported error(integrated)

Returned log is empty for errors that occurred due to untrusted or unknown issuer. This patch fixes it by setting proper code into verify node.
Attachment #363348 - Flags: review?(nelson)
Comment on attachment 363348 [details] [diff] [review]
patch to pkix_build.c - update log with reported error(integrated)

r=nelson
Attachment #363348 - Flags: review?(nelson) → review+
(Assignee)

Updated

8 years ago
Attachment #363348 - Attachment description: patch to pkix_build.c - update log with reported error → patch to pkix_build.c - update log with reported error(integrated)
What is the status of this bug?  Is it resolved?
(Assignee)

Comment 10

8 years ago
I don't think we need to do anything else at this point. Closing the bug.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.