9:10 p.m.
The first patch is a bug fix, the second is an improvement.
Michal Prívozník (2):
virsh: Fix logical error in cmdSetUserSSHKeys()
virsh: cmdSetUserSSHKeys: Error early if the file doesn't contain any
keys
tools/virsh-domain.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
--
2.26.2
9:10 p.m.
New subject: [PATCH 1/2] virsh: Fix logical error in cmdSetUserSSHKeys()
In v6.10.0-rc1~104 I've added a virsh command that exposes
virDomainAuthorizedSSHKeysSet() API under "set-user-sshkeys"
command. The command accepts mutually exclusive "--reset" and
"--remove" options (among others). While the former controls the
VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_APPEND flag, the latter
controls the VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_REMOVE flag.
These flags are also mutually exclusive. But the code that sets
them has a logical error which may result in both flags being
set. In fact, this results in user being not able to set just the
remove flag.
Fixes: 87d12effbea8b414c250b6fefd93154c62a99370
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1904674
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-domain.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1fb4189b4b..6266c7acd2 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -14375,17 +14375,18 @@ cmdSetUserSSHKeys(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
goto cleanup;
- if (!vshCommandOptBool(cmd, "reset")) {
- flags |= VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_APPEND;
-
- if (!from) {
- vshError(ctl, _("Option --file is required"));
- goto cleanup;
- }
- }
-
- if (vshCommandOptBool(cmd, "remove"))
+ if (vshCommandOptBool(cmd, "remove")) {
flags |= VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_REMOVE;
+ } else {
+ if (!vshCommandOptBool(cmd, "reset")) {
+ flags |= VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_APPEND;
+
+ if (!from) {
+ vshError(ctl, _("Option --file is required"));
+ goto cleanup;
+ }
+ }
+ }
if (from) {
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) {
--
2.26.2
9:10 p.m.
New subject: [PATCH 2/2] virsh: cmdSetUserSSHKeys: Error early if the file doesn't contain any keys
When removing SSH keys via set-user-sshkeys virsh command, then
files to remove are read from passed file. But when
experimenting, I've passed /dev/null as the file which resulted
in API checks which caught that @keys argument of
virDomainAuthorizedSSHKeysSet() can't be NULL. This is because if
the file is empty then its content is an empty string and thus
the buffer the file was read in to is not NULL.
Long story short, error is reported correctly, but it's not
necessary to go through public API to catch it.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-domain.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 6266c7acd2..befa8d2448 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -14398,6 +14398,10 @@ cmdSetUserSSHKeys(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
nkeys = virStringListLength((const char **) keys);
+ if (nkeys == 0) {
+ vshError(ctl, _("File %s contains no keys"), from);
+ goto cleanup;
+ }
}
if (virDomainAuthorizedSSHKeysSet(dom, user,
--
2.26.2
9:28 p.m.
On a Tuesday in 2020, Michal Privoznik wrote:
The first patch is a bug fix, the second is an improvement.
Michal Prívozník (2):
virsh: Fix logical error in cmdSetUserSSHKeys()
virsh: cmdSetUserSSHKeys: Error early if the file doesn't contain any
keys
tools/virsh-domain.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1426
days inactive
1426
days old
3 comments
2 participants
participants (2)
-
Ján Tomko -
Michal Privoznik