407064 - pkix_pl_LdapCertStore_BuildCrlList should not fail if a crl fails to be decoded

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Alexei Volkov]

Attachment: Patch v1

Function pkix_pl_CRL_CreateToList is modified to return an error if failed to decode der crl and add crl into a list.

There are two callers of this function:
      pkix_pl_HttpCertStore_ProcessCrlResponse - it suppose to fail if received a bad CRL.
      pkix_pl_LdapCertStore_BuildCrlList - which suppose to skip a bad CRL and continue adding others to a list of CRL.

The bug is in pkix_pl_LdapCertStore_BuildCrlList: the function aborts generating crl list if one crl fails to be decoded. 

The attached patch makes the function skip all but fatal errors returned by pkix_pl_CRL_CreateToList.

Comment 1

[Alexei Volkov]

The patch has syntax error: 
+                                pkix_pl_CRL_CreateToList(derCrlItem, crlList,
+                                                         plContext);
                                                                   ^^^
and should be:
+                                pkix_pl_CRL_CreateToList(derCrlItem, crlList,
+                                                         plContext),

I patch is approved, the error will be fixed before committing the patch.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

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

A Patch that doesn't compile is a patch that isn't tested.
Do you have any way to test this patch?  
Can you get a bad CRL to mix with others in a test?

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

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

r=nelson, once the syntax error is fixed.

Comment 4

[Alexei Volkov]

We have many ldap tests that are part of libpkix test suite. I believe that some tests try to fetch crl including bad ones.

I've filed the bug 396599 to restore ldap testing of libpkix, but the issue is not yet resolved. 

Comment 5

[Alexei Volkov]

/cvsroot/mozilla/security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_ldapcertstore.c,v  <--  pkix_pl_ldapcertstore.c
new revision: 1.5; previous revision: 1.4