diff -up ./lib/softoken/lgglue.c.read-write-db-locks ./lib/softoken/lgglue.c
--- ./lib/softoken/lgglue.c.read-write-db-locks 2023-06-04 10:42:53.000000000 +0200
+++ ./lib/softoken/lgglue.c 2023-06-21 10:59:44.940835332 +0200
@@ -184,13 +184,13 @@ sftkdb_encrypt_stub(PLArenaPool *arena,
}
/* not a key handle */
- if (handle == NULL || handle->passwordLock == NULL) {
+ if (handle == NULL || !sftkdb_passwordLockIsInited(handle)) {
return SECFailure;
}
- PZ_Lock(handle->passwordLock);
+ sftkdb_passwordReaderLock(handle);
if (handle->passwordKey.data == NULL) {
- PZ_Unlock(handle->passwordLock);
+ sftkdb_passwordReaderUnlock(handle);
/* PORT_SetError */
return SECFailure;
}
@@ -208,7 +208,7 @@ sftkdb_encrypt_stub(PLArenaPool *arena,
rv = sftkdb_EncryptAttribute(arena, handle, sdb, key, iterationCount,
CK_INVALID_HANDLE, CKT_INVALID_TYPE,
plainText, cipherText);
- PZ_Unlock(handle->passwordLock);
+ sftkdb_passwordReaderUnlock(handle);
return rv;
}
@@ -235,13 +235,13 @@ sftkdb_decrypt_stub(SDB *sdb, SECItem *c
}
/* not a key handle */
- if (handle == NULL || handle->passwordLock == NULL) {
+ if (handle == NULL || !sftkdb_passwordLockIsInited(handle)) {
return SECFailure;
}
- PZ_Lock(handle->passwordLock);
+ sftkdb_passwordReaderLock(handle);
if (handle->passwordKey.data == NULL) {
- PZ_Unlock(handle->passwordLock);
+ sftkdb_passwordReaderUnlock(handle);
/* PORT_SetError */
return SECFailure;
}
@@ -249,7 +249,7 @@ sftkdb_decrypt_stub(SDB *sdb, SECItem *c
CK_INVALID_HANDLE,
CKT_INVALID_TYPE,
cipherText, plainText);
- PZ_Unlock(handle->passwordLock);
+ sftkdb_passwordReaderUnlock(handle);
return rv;
}
diff -up ./lib/softoken/sftkdb.c.read-write-db-locks ./lib/softoken/sftkdb.c
--- ./lib/softoken/sftkdb.c.read-write-db-locks 2023-06-04 10:42:53.000000000 +0200
+++ ./lib/softoken/sftkdb.c 2023-06-21 12:54:41.007208477 +0200
@@ -413,9 +413,9 @@ sftkdb_fixupTemplateOut(CK_ATTRIBUTE *te
cipherText.data = ntemplate[i].pValue;
cipherText.len = ntemplate[i].ulValueLen;
- PZ_Lock(handle->passwordLock);
+ sftkdb_passwordReaderLock(handle);
if (handle->passwordKey.data == NULL) {
- PZ_Unlock(handle->passwordLock);
+ sftkdb_passwordReaderUnlock(handle);
template[i].ulValueLen = -1;
crv = CKR_USER_NOT_LOGGED_IN;
continue;
@@ -425,7 +425,7 @@ sftkdb_fixupTemplateOut(CK_ATTRIBUTE *te
objectID,
ntemplate[i].type,
&cipherText, &plainText);
- PZ_Unlock(handle->passwordLock);
+ sftkdb_passwordReaderUnlock(handle);
if (rv != SECSuccess) {
PORT_Memset(template[i].pValue, 0, template[i].ulValueLen);
template[i].ulValueLen = -1;
@@ -478,12 +478,12 @@ sftkdb_fixupTemplateOut(CK_ATTRIBUTE *te
* we do a second check holding the lock just in case the user
* loggout while we were trying to get the signature.
*/
- PZ_Lock(keyHandle->passwordLock);
+ sftkdb_passwordReaderLock(keyHandle);
if (keyHandle->passwordKey.data == NULL) {
/* if we are no longer logged in, no use checking the other
* Signatures either. */
checkSig = PR_FALSE;
- PZ_Unlock(keyHandle->passwordLock);
+ sftkdb_passwordReaderUnlock(keyHandle);
continue;
}
@@ -491,7 +491,7 @@ sftkdb_fixupTemplateOut(CK_ATTRIBUTE *te
&keyHandle->passwordKey,
objectID, ntemplate[i].type,
&plainText, &signText);
- PZ_Unlock(keyHandle->passwordLock);
+ sftkdb_passwordReaderUnlock(keyHandle);
if (rv != SECSuccess) {
PORT_Memset(template[i].pValue, 0, template[i].ulValueLen);
template[i].ulValueLen = -1;
@@ -590,9 +590,9 @@ sftk_signTemplate(PLArenaPool *arena, SF
plainText.data = template[i].pValue;
plainText.len = template[i].ulValueLen;
- PZ_Lock(keyHandle->passwordLock);
+ sftkdb_passwordReaderLock(keyHandle);
if (keyHandle->passwordKey.data == NULL) {
- PZ_Unlock(keyHandle->passwordLock);
+ sftkdb_passwordReaderUnlock(keyHandle);
crv = CKR_USER_NOT_LOGGED_IN;
goto loser;
}
@@ -601,7 +601,7 @@ sftk_signTemplate(PLArenaPool *arena, SF
keyHandle->defaultIterationCount,
objectID, template[i].type,
&plainText, &signText);
- PZ_Unlock(keyHandle->passwordLock);
+ sftkdb_passwordReaderUnlock(keyHandle);
if (rv != SECSuccess) {
crv = CKR_GENERAL_ERROR; /* better error code here? */
goto loser;
@@ -770,9 +770,9 @@ sftk_ExtractTemplate(PLArenaPool *arena,
plainText.data = tp->pValue;
plainText.len = tp->ulValueLen;
- PZ_Lock(handle->passwordLock);
+ sftkdb_passwordReaderLock(handle);
if (handle->passwordKey.data == NULL) {
- PZ_Unlock(handle->passwordLock);
+ sftkdb_passwordReaderUnlock(handle);
*crv = CKR_USER_NOT_LOGGED_IN;
break;
}
@@ -782,7 +782,7 @@ sftk_ExtractTemplate(PLArenaPool *arena,
objectID,
tp->type,
&plainText, &cipherText);
- PZ_Unlock(handle->passwordLock);
+ sftkdb_passwordReaderUnlock(handle);
if (rv == SECSuccess) {
tp->pValue = cipherText->data;
tp->ulValueLen = cipherText->len;
@@ -1638,15 +1638,15 @@ sftkdb_CloseDB(SFTKDBHandle *handle)
}
(*handle->db->sdb_Close)(handle->db);
}
- if (handle->passwordLock) {
- PZ_Lock(handle->passwordLock);
+ if (sftkdb_passwordLockIsInited(handle)) {
+ sftkdb_passwordWriterLock(handle);
}
if (handle->passwordKey.data) {
SECITEM_ZfreeItem(&handle->passwordKey, PR_FALSE);
}
- if (handle->passwordLock) {
- PZ_Unlock(handle->passwordLock);
- SKIP_AFTER_FORK(PZ_DestroyLock(handle->passwordLock));
+ if (sftkdb_passwordLockIsInited(handle)) {
+ sftkdb_passwordWriterUnlock(handle);
+ SKIP_AFTER_FORK(sftkdb_passwordLockDestroy(handle));
}
if (handle->updatePasswordKey) {
SECITEM_ZfreeItem(handle->updatePasswordKey, PR_TRUE);
@@ -2704,8 +2704,10 @@ sftk_NewDBHandle(SDB *sdb, int type, PRB
handle->passwordKey.data = NULL;
handle->passwordKey.len = 0;
handle->passwordLock = NULL;
+ handle->passwordWriterCond = NULL;
+ handle->passwordReaderCond = NULL;
if (type == SFTK_KEYDB_TYPE) {
- handle->passwordLock = PZ_NewLock(nssILockAttribute);
+ (void) sftkdb_passwordLockInit(handle);
}
sdb->app_private = handle;
return handle;
diff -up ./lib/softoken/sftkdbti.h.read-write-db-locks ./lib/softoken/sftkdbti.h
--- ./lib/softoken/sftkdbti.h.read-write-db-locks 2023-06-04 10:42:53.000000000 +0200
+++ ./lib/softoken/sftkdbti.h 2023-06-21 10:59:44.940835332 +0200
@@ -4,6 +4,7 @@
#ifndef SFTKDBTI_H
#define SFTKDBTI_H 1
+#include <prcvar.h>
/*
* private defines
@@ -19,6 +20,11 @@ struct SFTKDBHandleStr {
SECItem *oldKey;
SECItem *updatePasswordKey;
PZLock *passwordLock;
+ PRCondVar *passwordWriterCond;
+ PRCondVar *passwordReaderCond;
+ PRBool passwordWriterActive;
+ int passwordWriters;
+ int passwordReaders;
SFTKDBHandle *peerDB;
SDB *update;
char *updateID;
@@ -79,4 +85,13 @@ sftkdb_DestroyAttributeSignature(SFTKDBH
CK_OBJECT_HANDLE objectID,
CK_ATTRIBUTE_TYPE type);
+/* password lock functions */
+SECStatus sftkdb_passwordLockInit(SFTKDBHandle *keydb);
+void sftkdb_passwordLockDestroy(SFTKDBHandle *keydb);
+PRBool sftkdb_passwordLockIsInited(SFTKDBHandle *keydb);
+void sftkdb_passwordReaderLock(SFTKDBHandle *keydb);
+void sftkdb_passwordWriterLock(SFTKDBHandle *keydb);
+void sftkdb_passwordReaderUnlock(SFTKDBHandle *keydb);
+void sftkdb_passwordWriterUnlock(SFTKDBHandle *keydb);
+
#endif
diff -up ./lib/softoken/sftkpwd.c.read-write-db-locks ./lib/softoken/sftkpwd.c
--- ./lib/softoken/sftkpwd.c.read-write-db-locks 2023-06-04 10:42:53.000000000 +0200
+++ ./lib/softoken/sftkpwd.c 2023-06-21 10:59:44.940835332 +0200
@@ -632,6 +632,126 @@ loser:
return rv;
}
+SECStatus
+sftkdb_passwordLockInit(SFTKDBHandle *keydb)
+{
+ keydb->passwordLock = PZ_NewLock(nssILockAttribute);
+ if (keydb->passwordLock == NULL) {
+ return SECFailure;
+ }
+ keydb->passwordWriterCond = PR_NewCondVar(keydb->passwordLock);
+ if (keydb->passwordWriterCond == NULL) {
+ PZ_DestroyLock(keydb->passwordLock);
+ keydb->passwordLock = NULL;
+ return SECFailure;
+ }
+ keydb->passwordReaderCond = PR_NewCondVar(keydb->passwordLock);
+ if (keydb->passwordReaderCond == NULL) {
+ PR_DestroyCondVar(keydb->passwordWriterCond);
+ PZ_DestroyLock(keydb->passwordLock);
+ keydb->passwordWriterCond = NULL;
+ keydb->passwordLock = NULL;
+ return SECFailure;
+ }
+ keydb->passwordWriters = 0;
+ keydb->passwordReaders = 0;
+ keydb->passwordWriterActive = PR_FALSE;
+ return SECSuccess;
+}
+
+void
+sftkdb_passwordLockDestroy(SFTKDBHandle *keydb)
+{
+ PR_DestroyCondVar(keydb->passwordWriterCond);
+ keydb->passwordWriterCond = NULL;
+ PR_DestroyCondVar(keydb->passwordReaderCond);
+ keydb->passwordReaderCond = NULL;
+ PZ_DestroyLock(keydb->passwordLock);
+ keydb->passwordLock = NULL;
+}
+
+PRBool
+sftkdb_passwordLockIsInited(SFTKDBHandle *keydb)
+{
+ return (keydb->passwordLock) && (keydb->passwordWriterCond)
+ && (keydb->passwordReaderCond);
+}
+
+/* we need reader/writer locks for the database because we have servers
+ * that use the database key in their transactions on multi-cpu systems.
+ * This means that all the other cpus doing private key operations become
+ * effectively single threaded. We implement our specific reader/writer
+ * locks because we want writer locks to interrupt reader streams to
+ * prevent starvation on the writer side. */
+void
+sftkdb_passwordReaderLock(SFTKDBHandle *keydb)
+{
+ PZ_Lock(keydb->passwordLock);
+ /* if there is a writer waiting or running wait until the
+ * writer has completed before we continue. This prevents
+ * writer starvation in a loaded system */
+ while (keydb->passwordWriters) {
+ PR_WaitCondVar(keydb->passwordReaderCond, PR_INTERVAL_NO_TIMEOUT);
+ }
+ /* allow multiple readers */
+ keydb->passwordReaders++;
+ PORT_Assert(keydb->passwordWriterActive == PR_FALSE);
+ PZ_Unlock(keydb->passwordLock);
+}
+
+/* We can only have 1 active writer or multiple active readers.
+ * Readers will wait for an active writer. This means lots of write
+ * operations can stall the readers. Write operations are rare, though and
+ * only happen on database update, or on initial login.
+ */
+void
+sftkdb_passwordWriterLock(SFTKDBHandle *keydb)
+{
+ PZ_Lock(keydb->passwordLock);
+ keydb->passwordWriters++;
+ /* if we have any readers or writers, wait for the writer condition */
+ while (keydb->passwordReaders || keydb->passwordWriterActive) {
+ PR_WaitCondVar(keydb->passwordWriterCond, PR_INTERVAL_NO_TIMEOUT);
+ }
+ /* only one writer allowed at a time */
+ keydb->passwordWriterActive = PR_TRUE;
+ PZ_Unlock(keydb->passwordLock);
+}
+
+/* unlock, decrements the reader counter, and if we have not active readers,
+ * notify any waiting writers. */
+void
+sftkdb_passwordReaderUnlock(SFTKDBHandle *keydb)
+{
+ PZ_Lock(keydb->passwordLock);
+ PORT_Assert(keydb->passwordReaders);
+ keydb->passwordReaders--;
+ if (keydb->passwordReaders == 0) {
+ if (keydb->passwordWriters) {
+ PR_NotifyCondVar(keydb->passwordWriterCond);
+ }
+ }
+ PZ_Unlock(keydb->passwordLock);
+}
+
+/* unlock, if there are more writers, wake one of them up. If there or no more
+ * writers but readers, wake up all the readers. */
+void
+sftkdb_passwordWriterUnlock(SFTKDBHandle *keydb)
+{
+ PZ_Lock(keydb->passwordLock);
+ PORT_Assert(keydb->passwordWriters);
+ PORT_Assert(keydb->passwordWriterActive);
+ keydb->passwordWriterActive = PR_FALSE;
+ keydb->passwordWriters--;
+ if (keydb->passwordWriters) {
+ PR_NotifyCondVar(keydb->passwordWriterCond);
+ } else {
+ PR_NotifyAllCondVar(keydb->passwordReaderCond);
+ }
+ PZ_Unlock(keydb->passwordLock);
+}
+
/*
* safely swith the passed in key for the one caches in the keydb handle
*
@@ -645,13 +765,13 @@ sftkdb_switchKeys(SFTKDBHandle *keydb, S
unsigned char *data;
int len;
- if (keydb->passwordLock == NULL) {
+ if (!sftkdb_passwordLockIsInited(keydb)) {
PORT_Assert(keydb->type != SFTK_KEYDB_TYPE);
return;
}
/* an atomic pointer set would be nice */
- SKIP_AFTER_FORK(PZ_Lock(keydb->passwordLock));
+ SKIP_AFTER_FORK(sftkdb_passwordWriterLock(keydb));
data = keydb->passwordKey.data;
len = keydb->passwordKey.len;
keydb->passwordKey.data = passKey->data;
@@ -659,7 +779,7 @@ sftkdb_switchKeys(SFTKDBHandle *keydb, S
keydb->defaultIterationCount = iterationCount;
passKey->data = data;
passKey->len = len;
- SKIP_AFTER_FORK(PZ_Unlock(keydb->passwordLock));
+ SKIP_AFTER_FORK(sftkdb_passwordWriterUnlock(keydb));
}
/*
@@ -705,11 +825,11 @@ sftkdb_GetUpdatePasswordKey(SFTKDBHandle
return NULL;
}
- PZ_Lock(handle->passwordLock);
+ sftkdb_passwordReaderLock(handle);
if (handle->updatePasswordKey) {
key = SECITEM_DupItem(handle->updatePasswordKey);
}
- PZ_Unlock(handle->passwordLock);
+ sftkdb_passwordReaderUnlock(handle);
return key;
}
@@ -732,12 +852,12 @@ sftkdb_FreeUpdatePasswordKey(SFTKDBHandl
return;
}
- PZ_Lock(handle->passwordLock);
+ sftkdb_passwordWriterLock(handle);
if (handle->updatePasswordKey) {
key = handle->updatePasswordKey;
handle->updatePasswordKey = NULL;
}
- PZ_Unlock(handle->passwordLock);
+ sftkdb_passwordWriterUnlock(handle);
if (key) {
SECITEM_ZfreeItem(key, PR_TRUE);
@@ -1004,7 +1124,7 @@ sftkdb_finishPasswordCheck(SFTKDBHandle
* update, as we now have both required passwords.
*
*/
- PZ_Lock(keydb->passwordLock);
+ sftkdb_passwordWriterLock(keydb);
if (sftkdb_NeedUpdateDBPassword(keydb)) {
/* Squirrel this special key away.
* This has the side effect of turning sftkdb_NeedLegacyPW off,
@@ -1012,7 +1132,7 @@ sftkdb_finishPasswordCheck(SFTKDBHandle
* SFTK_GET_PW_DB (thus effecting both sftkdb_CheckPassword()
* and sftkdb_HasPasswordSet()) */
keydb->updatePasswordKey = SECITEM_DupItem(key);
- PZ_Unlock(keydb->passwordLock);
+ sftkdb_passwordWriterUnlock(keydb);
if (keydb->updatePasswordKey == NULL) {
/* PORT_Error set by SECITEM_DupItem */
rv = SECFailure;
@@ -1077,7 +1197,7 @@ sftkdb_finishPasswordCheck(SFTKDBHandle
* update case. */
}
} else {
- PZ_Unlock(keydb->passwordLock);
+ sftkdb_passwordWriterUnlock(keydb);
}
/* load the keys, so the keydb can parse it's key set */
sftkdb_switchKeys(keydb, key, iterationCount);