Bug 1612881 - Maintain PKCS11 C_GetAttributeValue semantics on attributes that lack NSS database columns r=keeler,rrelyea

`sdb_GetAttributeValueNoLock` builds a query string from a list of attributes in the input template. Unfortunately, `sqlite3_prepare_v2` will fail the entire query if one of the attributes is missing from the underlying table. The PKCS #11 spec [[ https://www.cryptsoft.com/pkcs11doc/v220/pkcs11__all_8h.html#aC_GetAttributeValue | requires ]] setting the output `ulValueLen` field to -1 for such invalid attributes.

This patch reads and stores the columns of nssPublic/nssPrivate when opened, then filters an input template in `sdb_GetAttributeValueNoLock` for unbacked/invalid attributes, removing them from the query and setting their template output lengths to -1.

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

6 files changed, 441 insertions, 62 deletions

diff --git a/automation/abi-check/expected-report-libnss3.so.txt b/automation/abi-check/expected-report-libnss3.so.txt
index 1dbcfa1be..54c5d0026 100644
--- a/automation/abi-check/expected-report-libnss3.so.txt
+++ b/automation/abi-check/expected-report-libnss3.so.txt
@@ -1,8 +1,9 @@
-6 Added functions:
+7 Added functions:
 
   [A] 'function SECStatus PK11_AEADOp(PK11Context*, CK_GENERATOR_FUNCTION, int, unsigned char*, int, const unsigned char*, int, unsigned char*, int*, int, unsigned char*, int, const unsigned char*, int)'    {PK11_AEADOp@@NSS_3.52}
   [A] 'function SECStatus PK11_AEADRawOp(PK11Context*, void*, int, const unsigned char*, int, unsigned char*, int*, int, const unsigned char*, int)'    {PK11_AEADRawOp@@NSS_3.52}
   [A] 'function CK_OBJECT_HANDLE PK11_GetObjectHandle(PK11ObjectType, void*, PK11SlotInfo**)'    {PK11_GetObjectHandle@@NSS_3.52}
+  [A] 'function SECStatus PK11_ReadRawAttributes(PLArenaPool*, PK11ObjectType, void*, CK_ATTRIBUTE*, unsigned int)'    {PK11_ReadRawAttributes@@NSS_3.52}
   [A] 'function SECStatus PK11_SymKeysToSameSlot(CK_MECHANISM_TYPE, CK_ATTRIBUTE_TYPE, CK_ATTRIBUTE_TYPE, PK11SymKey*, PK11SymKey*, PK11SymKey**, PK11SymKey**)'    {PK11_SymKeysToSameSlot@@NSS_3.52}
   [A] 'function PRBool _PK11_ContextGetAEADSimulation(PK11Context*)'    {_PK11_ContextGetAEADSimulation@@NSS_3.52}
   [A] 'function SECStatus _PK11_ContextSetAEADSimulation(PK11Context*)'    {_PK11_ContextSetAEADSimulation@@NSS_3.52}
diff --git a/gtests/softoken_gtest/softoken_gtest.cc b/gtests/softoken_gtest/softoken_gtest.cc
index 17949800a..f8b5abb44 100644
--- a/gtests/softoken_gtest/softoken_gtest.cc
+++ b/gtests/softoken_gtest/softoken_gtest.cc
@@ -100,6 +100,175 @@ static const CK_ATTRIBUTE attributes[] = {
     {CKA_TRUST_STEP_UP_APPROVED, (void *)&ck_false,
      (PRUint32)sizeof(CK_BBOOL)}};
 
