This change is for NSS_LIBPKIX_BRANCH . In the interest of performance, we want to allow libpkix to make use of the existing NSS CRL cache. It will be used primarily as a content cache, for decoded CRLs and hash table of entries. The validation of CRLs will happen within libpkix. Some new functionality was identified as a requirement. The new CRL cache functionality will be private, as libpkix will be statically linked with libnss . The changes are : 1) At the CRLDPCache level, there should be a function to return all the CERTSignedCrl objects available for the particular issuer/DP. This function will need to acquire/release a cache lock, and increment the refcount on all the CERTSignedCrls . It should probably take an arena to allocate a NULL-termindated array of CERTSignedCrl*. 2) Some DPCache state information should be made avaialble to libpkix . The following errors are currently defined : #define CRL_CACHE_INVALID_CRLS 0x0001 /* this state will be set if we have CRL objects with an invalid DER or signature. Can be cleared if the invalid objects are deleted from the token */ #define CRL_CACHE_LAST_FETCH_FAILED 0x0002 /* this state will be set if the last CRL fetch encountered an error. Can be cleared if a new fetch succeeds */ #define CRL_CACHE_OUT_OF_MEMORY 0x0004 /* this state will be set if we don't have enough memory to build the hash table of entries */ The first error is irrelevant to libpkix, since it will do its own CRL validation. But it probably cares about the other errors . 3) There should be a function to return a CRL entry from a CERTSignedCrl* . A DPCache lock will need to be held before entering this function. This will be achieved by libpkix calling AcquireDPCache / ReleaseDPCache around it. This function will first check that the CERTSignedCrl still exists in the DPCache, and if it finds a CachedCrl for it, it will then use its hash table to find the entry. Otherwise, it will fail with a new error (SEC_ERROR_CRL_NO_LONGER_IN_CACHE ?), which should probably be a private code for libpkix. The reason this function shouldn't acquire the DPCache lock is for efficiency. libpkix might check for entries from multiple CERTSignedCrl coming from the same DPCache. It shouldn't need to acquire/release the cache lock on the DPCache for each lookup.
The new prototypes will be : /* this function assumes the caller holds a read lock on the DPCache */ (I changed my mind and think it is cleaner to have the caller call AcquireDPCache / ReleaseDPCache around this call too). SECStatus DPCache_GetAllCRLs(CRLDPCache* dpc, PRArenaPool* arena, CertSignedCrl*** crls, PRUint16* status) /* this function assumes the caller holds a read lock on the DPCache */ SECStatus DPCache_GetCRLEntry(CRLDPCache* dpc, CertSignedCrl* crl, SECItem* sn, CERTCrlEntry** returned) Having the CRL verification completely decoupled from the CRL cache has an unfortunate consequence . The CRL cache is now doing partial decoding of CRLs when it fetches them from a token or gets them from an application. It decodes them fully and builds the hash table of entries only after the CRL signature validation succeeds, and the CRL has been determined to be the most recent, and thus selected to be used for validations. Right now, only one hash table is ever kept. The hash table for an old CRL is freed when a new CRL appears. The CRL entries themselves cannot be freed once a CRL is fully decoded, because they are allocated in the CRL's arena. This behavior will need to change now, because the CRL cache won't know which CRLs libpkix will need to access. - The logic for which CRLs to cache will need to be changed. When libpkix requests an entry from a CRL, if it's not already been cached, a write lock will need to be acquired, and the CRL will need to be fully decoded, and a table created. This will be a significant loss in terms of memory footprint. It will not be a performance loss otherwise, except for the 1-time per issuer cost of building the tables. Proper scoping of this case should ensure that it doesn't affect applications that use CERT_VerifyCert but none of the new libpkix APIs. - the purging of CRL hash tables from old CRLs when a new CRL object appears will need to be eliminated, since the CRL cache won't know when libpkix might no longer need it. The hash table will still be purged when the cache detects that the object has been removed from a token, or the application called CERT_UncacheCRL .
Created attachment 188645 [details] [diff] [review] add new CRL cache functions Hanfei, I finished coding the new functions you need for libpkix. I have done no runtime testing of them, so I need feedback to see if they work and if they provided the expected increase in performance for the libpkix validation function. I checked in the patch to the NSS_LIBPKIX_BRANCH . It should also apply to a 3.10 tree; but not 3.9 .
There was a bug in DPCache_GetAllCrls in the attached patch. The fix is : - allcrls[index] = cachedcrl->crl; + allcrls[index] = SEC_DupCrl(cachedcrl->crl); } + *crls = allcrls; return SECSuccess; I have checked it in to NSS_LIBPKIX_BRANCH . Checking in crl.c; /cvsroot/mozilla/security/nss/lib/certdb/crl.c,v <-- crl.c new revision: 18.104.22.168; previous revision: 22.214.171.124
Created attachment 190184 [details] [diff] [review] define ReleaseDPCache in certi.h This function in needed in order for anyone to be able to call the new APIs defined in the previous patch . As checked in on NSS_LIBPKIX_BRANCH . Checking in certi.h; /cvsroot/mozilla/security/nss/lib/certdb/certi.h,v <-- certi.h new revision: 126.96.36.199; previous revision: 188.8.131.52 done
Comment on attachment 188645 [details] [diff] [review] add new CRL cache functions Removing review, since the code has been tested and works.
This bug was fixed as part of the checkin for bug 358785 .