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 | a88634ea3f24f74fbba87eb3cfd1b8e812120ea3 (patch) | |
| tree | 3724b9c5253b06e3672a568e75217502efea036c | |
| parent | d8703e74896b5da7926b27f2565690a56594430a (diff) | |
| download | nss-hg-a88634ea3f24f74fbba87eb3cfd1b8e812120ea3.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) { |