420991 - libPKIX returns wrong NSS error code

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

There are some certs attached to bug 390381 that constitute a chain.
The SSL server cert in that chain is now expired. 

Whenever I attempt to validate that chain with libPKIX using either of the 
two commands shown below, the validation fails (as expected).  The error code 
returned is  -8164: This certificate is not valid.

It SHOULD be -8181: Peer's Certificate has expired.

> NSS_ENABLE_PKIX_VERIFY=1 vfychain -u 1 -v cert.000 cert.001
That commend uses libPKIX the old CERT_VerifyCert API.

> vfychain -u 1 -v -p cert.000 cert.001
tests the above chain using the new CERT_PKIXVerifyCert function

This is Major because if NSS returns meaningless error codes, we will be 
INUNDATED with unwanted questions asking "what does this mean?"

Comment 1

[Alexei Volkov]

Generalizing the subject of the bug.

Found another case, when pkix return incorrect nss error code. Cert is attached.

Returns:
 -8174 - security library: bad database.

Should be:
 -8179: Peer's Certificate issuer is not recognized.

Comment 2

[Alexei Volkov]

Attachment: Cert1

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

There are many cases where the wrong error code is reported.  If we try to 
combine all of them into just one bug, that bug will never get completely
fixed.  So, let's create separate bugs for other cases where the wrong 
error code is returned.  

Comment 4

[Alexei Volkov]

Comment on attachment 312399 [details]
Cert1

libpkix now produce the correct error code while validating the cert.

Comment 5

[Alexei Volkov]

Found one more case when returned error code is incorrect. This happens when validating a cert that was not approved for a particular usage of a key defined in extended key usage extension. In this case libpkix report the default error code that tells that certificate issuer was not found. Instead, it should report

    -8101 = Certificate type not approved for application.

Comment 6

[Alexei Volkov]

Attachment: Patch v1 - make cert selector to report the reason why a cert was filtered out

The patch modifies cert selector to return pkix error every time it rejected a cert. Passing verify node into cert selector will help deliver multiple cert selector errors to the upper level of the code.

Comment 7

[Alexei Volkov]

Attachment: Patch v2 - make cert selector to report the reason why a cert was filtered out

Code is adjusted to new changes to cert selector api.

Comment 8

[Alexei Volkov]

A memory leak was found during the review. r-

Comment 9

[Alexei Volkov]

Attachment: Patch v3 - make cert selector to report the reason why a cert was filtered out (checked in)

Fix memory leak(leaking pkix error object) that may happen in case of error returned by cert selector.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 371749 [details] [diff] [review]
Patch v3 - make cert selector to report the reason why a cert was filtered out (checked in)

r=nelson

Comment 11

[Julien Pierre]

Alexei,
Your checkin broke the build.

cc -o SunOS5.10_i86pc_DBG.OBJ/pkix_targetcertchecker.o -c -g -KPIC -DSVR4 -DSYSV -D__svr4 -D__svr4__ -DSOLARIS -D_REENTRANT -Di386 -DSOLARIS2_10 -D_SVID_GETTOD  -xs -DXP_UNIX -DDEBUG -UNDEBUG -DDEBUG_jp96085 -DNSS_ENABLE_ECC -DNSS_ECC_MORE_THAN_SUITE_B -DUSE_UTIL_DIRECTLY -I/usr/dt/include -I/usr/openwin/include -I../../../../../../dist/SunOS5.10_i86pc_DBG.OBJ/include -I../../../../../../dist/public/nss -I../../../../../../dist/private/nss -I../../../../../../dist/public/dbm  pkix_targetcertchecker.c
"pkix_targetcertchecker.c", line 382: prototype mismatch: 4 args passed, 3 expected
cc: acomp failed for pkix_targetcertchecker.c
gmake[4]: *** [SunOS5.10_i86pc_DBG.OBJ/pkix_targetcertchecker.o] Error 2
gmake[4]: Leaving directory `/h/monstre/export/home/julien/nss/virgin/mozilla/security/nss/lib/libpkix/pkix/checker'
gmake[3]: *** [libs] Error 2
gmake[3]: Leaving directory `/h/monstre/export/home/julien/nss/virgin/mozilla/security/nss/lib/libpkix/pkix'
gmake[2]: *** [libs] Error 2
gmake[2]: Leaving directory `/h/monstre/export/home/julien/nss/virgin/mozilla/security/nss/lib/libpkix'
gmake[1]: *** [libs] Error 2
gmake[1]: Leaving directory `/h/monstre/export/home/julien/nss/virgin/mozilla/security/nss/lib'
gmake: *** [libs] Error 2
28.57u 5.02s 0:52.22 64.3%

Comment 12

[Julien Pierre]

Attachment: Fix build

I am not sure if this is the correct fix, and I have not tested the runtime yet. But at least it will build.

Comment 13

[Julien Pierre]

Comment on attachment 372130 [details] [diff] [review]
Fix build

Cancelling review, since Alexei checked in a fix.
all.sh is green on my machine (without IOPR or NIST PKITS tests).