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.
  */
 
 /*
  * Functions used for manipulating the keystore
  */

@@ -468,12 +469,11 @@
         }
 
         (void) lock_file(fd, B_FALSE, B_FALSE);
 
         (void) close(fd);
-        if (hashed_pin_salt)
-                free(hashed_pin_salt);
+        freezero(hashed_pin_salt, hashed_pin_salt_len);
         return (0);
 
 cleanup:
         (void) lock_file(fd, B_FALSE, B_FALSE);
         (void) unlink(ks_desc_file);

@@ -890,11 +890,11 @@
                 return (CKR_HOST_MEMORY);
         }
 
         if ((readn_nointr(fd, *hashed_pin, hashed_pin_size))
             != (ssize_t)hashed_pin_size) {
-                free(*hashed_pin);
+                freezero(*hashed_pin, hashed_pin_size + 1);
                 *hashed_pin = NULL;
                 return (CKR_FUNCTION_FAILED);
         }
         (*hashed_pin)[hashed_pin_size] = '\0';
         return (CKR_OK);

@@ -1318,80 +1318,79 @@
 
         /* decrypt data using old key */
         decrypted_len = 0;
         if (soft_keystore_crypt(enc_key, old_iv, B_FALSE, buf, nread,
             NULL, &decrypted_len) != CKR_OK) {
-                free(buf);
+                freezero(buf, nread);
                 goto cleanup;
         }
 
         decrypted_buf = malloc(decrypted_len);
         if (decrypted_buf == NULL) {
-                free(buf);
+                freezero(buf, nread);
                 goto cleanup;
         }
 
         if (soft_keystore_crypt(enc_key, old_iv, B_FALSE, buf, nread,
             decrypted_buf, &decrypted_len) != CKR_OK) {
-                free(buf);
-                free(decrypted_buf);
-                goto cleanup;
+                freezero(buf, nread);
+                freezero(decrypted_buf, decrypted_len);
         }
 
-        free(buf);
+        freezero(buf, nread);
 
         /* re-encrypt with new key */
         encrypted_len = 0;
         if (soft_keystore_crypt(new_enc_key, iv, B_TRUE, decrypted_buf,
             decrypted_len, NULL, &encrypted_len) != CKR_OK) {
-                free(decrypted_buf);
+                freezero(decrypted_buf, decrypted_len);
                 goto cleanup;
         }
 
         buf = malloc(encrypted_len);
         if (buf == NULL) {
-                free(decrypted_buf);
+                freezero(decrypted_buf, decrypted_len);
                 goto cleanup;
         }
 
         if (soft_keystore_crypt(new_enc_key, iv, B_TRUE, decrypted_buf,
             decrypted_len, buf, &encrypted_len) != CKR_OK) {
-                free(buf);
-                free(decrypted_buf);
+                freezero(buf, encrypted_len);
+                freezero(buf, decrypted_len);
                 goto cleanup;
         }
 
-        free(decrypted_buf);
+        freezero(decrypted_buf, decrypted_len);
 
         /* calculate hmac on re-encrypted data using new hmac key */
         hmac_len = OBJ_HMAC_SIZE;
         if (soft_keystore_hmac(new_hmac_key, B_TRUE, buf,
             encrypted_len, hmac, &hmac_len) != CKR_OK) {
-                free(buf);
+                freezero(buf, encrypted_len);
                 goto cleanup;
         }
 
         /* just for sanity check */
         if (hmac_len != OBJ_HMAC_SIZE) {
-                free(buf);
+                freezero(buf, encrypted_len);
                 goto cleanup;
         }
 
         /* write new hmac */
         if (writen_nointr(new_fd, (char *)hmac, OBJ_HMAC_SIZE)
             != OBJ_HMAC_SIZE) {
-                free(buf);
+                freezero(buf, encrypted_len);
                 goto cleanup;
         }
 
         /* write re-encrypted buffer to temp file */
         if (writen_nointr(new_fd, (void *)buf, encrypted_len)
             != encrypted_len) {
-                free(buf);
+                freezero(buf, encrypted_len);
                 goto cleanup;
         }
-        free(buf);
+        freezero(buf, encrypted_len);
         ret_val = 0;
 
 cleanup:
         /* unlock the files */
         (void) lock_file(old_fd, B_TRUE, B_FALSE);

