Bug 1774657 Updating an old dbm with lots of certs with keys to sql results in a database that is slow to access.

This patch solves the problems in 3 ways:
1) The initial underlying issue is solved by not generating a trust record for user certs if they have default trust values (lib/softoken/legacydb/pcertdb.c).
This will cause new databases created from old dbm databases to function normally.

2) Skip the integrity check if the record we are reading is already the default trust value (lib/softoken/sftkdb.c). This will increase the performance of reading sqlite databases created from the old dbm databases before patch 1 to perform reasonably.

3) Increase the cashe count. (lib/softoken/lowpbe.c). This affects applications which do multiple private key operations on the same private keys. Usually high speed operations would copy the keys to a session key for better performance, but sometimes that's not possible. This allows up to 20 RSA keys to be references by the application without a performance hit from the PBE checking integrity and/or decrypting the key entry.

Differential Revision: https://phabricator.services.mozilla.com/D165221

8 files changed, 68 insertions, 8 deletions

diff --git a/lib/dev/ckhelper.c b/lib/dev/ckhelper.c
index 4f3972653..8ebf58fab 100644
--- a/lib/dev/ckhelper.c
+++ b/lib/dev/ckhelper.c
@@ -398,6 +398,9 @@ nssCryptokiTrust_GetAttributes(
     NSSSlot *slot;
     nssSession *session;
     CK_BBOOL isToken = PR_FALSE;
+    /* default values if the trust is record does not exist. In the highly
+     * unlikely case that these change, be sure to update softoken's
+     * 'sftkdb_isNullTrust()' function */
     CK_BBOOL stepUp = PR_FALSE;
     CK_TRUST saTrust = CKT_NSS_TRUST_UNKNOWN;
     CK_TRUST caTrust = CKT_NSS_TRUST_UNKNOWN;
diff --git a/lib/softoken/legacydb/pcertdb.c b/lib/softoken/legacydb/pcertdb.c
index 47778190d..488d0f06c 100644
--- a/lib/softoken/legacydb/pcertdb.c
+++ b/lib/softoken/legacydb/pcertdb.c
@@ -4277,6 +4277,7 @@ CreateTrust(void)
     if (trust) {
         trustListCount--;
         trustListHead = trust->next;
+        trust->next = NULL;
     }
     PORT_Assert(trustListCount >= 0);
     nsslowcert_UnlockFreeList();
@@ -5160,9 +5161,11 @@ nsslowcert_hasTrust(NSSLOWCERTCertTrust *trust)
     if (trust == NULL) {
         return PR_FALSE;
     }
-    return !((trust->sslFlags & CERTDB_TRUSTED_UNKNOWN) &&
-             (trust->emailFlags & CERTDB_TRUSTED_UNKNOWN) &&
-             (trust->objectSigningFlags & CERTDB_TRUSTED_UNKNOWN));
+    /* if we only have CERTDB__USER and CERTDB_TRUSTED_UNKNOWN bits, then
+     * we don't have a trust record. */
+    return !(((trust->sslFlags & ~(CERTDB_USER | CERTDB_TRUSTED_UNKNOWN)) == 0) &&
+             ((trust->emailFlags & ~(CERTDB_USER | CERTDB_TRUSTED_UNKNOWN)) == 0) &&
+             ((trust->objectSigningFlags & ~(CERTDB_USER | CERTDB_TRUSTED_UNKNOWN)) == 0));
 }
 
 /*
diff --git a/lib/softoken/lowpbe.c b/lib/softoken/lowpbe.c
index fae9e1828..ff80f573f 100644
--- a/lib/softoken/lowpbe.c
+++ b/lib/softoken/lowpbe.c
@@ -566,7 +566,7 @@ typedef struct KDFCacheItemStr KDFCacheItem;
 /* Bug 1606992 - Cache the hash result for the common case that we're
  * asked to repeatedly compute the key for the same password item,
  * hash, iterations and salt. */
-#define KDF2_CACHE_COUNT 3
+#define KDF2_CACHE_COUNT 150
 static struct {
     PZLock *lock;
     struct {
diff --git a/lib/softoken/sftkdb.c b/lib/softoken/sftkdb.c
index 8542a2d56..8827902ca 100644
--- a/lib/softoken/sftkdb.c
+++ b/lib/softoken/sftkdb.c
@@ -124,7 +124,6 @@ sftkdb_isAuthenticatedAttribute(CK_ATTRIBUTE_TYPE type)
     }
     return PR_FALSE;
 }
-
 /*
  * convert a native ULONG to a database ulong. Database ulong's
  * are all 4 byte big endian values.
@@ -141,7 +140,7 @@ sftk_ULong2SDBULong(unsigned char *data, CK_ULONG value)
 
 /*
  * convert a database ulong back to a native ULONG. (reverse of the above
- * function.
+ * function).
  */
 static CK_ULONG
 sftk_SDBULong2ULong(unsigned char *data)
@@ -155,6 +154,39 @@ sftk_SDBULong2ULong(unsigned char *data)
     return value;
 }
 
