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

@@ -20,10 +20,11 @@
  */
 /*
  * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  * Copyright 2012 Milan Jurik. All rights reserved.
+ * Copyright (c) 2018, Joyent, Inc.
  */
 
 #include <stdlib.h>
 #include <string.h>
 #include <security/cryptoki.h>

@@ -320,15 +321,12 @@
 }
 
 static void
 cleanup_cert_attr(cert_attr_t *attr)
 {
-        if (attr) {
-                if (attr->value) {
-                        (void) memset(attr->value, 0, attr->length);
-                        free(attr->value);
-                }
+        if (attr != NULL) {
+                freezero(attr->value, attr->length);
                 attr->value = NULL;
                 attr->length = 0;
         }
 }
 

@@ -343,12 +341,11 @@
         if (src_attr->value == NULL)
                 return (CKR_HOST_MEMORY);
 
         /* free memory if its already allocated */
         if (*dest_attr != NULL) {
-                if ((*dest_attr)->value != (CK_BYTE *)NULL)
-                        free((*dest_attr)->value);
+                cleanup_cert_attr(*dest_attr);
         } else {
                 *dest_attr = malloc(sizeof (cert_attr_t));
                 if (*dest_attr == NULL)
                         return (CKR_HOST_MEMORY);
         }

@@ -419,18 +416,20 @@
         CK_ATTRIBUTE_INFO_PTR tmp;
 
         extra_attr = object_p->extra_attrlistp;
         while (extra_attr) {
                 tmp = extra_attr->next;
-                if (extra_attr->attr.pValue)
+                if (extra_attr->attr.pValue != NULL) {
                         /*
                          * All extra attributes in the extra attribute
                          * list have pValue points to the value of the
                          * attribute (with simple byte array type).
                          * Free the storage for the value of the attribute.
                          */
-                        free(extra_attr->attr.pValue);
+                        freezero(extra_attr->attr.pValue,
+                            extra_attr->attr.ulValueLen);
+                }
 
                 /* Free the storage for the attribute_info struct. */
                 free(extra_attr);
                 extra_attr = tmp;
         }

@@ -670,13 +669,15 @@
         /* We found the attribute in the extra attribute list. */
         if ((template->pValue != NULL) &&
             (template->ulValueLen > 0)) {
                 if (template->ulValueLen > extra_attr->attr.ulValueLen) {
                         /* The old buffer is too small to hold the new value. */
-                        if (extra_attr->attr.pValue != NULL)
+                        if (extra_attr->attr.pValue != NULL) {
                                 /* Free storage for the old attribute value. */
-                                free(extra_attr->attr.pValue);
+                                freezero(extra_attr->attr.pValue,
+                                    extra_attr->attr.ulValueLen);
+                        }
 
                         /* Allocate storage for the new attribute value. */
                         extra_attr->attr.pValue = malloc(template->ulValueLen);
                         if (extra_attr->attr.pValue == NULL) {
                                 return (CKR_HOST_MEMORY);

@@ -928,15 +929,11 @@
                 /*
                  * If the attribute was already set, clear out the
                  * existing value and release the memory.
                  */
                 if (*dest != NULL) {
-                        if ((*dest)->value != NULL) {
-                                (void) memset((*dest)->value, 0,
-                                    (*dest)->length);
-                                free((*dest)->value);
-                        }
+                        cleanup_cert_attr(*dest);
                 } else {
                         *dest = malloc(sizeof (cert_attr_t));
                         if (*dest == NULL) {
                                 return (CKR_HOST_MEMORY);
                         }

@@ -985,16 +982,13 @@
 }
 
 void
 string_attr_cleanup(CK_ATTRIBUTE_PTR template)
 {
-
-        if (template->pValue) {
-                free(template->pValue);
+        freezero(template->pValue, template->ulValueLen);
                 template->pValue = NULL;
                 template->ulValueLen = 0;
-        }
 }
 
 /*
  * Release the storage allocated for object attribute with big integer
  * value.

@@ -1004,16 +998,13 @@
 {
 
         if (big == NULL)
                 return;
 
-        if (big->big_value) {
-                (void) memset(big->big_value, 0, big->big_value_len);
-                free(big->big_value);
+        freezero(big->big_value, big->big_value_len);
                 big->big_value = NULL;
                 big->big_value_len = 0;
-        }
 }
 
 
 /*
  * Clean up and release all the storage allocated to hold the big integer

@@ -1149,20 +1140,18 @@
         case CKO_SECRET_KEY:
                 if (OBJ_SEC(object_p)) {
                         /* cleanup key data area */
                         if (OBJ_SEC_VALUE(object_p) != NULL &&
                             OBJ_SEC_VALUE_LEN(object_p) > 0) {
-                                (void) memset(OBJ_SEC_VALUE(object_p), 0,
+                                freezero(OBJ_SEC_VALUE(object_p),
                                     OBJ_SEC_VALUE_LEN(object_p));
-                                free(OBJ_SEC_VALUE(object_p));
                         }
                         /* cleanup key schedule data area */
                         if (OBJ_KEY_SCHED(object_p) != NULL &&
                             OBJ_KEY_SCHED_LEN(object_p) > 0) {
-                                (void) memset(OBJ_KEY_SCHED(object_p), 0,
+                                freezero(OBJ_KEY_SCHED(object_p),
                                     OBJ_KEY_SCHED_LEN(object_p));
-                                free(OBJ_KEY_SCHED(object_p));
                         }
 
                         /* Release Secret Key Object struct. */
                         free(OBJ_SEC(object_p));
                         OBJ_SEC(object_p) = NULL;

@@ -6317,11 +6306,11 @@
                 return (CKR_HOST_MEMORY);
         }
         (void) memcpy(sk, old_secret_key_obj_p, sizeof (secret_key_obj_t));
 
         /* copy the secret key value */
-        sk->sk_value = malloc((sizeof (CK_BYTE) * sk->sk_value_len));
+        sk->sk_value = malloc(sk->sk_value_len);
         if (sk->sk_value == NULL) {
                 free(sk);
                 return (CKR_HOST_MEMORY);
         }
         (void) memcpy(sk->sk_value, old_secret_key_obj_p->sk_value,

@@ -6332,10 +6321,11 @@
          */
         if (old_secret_key_obj_p->key_sched != NULL &&
             old_secret_key_obj_p->keysched_len > 0) {
                 sk->key_sched = malloc(old_secret_key_obj_p->keysched_len);
                 if (sk->key_sched == NULL) {
+                        freezero(sk->sk_value, sk->sk_value_len);
                         free(sk);
                         return (CKR_HOST_MEMORY);
                 }
                 sk->keysched_len = old_secret_key_obj_p->keysched_len;
                 (void) memcpy(sk->key_sched, old_secret_key_obj_p->key_sched,