@@ -1545,27 +1544,29 @@
                     != CKR_OK) {
                         goto cleanup;
                 }
                 if (writen_nointr(tmp_ks_fd, (void *)new_crypt_salt,
                     KS_KEY_SALT_SIZE) != KS_KEY_SALT_SIZE) {
-                        free(new_crypt_salt);
+                        freezero(new_crypt_salt,
+                            KS_KEY_SALT_SIZE);
                         (void) soft_cleanup_object(new_crypt_key);
                         goto cleanup;
                 }
-                free(new_crypt_salt);
+                freezero(new_crypt_salt, KS_KEY_SALT_SIZE);
 
                 if (soft_gen_hmac_key(newpin, &new_hmac_key, &new_hmac_salt)
                     != CKR_OK) {
                         (void) soft_cleanup_object(new_crypt_key);
                         goto cleanup;
                 }
                 if (writen_nointr(tmp_ks_fd, (void *)new_hmac_salt,
                     KS_HMAC_SALT_SIZE) != KS_HMAC_SALT_SIZE) {
-                        free(new_hmac_salt);
+                        freezero(new_hmac_salt,
+                            KS_HMAC_SALT_SIZE);
                         goto cleanup3;
                 }
-                free(new_hmac_salt);
+                freezero(new_hmac_salt, KS_HMAC_SALT_SIZE);
         } else {
                 if (soft_gen_crypt_key(newpin, &new_crypt_key,
                     (CK_BYTE **)&crypt_salt) != CKR_OK) {
                         goto cleanup;
                 }

@@ -1610,30 +1611,33 @@
                 goto cleanup3;
         }
 
         if ((readn_nointr(fd, hashed_pin_salt, hashed_pin_salt_length)) !=
             (ssize_t)hashed_pin_salt_length) {
-                free(hashed_pin_salt);
+                freezero(hashed_pin_salt,
+                    hashed_pin_salt_length + 1);
                 goto cleanup3;
         }
 
         if ((writen_nointr(tmp_ks_fd, hashed_pin_salt, hashed_pin_salt_length))
             != (ssize_t)hashed_pin_salt_length) {
-                free(hashed_pin_salt);
+                freezero(hashed_pin_salt,
+                    hashed_pin_salt_length + 1);
                 goto cleanup3;
         }
 
         hashed_pin_salt[hashed_pin_salt_length] = '\0';
 
         /* old hashed pin length and value can be ignored, generate new one */
         if (soft_gen_hashed_pin(newpin, &new_hashed_pin,
             &hashed_pin_salt) < 0) {
-                free(hashed_pin_salt);
+                freezero(hashed_pin_salt,
+                    hashed_pin_salt_length + 1);
                 goto cleanup3;
         }
 
-        free(hashed_pin_salt);
+        freezero(hashed_pin_salt, hashed_pin_salt_length + 1);
 
         if (new_hashed_pin == NULL) {
                 goto cleanup3;
         }
 

@@ -1761,16 +1765,12 @@
         if (!lock_held) {
                 if (lock_file(fd, B_FALSE, B_FALSE) < 0) {
                         ret_val = 1;
                 }
         }
-        if (crypt_salt != NULL) {
-                free(crypt_salt);
-        }
-        if (hmac_salt != NULL) {
-                free(hmac_salt);
-        }
+        freezero(crypt_salt, KS_KEY_SALT_SIZE);
+        freezero(hmac_salt, KS_HMAC_SALT_SIZE);
         (void) close(fd);
         (void) close(tmp_ks_fd);
         if (ret_val != 0) {
                 (void) remove(tmp_ks_desc_name);
         }

@@ -1853,16 +1853,12 @@
 
 cleanup:
         /* unlock the file */
         (void) lock_file(fd, B_TRUE, B_FALSE);
         (void) close(fd);
-        if (crypt_salt != NULL) {
-                free(crypt_salt);
-        }
-        if (hmac_salt != NULL) {
-                free(hmac_salt);
-        }
+        freezero(crypt_salt, KS_KEY_SALT_SIZE);
+        freezero(hmac_salt, KS_HMAC_SALT_SIZE);
         return (ret_val);
 }
 
 /*
  *      FUNCTION: soft_keystore_get_objs

@@ -1978,11 +1974,11 @@
 
         /* free all the objects found before hitting the error */
         tmp = *result_obj_list;
         while (tmp) {
                 *result_obj_list = tmp->next;
-                free(tmp->buf);
+                freezero(tmp->buf, tmp->size);
                 free(tmp);
                 tmp = *result_obj_list;
         }
         *result_obj_list = NULL;
         return (rv);

