Add a command which allows to read a secret value from terminal.
'secret-passwd' is chosen as a name as the password has limitations as
passwords do have (printable, terminated by newline which is not
contained in the value). This makes it way more user-friendly to use the
secret driver with virsh when setting a value.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 docs/manpages/virsh.rst | 15 +++++++++
 tools/virsh-secret.c    | 70 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 6446a903ca..03364684b5 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -6564,6 +6564,21 @@ Set the value associated with *secret* (specified by its UUID) to the value
 Base64-encoded value *base64*.


+secret-passwd
+----------------
+
+**Syntax:**
+
+.. code-block::
+
+   secret-passwd secret
+
+Set the value associated with *secret* (specified by its UUID) to a string
+read from stdin. Note that input is terminated by a newline and the secret
+can't contain non-printable characters. Use *secret-set-value* for generic
+secrets. Note that this requires a terminal associated with virsh to read
+the password.
+
 secret-get-value
 ----------------

diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c
index 66369a25dc..9f64be6b14 100644
--- a/tools/virsh-secret.c
+++ b/tools/virsh-secret.c
@@ -219,6 +219,70 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd)
     return ret;
 }

+
+/*
+ * "secret-passwd" command
+ */
+static const vshCmdInfo info_secret_passwd[] = {
+    {.name = "help",
+     .data = N_("set a secret value from stdin")
+    },
+    {.name = "desc",
+     .data = N_("Set a secret value from stdin")
+    },
+    {.name = NULL}
+};
+
+static const vshCmdOptDef opts_secret_passwd[] = {
+    {.name = "secret",
+     .type = VSH_OT_DATA,
+     .flags = VSH_OFLAG_REQ,
+     .help = N_("secret UUID"),
+     .completer = virshSecretUUIDCompleter,
+    },
+    {.name = NULL}
+};
+
+static bool
+cmdSecretPasswd(vshControl *ctl,
+                const vshCmd *cmd)
+{
+    virSecretPtr secret;
+    g_autofree char *value = NULL;
+    int res;
+    bool ret = false;
+
+    if (!ctl->istty) {
+        vshError(ctl, "%s", _("secret-passwd requires a terminal"));
+        return false;
+    }
+
+    if (!(secret = virshCommandOptSecret(ctl, cmd, NULL)))
+        return false;
+
+    vshPrint(ctl, "%s", _("Enter new value for secret:"));
+    fflush(stdout);
+
+    if (!(value = getpass(""))) {
+        vshError(ctl, "%s", _("Failed to read secret"));
+        goto cleanup;
+    }
+
+    res = virSecretSetValue(secret, (unsigned char *) value, strlen(value), 0);
+
+    if (res != 0) {
+        vshError(ctl, "%s", _("Failed to set secret value"));
+        goto cleanup;
+    }
+    vshPrintExtra(ctl, "%s", _("Secret value set\n"));
+    ret = true;
+
+ cleanup:
+    virSecretFree(secret);
+    return ret;
+}
+
+
 /*
  * "secret-get-value" command
  */
@@ -805,6 +869,12 @@ const vshCmdDef secretCmds[] = {
      .info = info_secret_set_value,
      .flags = 0
     },
