9:42 a.m.
The currently existing virsh APIs for secrets are awful for human use
and don't promote security.
Peter Krempa (4):
virsh: secret: Add 'secret-passwd' command
virsh: secret: Allow getting secret's value without base64 encoding
virsh: secret: Allow setting secrets from file
docs: secret: Unify and sanitize examples on how to set secret value
docs/formatsecret.html.in | 86 ++++++++++++++++++----------
docs/manpages/virsh.rst | 26 ++++++++-
tools/virsh-secret.c | 116 ++++++++++++++++++++++++++++++++++++--
3 files changed, 189 insertions(+), 39 deletions(-)
--
2.24.1
9:42 a.m.
New subject: [libvirt] [PATCH 1/4] virsh: secret: Add 'secret-passwd' command
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(a)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
7:34 a.m.
New subject: [libvirt] [PATCH 1/4] virsh: secret: Add 'secret-passwd' command
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(a)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(a)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 :|
7:37 a.m.
New subject: [libvirt] [PATCH 1/4] virsh: secret: Add 'secret-passwd' command
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.
9:42 a.m.
New subject: [libvirt] [PATCH 2/4] virsh: secret: Allow getting secret's value without base64 encoding
Users might want to get the raw value instead of dealing with base64
encoding. This might be useful for redirection to file and also for
simple human-readable secrets.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/manpages/virsh.rst | 6 +++++-
tools/virsh-secret.c | 16 ++++++++++++++--
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 03364684b5..fcc8ef6758 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -6586,11 +6586,15 @@ secret-get-value
.. code-block::
- secret-get-value secret
+ secret-get-value [--plain] secret
Output the value associated with *secret* (specified by its UUID) to stdout,
encoded using Base64.
+If the *--plain* flag is used the value is not base64 encoded, but rather
+printed raw. Note that unless virsh is started in quiet mode (*virsh -q*) it
+prints a newline at the end of the command. This newline is not part of the
+secret.
secret-undefine
---------------
diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c
index 9f64be6b14..6d95ed9d5d 100644
--- a/tools/virsh-secret.c
+++ b/tools/virsh-secret.c
@@ -303,6 +303,10 @@ static const vshCmdOptDef opts_secret_get_value[] = {
.help = N_("secret UUID"),
.completer = virshSecretUUIDCompleter,
},
+ {.name = "plain",
+ .type = VSH_OT_BOOL,
+ .help = N_("get value without converting to base64")
+ },
{.name = NULL}
};
@@ -313,6 +317,7 @@ cmdSecretGetValue(vshControl *ctl, const vshCmd *cmd)
VIR_AUTODISPOSE_STR base64 = NULL;
unsigned char *value;
size_t value_size;
+ bool plain = vshCommandOptBool(cmd, "plain");
bool ret = false;
secret = virshCommandOptSecret(ctl, cmd, NULL);
@@ -323,9 +328,16 @@ cmdSecretGetValue(vshControl *ctl, const vshCmd *cmd)
if (value == NULL)
goto cleanup;
- base64 = g_base64_encode(value, value_size);
+ if (plain) {
+ if (fwrite(value, 1, value_size, stdout) != value_size) {
+ vshError(ctl, "failed to write secret");
+ goto cleanup;
+ }
+ } else {
+ base64 = g_base64_encode(value, value_size);
- vshPrint(ctl, "%s", base64);
+ vshPrint(ctl, "%s", base64);
+ }
ret = true;
cleanup:
--
2.24.1
7:35 a.m.
New subject: [libvirt] [PATCH 2/4] virsh: secret: Allow getting secret's value without base64 encoding
On Fri, Jan 10, 2020 at 04:42:42PM +0100, Peter Krempa wrote:
Users might want to get the raw value instead of dealing with base64
encoding. This might be useful for redirection to file and also for
simple human-readable secrets.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/manpages/virsh.rst | 6 +++++-
tools/virsh-secret.c | 16 ++++++++++++++--
2 files changed, 19 insertions(+), 3 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
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 :|
9:42 a.m.
New subject: [libvirt] [PATCH 3/4] virsh: secret: Allow setting secrets from file
The necessity to specify the secret value as command argument is
insecure. Allow reading the secret from a file.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/manpages/virsh.rst | 5 +++--
tools/virsh-secret.c | 30 +++++++++++++++++++++++++++---
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index fcc8ef6758..992b1daf90 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -6558,10 +6558,11 @@ secret-set-value
.. code-block::
- secret-set-value secret base64
+ secret-set-value secret (--file filename | base64)
Set the value associated with *secret* (specified by its UUID) to the value
-Base64-encoded value *base64*.
+Base64-encoded value *base64* or from file named *filename*. Note that *--file*
+and *base64* options are mutually exclusive.
secret-passwd
diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c
index 6d95ed9d5d..dd6bf75705 100644
--- a/tools/virsh-secret.c
+++ b/tools/virsh-secret.c
@@ -177,9 +177,13 @@ static const vshCmdOptDef opts_secret_set_value[] = {
.help = N_("secret UUID"),
.completer = virshSecretUUIDCompleter,
},
+ {.name = "file",
+ .type = VSH_OT_STRING,
+ .flags = VSH_OFLAG_REQ_OPT,
+ .help = N_("read secret from file"),
+ },
{.name = "base64",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
+ .type = VSH_OT_STRING,
.help = N_("base64-encoded secret value")
},
{.name = NULL}
@@ -191,17 +195,37 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd)
virSecretPtr secret;
size_t value_size;
const char *base64 = NULL;
+ const char *filename = NULL;
unsigned char *value;
int res;
bool ret = false;
+ VSH_EXCLUSIVE_OPTIONS("file", "base64");
+
if (!(secret = virshCommandOptSecret(ctl, cmd, NULL)))
return false;
if (vshCommandOptStringReq(ctl, cmd, "base64", &base64) < 0)
goto cleanup;
- value = g_base64_decode(base64, &value_size);
+ if (vshCommandOptStringReq(ctl, cmd, "file", &filename) < 0)
+ goto cleanup;
+
+ if (!base64 && !filename) {
+ vshError(ctl, _("Input secret value is missing"));
+ goto cleanup;
+ }
+
+ if (filename) {
+ ssize_t read_len;
+ if ((read_len = virFileReadAll(filename, 1024, (char **) &value)) < 0) {
+ vshSaveLibvirtError();
+ goto cleanup;
+ }
+ value_size = read_len;
+ } else {
+ value = g_base64_decode(base64, &value_size);
+ }
res = virSecretSetValue(secret, value, value_size, 0);
memset(value, 0, value_size);
--
2.24.1
7:38 a.m.
New subject: [libvirt] [PATCH 3/4] virsh: secret: Allow setting secrets from file
On Fri, Jan 10, 2020 at 04:42:43PM +0100, Peter Krempa wrote:
The necessity to specify the secret value as command argument is
insecure. Allow reading the secret from a file.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/manpages/virsh.rst | 5 +++--
tools/virsh-secret.c | 30 +++++++++++++++++++++++++++---
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index fcc8ef6758..992b1daf90 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -6558,10 +6558,11 @@ secret-set-value
.. code-block::
- secret-set-value secret base64
+ secret-set-value secret (--file filename | base64)
Set the value associated with *secret* (specified by its UUID) to the value
-Base64-encoded value *base64*.
+Base64-encoded value *base64* or from file named *filename*. Note that *--file*
+and *base64* options are mutually exclusive.
You added a --plain option to secret-get-value.
It would naturally suggest that we do the same here, then we can
support
secret-set-value $BASE64STR
secret-set-value --plain $RAWSTR
secret-set-value --file FILENAME-WITH-BASE64-STR
secret-set-value --plain --file FILENAME-WITH-RAW-STR
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 :|
7:43 a.m.
New subject: [libvirt] [PATCH 3/4] virsh: secret: Allow setting secrets from file
On Tue, Jan 21, 2020 at 13:38:13 +0000, Daniel Berrange wrote:
On Fri, Jan 10, 2020 at 04:42:43PM +0100, Peter Krempa wrote:
> The necessity to specify the secret value as command argument is
> insecure. Allow reading the secret from a file.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> docs/manpages/virsh.rst | 5 +++--
> tools/virsh-secret.c | 30 +++++++++++++++++++++++++++---
> 2 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index fcc8ef6758..992b1daf90 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -6558,10 +6558,11 @@ secret-set-value
>
> .. code-block::
>
> - secret-set-value secret base64
> + secret-set-value secret (--file filename | base64)
>
> Set the value associated with *secret* (specified by its UUID) to the value
> -Base64-encoded value *base64*.
> +Base64-encoded value *base64* or from file named *filename*. Note that *--file*
> +and *base64* options are mutually exclusive.
You added a --plain option to secret-get-value.
It would naturally suggest that we do the same here, then we can
support
secret-set-value $BASE64STR
secret-set-value --plain $RAWSTR
I think that both of the above should not have existed in the first
place. Adding the possibility to add plain secrets via argument looks to
me as a step back. If I could do it, I'd remove the base64 via command
line arguments as well.
secret-set-value --file FILENAME-WITH-BASE64-STR
This seems a bit pointless to me.
secret-set-value --plain --file FILENAME-WITH-RAW-STR
7:53 a.m.
New subject: [libvirt] [PATCH 3/4] virsh: secret: Allow setting secrets from file
On Tue, Jan 21, 2020 at 02:43:44PM +0100, Peter Krempa wrote:
On Tue, Jan 21, 2020 at 13:38:13 +0000, Daniel Berrange wrote:
> On Fri, Jan 10, 2020 at 04:42:43PM +0100, Peter Krempa wrote:
> > The necessity to specify the secret value as command argument is
> > insecure. Allow reading the secret from a file.
> >
> > Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> > ---
> > docs/manpages/virsh.rst | 5 +++--
> > tools/virsh-secret.c | 30 +++++++++++++++++++++++++++---
> > 2 files changed, 30 insertions(+), 5 deletions(-)
> >
> > diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> > index fcc8ef6758..992b1daf90 100644
> > --- a/docs/manpages/virsh.rst
> > +++ b/docs/manpages/virsh.rst
> > @@ -6558,10 +6558,11 @@ secret-set-value
> >
> > .. code-block::
> >
> > - secret-set-value secret base64
> > + secret-set-value secret (--file filename | base64)
> >
> > Set the value associated with *secret* (specified by its UUID) to the value
> > -Base64-encoded value *base64*.
> > +Base64-encoded value *base64* or from file named *filename*. Note that
*--file*
> > +and *base64* options are mutually exclusive.
>
> You added a --plain option to secret-get-value.
>
> It would naturally suggest that we do the same here, then we can
> support
>
> secret-set-value $BASE64STR
> secret-set-value --plain $RAWSTR
I think that both of the above should not have existed in the first
place. Adding the possibility to add plain secrets via argument looks to
me as a step back. If I could do it, I'd remove the base64 via command
line arguments as well.
True, we probably should forbid --plain RAWSTR.
Removing existing option is not viable, but perhaps we can at least print
a warning message to stderr
> secret-set-value --file FILENAME-WITH-BASE64-STR
This seems a bit pointless to me.
Bear in mind that the place you are getting the secret from may already
be giving it to you in base64 format, so it is useful to write it to the
file as-is, as you would do today when using set-value base64
> secret-set-value --plain --file FILENAME-WITH-RAW-STR
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 :|
7:52 a.m.
New subject: [libvirt] [PATCH 3/4] virsh: secret: Allow setting secrets from file
On Fri, Jan 10, 2020 at 04:42:43PM +0100, Peter Krempa wrote:
The necessity to specify the secret value as command argument is
insecure. Allow reading the secret from a file.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/manpages/virsh.rst | 5 +++--
tools/virsh-secret.c | 30 +++++++++++++++++++++++++++---
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index fcc8ef6758..992b1daf90 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -6558,10 +6558,11 @@ secret-set-value
.. code-block::
- secret-set-value secret base64
+ secret-set-value secret (--file filename | base64)
Set the value associated with *secret* (specified by its UUID) to the value
-Base64-encoded value *base64*.
+Base64-encoded value *base64* or from file named *filename*. Note that *--file*
+and *base64* options are mutually exclusive.
secret-passwd
Please include a way to read the secret from an EBCDIC-encoded file,
just for completeness.
Jano
9:42 a.m.
New subject: [libvirt] [PATCH 4/4] docs: secret: Unify and sanitize examples on how to set secret value
Discourage passing secrets as commandline arguments.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/formatsecret.html.in | 86 +++++++++++++++++++++++++--------------
1 file changed, 55 insertions(+), 31 deletions(-)
diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in
index 8f5383cf64..61a8396682 100644
--- a/docs/formatsecret.html.in
+++ b/docs/formatsecret.html.in
@@ -76,13 +76,13 @@
<pre>
# virsh secret-define volume-secret.xml
Secret 0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f created
-#
-# MYSECRET=`printf %s "open sesame" | base64`
-# virsh secret-set-value 0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f $MYSECRET
-Secret value set
-#
</pre>
+ <p>
+ See <a href="#settingSecrets">virsh secret-set-value</a> on
how
+ to set the value of the secret.
+ </p>
+
<p>
The volume type secret can be supplied either in volume XML during
creation of a <a href="formatstorage.html#StorageVol">storage
volume</a>
@@ -103,12 +103,11 @@ Secret value set
# virsh secret-define luks-secret.xml
Secret f52a81b2-424e-490c-823d-6bd4235bc57 created
-#
-# MYSECRET=`printf %s "letmein" | base64`
-# virsh secret-set-value f52a81b2-424e-490c-823d-6bd4235bc57 $MYSECRET
-Secret value set
-#
</pre>
+ <p>
+ See <a href="#settingSecrets">virsh secret-set-value</a> on
how
+ to set the value of the secret.
+ </p>
<p>
The volume type secret can be supplied in domain XML for a luks storage
@@ -156,13 +155,11 @@ Secret 1b40a534-8301-45d5-b1aa-11894ebb1735 created
UUID Usage
-----------------------------------------------------------
1b40a534-8301-45d5-b1aa-11894ebb1735 cephx ceph_example
-#
-# CEPHPHRASE=`printf %s "pass phrase" | base64`
-# virsh secret-set-value 1b40a534-8301-45d5-b1aa-11894ebb1735 $CEPHPHRASE
-Secret value set
-
-#
</pre>
+ <p>
+ See <a href="#settingSecrets">virsh secret-set-value</a> on
how
+ to set the value of the secret.
+ </p>
<p>
The ceph secret can then be used by UUID or by the
@@ -229,7 +226,9 @@ incominguser myname mysecret
<p>
Next, use <code>virsh secret-define iscsi-secret.xml</code> to define
- the secret and <code>virsh secret-set-value</code> using the generated
+ the secret and
+ <code><a href="#settingSecrets">virsh
secret-set-value</a></code>
+ using the generated
UUID value and a base64 generated secret value in order to define the
chosen secret pass phrase. The pass phrase must match the password
used in the iSCSI authentication configuration file.
@@ -243,12 +242,13 @@ Secret c4dbe20b-b1a3-4ac1-b6e6-2ac97852ebb6 created
-----------------------------------------------------------
c4dbe20b-b1a3-4ac1-b6e6-2ac97852ebb6 iscsi libvirtiscsi
-# MYSECRET=`printf %s "mysecret" | base64`
-# virsh secret-set-value c4dbe20b-b1a3-4ac1-b6e6-2ac97852ebb6 $MYSECRET
-Secret value set
-#
</pre>
+ <p>
+ See <a href="#settingSecrets">virsh secret-set-value</a> on
how
+ to set the value of the secret.
+ </p>
+
<p>
The iSCSI secret can then be used by UUID or by the
usage name via the <code><auth></code> element in a
domain's
@@ -313,19 +313,13 @@ Secret 718c71bd-67b5-4a2b-87ec-a24e8ca200dc created
Once the secret is defined, a secret value will need to be set. The
secret would be the passphrase used to access the TLS credentials.
The following is a simple example of using
- <code>virsh secret-set-value</code> to set the secret value. The
+ <code><a href="#settingSecrets">virsh
secret-set-value</a></code> to set
+ the secret value. The
<a href="html/libvirt-libvirt-secret.html#virSecretSetValue">
<code>virSecretSetValue</code></a> API may also be used to set
a more secure secret without using printable/readable characters.
</p>
- <pre>
-# MYSECRET=`printf %s "letmein" | base64`
-# virsh secret-set-value 718c71bd-67b5-4a2b-87ec-a24e8ca200dc $MYSECRET
-Secret value set
-
- </pre>
-
<h3><a id="vTPMUsageType">Usage type
"vtpm"</a></h3>
<p>
@@ -370,17 +364,47 @@ Secret 6dd3e4a5-1d76-44ce-961f-f119f5aad935 created
Once the secret is defined, a secret value will need to be set. The
secret would be the passphrase used to decrypt the vTPM state.
The following is a simple example of using
- <code>virsh secret-set-value</code> to set the secret value. The
+ <code><a href="#settingSecrets">virsh
secret-set-value</a></code>
+ to set the secret value. The
<a href="html/libvirt-libvirt-secret.html#virSecretSetValue">
<code>virSecretSetValue</code></a> API may also be used to set
a more secure secret without using printable/readable characters.
</p>
+ <h2><a id="settingSecrets">Setting secret values in
virsh</a></h2>
+
+ <p>
+ To set the value of the secret you can use the following virsh commands.
+ If the secret is a password-like string (printable characters, no newline)
+ you can use:
+ </p>
+ <pre>
+# virsh secret-passwd 6dd3e4a5-1d76-44ce-961f-f119f5aad935
+Enter new value for secret:
+Secret value set
+ </pre>
+
+ <p>
+ Another secure option is to read the secret from a file. This way the
+ secret can contain any bytes (even NUL and non-printable characters). The
+ length of the secret is the length of the input file.
+ </p>
+
+ <pre>
+# virsh secret-set-value 6dd3e4a5-1d76-44ce-961f-f119f5aad935 --file secretinfile
+Secret value set
+ </pre>
+
+ <p>
+ The secret can also be set via an argument, but note that other users
+ may see it in the process listing output. The secret must be base64
+ encoded.
+ </p>
+
<pre>
# MYSECRET=`printf %s "open sesame" | base64`
# virsh secret-set-value 6dd3e4a5-1d76-44ce-961f-f119f5aad935 $MYSECRET
Secret value set
-
</pre>
</body>
--
2.24.1
7:39 a.m.
New subject: [libvirt] [PATCH 4/4] docs: secret: Unify and sanitize examples on how to set secret value
On Fri, Jan 10, 2020 at 04:42:44PM +0100, Peter Krempa wrote:
Discourage passing secrets as commandline arguments.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/formatsecret.html.in | 86 +++++++++++++++++++++++++--------------
1 file changed, 55 insertions(+), 31 deletions(-)
diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in
index 8f5383cf64..61a8396682 100644
--- a/docs/formatsecret.html.in
+++ b/docs/formatsecret.html.in
@@ -76,13 +76,13 @@
<pre>
# virsh secret-define volume-secret.xml
Secret 0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f created
-#
-# MYSECRET=`printf %s "open sesame" | base64`
-# virsh secret-set-value 0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f $MYSECRET
-Secret value set
-#
</pre>
+ <p>
+ See <a href="#settingSecrets">virsh secret-set-value</a> on
how
+ to set the value of the secret.
+ </p>
+
<p>
The volume type secret can be supplied either in volume XML during
creation of a <a href="formatstorage.html#StorageVol">storage
volume</a>
@@ -103,12 +103,11 @@ Secret value set
# virsh secret-define luks-secret.xml
Secret f52a81b2-424e-490c-823d-6bd4235bc57 created
-#
-# MYSECRET=`printf %s "letmein" | base64`
-# virsh secret-set-value f52a81b2-424e-490c-823d-6bd4235bc57 $MYSECRET
-Secret value set
-#
</pre>
+ <p>
+ See <a href="#settingSecrets">virsh secret-set-value</a> on
how
+ to set the value of the secret.
+ </p>
<p>
The volume type secret can be supplied in domain XML for a luks storage
@@ -156,13 +155,11 @@ Secret 1b40a534-8301-45d5-b1aa-11894ebb1735 created
UUID Usage
-----------------------------------------------------------
1b40a534-8301-45d5-b1aa-11894ebb1735 cephx ceph_example
-#
-# CEPHPHRASE=`printf %s "pass phrase" | base64`
-# virsh secret-set-value 1b40a534-8301-45d5-b1aa-11894ebb1735 $CEPHPHRASE
-Secret value set
-
-#
</pre>
+ <p>
+ See <a href="#settingSecrets">virsh secret-set-value</a> on
how
+ to set the value of the secret.
+ </p>
<p>
The ceph secret can then be used by UUID or by the
@@ -229,7 +226,9 @@ incominguser myname mysecret
<p>
Next, use <code>virsh secret-define iscsi-secret.xml</code> to define
- the secret and <code>virsh secret-set-value</code> using the
generated
+ the secret and
+ <code><a href="#settingSecrets">virsh
secret-set-value</a></code>
+ using the generated
UUID value and a base64 generated secret value in order to define the
chosen secret pass phrase. The pass phrase must match the password
used in the iSCSI authentication configuration file.
@@ -243,12 +242,13 @@ Secret c4dbe20b-b1a3-4ac1-b6e6-2ac97852ebb6 created
-----------------------------------------------------------
c4dbe20b-b1a3-4ac1-b6e6-2ac97852ebb6 iscsi libvirtiscsi
-# MYSECRET=`printf %s "mysecret" | base64`
-# virsh secret-set-value c4dbe20b-b1a3-4ac1-b6e6-2ac97852ebb6 $MYSECRET
-Secret value set
-#
</pre>
+ <p>
+ See <a href="#settingSecrets">virsh secret-set-value</a> on
how
+ to set the value of the secret.
+ </p>
+
<p>
The iSCSI secret can then be used by UUID or by the
usage name via the <code><auth></code> element in a
domain's
@@ -313,19 +313,13 @@ Secret 718c71bd-67b5-4a2b-87ec-a24e8ca200dc created
Once the secret is defined, a secret value will need to be set. The
secret would be the passphrase used to access the TLS credentials.
The following is a simple example of using
- <code>virsh secret-set-value</code> to set the secret value. The
+ <code><a href="#settingSecrets">virsh
secret-set-value</a></code> to set
+ the secret value. The
<a href="html/libvirt-libvirt-secret.html#virSecretSetValue">
<code>virSecretSetValue</code></a> API may also be used to set
a more secure secret without using printable/readable characters.
</p>
- <pre>
-# MYSECRET=`printf %s "letmein" | base64`
-# virsh secret-set-value 718c71bd-67b5-4a2b-87ec-a24e8ca200dc $MYSECRET
-Secret value set
-
- </pre>
-
<h3><a id="vTPMUsageType">Usage type
"vtpm"</a></h3>
<p>
@@ -370,17 +364,47 @@ Secret 6dd3e4a5-1d76-44ce-961f-f119f5aad935 created
Once the secret is defined, a secret value will need to be set. The
secret would be the passphrase used to decrypt the vTPM state.
The following is a simple example of using
- <code>virsh secret-set-value</code> to set the secret value. The
+ <code><a href="#settingSecrets">virsh
secret-set-value</a></code>
+ to set the secret value. The
<a href="html/libvirt-libvirt-secret.html#virSecretSetValue">
<code>virSecretSetValue</code></a> API may also be used to set
a more secure secret without using printable/readable characters.
</p>
+ <h2><a id="settingSecrets">Setting secret values in
virsh</a></h2>
+
+ <p>
+ To set the value of the secret you can use the following virsh commands.
+ If the secret is a password-like string (printable characters, no newline)
+ you can use:
+ </p>
+ <pre>
+# virsh secret-passwd 6dd3e4a5-1d76-44ce-961f-f119f5aad935
+Enter new value for secret:
+Secret value set
+ </pre>
+
+ <p>
+ Another secure option is to read the secret from a file. This way the
+ secret can contain any bytes (even NUL and non-printable characters). The
+ length of the secret is the length of the input file.
+ </p>
+
+ <pre>
+# virsh secret-set-value 6dd3e4a5-1d76-44ce-961f-f119f5aad935 --file secretinfile
+Secret value set
+ </pre>
+
+ <p>
+ The secret can also be set via an argument, but note that other users
+ may see it in the process listing output. The secret must be base64
+ encoded.
+ </p>
This paragraph will need updating based on my prev patch comments.
Overall this patch looks good though.
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 :|
6:33 a.m.
On Fri, Jan 10, 2020 at 16:42:40 +0100, Peter Krempa wrote:
The currently existing virsh APIs for secrets are awful for human
use
and don't promote security.
Peter Krempa (4):
virsh: secret: Add 'secret-passwd' command
virsh: secret: Allow getting secret's value without base64 encoding
virsh: secret: Allow setting secrets from file
docs: secret: Unify and sanitize examples on how to set secret value
Ping
6:57 a.m.
On 1/10/20 12:42 PM, Peter Krempa wrote:
The currently existing virsh APIs for secrets are awful for human
use
and don't promote security.
Peter Krempa (4):
virsh: secret: Add 'secret-passwd' command
virsh: secret: Allow getting secret's value without base64 encoding
virsh: secret: Allow setting secrets from file
docs: secret: Unify and sanitize examples on how to set secret value
docs/formatsecret.html.in | 86 ++++++++++++++++++----------
docs/manpages/virsh.rst | 26 ++++++++-
tools/virsh-secret.c | 116 ++++++++++++++++++++++++++++++++++++--
3 files changed, 189 insertions(+), 39 deletions(-)
Code-wise LGTM. I have a question about the design though.
Shouldn't we ask for a password confirmation when setting the secret
via secret-passwd? This would be more on par with how 'passwd' works
in Linux, and can also help to prevent user typos when setting a
secret.
Thanks,
DHB
7:03 a.m.
On Tue, Jan 21, 2020 at 09:57:22 -0300, Daniel Henrique Barboza wrote:
On 1/10/20 12:42 PM, Peter Krempa wrote:
> The currently existing virsh APIs for secrets are awful for human use
> and don't promote security.
>
> Peter Krempa (4):
> virsh: secret: Add 'secret-passwd' command
> virsh: secret: Allow getting secret's value without base64 encoding
> virsh: secret: Allow setting secrets from file
> docs: secret: Unify and sanitize examples on how to set secret value
>
> docs/formatsecret.html.in | 86 ++++++++++++++++++----------
> docs/manpages/virsh.rst | 26 ++++++++-
> tools/virsh-secret.c | 116 ++++++++++++++++++++++++++++++++++++--
> 3 files changed, 189 insertions(+), 39 deletions(-)
>
Code-wise LGTM. I have a question about the design though.
Shouldn't we ask for a password confirmation when setting the secret
via secret-passwd? This would be more on par with how 'passwd' works
in Linux, and can also help to prevent user typos when setting a
secret.
I don't really think that it's necessary. When you mess up changing of
your account password you won't be able to log in thus it's a good idea
to make more sure that you didn't miss-type it.
With libvirt secrets, it doesn't prevent you from fixing it later as you
aren't even asked for the previous value of the secret.
7:38 a.m.
On 1/21/20 10:03 AM, Peter Krempa wrote:
On Tue, Jan 21, 2020 at 09:57:22 -0300, Daniel Henrique Barboza
wrote:
>
>
> On 1/10/20 12:42 PM, Peter Krempa wrote:
>> The currently existing virsh APIs for secrets are awful for human use
>> and don't promote security.
>>
>> Peter Krempa (4):
>> virsh: secret: Add 'secret-passwd' command
>> virsh: secret: Allow getting secret's value without base64 encoding
>> virsh: secret: Allow setting secrets from file
>> docs: secret: Unify and sanitize examples on how to set secret value
>>
>> docs/formatsecret.html.in | 86 ++++++++++++++++++----------
>> docs/manpages/virsh.rst | 26 ++++++++-
>> tools/virsh-secret.c | 116 ++++++++++++++++++++++++++++++++++++--
>> 3 files changed, 189 insertions(+), 39 deletions(-)
>>
>
>
> Code-wise LGTM. I have a question about the design though.
>
> Shouldn't we ask for a password confirmation when setting the secret
> via secret-passwd? This would be more on par with how 'passwd' works
> in Linux, and can also help to prevent user typos when setting a
> secret.
I don't really think that it's necessary. When you mess up changing of
your account password you won't be able to log in thus it's a good idea
to make more sure that you didn't miss-type it.
With libvirt secrets, it doesn't prevent you from fixing it later as you
aren't even asked for the previous value of the secret.
Those are fair points. I agree.
All patches:
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
1817
days inactive
1828
days old
16 comments
4 participants
participants (4)
-
Daniel Henrique Barboza -
Daniel P. Berrangé -
Ján Tomko -
Peter Krempa