@@ -2085,34 +2081,34 @@
 
                 /* verify HMAC of the object, make sure it matches */
                 hmac_size = OBJ_HMAC_SIZE;
                 if (soft_keystore_hmac(hmac_key, B_FALSE, buf,
                     nread, obj_hmac, &hmac_size) != CKR_OK) {
-                        free(buf);
+                        freezero(buf, nread);
                         rv = CKR_FUNCTION_FAILED;
                         goto cleanup;
                 }
 
                 /* decrypt object */
                 if (soft_keystore_crypt(enc_key, iv, B_FALSE, buf, nread,
                     NULL, &out_len) != CKR_OK) {
-                        free(buf);
+                        freezero(buf, nread);
                         rv = CKR_FUNCTION_FAILED;
                         goto cleanup;
                 }
 
                 decrypted_buf = malloc(sizeof (uchar_t) * out_len);
                 if (decrypted_buf == NULL) {
-                        free(buf);
+                        freezero(buf, nread);
                         rv = CKR_HOST_MEMORY;
                         goto cleanup;
                 }
 
                 if (soft_keystore_crypt(enc_key, iv, B_FALSE, buf, nread,
                     decrypted_buf, &out_len) != CKR_OK) {
-                        free(decrypted_buf);
-                        free(buf);
+                        freezero(buf, nread);
+                        freezero(decrypted_buf, out_len);
                         rv = CKR_FUNCTION_FAILED;
                         goto cleanup;
                 }
 
                 obj->size = out_len - MAXPATHLEN;

@@ -2124,18 +2120,18 @@
                  * See prepare_data_for_encrypt() function in the file
                  * to understand how and why the pathname is added.
                  */
                 obj->buf = malloc(sizeof (uchar_t) * (out_len - MAXPATHLEN));
                 if (obj->buf == NULL) {
-                        free(decrypted_buf);
-                        free(buf);
+                        freezero(buf, nread);
+                        freezero(decrypted_buf, out_len);
                         rv = CKR_HOST_MEMORY;
                         goto cleanup;
                 }
                 (void) memcpy(obj->buf, decrypted_buf + MAXPATHLEN, obj->size);
-                free(decrypted_buf);
-                free(buf);
+                freezero(buf, nread);
+                freezero(decrypted_buf, out_len);
                 *return_obj = obj;
         }
 
 cleanup:
 

@@ -2334,57 +2330,57 @@
                 }
 
                 if (soft_keystore_crypt(enc_key, iv,
                     B_TRUE, prepared_buf, prepared_len,
                     NULL, &out_len) != CKR_OK) {
-                        free(prepared_buf);
+                        freezero(prepared_buf, prepared_len);
                         goto cleanup2;
                 }
 
                 encrypted_buf = malloc(out_len * sizeof (char));
                 if (encrypted_buf == NULL) {
-                        free(prepared_buf);
+                        freezero(prepared_buf, prepared_len);
                         goto cleanup2;
                 }
 
                 if (soft_keystore_crypt(enc_key, iv,
                     B_TRUE, prepared_buf, prepared_len,
                     encrypted_buf, &out_len) != CKR_OK) {
-                        free(encrypted_buf);
-                        free(prepared_buf);
+                        freezero(encrypted_buf, out_len);
+                        freezero(prepared_buf, prepared_len);
                         goto cleanup2;
                 }
-                free(prepared_buf);
+                freezero(prepared_buf, prepared_len);
 
                 /* calculate HMAC of encrypted object */
                 hmac_size = OBJ_HMAC_SIZE;
                 if (soft_keystore_hmac(hmac_key, B_TRUE, encrypted_buf,
                     out_len, obj_hmac, &hmac_size) != CKR_OK) {
-                        free(encrypted_buf);
+                        freezero(encrypted_buf, out_len);
                         goto cleanup2;
                 }
 
                 if (hmac_size != OBJ_HMAC_SIZE) {
-                        free(encrypted_buf);
+                        freezero(encrypted_buf, out_len);
                         goto cleanup2;
                 }
 
                 /* write hmac */
                 if (writen_nointr(obj_fd, (void *)obj_hmac,
                     sizeof (obj_hmac)) != sizeof (obj_hmac)) {
-                        free(encrypted_buf);
+                        freezero(encrypted_buf, out_len);
                         goto cleanup2;
                 }
 
                 /* write encrypted object */
                 if (writen_nointr(obj_fd, (void *)encrypted_buf, out_len)
                     != out_len) {
-                        free(encrypted_buf);
+                        freezero(encrypted_buf, out_len);
                         goto cleanup2;
                 }
 
