summaryrefslogtreecommitdiff
path: root/lib/ssl
diff options
context:
space:
mode:
authorMartin Thomson <mt@lowentropy.net>2019-06-25 12:25:40 +1000
committerMartin Thomson <mt@lowentropy.net>2019-06-25 12:25:40 +1000
commitaf023085c60058746b5f15e7042bba598f318197 (patch)
treeb2e916b3fbd95df0f9e0174677c055860c26035d /lib/ssl
parent20ded1736ed1abf64ca66de0c70c7c4385549c2a (diff)
downloadnss-hg-af023085c60058746b5f15e7042bba598f318197.tar.gz
Bug 1556591 - Eliminate races in uses of PK11_SetWrapKey, r=kjacobs
These functions are terrible, but in the spirit of ABI compatibility, I can't really change their signature. And it isn't important enough to fix them, so this introduces a quick hack. This is wrapped up in the mess that is Bug 1556588, but I'm proceeding here under the assumption that unconditional use of the lock is safe and that a non-thread-safe module will explode in many other ways. Differential Revision: https://phabricator.services.mozilla.com/D34650
Diffstat (limited to 'lib/ssl')
-rw-r--r--lib/ssl/ssl3con.c12
1 files changed, 12 insertions, 0 deletions
diff --git a/lib/ssl/ssl3con.c b/lib/ssl/ssl3con.c
index d9f4b5234..67f162955 100644
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -11378,7 +11378,19 @@ ssl3_CacheWrappedSecret(sslSocket *ss, sslSessionID *sid,
wrappingKey = PK11_KeyGen(symKeySlot, mechanism, NULL,
keyLength, pwArg);
if (wrappingKey) {
+ /* The thread safety characteristics of PK11_[SG]etWrapKey is
+ * abominable. This protects against races in calling
+ * PK11_SetWrapKey by dropping and re-acquiring the canonical
+ * value once it is set. The mutex in PK11_[SG]etWrapKey will
+ * ensure that races produce the same value in the end. */
PK11_SetWrapKey(symKeySlot, wrapKeyIndex, wrappingKey);
+ PK11_FreeSymKey(wrappingKey);
+ wrappingKey = PK11_GetWrapKey(symKeySlot, wrapKeyIndex,
+ CKM_INVALID_MECHANISM, incarnation, pwArg);
+ if (!wrappingKey) {
+ PK11_FreeSlot(symKeySlot);
+ return SECFailure;
+ }
}
}
} else {