+    {.name = "secret-passwd",
+     .handler = cmdSecretPasswd,
+     .opts = opts_secret_passwd,
+     .info = info_secret_passwd,
+     .flags = 0
+    },
     {.name = "secret-undefine",
      .handler = cmdSecretUndefine,
      .opts = opts_secret_undefine,
-- 
2.24.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Jan 10, 2020 at 04:42:41PM +0100, Peter Krempa wrote:
> Add a command which allows to read a secret value from terminal.
> 'secret-passwd' is chosen as a name as the password has limitations as
> passwords do have (printable, terminated by newline which is not
> contained in the value). This makes it way more user-friendly to use the
> secret driver with virsh when setting a value.

In a later patch you already extend  secret-set-value to have a new
"--filename PATH" arg. I think we should do the same for interactive
usage, eg

 $ virsh secret-set-value --prompt

or --interactive / -i

> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  docs/manpages/virsh.rst | 15 +++++++++
>  tools/virsh-secret.c    | 70 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 85 insertions(+)
> 
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index 6446a903ca..03364684b5 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -6564,6 +6564,21 @@ Set the value associated with *secret* (specified by its UUID) to the value
>  Base64-encoded value *base64*.
> 
> 
> +secret-passwd
> +----------------
> +
> +**Syntax:**
> +
> +.. code-block::
> +
> +   secret-passwd secret
> +
> +Set the value associated with *secret* (specified by its UUID) to a string
> +read from stdin. Note that input is terminated by a newline and the secret
> +can't contain non-printable characters. Use *secret-set-value* for generic
> +secrets. Note that this requires a terminal associated with virsh to read
> +the password.
> +
>  secret-get-value
>  ----------------
> 
> diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c
> index 66369a25dc..9f64be6b14 100644
> --- a/tools/virsh-secret.c
> +++ b/tools/virsh-secret.c
> @@ -219,6 +219,70 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd)
>      return ret;
>  }
> 
> +
> +/*
> + * "secret-passwd" command
> + */
> +static const vshCmdInfo info_secret_passwd[] = {
> +    {.name = "help",
> +     .data = N_("set a secret value from stdin")
> +    },
> +    {.name = "desc",
> +     .data = N_("Set a secret value from stdin")
> +    },
> +    {.name = NULL}
> +};
> +
> +static const vshCmdOptDef opts_secret_passwd[] = {
> +    {.name = "secret",
> +     .type = VSH_OT_DATA,
> +     .flags = VSH_OFLAG_REQ,
> +     .help = N_("secret UUID"),
> +     .completer = virshSecretUUIDCompleter,
> +    },
> +    {.name = NULL}
> +};
> +
> +static bool
> +cmdSecretPasswd(vshControl *ctl,
> +                const vshCmd *cmd)
> +{
> +    virSecretPtr secret;
> +    g_autofree char *value = NULL;
> +    int res;
> +    bool ret = false;
> +
> +    if (!ctl->istty) {
> +        vshError(ctl, "%s", _("secret-passwd requires a terminal"));
> +        return false;
> +    }
> +
> +    if (!(secret = virshCommandOptSecret(ctl, cmd, NULL)))
> +        return false;
> +
> +    vshPrint(ctl, "%s", _("Enter new value for secret:"));
> +    fflush(stdout);
> +
> +    if (!(value = getpass(""))) {
> +        vshError(ctl, "%s", _("Failed to read secret"));
> +        goto cleanup;
> +    }
> +
> +    res = virSecretSetValue(secret, (unsigned char *) value, strlen(value), 0);
> +
> +    if (res != 0) {
> +        vshError(ctl, "%s", _("Failed to set secret value"));
> +        goto cleanup;
> +    }
> +    vshPrintExtra(ctl, "%s", _("Secret value set\n"));
> +    ret = true;
> +
> + cleanup:
> +    virSecretFree(secret);
> +    return ret;
> +}
> +
> +
>  /*
>   * "secret-get-value" command
>   */
> @@ -805,6 +869,12 @@ const vshCmdDef secretCmds[] = {
>       .info = info_secret_set_value,
>       .flags = 0
>      },
> +    {.name = "secret-passwd",
> +     .handler = cmdSecretPasswd,
> +     .opts = opts_secret_passwd,
> +     .info = info_secret_passwd,
> +     .flags = 0
> +    },
>      {.name = "secret-undefine",
>       .handler = cmdSecretUndefine,
>       .opts = opts_secret_undefine,
> -- 
> 2.24.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Tue, Jan 21, 2020 at 13:34:27 +0000, Daniel Berrange wrote:
> On Fri, Jan 10, 2020 at 04:42:41PM +0100, Peter Krempa wrote:
> > Add a command which allows to read a secret value from terminal.
> > 'secret-passwd' is chosen as a name as the password has limitations as
> > passwords do have (printable, terminated by newline which is not
> > contained in the value). This makes it way more user-friendly to use the
> > secret driver with virsh when setting a value.
> 
> In a later patch you already extend  secret-set-value to have a new
> "--filename PATH" arg. I think we should do the same for interactive
> usage, eg
> 
>  $ virsh secret-set-value --prompt
> 
> or --interactive / -i

Hmm, yeah. Originally I've added this one first for simplicity and then
dealt with the --filename thing, but you are right that this is the
better option.