Print this page
9642 PKCS#11 softtoken should use explicit_bzero
Reviewed by: Dan McDonald <danmcd@joyent.com>
Reviewed by: Alex Wilson <alex.wilson@joyent.com>

@@ -18,10 +18,11 @@
  *
  * CDDL HEADER END
  */
 /*
  * Copyright (c) 2004, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright 2018, Joyent, Inc.
  */
 
 #include <crypt.h>
 #include <cryptoutil.h>
 #include <pwd.h>

@@ -96,12 +97,15 @@
                         return (-1);
                 }
         }
 
         if ((*result = crypt((char *)pPin, *salt)) == NULL) {
-                if (new_salt)
-                        free(*salt);
+                if (new_salt) {
+                        size_t saltlen = strlen(*salt) + 1;
+
+                        freezero(*salt, saltlen);
+                }
                 return (-1);
         }
 
         return (0);
 }

@@ -117,10 +121,11 @@
         char    *ks_cryptpin = NULL;
         char    *salt = NULL;
         uchar_t *tmp_pin = NULL;
         boolean_t pin_initialized = B_FALSE;
         CK_RV   rv = CKR_OK;
+        size_t  len = 0;
 
         /*
          * Check to see if keystore is initialized.
          */
         rv = soft_keystore_pin_initialized(&pin_initialized, &ks_cryptpin,

@@ -187,17 +192,22 @@
                 soft_slot.userpin_change_needed = 1;
                 rv = CKR_OK;
         }
 
 cleanup:
-        if (salt)
-                free(salt);
-        if (tmp_pin)
-                free(tmp_pin);
-        if (ks_cryptpin)
-                free(ks_cryptpin);
-
+        if (salt) {
+                len = strlen(salt) + 1;
+                freezero(salt, len);
+        }
+        if (tmp_pin) {
+                len = strlen((char *)tmp_pin) + 1;
+                freezero(tmp_pin, len);
+        }
+        if (ks_cryptpin) {
+                len = strlen(ks_cryptpin) + 1;
+                freezero(ks_cryptpin, len);
+        }
         return (rv);
 }
 
 /*
  * The second level C_SetPIN function.

@@ -211,10 +221,11 @@
         char    *ks_cryptpin = NULL;
         char    *salt = NULL;
         boolean_t pin_initialized = B_FALSE;
         uchar_t *tmp_old_pin = NULL, *tmp_new_pin = NULL;
         CK_RV   rv = CKR_OK;
+        size_t  len = 0;
 
         /*
          * Check to see if keystore is initialized.
          */
         rv = soft_keystore_pin_initialized(&pin_initialized, &ks_cryptpin,

@@ -288,18 +299,26 @@
                 (void) pthread_mutex_unlock(&soft_giant_mutex);
                 rv = CKR_OK;
         }
 
 cleanup:
