External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2025-02-22 11:22:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

storage_backend_iscsi(_direct): Properly clear secrets

Browse Source 
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@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Peter Krempa 2021-02-01 12:15:57 +01:00
parent 8d6353a066
commit fabd172065
2 changed files with 18 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/storage/storage_backend_iscsi.c
View File

@ -41,6 +41,7 @@
#include "virsecret.h"
#include "storage_util.h"
#include "virutil.h"
#include "virsecureerase.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@ -256,8 +257,9 @@ static int
virStorageBackendISCSISetAuth(const char *portal,
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;
@ -282,10 +284,10 @@ virStorageBackendISCSISetAuth(const char *portal,
&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);
virSecureErase(secret_value, secret_size);
secret_str[secret_size] = '\0';
if (virISCSINodeUpdate(portal,
source->devices[0].path,
@ -298,13 +300,13 @@ virStorageBackendISCSISetAuth(const char *portal,
virISCSINodeUpdate(portal,
source->devices[0].path,
"node.session.auth.password",
(const char *)secret_value) < 0)
secret_str) < 0)
goto cleanup;
ret = 0;
cleanup:
VIR_DISPOSE_N(secret_value, secret_size);
virSecureErase(secret_str, secret_size);
virObjectUnref(conn);
return ret;
}

17
src/storage/storage_backend_iscsi_direct.c
View File

@ -34,6 +34,7 @@
#include "virstring.h"
#include "virtime.h"
#include "viruuid.h"
#include "virsecureerase.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@ -87,8 +88,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 +115,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);
virSecureErase(secret_value, 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 +130,7 @@ virStorageBackendISCSIDirectSetAuth(struct iscsi_context *iscsi,
ret = 0;
cleanup:
VIR_DISPOSE_N(secret_value, secret_size);
virSecureErase(secret_str, secret_size);
virObjectUnref(conn);
return ret;
}