[PATCH 08/14] virsh: cmdSecretSetValue: Rework handling of the secret value
Use a single buffer for the secret to make it easier to follow it's
lifecycle. For base64 decoding use a local temporary buffer which will
be cleared right away.
This also uses memset for clearing the bufer instead of VIR_DISPOSE_N
which is being phased out.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tools/virsh-secret.c | 34 ++++++++++++++++------------------
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c
index 5d656151e8..23bbd61698 100644
--- a/tools/virsh-secret.c
+++ b/tools/virsh-secret.c
@@ -202,10 +202,8 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd)
g_autoptr(virshSecret) secret = NULL;
const char *base64 = NULL;
const char *filename = NULL;
- char *file_buf = NULL;
- size_t file_len = 0;
- unsigned char *value;
- size_t value_size;
+ g_autofree char *secret_val = NULL;
+ size_t secret_len = 0;
bool plain = vshCommandOptBool(cmd, "plain");
bool interactive = vshCommandOptBool(cmd, "interactive");
int res;
@@ -228,41 +226,41 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd)
if (base64) {
/* warn users that the --base64 option passed from command line is wrong */
vshError(ctl, _("Passing secret value as command-line argument is
insecure!"));
+ secret_val = g_strdup(base64);
+ secret_len = strlen(secret_val);
} else if (filename) {
ssize_t read_ret;
- if ((read_ret = virFileReadAll(filename, 1024, &file_buf)) < 0) {
+ if ((read_ret = virFileReadAll(filename, 1024, &secret_val)) < 0) {
vshSaveLibvirtError();
return false;
}
- file_len = read_ret;
- base64 = file_buf;
+ secret_len = read_ret;
} else if (interactive) {
vshPrint(ctl, "%s", _("Enter new value for secret:"));
fflush(stdout);
- if (!(file_buf = virGetPassword())) {
+ if (!(secret_val = virGetPassword())) {
vshError(ctl, "%s", _("Failed to read secret"));
return false;
}
- file_len = strlen(file_buf);
+ secret_len = strlen(secret_val);
plain = true;
} else {
vshError(ctl, _("Input secret value is missing"));
return false;
}
- if (plain) {
- value = g_steal_pointer(&file_buf);
- value_size = file_len;
- file_len = 0;
- } else {
- value = g_base64_decode(base64, &value_size);
+ if (!plain) {
+ g_autofree char *tmp = g_steal_pointer(&secret_val);
+ size_t tmp_len = secret_len;
+
+ secret_val = (char *) g_base64_decode(tmp, &secret_len);
+ memset(tmp, 0, tmp_len);
}
- res = virSecretSetValue(secret, value, value_size, 0);
- VIR_DISPOSE_N(value, value_size);
- VIR_DISPOSE_N(file_buf, file_len);
+ res = virSecretSetValue(secret, (unsigned char *) secret_val, secret_len, 0);
+ memset(secret_val, 0, secret_len);
if (res != 0) {
vshError(ctl, "%s", _("Failed to set secret value"));
--
2.29.2