-        if (salt)
-                free(salt);
-        if (ks_cryptpin)
-                free(ks_cryptpin);
-        if (tmp_old_pin)
-                free(tmp_old_pin);
-        if (tmp_new_pin)
-                free(tmp_new_pin);
+        if (salt) {
+                len = strlen(salt) + 1;
+                freezero(salt, len);
+        }
+        if (ks_cryptpin) {
+                len = strlen(ks_cryptpin) + 1;
+                freezero(ks_cryptpin, len);
+        }
+        if (tmp_old_pin) {
+                len = strlen((char *)tmp_old_pin) + 1;
+                freezero(tmp_old_pin, len);
+        }
+        if (tmp_new_pin) {
+                len = strlen((char *)tmp_new_pin) + 1;
+                freezero(tmp_new_pin, len);
+        }
 
         return (rv);
 }
 
 /*

@@ -473,13 +492,11 @@
                         (void) memcpy(template.pValue, buf,
                             template.ulValueLen);
                 }
 
                 rv = soft_add_extra_attr(&template, obj);
-                if (template.pValue) {
-                        free(template.pValue);
-                }
+                freezero(template.pValue, template.ulValueLen);
 
                 if (rv != CKR_OK) {
                         return (rv);
                 }
 

@@ -541,11 +558,11 @@
                 rv = get_cert_attr_from_template(cert_dest, &template);
         } else {
                 rv = get_bigint_attr_from_template(key_dest, &template);
         }
 
-        free(template.pValue);
+        freezero(template.pValue, template.ulValueLen);
         if (rv != CKR_OK) {
                 return (rv);
         }
 
         *offset = sizeof (uint64_t) + template.ulValueLen;

@@ -1855,29 +1872,19 @@
         rv = soft_keystore_pack_obj(objp, &buf, &len);
         if (rv != CKR_OK)
                 return (rv);
 
         (void) pthread_mutex_lock(&soft_slot.slot_mutex);
-        if (objp->object_type == TOKEN_PUBLIC) {
-                if ((soft_keystore_put_new_obj(buf, len, B_TRUE,
-                    B_FALSE, &objp->ks_handle)) == -1) {
-                        (void) pthread_mutex_unlock(&soft_slot.slot_mutex);
-                        free(buf);
-                        return (CKR_FUNCTION_FAILED);
+        if (soft_keystore_put_new_obj(buf, len,
+            !!(objp->object_type == TOKEN_PUBLIC), B_FALSE,
+            &objp->ks_handle) == -1) {
+                rv = CKR_FUNCTION_FAILED;
                 }
-        } else {
-                if ((soft_keystore_put_new_obj(buf, len, B_FALSE,
-                    B_FALSE, &objp->ks_handle)) == -1) {
                         (void) pthread_mutex_unlock(&soft_slot.slot_mutex);
-                        free(buf);
-                        return (CKR_FUNCTION_FAILED);
-                }
-        }
-        (void) pthread_mutex_unlock(&soft_slot.slot_mutex);
-        free(buf);
-        return (CKR_OK);
 
+        freezero(buf, len);
+        return (rv);
 }
 
 /*
  * Modify the in-core token object and then write it to
  * a keystore file.

@@ -1895,15 +1902,15 @@
                 return (rv);
 
         /* B_TRUE: caller has held a writelock on the keystore */
         if (soft_keystore_modify_obj(&objp->ks_handle, buf, len,
             B_TRUE) < 0) {
-                return (CKR_FUNCTION_FAILED);
+                rv = CKR_FUNCTION_FAILED;
         }
 
-        free(buf);
-        return (CKR_OK);
+        freezero(buf, len);
+        return (rv);
 
 }
 
 /*
  * Read the token object from the keystore file.

@@ -1940,22 +1947,21 @@
 
                 soft_add_token_object_to_slot(new_objp);
 
                 /* Free the ks_obj list */
                 ks_obj_next = ks_obj->next;
-                if (ks_obj->buf)
-                        free(ks_obj->buf);
+                freezero(ks_obj->buf, ks_obj->size);
                 free(ks_obj);
                 ks_obj = ks_obj_next;
         }
 
         return (CKR_OK);
 
 cleanup:
         while (ks_obj) {
                 ks_obj_next = ks_obj->next;
-                free(ks_obj->buf);
+                freezero(ks_obj->buf, ks_obj->size);
                 free(ks_obj);
                 ks_obj = ks_obj_next;
         }
         return (rv);
 }

@@ -2302,13 +2308,12 @@
                 soft_aes_ctx->aes_cbc = (void *)aes_cbc_ctx_init(
                     soft_aes_ctx->key_sched, soft_aes_ctx->keysched_len,
                     soft_aes_ctx->ivec);
 
                 if (soft_aes_ctx->aes_cbc == NULL) {
-                        bzero(soft_aes_ctx->key_sched,
+                        freezero(soft_aes_ctx->key_sched,
                             soft_aes_ctx->keysched_len);
-                        free(soft_aes_ctx->key_sched);
                         if (encrypt) {
                                 free(token_session.encrypt.context);
                                 token_session.encrypt.context = NULL;
                         } else {
                                 free(token_session.encrypt.context);