+/* certain trust records are default values, which are the values
+ * returned if the signature check fails anyway.
+ * In those cases, we can skip the signature check. */
+PRBool
+sftkdb_isNullTrust(const CK_ATTRIBUTE *template)
+{
+    switch (template->type) {
+        case CKA_TRUST_SERVER_AUTH:
+        case CKA_TRUST_CLIENT_AUTH:
+        case CKA_TRUST_EMAIL_PROTECTION:
+        case CKA_TRUST_CODE_SIGNING:
+            if (template->ulValueLen != SDB_ULONG_SIZE) {
+                break;
+            }
+            if (sftk_SDBULong2ULong(template->pValue) ==
+                CKT_NSS_TRUST_UNKNOWN) {
+                return PR_TRUE;
+            }
+            break;
+        case CKA_TRUST_STEP_UP_APPROVED:
+            if (template->ulValueLen != 1) {
+                break;
+            }
+            if (*((unsigned char *)(template->pValue)) == 0) {
+                return PR_TRUE;
+            }
+            break;
+        default:
+            break;
+    }
+    return PR_FALSE;
+}
+
 /*
  * fix up the input templates. Our fixed up ints are stored in data and must
  * be freed by the caller. The new template must also be freed. If there are no
@@ -415,7 +447,7 @@ sftkdb_fixupTemplateOut(CK_ATTRIBUTE *template, CK_OBJECT_HANDLE objectID,
             SECITEM_ZfreeItem(plainText, PR_TRUE);
         }
         /* make sure signed attributes are valid */
-        if (checkSig && sftkdb_isAuthenticatedAttribute(ntemplate[i].type)) {
+        if (checkSig && sftkdb_isAuthenticatedAttribute(ntemplate[i].type) && !sftkdb_isNullTrust(&ntemplate[i])) {
             SECStatus rv;
             CK_RV local_crv;
             SECItem signText;
@@ -2395,7 +2427,6 @@ sftkdb_mergeObject(SFTKDBHandle *handle, CK_OBJECT_HANDLE id,
 
     objectType = sftkdb_getULongFromTemplate(CKA_CLASS, ptemplate,
                                              max_attributes);
-
     /*
      * Update Object updates the object template if necessary then returns
      * whether or not we need to actually write the object out to our target
diff --git a/tests/dbtests/bigdb/cert9.db b/tests/dbtests/bigdb/cert9.db
new file mode 100644
index 000000000..461c26c41
--- /dev/null
+++ b/tests/dbtests/bigdb/cert9.db
diff --git a/tests/dbtests/bigdb/key4.db b/tests/dbtests/bigdb/key4.db
new file mode 100644
index 000000000..b45918d7f
--- /dev/null
+++ b/tests/dbtests/bigdb/key4.db
diff --git a/tests/dbtests/bigdb/pkcs11.txt b/tests/dbtests/bigdb/pkcs11.txt
new file mode 100644
index 000000000..adbb159ba
--- /dev/null
+++ b/tests/dbtests/bigdb/pkcs11.txt
@@ -0,0 +1,5 @@
+library=
+name=NSS Internal PKCS #11 Module
+parameters=configdir='sql:/tmp/nssdb' certPrefix='' keyPrefix='' secmod='secmod.db' flags= updatedir='' updateCertPrefix='' updateKeyPrefix='' updateid='' updateTokenDescription='' 
+NSS=trustOrder=75 cipherOrder=100 slotParams={0x00000001=[slotFlags=RSA,RC4,RC2,DES,DH,SHA1,MD5,MD2,SSL,TLS,AES,SHA256,SHA512,Camellia,SEED,ECC,RANDOM askpw=any timeout=30 ] }  Flags=internal,critical
+
diff --git a/tests/dbtests/dbtests.sh b/tests/dbtests/dbtests.sh
index 00740d092..b0b195d4d 100755
--- a/tests/dbtests/dbtests.sh
+++ b/tests/dbtests/dbtests.sh
@@ -51,6 +51,7 @@ dbtest_init()
   EMPTY_DIR=${HOSTDIR}/emptydir
   CONFLICT_DIR=${HOSTDIR}/conflictdir
   THREAD_DIR=${HOSTDIR}/threadir
+  BIG_DIR=${HOSTDIR}/bigdir
 
   html_head "CERT and Key DB Tests"
 
@@ -351,6 +352,23 @@ dbtest_main()
       *)
          html_failed "sdbthreadtst failed with an unrecognized error code."
       esac
+
+      # now verify that we can quickly dump a database that has explicit
+      # default trust values (generated by updating a dbm database with
+      # to a sql database with and older version of NSS).
+      mkdir -p ${BIG_DIR}
+      cp ${QADIR}/dbtests/bigdb/* ${BIG_DIR}/
+      echo "time certutil -K -d ${BIG_DIR} -f ${R_PWFILE}"
+      dtime=$(time -p (certutil -K -d ${BIG_DIR} -f ${R_PWFILE}) 2>&1 1>/dev/null)
+      echo "------------- time ----------------------"
+      echo $dtime
+      # now parse the real time to make sure it's subsecond
+      RARRAY=($dtime)
+      TIMEARRAY=(${RARRAY[1]//./ })
+      echo "${TIMEARRAY[0]} seconds"
+      test ${TIMEARRAY[0]} -lt 2
+      ret=$?
+      html_msg ${ret} 0 "certutil dump keys with explicit default trust flags"
     fi
 }