-                free(encrypted_buf);
+                freezero(encrypted_buf, out_len);
         }
 
 
         (void) close(obj_fd);
         (void) snprintf((char *)keyhandle->name, sizeof (keyhandle->name),

@@ -2419,10 +2415,12 @@
                         (void) close(fd);
                         return (-1);
                 }
         }
         (void) close(fd);
+        explicit_bzero(obj_hmac, sizeof (obj_hmac));
+        explicit_bzero(iv, sizeof (iv));
         return (0);
 
 cleanup2:
 
         /* remove object file.  No need to remove lock first */

@@ -2436,10 +2434,12 @@
                 /* release lock on description file */
                 (void) lock_file(fd, B_FALSE, B_FALSE);
         }
 
         (void) close(fd);
+        explicit_bzero(obj_hmac, sizeof (obj_hmac));
+        explicit_bzero(iv, sizeof (iv));
         return (-1);
 }
 
 /*
  *      FUNCTION: soft_keystore_modify_obj

@@ -2589,48 +2589,48 @@
                         goto cleanup2;
                 }
 
                 encrypted_buf = malloc(out_len * sizeof (char));
                 if (encrypted_buf == NULL) {
-                        free(prepared_buf);
+                        freezero(prepared_buf, prepared_len);
                         goto cleanup2;
                 }
 
                 if (soft_keystore_crypt(enc_key, iv, B_TRUE, prepared_buf,
                     prepared_len, encrypted_buf, &out_len) != CKR_OK) {
-                        free(encrypted_buf);
-                        free(prepared_buf);
+                        freezero(prepared_buf, prepared_len);
+                        freezero(encrypted_buf, out_len);
                         goto cleanup2;
                 }
 
-                free(prepared_buf);
+                freezero(prepared_buf, prepared_len);
 
                 /* calculate hmac on encrypted buf */
                 hmac_size = OBJ_HMAC_SIZE;
                 if (soft_keystore_hmac(hmac_key, B_TRUE, encrypted_buf,
                     out_len, obj_hmac, &hmac_size) != CKR_OK) {
-                        free(encrypted_buf);
+                        freezero(encrypted_buf, out_len);
                         goto cleanup2;
                 }
 
                 if (hmac_size != OBJ_HMAC_SIZE) {
-                        free(encrypted_buf);
+                        freezero(encrypted_buf, out_len);
                         goto cleanup2;
                 }
 
                 if (writen_nointr(tmp_fd, (char *)obj_hmac, OBJ_HMAC_SIZE)
                     != OBJ_HMAC_SIZE) {
-                        free(encrypted_buf);
+                        freezero(encrypted_buf, out_len);
                         goto cleanup2;
                 }
 
                 if (writen_nointr(tmp_fd, (void *)encrypted_buf, out_len)
                     != out_len) {
-                        free(encrypted_buf);
+                        freezero(encrypted_buf, out_len);
                         goto cleanup2;
                 }
-                free(encrypted_buf);
+                freezero(encrypted_buf, out_len);
         }
         (void) close(tmp_fd);
 
         /* rename updated temporary object file */
         if (rename(tmp_name, orig_name) != 0) {

@@ -2663,10 +2663,12 @@
 
         (void) close(ks_fd);
 
         (void) close(fd);
 
+        explicit_bzero(iv, sizeof (iv));
+        explicit_bzero(obj_hmac, sizeof (obj_hmac));
         return (0); /* All operations completed successfully */
 
 cleanup2:
         (void) close(tmp_fd);
         (void) remove(tmp_name);

@@ -2677,10 +2679,12 @@
 cleanup:
         /* unlock keystore description file */
         (void) lock_file(ks_fd, B_FALSE, B_FALSE);
         (void) close(ks_fd);
         (void) remove(tmp_ks_name);
+        explicit_bzero(iv, sizeof (iv));
+        explicit_bzero(obj_hmac, sizeof (obj_hmac));
         return (-1);
 }
 
 /*
  *      FUNCTION: soft_keystore_del_obj

@@ -2801,11 +2805,11 @@
                 goto cleanup;
         }
 
         if ((readn_nointr(fd, *salt, hashed_pin_salt_size))
             != (ssize_t)hashed_pin_salt_size) {
-                free(*salt);
+                freezero(*salt, hashed_pin_salt_size + 1);
                 goto cleanup;
         }
         (*salt)[hashed_pin_salt_size] = '\0';
 
         ret_val = 0;