diff options
author | Robert Relyea <rrelyea@redhat.com> | 2020-03-13 10:58:55 -0700 |
---|---|---|
committer | Robert Relyea <rrelyea@redhat.com> | 2020-03-13 10:58:55 -0700 |
commit | 35f142ab8e3bc4cfe98e4b9b096250bb43a59f8c (patch) | |
tree | c8127b1d298b760fbf17f133c2954171f02bdfb1 | |
parent | aca232e85ee16df545f7c6e47356ca44f39ef371 (diff) | |
download | nss-hg-35f142ab8e3bc4cfe98e4b9b096250bb43a59f8c.tar.gz |
Bug 1608245 KBKDF - Consistently handle NULL slot/session r=kjacobs
Patch by cipherboy, review by kjacobs.
https://phabricator.services.mozilla.com/D59409
Per Bug 1607955, the KBKDF code introduced in Bug 1599603 confused
Coverity with a elided NULL check on sftk_SlotFromSessionHandle(...).
While Coverity is incorrect (and the behavior is fine as-is), it isn't
consistent with the KBKDF code's handling of sftk_SessionFromHandle(...)
(which is NULL checked).
This brings these two call sites into internal consistency.
-rw-r--r-- | lib/softoken/kbkdf.c | 16 |
1 files changed, 10 insertions, 6 deletions
diff --git a/lib/softoken/kbkdf.c b/lib/softoken/kbkdf.c index 44cba084b..15761601a 100644 --- a/lib/softoken/kbkdf.c +++ b/lib/softoken/kbkdf.c @@ -599,12 +599,16 @@ kbkdf_SaveKey(SFTKObject *key, unsigned char *key_buffer, unsigned int key_len) CK_RV kbkdf_CreateKey(CK_MECHANISM_TYPE kdf_mech, CK_SESSION_HANDLE hSession, CK_DERIVED_KEY_PTR derived_key, SFTKObject **ret_key) { - /* Largely duplicated from NSC_Derive(...) */ + /* Largely duplicated from NSC_DeriveKey(...) */ CK_RV ret = CKR_HOST_MEMORY; SFTKObject *key = NULL; SFTKSlot *slot = sftk_SlotFromSessionHandle(hSession); size_t offset = 0; + /* Slot should be non-NULL because NSC_DeriveKey(...) has already + * performed a sftk_SlotFromSessionHandle(...) call on this session + * handle. However, Coverity incorrectly flagged this (see 1607955). */ + PR_ASSERT(slot != NULL); PR_ASSERT(ret_key != NULL); PR_ASSERT(derived_key != NULL); PR_ASSERT(derived_key->phKey != NULL); @@ -646,7 +650,7 @@ kbkdf_CreateKey(CK_MECHANISM_TYPE kdf_mech, CK_SESSION_HANDLE hSession, CK_DERIV CK_RV kbkdf_FinalizeKey(CK_SESSION_HANDLE hSession, CK_DERIVED_KEY_PTR derived_key, SFTKObject *key) { - /* Largely duplicated from NSC_Derive(...) */ + /* Largely duplicated from NSC_DeriveKey(...) */ CK_RV ret = CKR_HOST_MEMORY; SFTKSession *session = NULL; @@ -657,10 +661,10 @@ kbkdf_FinalizeKey(CK_SESSION_HANDLE hSession, CK_DERIVED_KEY_PTR derived_key, SF sessionForKey->wasDerived = PR_TRUE; session = sftk_SessionFromHandle(hSession); - if (session == NULL) { - ret = CKR_HOST_MEMORY; - goto done; - } + + /* Session should be non-NULL because NSC_DeriveKey(...) has already + * performed a sftk_SessionFromHandle(...) call on this session handle. */ + PR_ASSERT(session != NULL); ret = sftk_handleObject(key, session); if (ret != CKR_OK) { |