Re: [PATCH v2 09/27] storage_backend_iscsi(_direct): Properly clear secrets

The code pretends that it cares about clearing the secret values, but passes the secret value to a realloc, which may copy the value somewhere else and doesn't sanitize the original location when it does so. Since we want to construct a string from the value, let's copy it to a new piece of memory which has the space for the 'NUL' byte ourselves, to prevent a random realloc keeping the data around. While at it, use virSecureErase instead of VIR_DISPOSE_N since it's being phased out. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/storage/storage_backend_iscsi.c | 16 +++++++++------- src/storage/storage_backend_iscsi_direct.c | 17 +++++++++-------- 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/storage_backend_iscsi_direct.c index 12b075db0b..78b12f057f 100644 --- a/src/storage/storage_backend_iscsi_direct.c +++ b/src/storage/storage_backend_iscsi_direct.c @@ -87,8 +87,9 @@ static int virStorageBackendISCSIDirectSetAuth(struct iscsi_context *iscsi, virStoragePoolSourcePtr source) { - unsigned char *secret_value = NULL; + g_autofree unsigned char *secret_value = NULL; size_t secret_size; + g_autofree char *secret_str = NULL; virStorageAuthDefPtr authdef = source->auth; int ret = -1; virConnectPtr conn = NULL; @@ -113,14 +114,13 @@ virStorageBackendISCSIDirectSetAuth(struct iscsi_context *iscsi, &secret_value, &secret_size) < 0) goto cleanup; - if (VIR_REALLOC_N(secret_value, secret_size + 1) < 0) - goto cleanup; - - secret_value[secret_size] = '\0'; + secret_str = g_new0(char, secret_size + 1); + memcpy(secret_str, secret_value, secret_size); + memset(secret_value, 0, secret_size); + secret_str[secret_size] = '\0'; if (iscsi_set_initiator_username_pwd(iscsi, - authdef->username, - (const char *)secret_value) < 0) { + authdef->username, secret_str) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to set credential: %s"), iscsi_get_error(iscsi)); @@ -129,7 +129,8 @@ virStorageBackendISCSIDirectSetAuth(struct iscsi_context *iscsi, ret = 0; cleanup: - VIR_DISPOSE_N(secret_value, secret_size); + if (secret_str) + memset(secret_str, 0, secret_size);