+TEST_F(SoftokenTest, GetInvalidAttribute) {
+  ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
+  ASSERT_TRUE(slot);
+  EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, "password"));
+  ScopedPK11GenericObject obj(PK11_CreateGenericObject(
+      slot.get(), attributes, PR_ARRAY_SIZE(attributes), true));
+  ASSERT_NE(nullptr, obj);
+  SECItem out = {siBuffer, nullptr, 0};
+  SECStatus rv = PK11_ReadRawAttribute(PK11_TypeGeneric, obj.get(),
+                                       CKA_ALLOWED_MECHANISMS, &out);
+  EXPECT_EQ(SECFailure, rv);
+  // CKR_ATTRIBUTE_TYPE_INVALID maps to SEC_ERROR_BAD_DATA.
+  EXPECT_EQ(SEC_ERROR_BAD_DATA, PORT_GetError());
+}
+
+TEST_F(SoftokenTest, GetValidAttributes) {
+  ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
+  ASSERT_TRUE(slot);
+  EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, "password"));
+  ScopedPK11GenericObject obj(PK11_CreateGenericObject(
+      slot.get(), attributes, PR_ARRAY_SIZE(attributes), true));
+  ASSERT_NE(nullptr, obj);
+
+  CK_ATTRIBUTE template_attrs[] = {
+      {CKA_LABEL, NULL, 0},
+      {CKA_CERT_SHA1_HASH, NULL, 0},
+      {CKA_ISSUER, NULL, 0},
+  };
+  SECStatus rv =
+      PK11_ReadRawAttributes(nullptr, PK11_TypeGeneric, obj.get(),
+                             template_attrs, PR_ARRAY_SIZE(template_attrs));
+  EXPECT_EQ(SECSuccess, rv);
+  ASSERT_EQ(attributes[4].ulValueLen, template_attrs[0].ulValueLen);
+  EXPECT_EQ(0, memcmp(attributes[4].pValue, template_attrs[0].pValue,
+                      template_attrs[0].ulValueLen));
+  ASSERT_EQ(attributes[5].ulValueLen, template_attrs[1].ulValueLen);
+  EXPECT_EQ(0, memcmp(attributes[5].pValue, template_attrs[1].pValue,
+                      template_attrs[1].ulValueLen));
+  ASSERT_EQ(attributes[7].ulValueLen, template_attrs[2].ulValueLen);
+  EXPECT_EQ(0, memcmp(attributes[7].pValue, template_attrs[2].pValue,
+                      template_attrs[2].ulValueLen));
+  for (unsigned int i = 0; i < PR_ARRAY_SIZE(template_attrs); i++) {
+    PORT_Free(template_attrs[i].pValue);
+  }
+}
+
+TEST_F(SoftokenTest, GetOnlyInvalidAttributes) {
+  ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
+  ASSERT_TRUE(slot);
+  EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, "password"));
+  ScopedPK11GenericObject obj(PK11_CreateGenericObject(
+      slot.get(), attributes, PR_ARRAY_SIZE(attributes), true));
+  ASSERT_NE(nullptr, obj);
+
+  // Provide buffers of sufficient size, so that token
+  // will write the data. This is annoying, but PK11_GetAttributes
+  // won't allocate in the cases below when a single attribute
+  // is missing. So, just put them all on the stack.
+  unsigned char buf1[100];
+  unsigned char buf2[100];
+  CK_ATTRIBUTE template_attrs[] = {{0xffffffffUL, buf1, sizeof(buf1)},
+                                   {0xfffffffeUL, buf2, sizeof(buf2)}};
+  SECStatus rv =
+      PK11_ReadRawAttributes(nullptr, PK11_TypeGeneric, obj.get(),
+                             template_attrs, PR_ARRAY_SIZE(template_attrs));
+  EXPECT_EQ(SECFailure, rv);
+  EXPECT_EQ(SEC_ERROR_BAD_DATA, PORT_GetError());
+
+  // MSVC rewards -1UL with a C4146 warning...
+  ASSERT_EQ(0UL, template_attrs[0].ulValueLen + 1);
+  ASSERT_EQ(0UL, template_attrs[1].ulValueLen + 1);
+}
+
+TEST_F(SoftokenTest, GetAttributesInvalidInterspersed1) {
+  ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
+  ASSERT_TRUE(slot);
+  EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, "password"));
+  ScopedPK11GenericObject obj(PK11_CreateGenericObject(
+      slot.get(), attributes, PR_ARRAY_SIZE(attributes), true));
+  ASSERT_NE(nullptr, obj);
+
+  unsigned char buf1[100];
+  unsigned char buf2[100];
+  unsigned char buf3[200];
+  CK_ATTRIBUTE template_attrs[] = {{0xffffffff, buf1, sizeof(buf1)},
+                                   {CKA_CERT_SHA1_HASH, buf2, sizeof(buf2)},
+                                   {CKA_ISSUER, buf3, sizeof(buf3)}};
+  SECStatus rv =
+      PK11_ReadRawAttributes(nullptr, PK11_TypeGeneric, obj.get(),
+                             template_attrs, PR_ARRAY_SIZE(template_attrs));
+  EXPECT_EQ(SECFailure, rv);
+  EXPECT_EQ(SEC_ERROR_BAD_DATA, PORT_GetError());
+  ASSERT_EQ(0UL, template_attrs[0].ulValueLen + 1);
+  ASSERT_EQ(attributes[5].ulValueLen, template_attrs[1].ulValueLen);
+  EXPECT_EQ(0, memcmp(attributes[5].pValue, template_attrs[1].pValue,
+                      template_attrs[1].ulValueLen));
+  ASSERT_EQ(attributes[7].ulValueLen, template_attrs[2].ulValueLen);
+  EXPECT_EQ(0, memcmp(attributes[7].pValue, template_attrs[2].pValue,
+                      template_attrs[2].ulValueLen));
+}
+
+TEST_F(SoftokenTest, GetAttributesInvalidInterspersed2) {
+  ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
+  ASSERT_TRUE(slot);
+  EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, "password"));
+  ScopedPK11GenericObject obj(PK11_CreateGenericObject(
+      slot.get(), attributes, PR_ARRAY_SIZE(attributes), true));
+  ASSERT_NE(nullptr, obj);
+
+  unsigned char buf1[100];
+  unsigned char buf2[100];
+  unsigned char buf3[100];
+  CK_ATTRIBUTE template_attrs[] = {{CKA_LABEL, buf1, sizeof(buf1)},
+                                   {CKA_CERT_SHA1_HASH, buf2, sizeof(buf2)},
+                                   {0xffffffffUL, buf3, sizeof(buf3)}};
+  SECStatus rv =
+      PK11_ReadRawAttributes(nullptr, PK11_TypeGeneric, obj.get(),
+                             template_attrs, PR_ARRAY_SIZE(template_attrs));
+  EXPECT_EQ(SECFailure, rv);
+  EXPECT_EQ(SEC_ERROR_BAD_DATA, PORT_GetError());
+  ASSERT_EQ(attributes[4].ulValueLen, template_attrs[0].ulValueLen);
+  EXPECT_EQ(0, memcmp(attributes[4].pValue, template_attrs[0].pValue,
+                      template_attrs[0].ulValueLen));
+  ASSERT_EQ(attributes[5].ulValueLen, template_attrs[1].ulValueLen);
+  EXPECT_EQ(0, memcmp(attributes[5].pValue, template_attrs[1].pValue,
+                      template_attrs[1].ulValueLen));
+  ASSERT_EQ(0UL, template_attrs[2].ulValueLen + 1);
+}
+
+TEST_F(SoftokenTest, GetAttributesInvalidInterspersed3) {
+  ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
+  ASSERT_TRUE(slot);
+  EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, "password"));
+  ScopedPK11GenericObject obj(PK11_CreateGenericObject(
+      slot.get(), attributes, PR_ARRAY_SIZE(attributes), true));
+  ASSERT_NE(nullptr, obj);
+
+  unsigned char buf1[100];
+  unsigned char buf2[100];
+  unsigned char buf3[100];
+  unsigned char buf4[100];
+  unsigned char buf5[100];
+  unsigned char buf6[200];
+  CK_ATTRIBUTE template_attrs[6] = {{CKA_LABEL, buf1, sizeof(buf1)},
+                                    {0xffffffffUL, buf2, sizeof(buf2)},
+                                    {0xfffffffeUL, buf3, sizeof(buf3)},
+                                    {CKA_CERT_SHA1_HASH, buf4, sizeof(buf4)},
+                                    {0xfffffffdUL, buf5, sizeof(buf5)},
+                                    {CKA_ISSUER, buf6, sizeof(buf6)}};
+  SECStatus rv =
+      PK11_ReadRawAttributes(nullptr, PK11_TypeGeneric, obj.get(),
+                             template_attrs, PR_ARRAY_SIZE(template_attrs));
+  EXPECT_EQ(SECFailure, rv);
+  EXPECT_EQ(SEC_ERROR_BAD_DATA, PORT_GetError());
+
+  ASSERT_EQ(attributes[4].ulValueLen, template_attrs[0].ulValueLen);
+  EXPECT_EQ(0, memcmp(attributes[4].pValue, template_attrs[0].pValue,
+                      template_attrs[0].ulValueLen));
+  ASSERT_EQ(0UL, template_attrs[1].ulValueLen + 1);
+  ASSERT_EQ(0UL, template_attrs[2].ulValueLen + 1);
+  ASSERT_EQ(attributes[5].ulValueLen, template_attrs[3].ulValueLen);
+  EXPECT_EQ(0, memcmp(attributes[5].pValue, template_attrs[3].pValue,
+                      template_attrs[3].ulValueLen));
+  ASSERT_EQ(0UL, template_attrs[4].ulValueLen + 1);
+  ASSERT_EQ(attributes[7].ulValueLen, template_attrs[5].ulValueLen);
+  EXPECT_EQ(0, memcmp(attributes[7].pValue, template_attrs[5].pValue,
+                      template_attrs[5].ulValueLen));
+}
+
 TEST_F(SoftokenTest, CreateObjectNonEmptyPassword) {
   ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
   ASSERT_TRUE(slot);
diff --git a/lib/nss/nss.def b/lib/nss/nss.def
index 88572fa7b..49b8c1c1b 100644
--- a/lib/nss/nss.def
+++ b/lib/nss/nss.def
@@ -1170,6 +1170,7 @@ _PK11_ContextSetAEADSimulation;
 PK11_AEADOp;
 PK11_AEADRawOp;
 PK11_GetObjectHandle;
+PK11_ReadRawAttributes;
 PK11_SymKeysToSameSlot;
 ;+    local:
 ;+       *;
diff --git a/lib/pk11wrap/pk11obj.c b/lib/pk11wrap/pk11obj.c
index 78c46dfdb..37eea41f8 100644
--- a/lib/pk11wrap/pk11obj.c
+++ b/lib/pk11wrap/pk11obj.c
@@ -1783,6 +1783,26 @@ PK11_ReadRawAttribute(PK11ObjectType objType, void *objSpec,
     return PK11_ReadAttribute(slot, handle, attrType, NULL, item);
 }
 
+SECStatus
+PK11_ReadRawAttributes(PLArenaPool *arena, PK11ObjectType objType, void *objSpec,
+                       CK_ATTRIBUTE *pTemplate, unsigned int count)
+{
+    PK11SlotInfo *slot = NULL;
+    CK_OBJECT_HANDLE handle = 0;
+
+    handle = PK11_GetObjectHandle(objType, objSpec, &slot);
+    if (handle == CK_INVALID_HANDLE) {
+        PORT_SetError(SEC_ERROR_UNKNOWN_OBJECT_TYPE);
+        return SECFailure;
+    }
+    CK_RV crv = PK11_GetAttributes(arena, slot, handle, pTemplate, count);
+    if (crv != CKR_OK) {
+        PORT_SetError(PK11_MapError(crv));
+        return SECFailure;
+    }
+    return SECSuccess;
+}
+
 /*
  * return the object handle that matches the template
  */
diff --git a/lib/pk11wrap/pk11pub.h b/lib/pk11wrap/pk11pub.h
index e766e751d..bc6c3169e 100644
--- a/lib/pk11wrap/pk11pub.h
+++ b/lib/pk11wrap/pk11pub.h
@@ -893,6 +893,8 @@ PK11GenericObject *PK11_CreateGenericObject(PK11SlotInfo *slot,
  */
 SECStatus PK11_ReadRawAttribute(PK11ObjectType type, void *object,
                                 CK_ATTRIBUTE_TYPE attr, SECItem *item);
+SECStatus PK11_ReadRawAttributes(PLArenaPool *arena, PK11ObjectType type, void *object,
+                                 CK_ATTRIBUTE *pTemplate, unsigned int count);
 SECStatus PK11_WriteRawAttribute(PK11ObjectType type, void *object,
                                  CK_ATTRIBUTE_TYPE attr, SECItem *item);
 /* get the PKCS #11 handle and slot for a generic object */
diff --git a/lib/softoken/sdb.c b/lib/softoken/sdb.c
index a908d1a58..de0fd1f12 100644
--- a/lib/softoken/sdb.c
+++ b/lib/softoken/sdb.c
@@ -90,38 +90,6 @@ typedef enum {
 #define SDB_MAX_BUSY_RETRIES 10
 
 /*
- * Note on use of sqlReadDB: Only one thread at a time may have an actual
- * operation going on given sqlite3 * database. An operation is defined as
- * the time from a sqlite3_prepare() until the sqlite3_finalize().
- * Multiple sqlite3 * databases can be open and have simultaneous operations
- * going. We use the sqlXactDB for all write operations. This database
- * is only opened when we first create a transaction and closed when the
- * transaction is complete. sqlReadDB is open when we first opened the database
- * and is used for all read operation. It's use is protected by a monitor. This
- * is because an operation can span the use of FindObjectsInit() through the
- * call to FindObjectsFinal(). In the intermediate time it is possible to call
- * other operations like NSC_GetAttributeValue */
-
-struct SDBPrivateStr {
-    char *sqlDBName;               /* invariant, path to this database */
-    sqlite3 *sqlXactDB;            /* access protected by dbMon, use protected
-                                    * by the transaction. Current transaction db*/
-    PRThread *sqlXactThread;       /* protected by dbMon,
-                                    * current transaction thread */
-    sqlite3 *sqlReadDB;            /* use protected by dbMon, value invariant */
-    PRIntervalTime lastUpdateTime; /* last time the cache was updated */
-    PRIntervalTime updateInterval; /* how long the cache can go before it
-                                    * must be updated again */
-    sdbDataType type;              /* invariant, database type */
-    char *table;                   /* invariant, SQL table which contains the db */
-    char *cacheTable;              /* invariant, SQL table cache of db */
-    PRMonitor *dbMon;              /* invariant, monitor to protect
-                                    * sqlXact* fields, and use of the sqlReadDB */
-};
-
-typedef struct SDBPrivateStr SDBPrivate;
-
-/*
  * known attributes
  */
 static const CK_ATTRIBUTE_TYPE known_attributes[] = {
@@ -135,35 +103,68 @@ static const CK_ATTRIBUTE_TYPE known_attributes[] = {
     CKA_VERIFY, CKA_VERIFY_RECOVER, CKA_DERIVE, CKA_START_DATE, CKA_END_DATE,
     CKA_MODULUS, CKA_MODULUS_BITS, CKA_PUBLIC_EXPONENT, CKA_PRIVATE_EXPONENT,
     CKA_PRIME_1, CKA_PRIME_2, CKA_EXPONENT_1, CKA_EXPONENT_2, CKA_COEFFICIENT,
-    CKA_PRIME, CKA_SUBPRIME, CKA_BASE, CKA_PRIME_BITS,
+    CKA_PUBLIC_KEY_INFO, CKA_PRIME, CKA_SUBPRIME, CKA_BASE, CKA_PRIME_BITS,
     CKA_SUB_PRIME_BITS, CKA_VALUE_BITS, CKA_VALUE_LEN, CKA_EXTRACTABLE,
     CKA_LOCAL, CKA_NEVER_EXTRACTABLE, CKA_ALWAYS_SENSITIVE,
     CKA_KEY_GEN_MECHANISM, CKA_MODIFIABLE, CKA_EC_PARAMS,
     CKA_EC_POINT, CKA_SECONDARY_AUTH, CKA_AUTH_PIN_FLAGS,
-    CKA_ALWAYS_AUTHENTICATE, CKA_WRAP_WITH_TRUSTED, CKA_WRAP_TEMPLATE,
-    CKA_UNWRAP_TEMPLATE, CKA_HW_FEATURE_TYPE, CKA_RESET_ON_INIT,
-    CKA_HAS_RESET, CKA_PIXEL_X, CKA_PIXEL_Y, CKA_RESOLUTION, CKA_CHAR_ROWS,
-    CKA_CHAR_COLUMNS, CKA_COLOR, CKA_BITS_PER_PIXEL, CKA_CHAR_SETS,
-    CKA_ENCODING_METHODS, CKA_MIME_TYPES, CKA_MECHANISM_TYPE,
-    CKA_REQUIRED_CMS_ATTRIBUTES, CKA_DEFAULT_CMS_ATTRIBUTES,
-    CKA_SUPPORTED_CMS_ATTRIBUTES, CKA_NSS_URL, CKA_NSS_EMAIL,
-    CKA_NSS_SMIME_INFO, CKA_NSS_SMIME_TIMESTAMP,
+    CKA_ALWAYS_AUTHENTICATE, CKA_WRAP_WITH_TRUSTED, CKA_HW_FEATURE_TYPE,
+    CKA_RESET_ON_INIT, CKA_HAS_RESET, CKA_PIXEL_X, CKA_PIXEL_Y,
+    CKA_RESOLUTION, CKA_CHAR_ROWS, CKA_CHAR_COLUMNS, CKA_COLOR,
+    CKA_BITS_PER_PIXEL, CKA_CHAR_SETS, CKA_ENCODING_METHODS, CKA_MIME_TYPES,
+    CKA_MECHANISM_TYPE, CKA_REQUIRED_CMS_ATTRIBUTES,
+    CKA_DEFAULT_CMS_ATTRIBUTES, CKA_SUPPORTED_CMS_ATTRIBUTES,
+    CKA_WRAP_TEMPLATE, CKA_UNWRAP_TEMPLATE, CKA_NSS_TRUST, CKA_NSS_URL,
+    CKA_NSS_EMAIL, CKA_NSS_SMIME_INFO, CKA_NSS_SMIME_TIMESTAMP,
     CKA_NSS_PKCS8_SALT, CKA_NSS_PASSWORD_CHECK, CKA_NSS_EXPIRES,
     CKA_NSS_KRL, CKA_NSS_PQG_COUNTER, CKA_NSS_PQG_SEED,
     CKA_NSS_PQG_H, CKA_NSS_PQG_SEED_BITS, CKA_NSS_MODULE_SPEC,
-    CKA_TRUST_DIGITAL_SIGNATURE, CKA_TRUST_NON_REPUDIATION,
-    CKA_TRUST_KEY_ENCIPHERMENT, CKA_TRUST_DATA_ENCIPHERMENT,
-    CKA_TRUST_KEY_AGREEMENT, CKA_TRUST_KEY_CERT_SIGN, CKA_TRUST_CRL_SIGN,
-    CKA_TRUST_SERVER_AUTH, CKA_TRUST_CLIENT_AUTH, CKA_TRUST_CODE_SIGNING,
-    CKA_TRUST_EMAIL_PROTECTION, CKA_TRUST_IPSEC_END_SYSTEM,
-    CKA_TRUST_IPSEC_TUNNEL, CKA_TRUST_IPSEC_USER, CKA_TRUST_TIME_STAMPING,
-    CKA_TRUST_STEP_UP_APPROVED, CKA_CERT_SHA1_HASH, CKA_CERT_MD5_HASH,
-    CKA_NSS_DB, CKA_NSS_TRUST, CKA_NSS_OVERRIDE_EXTENSIONS,
-    CKA_PUBLIC_KEY_INFO, CKA_NSS_SERVER_DISTRUST_AFTER, CKA_NSS_EMAIL_DISTRUST_AFTER
+    CKA_NSS_OVERRIDE_EXTENSIONS, CKA_NSS_SERVER_DISTRUST_AFTER,
+    CKA_NSS_EMAIL_DISTRUST_AFTER, CKA_TRUST_DIGITAL_SIGNATURE,
+    CKA_TRUST_NON_REPUDIATION, CKA_TRUST_KEY_ENCIPHERMENT,
+    CKA_TRUST_DATA_ENCIPHERMENT, CKA_TRUST_KEY_AGREEMENT,
+    CKA_TRUST_KEY_CERT_SIGN, CKA_TRUST_CRL_SIGN, CKA_TRUST_SERVER_AUTH,
+    CKA_TRUST_CLIENT_AUTH, CKA_TRUST_CODE_SIGNING, CKA_TRUST_EMAIL_PROTECTION,
+    CKA_TRUST_IPSEC_END_SYSTEM, CKA_TRUST_IPSEC_TUNNEL, CKA_TRUST_IPSEC_USER,
+    CKA_TRUST_TIME_STAMPING, CKA_TRUST_STEP_UP_APPROVED, CKA_CERT_SHA1_HASH,
+    CKA_CERT_MD5_HASH, CKA_NSS_DB
+};
+
+static const int known_attributes_size = PR_ARRAY_SIZE(known_attributes);
+
+/*
+ * Note on use of sqlReadDB: Only one thread at a time may have an actual
+ * operation going on given sqlite3 * database. An operation is defined as
+ * the time from a sqlite3_prepare() until the sqlite3_finalize().
+ * Multiple sqlite3 * databases can be open and have simultaneous operations
+ * going. We use the sqlXactDB for all write operations. This database
+ * is only opened when we first create a transaction and closed when the
+ * transaction is complete. sqlReadDB is open when we first opened the database
+ * and is used for all read operation. It's use is protected by a monitor. This
+ * is because an operation can span the use of FindObjectsInit() through the
+ * call to FindObjectsFinal(). In the intermediate time it is possible to call
+ * other operations like NSC_GetAttributeValue */
+
+struct SDBPrivateStr {
+    char *sqlDBName;                /* invariant, path to this database */
+    sqlite3 *sqlXactDB;             /* access protected by dbMon, use protected
+                                    * by the transaction. Current transaction db*/
+    PRThread *sqlXactThread;        /* protected by dbMon,
+                                    * current transaction thread */
+    sqlite3 *sqlReadDB;             /* use protected by dbMon, value invariant */
+    PRIntervalTime lastUpdateTime;  /* last time the cache was updated */
+    PRIntervalTime updateInterval;  /* how long the cache can go before it
+                                    * must be updated again */
+    sdbDataType type;               /* invariant, database type */
+    char *table;                    /* invariant, SQL table which contains the db */
+    char *cacheTable;               /* invariant, SQL table cache of db */
+    PRMonitor *dbMon;               /* invariant, monitor to protect
+                                    * sqlXact* fields, and use of the sqlReadDB */
+    CK_ATTRIBUTE_TYPE *schemaAttrs; /* Attribute columns that exist in the table. */
+    unsigned int numSchemaAttrs;
 };
 
-static int known_attributes_size = sizeof(known_attributes) /
-                                   sizeof(known_attributes[0]);
+typedef struct SDBPrivateStr SDBPrivate;
 
 /* Magic for an explicit NULL. NOTE: ideally this should be
  * out of band data. Since it's not completely out of band, pick
@@ -891,9 +892,9 @@ sdb_FindObjectsFinal(SDB *sdb, SDBFind *sdbFind)
     return sdb_mapSQLError(sdb_p->type, sqlerr);
 }
 
-CK_RV
-sdb_GetAttributeValueNoLock(SDB *sdb, CK_OBJECT_HANDLE object_id,
-                            CK_ATTRIBUTE *template, CK_ULONG count)
+static CK_RV
+sdb_GetValidAttributeValueNoLock(SDB *sdb, CK_OBJECT_HANDLE object_id,
+                                 CK_ATTRIBUTE *template, CK_ULONG count)
 {
     SDBPrivate *sdb_p = sdb->private;
     sqlite3 *sqlDB = NULL;
@@ -1025,19 +1026,109 @@ loser:
     return error;
 }
 
+/* NOTE: requires sdb_p->schemaAttrs to be sorted asc. */
+inline static PRBool
+sdb_attributeExists(SDB *sdb, CK_ATTRIBUTE_TYPE attr)
+{
+    SDBPrivate *sdb_p = sdb->private;
+    int first = 0;
+    int last = (int)sdb_p->numSchemaAttrs - 1;
+    while (last >= first) {
+        int mid = first + (last - first) / 2;
+        if (sdb_p->schemaAttrs[mid] == attr) {
+            return PR_TRUE;
+        }
+        if (attr > sdb_p->schemaAttrs[mid]) {
+            first = mid + 1;
+        } else {
+            last = mid - 1;
+        }
+    }
+
+    return PR_FALSE;
+}
+
 CK_RV
 sdb_GetAttributeValue(SDB *sdb, CK_OBJECT_HANDLE object_id,
                       CK_ATTRIBUTE *template, CK_ULONG count)
 {
-    CK_RV crv;
+    CK_RV crv = CKR_OK;
+    unsigned int tmplIdx;
+    unsigned int resIdx = 0;
+    unsigned int validCount = 0;
+    unsigned int i;
 
     if (count == 0) {
-        return CKR_OK;
+        return crv;
+    }
+
+    CK_ATTRIBUTE *validTemplate;
+    PRBool invalidExists = PR_FALSE;
+    for (tmplIdx = 0; tmplIdx < count; tmplIdx++) {
+        if (!sdb_attributeExists(sdb, template[tmplIdx].type)) {
+            template[tmplIdx].ulValueLen = -1;
+            crv = CKR_ATTRIBUTE_TYPE_INVALID;
+            invalidExists = PR_TRUE;
+            break;
+        }
+    }
+
+    if (!invalidExists) {
+        validTemplate = template;
+        validCount = count;
+    } else {
+        /* Create a new template containing only the valid subset of
+         * input |template|, and query with that. */
+        validCount = tmplIdx;
+        validTemplate = malloc(sizeof(CK_ATTRIBUTE) * count);
+        if (!validTemplate) {
+            return CKR_HOST_MEMORY;
+        }
+        /* Copy in what we already know is valid. */
+        for (i = 0; i < validCount; i++) {
+            validTemplate[i] = template[i];
+        }
+
+        /* tmplIdx was left at the index of the first invalid
+         * attribute, which has been handled. We only need to
+         * deal with the remainder. */
+        tmplIdx++;
+        for (; tmplIdx < count; tmplIdx++) {
+            if (sdb_attributeExists(sdb, template[tmplIdx].type)) {
+                validTemplate[validCount++] = template[tmplIdx];
+            } else {
+                template[tmplIdx].ulValueLen = -1;
+            }
+        }
+    }
+
+    if (validCount) {
+        LOCK_SQLITE()
+        CK_RV crv2 = sdb_GetValidAttributeValueNoLock(sdb, object_id, validTemplate, validCount);
+        UNLOCK_SQLITE()
+
+        /* If an invalid attribute was removed above, let
+         * the caller know. Any other error from the actual
+         * query should propogate. */
+        crv = (crv2 == CKR_OK) ? crv : crv2;
+    }
+
+    if (invalidExists) {
+        /* Copy out valid lengths. */
+        tmplIdx = 0;
+        for (resIdx = 0; resIdx < validCount; resIdx++) {
+            for (; tmplIdx < count; tmplIdx++) {
+                if (template[tmplIdx].type != validTemplate[resIdx].type) {
+                    continue;
+                }
+                template[tmplIdx].ulValueLen = validTemplate[resIdx].ulValueLen;
+                tmplIdx++;
+                break;
+            }
+        }
+        free(validTemplate);
     }
 
-    LOCK_SQLITE()
-    crv = sdb_GetAttributeValueNoLock(sdb, object_id, template, count);
-    UNLOCK_SQLITE()
     return crv;
 }
 
@@ -1148,7 +1239,7 @@ sdb_objectExists(SDB *sdb, CK_OBJECT_HANDLE candidate)
     CK_RV crv;
     CK_ATTRIBUTE template = { CKA_LABEL, NULL, 0 };
 
-    crv = sdb_GetAttributeValueNoLock(sdb, candidate, &template, 1);
+    crv = sdb_GetValidAttributeValueNoLock(sdb, candidate, &template, 1);
     if (crv == CKR_OBJECT_HANDLE_INVALID) {
         return PR_FALSE;
     }
@@ -1792,6 +1883,7 @@ sdb_Close(SDB *sdb)
     if (sdb_p->dbMon) {
         PR_DestroyMonitor(sdb_p->dbMon);
     }
+    free(sdb_p->schemaAttrs);
     free(sdb_p);
     free(sdb);
     return sdb_mapSQLError(type, sqlerr);
@@ -1829,6 +1921,18 @@ sdb_SetForkState(PRBool forked)
      * interface, we will need to set it and reset it from here */
 }
 
+static int
+sdb_attributeComparator(const void *a, const void *b)
+{
+    if (*(CK_ATTRIBUTE_TYPE *)a < *(CK_ATTRIBUTE_TYPE *)b) {
+        return -1;
+    }
+    if (*(CK_ATTRIBUTE_TYPE *)a > *(CK_ATTRIBUTE_TYPE *)b) {
+        return 1;
+    }
+    return 0;
+}
+
 /*
  * initialize a single database
  */
@@ -1842,6 +1946,7 @@ sdb_init(char *dbname, char *table, sdbDataType type, int *inUpdate,
     int i;
     char *initStr = NULL;
     char *newStr;
+    char *queryStr = NULL;
     int inTransaction = 0;
     SDB *sdb = NULL;
     SDBPrivate *sdb_p = NULL;
@@ -2095,7 +2200,85 @@ sdb_init(char *dbname, char *table, sdbDataType type, int *inUpdate,
     }
 
     sdb = (SDB *)malloc(sizeof(SDB));
+    if (!sdb) {
+        error = CKR_HOST_MEMORY;
+        goto loser;
+    }
     sdb_p = (SDBPrivate *)malloc(sizeof(SDBPrivate));
+    if (!sdb_p) {
+        error = CKR_HOST_MEMORY;
+        goto loser;
+    }
+
+    /* Cache the attributes that are held in the table, so we can later check
+     * that queried attributes actually exist. We don't assume the schema
+     * to be exactly |known_attributes|, as it may change over time. */
+    sdb_p->schemaAttrs = NULL;
+    if (!PORT_Strcmp("nssPublic", table) ||
+        !PORT_Strcmp("nssPrivate", table)) {
+        sqlite3_stmt *stmt = NULL;
+        int retry = 0;
+        unsigned int backedAttrs = 0;
+
+        /* Can't bind parameters to a PRAGMA. */
+        queryStr = sqlite3_mprintf("PRAGMA table_info(%s);", table);
+        if (queryStr == NULL) {
+            error = CKR_HOST_MEMORY;
+            goto loser;
+        }
+        sqlerr = sqlite3_prepare_v2(sqlDB, queryStr, -1, &stmt, NULL);
+        sqlite3_free(queryStr);
+        queryStr = NULL;
+        if (sqlerr != SQLITE_OK) {
+            goto loser;
+        }
+        unsigned int schemaAttrsCapacity = known_attributes_size;
+        sdb_p->schemaAttrs = malloc(schemaAttrsCapacity * sizeof(CK_ATTRIBUTE));
+        if (!sdb_p->schemaAttrs) {
+            error = CKR_HOST_MEMORY;
+            goto loser;
+        }
+        do {
+            sqlerr = sqlite3_step(stmt);
+            if (sqlerr == SQLITE_BUSY) {
+                PR_Sleep(SDB_BUSY_RETRY_TIME);
+            }
+            if (sqlerr == SQLITE_ROW) {
+                if (backedAttrs == schemaAttrsCapacity) {
+                    schemaAttrsCapacity += known_attributes_size;
+                    sdb_p->schemaAttrs = realloc(sdb_p->schemaAttrs,
+                                                 schemaAttrsCapacity * sizeof(CK_ATTRIBUTE));
+                    if (!sdb_p->schemaAttrs) {
+                        error = CKR_HOST_MEMORY;
+                        goto loser;
+                    }
+                }
+                /* Record the ULONG attribute value. */
+                char *val = (char *)sqlite3_column_text(stmt, 1);
+                if (val && val[0] == 'a') {
+                    CK_ATTRIBUTE_TYPE attr = strtoul(&val[1], NULL, 16);
+                    sdb_p->schemaAttrs[backedAttrs++] = attr;
+                }
+            }
+        } while (!sdb_done(sqlerr, &retry));
+        if (sqlerr != SQLITE_DONE) {
+            goto loser;
+        }
+        sqlerr = sqlite3_reset(stmt);
+        if (sqlerr != SQLITE_OK) {
+            goto loser;
+        }
+        sqlerr = sqlite3_finalize(stmt);
+        if (sqlerr != SQLITE_OK) {
+            goto loser;
+        }
+
+        sdb_p->numSchemaAttrs = backedAttrs;
+
+        /* Sort these once so we can shortcut invalid attribute searches. */
+        qsort(sdb_p->schemaAttrs, sdb_p->numSchemaAttrs,
+              sizeof(CK_ATTRIBUTE_TYPE), sdb_attributeComparator);
+    }
 
     /* invariant fields */
     sdb_p->sqlDBName = PORT_Strdup(dbname);
@@ -2156,6 +2339,9 @@ loser:
         free(sdb);
     }
     if (sdb_p) {
+        if (sdb_p->schemaAttrs) {
+            free(sdb_p->schemaAttrs);
+        }
         free(sdb_p);
     }
     if (sqlDB) {