src/qemu/qemu_agent.c | 158 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 26 +++++++ tests/qemuagenttest.c | 80 +++++++++++++++++++++ 3 files changed, 264 insertions(+)
From: Marc-André Lureau <marcandre.lureau@redhat.com>
In QEMU 5.2, the guest agent learned to manipulate a user
~/.ssh/authorized_keys. Bind the JSON API to libvirt.
https://wiki.qemu.org/ChangeLog/5.2#Guest_agent
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1888537
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
src/qemu/qemu_agent.c | 158 ++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_agent.h | 26 +++++++
tests/qemuagenttest.c | 80 +++++++++++++++++++++
3 files changed, 264 insertions(+)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 7fbb4a9431..75e7fea9e4 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -2496,3 +2496,161 @@ qemuAgentSetResponseTimeout(qemuAgentPtr agent,
{
agent->timeout = timeout;
}
+
+void qemuAgentSSHAuthorizedKeyFree(qemuAgentSSHAuthorizedKeyPtr key)
+{
+ if (!key)
+ return;
+
+ g_free(key->key);
+ g_free(key);
+}
+
+/* Returns: 0 on success
+ * -2 when agent command is not supported by the agent and
+ * 'report_unsupported' is false (libvirt error is not reported)
+ * -1 otherwise (libvirt error is reported)
+ */
+int qemuAgentSSHGetAuthorizedKeys(qemuAgentPtr agent,
+ const char *user,
+ qemuAgentSSHAuthorizedKeyPtr **keys,
+ bool report_unsupported)
+{
+ g_autoptr(virJSONValue) cmd = NULL;
+ g_autoptr(virJSONValue) reply = NULL;
+ virJSONValuePtr data = NULL;
+ size_t ndata;
+ size_t i;
+ int rc;
+ qemuAgentSSHAuthorizedKeyPtr *keys_ret = NULL;
+
+ if (!(cmd = qemuAgentMakeCommand("guest-ssh-get-authorized-keys",
+ "s:username", user,
+ NULL)))
+ return -1;
+
+ if ((rc = qemuAgentCommandFull(agent, cmd, &reply, agent->timeout,
+ report_unsupported)) < 0)
+ return rc;
+
+ if (!(data = virJSONValueObjectGetArray(reply, "return"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("qemu agent didn't return an array of keys"));
+ return -1;
+ }
+ ndata = virJSONValueArraySize(data);
+
+ keys_ret = g_new0(qemuAgentSSHAuthorizedKeyPtr, ndata);
+
+ for (i = 0; i < ndata; i++) {
+ virJSONValuePtr entry = virJSONValueArrayGet(data, i);
+
+ if (!entry) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("array element missing in guest-ssh-get-authorized-keys return "
+ "value"));
+ goto cleanup;
+ }
+
+ keys_ret[i] = g_new0(qemuAgentSSHAuthorizedKey, 1);
+ keys_ret[i]->key = g_strdup(virJSONValueGetString(entry));
+ }
+
+ *keys = g_steal_pointer(&keys_ret);
+ return ndata;
+
+ cleanup:
+ if (keys_ret) {
+ for (i = 0; i < ndata; i++)
+ qemuAgentSSHAuthorizedKeyFree(keys_ret[i]);
+ g_free(keys_ret);
+ }
+ return -1;
+}
+
+static virJSONValuePtr
+makeJSONArrayFromKeys(qemuAgentSSHAuthorizedKeyPtr *keys,
+ size_t nkeys)
+{
+ g_autoptr(virJSONValue) jkeys = NULL;
+ size_t i;
+
+ jkeys = virJSONValueNewArray();
+
+ for (i = 0; i < nkeys; i++) {
+ qemuAgentSSHAuthorizedKeyPtr k = keys[i];
+
+ if (virJSONValueArrayAppendString(jkeys, k->key) < 0)
+ return NULL;
+ }
+
+ return g_steal_pointer(&jkeys);
+}
+
+/* Returns: 0 on success
+ * -2 when agent command is not supported by the agent and
+ * 'report_unsupported' is false (libvirt error is not reported)
+ * -1 otherwise (libvirt error is reported)
+ */
+int qemuAgentSSHAddAuthorizedKeys(qemuAgentPtr agent,
+ const char *user,
+ qemuAgentSSHAuthorizedKeyPtr *keys,
+ size_t nkeys,
+ bool reset,
+ bool report_unsupported)
+{
+ g_autoptr(virJSONValue) cmd = NULL;
+ g_autoptr(virJSONValue) reply = NULL;
+ g_autoptr(virJSONValue) jkeys = NULL;
+ int rc;
+
+ jkeys = makeJSONArrayFromKeys(keys, nkeys);
+ if (jkeys == NULL)
+ return -1;
+
+ if (!(cmd = qemuAgentMakeCommand("guest-ssh-add-authorized-keys",
+ "s:username", user,
+ "a:keys", &jkeys,
+ "b:reset", reset,
+ NULL)))
+ return -1;
+
+ if ((rc = qemuAgentCommandFull(agent, cmd, &reply, agent->timeout,
+ report_unsupported)) < 0)
+ return rc;
+
+ return 0;
+}
+
+/* Returns: 0 on success
+ * -2 when agent command is not supported by the agent and
+ * 'report_unsupported' is false (libvirt error is not reported)
+ * -1 otherwise (libvirt error is reported)
+ */
+int qemuAgentSSHRemoveAuthorizedKeys(qemuAgentPtr agent,
+ const char *user,
+ qemuAgentSSHAuthorizedKeyPtr *keys,
+ size_t nkeys,
+ bool report_unsupported)
+{
+ g_autoptr(virJSONValue) cmd = NULL;
+ g_autoptr(virJSONValue) reply = NULL;
+ g_autoptr(virJSONValue) jkeys = NULL;
+ int rc;
+
+ jkeys = makeJSONArrayFromKeys(keys, nkeys);
+ if (jkeys == NULL)
+ return -1;
+
+ if (!(cmd = qemuAgentMakeCommand("guest-ssh-remove-authorized-keys",
+ "s:username", user,
+ "a:keys", &jkeys,
+ NULL)))
+ return -1;
+
+ if ((rc = qemuAgentCommandFull(agent, cmd, &reply, agent->timeout,
+ report_unsupported)) < 0)
+ return rc;
+
+ return 0;
+}
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 2eeb376a68..3e70a55c19 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -67,6 +67,12 @@ typedef enum {
QEMU_AGENT_SHUTDOWN_LAST,
} qemuAgentShutdownMode;
+typedef struct _qemuAgentSSHAuthorizedKey qemuAgentSSHAuthorizedKey;
+typedef qemuAgentSSHAuthorizedKey *qemuAgentSSHAuthorizedKeyPtr;
+struct _qemuAgentSSHAuthorizedKey {
+ char *key;
+};
+
typedef struct _qemuAgentDiskInfo qemuAgentDiskInfo;
typedef qemuAgentDiskInfo *qemuAgentDiskInfoPtr;
struct _qemuAgentDiskInfo {
@@ -170,3 +176,23 @@ int qemuAgentGetTimezone(qemuAgentPtr mon,
void qemuAgentSetResponseTimeout(qemuAgentPtr mon,
int timeout);
+
+void qemuAgentSSHAuthorizedKeyFree(qemuAgentSSHAuthorizedKeyPtr key);
+
+int qemuAgentSSHGetAuthorizedKeys(qemuAgentPtr agent,
+ const char *user,
+ qemuAgentSSHAuthorizedKeyPtr **keys,
+ bool report_unsupported);
+
+int qemuAgentSSHAddAuthorizedKeys(qemuAgentPtr agent,
+ const char *user,
+ qemuAgentSSHAuthorizedKeyPtr *keys,
+ size_t nkeys,
+ bool reset,
+ bool report_unsupported);
+
+int qemuAgentSSHRemoveAuthorizedKeys(qemuAgentPtr agent,
+ const char *user,
+ qemuAgentSSHAuthorizedKeyPtr *keys,
+ size_t nkeys,
+ bool report_unsupported);
diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index 607bd97b5c..eda688850f 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -35,6 +35,85 @@
virQEMUDriver driver;
+static int
+testQemuAgentSSHKeys(const void *data)
+{
+ virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+ qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
+ qemuAgentSSHAuthorizedKeyPtr *keys = NULL;
+ int nkeys = 0, i;
+ int ret = -1;
+
+ if (!test)
+ return -1;
+
+ if (qemuMonitorTestAddAgentSyncResponse(test) < 0)
+ goto cleanup;
+
+ if (qemuMonitorTestAddItem(test, "guest-ssh-get-authorized-keys",
+ "{ \"return\" : [\"algo1 key1 comments1\","
+ " \"algo2 key2 comments2\"] }") < 0)
+ goto cleanup;
+
+ if (qemuMonitorTestAddAgentSyncResponse(test) < 0)
+ goto cleanup;
+
+ if (qemuMonitorTestAddItem(test, "guest-ssh-add-authorized-keys",
+ "{ \"return\" : {} }") < 0)
+ goto cleanup;
+
+ if (qemuMonitorTestAddAgentSyncResponse(test) < 0)
+ goto cleanup;
+
+ if (qemuMonitorTestAddItem(test, "guest-ssh-remove-authorized-keys",
+ "{ \"return\" : {} }") < 0)
+ goto cleanup;
+
+ if ((nkeys = qemuAgentSSHGetAuthorizedKeys(qemuMonitorTestGetAgent(test),
+ "user",
+ &keys,
+ true)) < 0)
+ goto cleanup;
+
+ if (nkeys != 2) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "expected 2 keys, got %d", nkeys);
+ ret = -1;
+ goto cleanup;
+ }
+
+ if (STRNEQ(keys[1]->key, "algo2 key2 comments2")) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "Unexpected key returned: %s", keys[1]->key);
+ ret = -1;
+ goto cleanup;
+ }
+
+ if ((ret = qemuAgentSSHAddAuthorizedKeys(qemuMonitorTestGetAgent(test),
+ "user",
+ keys,
+ nkeys,
+ true,
+ true)) < 0)
+ goto cleanup;
+
+ if ((ret = qemuAgentSSHRemoveAuthorizedKeys(qemuMonitorTestGetAgent(test),
+ "user",
+ keys,
+ nkeys,
+ true)) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ for (i = 0; i < nkeys; i++)
+ qemuAgentSSHAuthorizedKeyFree(keys[i]);
+ VIR_FREE(keys);
+ qemuMonitorTestFree(test);
+ return ret;
+}
+
+
static int
testQemuAgentFSFreeze(const void *data)
{
@@ -1315,6 +1394,7 @@ mymain(void)
DO_TEST(Users);
DO_TEST(OSInfo);
DO_TEST(Timezone);
+ DO_TEST(SSHKeys);
DO_TEST(Timeout); /* Timeout should always be called last */
--
2.29.0
On 11/7/20 10:12 AM, marcandre.lureau@redhat.com wrote: > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > In QEMU 5.2, the guest agent learned to manipulate a user > ~/.ssh/authorized_keys. Bind the JSON API to libvirt. > > https://wiki.qemu.org/ChangeLog/5.2#Guest_agent > > Fixes: > https://bugzilla.redhat.com/show_bug.cgi?id=1888537 > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > src/qemu/qemu_agent.c | 158 ++++++++++++++++++++++++++++++++++++++++++ > src/qemu/qemu_agent.h | 26 +++++++ > tests/qemuagenttest.c | 80 +++++++++++++++++++++ > 3 files changed, 264 insertions(+) > While you get bonus points for introducing tests we're still missing public APIs. And virsh commands. > diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c > index 7fbb4a9431..75e7fea9e4 100644 > --- a/src/qemu/qemu_agent.c > +++ b/src/qemu/qemu_agent.c > @@ -2496,3 +2496,161 @@ qemuAgentSetResponseTimeout(qemuAgentPtr agent, > { > agent->timeout = timeout; > } > + > +void qemuAgentSSHAuthorizedKeyFree(qemuAgentSSHAuthorizedKeyPtr key) > +{ > + if (!key) > + return; > + > + g_free(key->key); > + g_free(key); I wonder if we need to wrap this string into a struct. At least here on internal APIs level looks like we don't - we can change that anytime. And the public API - well, I don't think we need to break down the key string into its individual members, do we? I mean, "options, keytype, base64-encoded key, comment". The public API can accept just a single string and let sshd interpret it later. > +} > + > +/* Returns: 0 on success > + * -2 when agent command is not supported by the agent and > + * 'report_unsupported' is false (libvirt error is not reported) > + * -1 otherwise (libvirt error is reported) > + */ > +int qemuAgentSSHGetAuthorizedKeys(qemuAgentPtr agent, > + const char *user, > + qemuAgentSSHAuthorizedKeyPtr **keys, > + bool report_unsupported) I don't think we need to suppress CommandNotFound type of messages. Some wrappers have it because they are called from qemuDomainGetGuestInfo() which has a logic where if no specific type was requested then all available types are fetched (user list, os info, timezone, hostname, FS info). > +{ > + g_autoptr(virJSONValue) cmd = NULL; > + g_autoptr(virJSONValue) reply = NULL; > + virJSONValuePtr data = NULL; > + size_t ndata; > + size_t i; > + int rc; > + qemuAgentSSHAuthorizedKeyPtr *keys_ret = NULL; > + > + if (!(cmd = qemuAgentMakeCommand("guest-ssh-get-authorized-keys", > + "s:username", user, > + NULL))) > + return -1; > + > + if ((rc = qemuAgentCommandFull(agent, cmd, &reply, agent->timeout, > + report_unsupported)) < 0) > + return rc; > + > + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("qemu agent didn't return an array of keys")); > + return -1; > + } > + ndata = virJSONValueArraySize(data); > + > + keys_ret = g_new0(qemuAgentSSHAuthorizedKeyPtr, ndata); > + > + for (i = 0; i < ndata; i++) { > + virJSONValuePtr entry = virJSONValueArrayGet(data, i); > + > + if (!entry) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("array element missing in guest-ssh-get-authorized-keys return " > + "value")); > + goto cleanup; > + } > + > + keys_ret[i] = g_new0(qemuAgentSSHAuthorizedKey, 1); > + keys_ret[i]->key = g_strdup(virJSONValueGetString(entry)); > + } > + > + *keys = g_steal_pointer(&keys_ret); > + return ndata; > + > + cleanup: Technically, this should be 'error' because it's used only in case of failure ;-) > + if (keys_ret) { > + for (i = 0; i < ndata; i++) > + qemuAgentSSHAuthorizedKeyFree(keys_ret[i]); > + g_free(keys_ret); > + } > + return -1; > +} > + > +static virJSONValuePtr > +makeJSONArrayFromKeys(qemuAgentSSHAuthorizedKeyPtr *keys, > + size_t nkeys) If we'd go with plain strings then we could use qemuAgentMakeStringsArray() instead. > +{ > + g_autoptr(virJSONValue) jkeys = NULL; > + size_t i; > + > + jkeys = virJSONValueNewArray(); > + > + for (i = 0; i < nkeys; i++) { > + qemuAgentSSHAuthorizedKeyPtr k = keys[i]; > + > + if (virJSONValueArrayAppendString(jkeys, k->key) < 0) > + return NULL; > + } > + > + return g_steal_pointer(&jkeys); > +} I'm stopping my review here. The wrappers are okay, but we really need the public API and RPC first. I can work on that if you don't feel like it. Michal
Hi On Mon, Nov 9, 2020 at 4:44 PM Michal Privoznik <mprivozn@redhat.com> wrote: > > I'm stopping my review here. The wrappers are okay, but we really need > the public API and RPC first. I can work on that if you don't feel like it. I am on holiday this week and perhaps a few more days. If you have some free time, feel free to update the patch. Otherwise, it will wait a bit longer. thanks
© 2016 - 2024 Red Hat, Inc.