[PATCH v2 0/6] Introduce OpenSSH authorized key file mgmt APIs
v2 of: https://www.redhat.com/archives/libvir-list/2020-November/msg00444.html diff to v1: - Fixed issues raised by Peter (RO connection check, switched virsh from ARGV to --file, etc.) Marc-André Lureau (1): qemu_agent: add qemuAgentSSH{Add,Remove,Get}AuthorizedKeys Michal Prívozník (5): Introduce OpenSSH authorized key file mgmt APIs remote: Implement OpenSSH authorized key file mgmt APIs virsh: Expose OpenSSH authorized key file mgmt APIs qemu: Implement OpenSSH authorized key file mgmt APIs news: Document recent OpenSSH authorized key file mgmt APIs NEWS.rst | 6 + docs/manpages/virsh.rst | 37 ++++++ include/libvirt/libvirt-domain.h | 17 +++ src/driver-hypervisor.h | 15 +++ src/libvirt-domain.c | 133 ++++++++++++++++++++++ src/libvirt_public.syms | 6 + src/qemu/qemu_agent.c | 142 +++++++++++++++++++++++ src/qemu/qemu_agent.h | 15 +++ src/qemu/qemu_driver.c | 81 ++++++++++++++ src/remote/remote_daemon_dispatch.c | 82 ++++++++++++++ src/remote/remote_driver.c | 87 +++++++++++++++ src/remote/remote_protocol.x | 34 +++++- src/remote_protocol-structs | 22 ++++ tests/qemuagenttest.c | 79 +++++++++++++ tools/virsh-domain.c | 167 ++++++++++++++++++++++++++++ 15 files changed, 922 insertions(+), 1 deletion(-) -- 2.26.2
When setting up a new guest or when a management software wants to allow access to an existing guest the virDomainSetUserPassword() API can be used, but that might be not good enough if user want to ssh into the guest. Not only sshd has to be configured to accept password authentication (which is usually not the case for root), user have to type in their password. Using SSH keys is more convenient. Therefore, two new APIs are introduced: virDomainAuthorizedSSHKeysGet() which lists authorized keys for given user, and virDomainAuthorizedSSHKeysSet() which modifies the authorized keys file for given user (append, set or remove keys from the file). It's worth nothing that while authorized_keys file entries have some structure (as defined by sshd(8)), expressing that structure goes beyond libvirt's focus and thus "keys" are nothing but an opaque string to libvirt. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt-domain.h | 17 ++++ src/driver-hypervisor.h | 15 ++++ src/libvirt-domain.c | 133 +++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ 4 files changed, 171 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index e1095a193d..d81157ccaf 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -5101,4 +5101,21 @@ int virDomainBackupBegin(virDomainPtr domain, char *virDomainBackupGetXMLDesc(virDomainPtr domain, unsigned int flags); +int virDomainAuthorizedSSHKeysGet(virDomainPtr domain, + const char *user, + char ***keys, + unsigned int flags); + +typedef enum { + VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_APPEND = (1 << 0), /* don't truncate file, just append */ + VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_REMOVE = (1 << 1), /* remove keys, instead of adding them */ + +} virDomainAuthorizedSSHKeysSetFlags; + +int virDomainAuthorizedSSHKeysSet(virDomainPtr domain, + const char *user, + const char **keys, + int nkeys, + unsigned int flags); + #endif /* LIBVIRT_DOMAIN_H */ diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index bce023017d..5a5ea95c51 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1387,6 +1387,19 @@ typedef char * (*virDrvDomainBackupGetXMLDesc)(virDomainPtr domain, unsigned int flags); +typedef int +(*virDrvDomainAuthorizedSSHKeysGet)(virDomainPtr domain, + const char *user, + char ***keys, + unsigned int flags); + +typedef int +(*virDrvDomainAuthorizedSSHKeysSet)(virDomainPtr domain, + const char *user, + const char **keys, + int nkeys, + unsigned int flags); + typedef struct _virHypervisorDriver virHypervisorDriver; typedef virHypervisorDriver *virHypervisorDriverPtr; @@ -1650,4 +1663,6 @@ struct _virHypervisorDriver { virDrvDomainAgentSetResponseTimeout domainAgentSetResponseTimeout; virDrvDomainBackupBegin domainBackupBegin; virDrvDomainBackupGetXMLDesc domainBackupGetXMLDesc; + virDrvDomainAuthorizedSSHKeysGet domainAuthorizedSSHKeysGet; + virDrvDomainAuthorizedSSHKeysSet domainAuthorizedSSHKeysSet; }; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 3c5f55176a..47b821f7d4 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12758,3 +12758,136 @@ virDomainBackupGetXMLDesc(virDomainPtr domain, virDispatchError(conn); return NULL; } + + +/** + * virDomainAuthorizedSSHKeysGet: + * @domain: a domain object + * @user: user to list keys for + * @keys: pointer to a variable to store authorized keys + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * For given @user in @domain fetch list of public SSH authorized + * keys and store them into @keys array which is allocated upon + * successful return. The caller is responsible for freeing @keys + * when no longer needed. + * + * Keys are in OpenSSH format (see sshd(8)) but from libvirt's + * point of view are opaque strings, i.e. not interpreted. + * + * Please note that some hypervisors may require guest agent to + * be configured and running in order to be able to run this API. + * + * Returns: number of keys stored in @keys, + * -1 otherwise. + */ +int +virDomainAuthorizedSSHKeysGet(virDomainPtr domain, + const char *user, + char ***keys, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "user=%s, keys=%p, flags=0x%x", + user, keys, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + virCheckNonNullArgGoto(user, error); + virCheckNonNullArgGoto(keys, error); + + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->driver->domainAuthorizedSSHKeysGet) { + int ret; + ret = conn->driver->domainAuthorizedSSHKeysGet(domain, user, + keys, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + error: + virDispatchError(conn); + return -1; +} + + +/** + * virDomainAuthorizedSSHKeysSet: + * @domain: a domain object + * @user: user to add keys for + * @keys: authorized keys to set + * @nkeys: number of keys in @keys array + * @flags: bitwise or of virDomainAuthorizedSSHKeysSetFlags + * + * For given @user in @domain set @keys in authorized keys file. + * Any previous content of the file is overwritten with new keys. + * That is, if this API is called with @nkeys = 0, @keys = NULL + * and @flags = 0 then the authorized keys file for @user is + * cleared out. + * + * Keys are in OpenSSH format (see sshd(8)) but from libvirt's + * point of view are opaque strings, i.e. not interpreted. + * + * If VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_APPEND flag is set + * then the file is not overwritten and new @keys are appended + * instead. + * + * If VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_REMOVE flag is set then + * instead of adding any new keys, provided @keys are removed + * from the file. It's not considered error if the key doesn't + * exist. + * + * Please note that some hypervisors may require guest agent to + * be configured and running in order to be able to run this API. + * + * Returns: 0 on success, + * -1 otherwise. + */ +int +virDomainAuthorizedSSHKeysSet(virDomainPtr domain, + const char *user, + const char **keys, + int nkeys, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "user=%s, keys=%p, nkeys=%d, flags=0x%x", + user, keys, nkeys, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + virCheckNonNullArgGoto(user, error); + + if (flags & VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_APPEND || + flags & VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_REMOVE) + virCheckNonNullArgGoto(keys, error); + + VIR_EXCLUSIVE_FLAGS_RET(VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_APPEND, + VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_REMOVE, + -1); + + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->driver->domainAuthorizedSSHKeysSet) { + int ret; + ret = conn->driver->domainAuthorizedSSHKeysSet(domain, user, + keys, nkeys, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + error: + virDispatchError(conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 539d2e3943..cf31f937d5 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -873,4 +873,10 @@ LIBVIRT_6.0.0 { virDomainBackupGetXMLDesc; } LIBVIRT_5.10.0; +LIBVIRT_6.10.0 { + global: + virDomainAuthorizedSSHKeysGet; + virDomainAuthorizedSSHKeysSet; +} LIBVIRT_6.0.0; + # .... define new API here using predicted next version number .... -- 2.26.2
On Mon, Nov 16, 2020 at 13:20:58 +0100, Michal Privoznik wrote:
When setting up a new guest or when a management software wants to allow access to an existing guest the virDomainSetUserPassword() API can be used, but that might be not good enough if user want to ssh into the guest. Not only sshd has to be configured to accept password authentication (which is usually not the case for root), user have to type in their password. Using SSH keys is more convenient. Therefore, two new APIs are introduced:
virDomainAuthorizedSSHKeysGet() which lists authorized keys for given user, and
virDomainAuthorizedSSHKeysSet() which modifies the authorized keys file for given user (append, set or remove keys from the file).
It's worth nothing that while authorized_keys file entries have some structure (as defined by sshd(8)), expressing that structure goes beyond libvirt's focus and thus "keys" are nothing but an opaque string to libvirt.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt-domain.h | 17 ++++ src/driver-hypervisor.h | 15 ++++ src/libvirt-domain.c | 133 +++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ 4 files changed, 171 insertions(+)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On Mon, Nov 16, 2020 at 13:20:58 +0100, Michal Privoznik wrote:
When setting up a new guest or when a management software wants to allow access to an existing guest the virDomainSetUserPassword() API can be used, but that might be not good enough if user want to ssh into the guest. Not only sshd has to be configured to accept password authentication (which is usually not the case for root), user have to type in their password. Using SSH keys is more convenient. Therefore, two new APIs are introduced:
virDomainAuthorizedSSHKeysGet() which lists authorized keys for given user, and
virDomainAuthorizedSSHKeysSet() which modifies the authorized keys file for given user (append, set or remove keys from the file).
It's worth nothing that while authorized_keys file entries have some structure (as defined by sshd(8)), expressing that structure goes beyond libvirt's focus and thus "keys" are nothing but an opaque string to libvirt.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt-domain.h | 17 ++++ src/driver-hypervisor.h | 15 ++++ src/libvirt-domain.c | 133 +++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ 4 files changed, 171 insertions(+)
[..]
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 3c5f55176a..47b821f7d4 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12758,3 +12758,136 @@ virDomainBackupGetXMLDesc(virDomainPtr domain, virDispatchError(conn); return NULL; } + + +/** + * virDomainAuthorizedSSHKeysGet: + * @domain: a domain object + * @user: user to list keys for + * @keys: pointer to a variable to store authorized keys + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * For given @user in @domain fetch list of public SSH authorized + * keys and store them into @keys array which is allocated upon + * successful return. The caller is responsible for freeing @keys + * when no longer needed.
One nit. We tend to NULL-terminate such lists so that users can use various checks for iteration.
On 11/16/20 3:46 PM, Peter Krempa wrote:
On Mon, Nov 16, 2020 at 13:20:58 +0100, Michal Privoznik wrote:
When setting up a new guest or when a management software wants to allow access to an existing guest the virDomainSetUserPassword() API can be used, but that might be not good enough if user want to ssh into the guest. Not only sshd has to be configured to accept password authentication (which is usually not the case for root), user have to type in their password. Using SSH keys is more convenient. Therefore, two new APIs are introduced:
virDomainAuthorizedSSHKeysGet() which lists authorized keys for given user, and
virDomainAuthorizedSSHKeysSet() which modifies the authorized keys file for given user (append, set or remove keys from the file).
It's worth nothing that while authorized_keys file entries have some structure (as defined by sshd(8)), expressing that structure goes beyond libvirt's focus and thus "keys" are nothing but an opaque string to libvirt.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt-domain.h | 17 ++++ src/driver-hypervisor.h | 15 ++++ src/libvirt-domain.c | 133 +++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ 4 files changed, 171 insertions(+)
[..]
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 3c5f55176a..47b821f7d4 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12758,3 +12758,136 @@ virDomainBackupGetXMLDesc(virDomainPtr domain, virDispatchError(conn); return NULL; } + + +/** + * virDomainAuthorizedSSHKeysGet: + * @domain: a domain object + * @user: user to list keys for + * @keys: pointer to a variable to store authorized keys + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * For given @user in @domain fetch list of public SSH authorized + * keys and store them into @keys array which is allocated upon + * successful return. The caller is responsible for freeing @keys + * when no longer needed.
One nit. We tend to NULL-terminate such lists so that users can use various checks for iteration.
Ah, good point. I guess it makes sense to NULL terminate only if actually returning something, i.e. if the retval > 0. Michal
On Mon, Nov 16, 2020 at 18:41:57 +0100, Michal Privoznik wrote:
On 11/16/20 3:46 PM, Peter Krempa wrote:
On Mon, Nov 16, 2020 at 13:20:58 +0100, Michal Privoznik wrote:
When setting up a new guest or when a management software wants to allow access to an existing guest the virDomainSetUserPassword() API can be used, but that might be not good enough if user want to ssh into the guest. Not only sshd has to be configured to accept password authentication (which is usually not the case for root), user have to type in their password. Using SSH keys is more convenient. Therefore, two new APIs are introduced:
virDomainAuthorizedSSHKeysGet() which lists authorized keys for given user, and
virDomainAuthorizedSSHKeysSet() which modifies the authorized keys file for given user (append, set or remove keys from the file).
It's worth nothing that while authorized_keys file entries have some structure (as defined by sshd(8)), expressing that structure goes beyond libvirt's focus and thus "keys" are nothing but an opaque string to libvirt.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt-domain.h | 17 ++++ src/driver-hypervisor.h | 15 ++++ src/libvirt-domain.c | 133 +++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ 4 files changed, 171 insertions(+)
[..]
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 3c5f55176a..47b821f7d4 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12758,3 +12758,136 @@ virDomainBackupGetXMLDesc(virDomainPtr domain, virDispatchError(conn); return NULL; } + + +/** + * virDomainAuthorizedSSHKeysGet: + * @domain: a domain object + * @user: user to list keys for + * @keys: pointer to a variable to store authorized keys + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * For given @user in @domain fetch list of public SSH authorized + * keys and store them into @keys array which is allocated upon + * successful return. The caller is responsible for freeing @keys + * when no longer needed.
One nit. We tend to NULL-terminate such lists so that users can use various checks for iteration.
Ah, good point. I guess it makes sense to NULL terminate only if actually returning something, i.e. if the retval > 0.
As a data-point, virDomainListAllDomains always allocates the output array with the one extra element if success is returned. For a case of 0 returned object that results in just the NULL terminator being returned.
Since both APIs accept/return an array of strings we can't have client/server dispatch code generated. But implementation is fairly trivial, although verbose. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/remote/remote_daemon_dispatch.c | 82 +++++++++++++++++++++++++++ src/remote/remote_driver.c | 87 +++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 34 ++++++++++- src/remote_protocol-structs | 22 ++++++++ 4 files changed, 224 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index eb5f6ebb0c..46683aa4a7 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -7381,3 +7381,85 @@ remoteDispatchDomainGetGuestInfo(virNetServerPtr server G_GNUC_UNUSED, return rv; } + +static int +remoteDispatchDomainAuthorizedSshKeysGet(virNetServerPtr server G_GNUC_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg G_GNUC_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_authorized_ssh_keys_get_args *args, + remote_domain_authorized_ssh_keys_get_ret *ret) +{ + int rv = -1; + virConnectPtr conn = remoteGetHypervisorConn(client); + int nkeys = 0; + char **keys = NULL; + virDomainPtr dom = NULL; + + if (!conn) + goto cleanup; + + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + + if ((nkeys = virDomainAuthorizedSSHKeysGet(dom, args->user, + &keys, args->flags)) < 0) + goto cleanup; + + if (nkeys > REMOTE_DOMAIN_AUTHORIZED_SSH_KEYS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of keys %d, which exceeds max liit: %d"), + nkeys, REMOTE_DOMAIN_AUTHORIZED_SSH_KEYS_MAX); + goto cleanup; + } + + ret->keys.keys_val = g_steal_pointer(&keys); + ret->keys.keys_len = nkeys; + + rv = nkeys; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (nkeys > 0) + virStringListFreeCount(keys, nkeys); + virObjectUnref(dom); + + return rv; +} + +static int +remoteDispatchDomainAuthorizedSshKeysSet(virNetServerPtr server G_GNUC_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg G_GNUC_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_authorized_ssh_keys_set_args *args) +{ + int rv = -1; + virConnectPtr conn = remoteGetHypervisorConn(client); + virDomainPtr dom = NULL; + + if (!conn) + goto cleanup; + + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + + if (args->keys.keys_len > REMOTE_DOMAIN_AUTHORIZED_SSH_KEYS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of keys %d, which exceeds max liit: %d"), + args->keys.keys_len, REMOTE_DOMAIN_AUTHORIZED_SSH_KEYS_MAX); + goto cleanup; + } + + rv = virDomainAuthorizedSSHKeysSet(dom, args->user, + (const char **) args->keys.keys_val, + args->keys.keys_len, args->flags); + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virObjectUnref(dom); + + return rv; +} diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 9cd2fd36ae..0b8d1e753f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8027,6 +8027,91 @@ remoteDomainGetGuestInfo(virDomainPtr dom, return rv; } +static int +remoteDomainAuthorizedSSHKeysGet(virDomainPtr domain, + const char *user, + char ***keys, + unsigned int flags) +{ + int rv = -1; + size_t i; + struct private_data *priv = domain->conn->privateData; + remote_domain_authorized_ssh_keys_get_args args; + remote_domain_authorized_ssh_keys_get_ret ret; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.user = (char *) user; + args.flags = flags; + memset(&ret, 0, sizeof(ret)); + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_AUTHORIZED_SSH_KEYS_GET, + (xdrproc_t) xdr_remote_domain_authorized_ssh_keys_get_args, (char *)&args, + (xdrproc_t) xdr_remote_domain_authorized_ssh_keys_get_ret, (char *)&ret) == -1) { + goto cleanup; + } + + if (ret.keys.keys_len > REMOTE_DOMAIN_AUTHORIZED_SSH_KEYS_MAX) { + virReportError(VIR_ERR_RPC, "%s", + _("remoteDomainAuthorizedSSHKeysGet: " + "returned number of keys exceeds limit")); + goto cleanup; + } + + *keys = g_new0(char *, ret.keys.keys_len); + for (i = 0; i < ret.keys.keys_len; i++) + (*keys)[i] = g_strdup(ret.keys.keys_val[i]); + + rv = ret.keys.keys_len; + + cleanup: + remoteDriverUnlock(priv); + xdr_free((xdrproc_t)xdr_remote_domain_authorized_ssh_keys_get_ret, + (char *) &ret); + return rv; +} + +static int +remoteDomainAuthorizedSSHKeysSet(virDomainPtr domain, + const char *user, + const char **keys, + int nkeys, + unsigned int flags) +{ + int rv = -1; + struct private_data *priv = domain->conn->privateData; + remote_domain_authorized_ssh_keys_set_args args; + + remoteDriverLock(priv); + + if (nkeys > REMOTE_DOMAIN_AUTHORIZED_SSH_KEYS_MAX) { + virReportError(VIR_ERR_RPC, "%s", + _("remoteDomainAuthorizedSSHKeysSet: " + "returned number of keys exceeds limit")); + goto cleanup; + } + + make_nonnull_domain(&args.dom, domain); + args.user = (char *) user; + args.keys.keys_len = nkeys; + args.keys.keys_val = (char **) keys; + args.flags = flags; + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_AUTHORIZED_SSH_KEYS_SET, + (xdrproc_t) xdr_remote_domain_authorized_ssh_keys_set_args, (char *)&args, + (xdrproc_t) xdr_void, (char *) NULL) == -1) { + goto cleanup; + } + + rv = 0; + + cleanup: + remoteDriverUnlock(priv); + return rv; +} + + /* get_nonnull_domain and get_nonnull_network turn an on-wire * (name, uuid) pair into virDomainPtr or virNetworkPtr object. * These can return NULL if underlying memory allocations fail, @@ -8458,6 +8543,8 @@ static virHypervisorDriver hypervisor_driver = { .domainAgentSetResponseTimeout = remoteDomainAgentSetResponseTimeout, /* 5.10.0 */ .domainBackupBegin = remoteDomainBackupBegin, /* 6.0.0 */ .domainBackupGetXMLDesc = remoteDomainBackupGetXMLDesc, /* 6.0.0 */ + .domainAuthorizedSSHKeysGet = remoteDomainAuthorizedSSHKeysGet, /* 6.10.0 */ + .domainAuthorizedSSHKeysSet = remoteDomainAuthorizedSSHKeysSet, /* 6.10.0 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 5e5e781e76..2df38cef77 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -280,6 +280,9 @@ const REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX = 2048; */ const REMOTE_NETWORK_PORT_PARAMETERS_MAX = 16; +/* Upper limit on number of SSH keys */ +const REMOTE_DOMAIN_AUTHORIZED_SSH_KEYS_MAX = 2048; + /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN]; @@ -3779,6 +3782,23 @@ struct remote_domain_backup_get_xml_desc_ret { remote_nonnull_string xml; }; +struct remote_domain_authorized_ssh_keys_get_args { + remote_nonnull_domain dom; + remote_nonnull_string user; + unsigned int flags; +}; + +struct remote_domain_authorized_ssh_keys_get_ret { + remote_nonnull_string keys<REMOTE_DOMAIN_AUTHORIZED_SSH_KEYS_MAX>; +}; + +struct remote_domain_authorized_ssh_keys_set_args { + remote_nonnull_domain dom; + remote_nonnull_string user; + remote_nonnull_string keys<REMOTE_DOMAIN_AUTHORIZED_SSH_KEYS_MAX>; + unsigned int flags; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -6682,5 +6702,17 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_DOMAIN_EVENT_MEMORY_FAILURE = 423 + REMOTE_PROC_DOMAIN_EVENT_MEMORY_FAILURE = 423, + + /** + * @generate: none + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_AUTHORIZED_SSH_KEYS_GET = 424, + + /** + * @generate: none + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_AUTHORIZED_SSH_KEYS_SET = 425 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index c2ae411885..9bcd14603d 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -3142,6 +3142,26 @@ struct remote_domain_backup_get_xml_desc_args { struct remote_domain_backup_get_xml_desc_ret { remote_nonnull_string xml; }; +struct remote_domain_authorized_ssh_keys_get_args { + remote_nonnull_domain dom; + remote_nonnull_string user; + u_int flags; +}; +struct remote_domain_authorized_ssh_keys_get_ret { + struct { + u_int keys_len; + remote_nonnull_string * keys_val; + } keys; +}; +struct remote_domain_authorized_ssh_keys_set_args { + remote_nonnull_domain dom; + remote_nonnull_string user; + struct { + u_int keys_len; + remote_nonnull_string * keys_val; + } keys; + u_int flags; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -3566,4 +3586,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BACKUP_BEGIN = 421, REMOTE_PROC_DOMAIN_BACKUP_GET_XML_DESC = 422, REMOTE_PROC_DOMAIN_EVENT_MEMORY_FAILURE = 423, + REMOTE_PROC_DOMAIN_AUTHORIZED_SSH_KEYS_GET = 424, + REMOTE_PROC_DOMAIN_AUTHORIZED_SSH_KEYS_SET = 425, }; -- 2.26.2
On Mon, Nov 16, 2020 at 13:20:59 +0100, Michal Privoznik wrote:
Since both APIs accept/return an array of strings we can't have client/server dispatch code generated. But implementation is fairly trivial, although verbose.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/remote/remote_daemon_dispatch.c | 82 +++++++++++++++++++++++++++ src/remote/remote_driver.c | 87 +++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 34 ++++++++++- src/remote_protocol-structs | 22 ++++++++ 4 files changed, 224 insertions(+), 1 deletion(-)
[...]
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 9cd2fd36ae..0b8d1e753f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8027,6 +8027,91 @@ remoteDomainGetGuestInfo(virDomainPtr dom, return rv; }
+static int +remoteDomainAuthorizedSSHKeysGet(virDomainPtr domain, + const char *user, + char ***keys, + unsigned int flags) +{ + int rv = -1; + size_t i; + struct private_data *priv = domain->conn->privateData; + remote_domain_authorized_ssh_keys_get_args args; + remote_domain_authorized_ssh_keys_get_ret ret; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.user = (char *) user; + args.flags = flags; + memset(&ret, 0, sizeof(ret)); + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_AUTHORIZED_SSH_KEYS_GET, + (xdrproc_t) xdr_remote_domain_authorized_ssh_keys_get_args, (char *)&args, + (xdrproc_t) xdr_remote_domain_authorized_ssh_keys_get_ret, (char *)&ret) == -1) { + goto cleanup; + } + + if (ret.keys.keys_len > REMOTE_DOMAIN_AUTHORIZED_SSH_KEYS_MAX) { + virReportError(VIR_ERR_RPC, "%s", + _("remoteDomainAuthorizedSSHKeysGet: " + "returned number of keys exceeds limit")); + goto cleanup; + } + + *keys = g_new0(char *, ret.keys.keys_len);
Please over-allocate by 1 to ensure a NULL-terminated list.
+ for (i = 0; i < ret.keys.keys_len; i++) + (*keys)[i] = g_strdup(ret.keys.keys_val[i]); + + rv = ret.keys.keys_len; + + cleanup: + remoteDriverUnlock(priv); + xdr_free((xdrproc_t)xdr_remote_domain_authorized_ssh_keys_get_ret, + (char *) &ret); + return rv; +} +
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The new virsh commands are: get-user-sshkeys set-user-sshkeys Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/manpages/virsh.rst | 37 +++++++++ tools/virsh-domain.c | 167 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 204 insertions(+) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index bfd26e3120..18cee358fd 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -2636,6 +2636,21 @@ When *--timestamp* is used, a human-readable timestamp will be printed before the event. +get-user-sshkeys +---------------- + +**Syntax:** + +:: + + get-user-sshkeys domain user + +Print SSH authorized keys for given *user* in the guest *domain*. Please note, +that an entry in the file has internal structure as defined by *sshd(8)* and +virsh/libvirt does handle keys as opaque strings, i.e. does not interpret +them. + + guest-agent-timeout ------------------- @@ -4004,6 +4019,28 @@ For QEMU/KVM, this requires the guest agent to be configured and running. +set-user-sshkeys +---------------- + +**Syntax:** + +:: + + set-user-sshkeys domain user [--file FILE] [{--append | --remove}] + +Replace the contents of *user*'s SSH authorized keys file in the guest *domain* +with keys read from optional *FILE*. In the *FILE* keys must be on separate +lines and each line must follow authorized keys format as defined by *sshd(8)*. +If no *FILE* is specified then the guest authorized keys file is cleared out. + +If *--append* is specified, then the guest authorized keys file content is not +replaced and new keys from *FILE* are just appended. + +If *--remove* is specified, then instead of adding any new keys then keys read +from *FILE* are removed from the authorized keys file. It is not considered an +error if the key does not exist in the file. + + setmaxmem --------- diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 12b35c037d..2a775fd277 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -14263,6 +14263,161 @@ cmdGuestInfo(vshControl *ctl, const vshCmd *cmd) return ret; } +/* + * "get-user-sshkeys" command + */ +static const vshCmdInfo info_get_user_sshkeys[] = { + {.name = "help", + .data = N_("list authorized SSH keys for given user (via agent)") + }, + {.name = "desc", + .data = N_("Use the guest agent to query authorized SSH keys for given " + "user") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_get_user_sshkeys[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), + {.name = "user", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("user to list authorized keys for"), + }, + {.name = NULL} +}; + +static bool +cmdGetUserSSHKeys(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + const char *user; + char **keys = NULL; + int nkeys = 0; + size_t i; + const unsigned int flags = 0; + vshTablePtr table = NULL; + bool ret = false; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "user", &user) < 0) + goto cleanup; + + nkeys = virDomainAuthorizedSSHKeysGet(dom, user, &keys, flags); + if (nkeys < 0) + goto cleanup; + + if (!(table = vshTableNew(_("key"), NULL))) + goto cleanup; + + for (i = 0; i < nkeys; i++) { + if (vshTableRowAppend(table, keys[i], NULL) < 0) + goto cleanup; + } + + vshTablePrintToStdout(table, ctl); + + ret = true; + cleanup: + vshTableFree(table); + if (nkeys > 0) + virStringListFreeCount(keys, nkeys); + virshDomainFree(dom); + return ret; +} + + +/* + * "set-user-sshkeys" command + */ +static const vshCmdInfo info_set_user_sshkeys[] = { + {.name = "help", + .data = N_("manipulate authorized SSH keys file for given user (via agent)") + }, + {.name = "desc", + .data = N_("Append, reset or remove specified key from the authorized " + "keys file for given user") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_set_user_sshkeys[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), + {.name = "user", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("user to set authorized keys for"), + }, + {.name = "file", + .type = VSH_OT_STRING, + .help = N_("optional file to read keys from"), + }, + {.name = "append", + .type = VSH_OT_BOOL, + .help = N_("append keys to the authorized keys file"), + }, + {.name = "remove", + .type = VSH_OT_BOOL, + .help = N_("remove keys from the authorized keys file"), + }, + {.name = NULL} +}; + +static bool +cmdSetUserSSHKeys(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + const char *user; + const char *from; + g_autofree char *buffer = NULL; + VIR_AUTOSTRINGLIST keys = NULL; + int nkeys = 0; + unsigned int flags = 0; + bool ret = false; + + VSH_REQUIRE_OPTION("append", "file"); + VSH_REQUIRE_OPTION("remove", "file"); + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "user", &user) < 0) + goto cleanup; + + if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + goto cleanup; + + if (vshCommandOptBool(cmd, "append")) + flags |= VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_APPEND; + if (vshCommandOptBool(cmd, "remove")) + flags |= VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_REMOVE; + + if (from) { + if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) { + vshReportError(ctl); + goto cleanup; + } + + if (!(keys = virStringSplit(buffer, "\n", -1))) + goto cleanup; + + nkeys = virStringListLength((const char **) keys); + } + + if (virDomainAuthorizedSSHKeysSet(dom, user, + (const char **) keys, nkeys, flags) < 0) { + goto cleanup; + } + + ret = true; + cleanup: + virshDomainFree(dom); + return ret; +} + + const vshCmdDef domManagementCmds[] = { {.name = "attach-device", .handler = cmdAttachDevice, @@ -14530,6 +14685,12 @@ const vshCmdDef domManagementCmds[] = { .info = info_event, .flags = 0 }, + {.name = "get-user-sshkeys", + .handler = cmdGetUserSSHKeys, + .opts = opts_get_user_sshkeys, + .info = info_get_user_sshkeys, + .flags = 0 + }, {.name = "inject-nmi", .handler = cmdInjectNMI, .opts = opts_inject_nmi, @@ -14776,6 +14937,12 @@ const vshCmdDef domManagementCmds[] = { .info = info_setLifecycleAction, .flags = 0 }, + {.name = "set-user-sshkeys", + .handler = cmdSetUserSSHKeys, + .opts = opts_set_user_sshkeys, + .info = info_set_user_sshkeys, + .flags = 0 + }, {.name = "set-user-password", .handler = cmdSetUserPassword, .opts = opts_set_user_password, -- 2.26.2
On Mon, Nov 16, 2020 at 13:21:00 +0100, Michal Privoznik wrote:
The new virsh commands are:
get-user-sshkeys set-user-sshkeys
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/manpages/virsh.rst | 37 +++++++++ tools/virsh-domain.c | 167 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 204 insertions(+)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index bfd26e3120..18cee358fd 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -2636,6 +2636,21 @@ When *--timestamp* is used, a human-readable timestamp will be printed before the event.
+get-user-sshkeys +---------------- + +**Syntax:** + +:: + + get-user-sshkeys domain user + +Print SSH authorized keys for given *user* in the guest *domain*. Please note, +that an entry in the file has internal structure as defined by *sshd(8)* and +virsh/libvirt does handle keys as opaque strings, i.e. does not interpret +them. + + guest-agent-timeout -------------------
@@ -4004,6 +4019,28 @@ For QEMU/KVM, this requires the guest agent to be configured and running.
+set-user-sshkeys +---------------- + +**Syntax:** + +:: + + set-user-sshkeys domain user [--file FILE] [{--append | --remove}] + +Replace the contents of *user*'s SSH authorized keys file in the guest *domain* +with keys read from optional *FILE*. In the *FILE* keys must be on separate +lines and each line must follow authorized keys format as defined by *sshd(8)*. +If no *FILE* is specified then the guest authorized keys file is cleared out.
IMO this is a very dangerous mode of operation. The default at least for virsh should be the same as if --apend was specified when no flag is used. That way you won't accidentally delete or overwrite keys if you don't mean to. Any of the more destructive modes should have an explicit flag.
+If *--append* is specified, then the guest authorized keys file content is not +replaced and new keys from *FILE* are just appended. + +If *--remove* is specified, then instead of adding any new keys then keys read +from *FILE* are removed from the authorized keys file. It is not considered an +error if the key does not exist in the file. + + setmaxmem ---------
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 12b35c037d..2a775fd277 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c
[...]
+static bool +cmdGetUserSSHKeys(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + const char *user; + char **keys = NULL; + int nkeys = 0; + size_t i; + const unsigned int flags = 0; + vshTablePtr table = NULL; + bool ret = false; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "user", &user) < 0) + goto cleanup; + + nkeys = virDomainAuthorizedSSHKeysGet(dom, user, &keys, flags); + if (nkeys < 0) + goto cleanup; + + if (!(table = vshTableNew(_("key"), NULL)))
vshTable seems to be a bit overkill for printing just strings on a line.
+ goto cleanup; + + for (i = 0; i < nkeys; i++) { + if (vshTableRowAppend(table, keys[i], NULL) < 0) + goto cleanup; + } + + vshTablePrintToStdout(table, ctl); + + ret = true; + cleanup: + vshTableFree(table); + if (nkeys > 0) + virStringListFreeCount(keys, nkeys); + virshDomainFree(dom); + return ret; +} + + +/* + * "set-user-sshkeys" command + */ +static const vshCmdInfo info_set_user_sshkeys[] = { + {.name = "help", + .data = N_("manipulate authorized SSH keys file for given user (via agent)") + }, + {.name = "desc", + .data = N_("Append, reset or remove specified key from the authorized " + "keys file for given user") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_set_user_sshkeys[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), + {.name = "user", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("user to set authorized keys for"), + }, + {.name = "file", + .type = VSH_OT_STRING, + .help = N_("optional file to read keys from"), + }, + {.name = "append", + .type = VSH_OT_BOOL, + .help = N_("append keys to the authorized keys file"), + }, + {.name = "remove", + .type = VSH_OT_BOOL, + .help = N_("remove keys from the authorized keys file"), + }, + {.name = NULL} +}; + +static bool +cmdSetUserSSHKeys(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + const char *user; + const char *from; + g_autofree char *buffer = NULL; + VIR_AUTOSTRINGLIST keys = NULL; + int nkeys = 0; + unsigned int flags = 0; + bool ret = false; + + VSH_REQUIRE_OPTION("append", "file"); + VSH_REQUIRE_OPTION("remove", "file"); + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "user", &user) < 0) + goto cleanup; + + if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + goto cleanup; + + if (vshCommandOptBool(cmd, "append")) + flags |= VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_APPEND; + if (vshCommandOptBool(cmd, "remove")) + flags |= VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_REMOVE; + + if (from) { + if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) { + vshReportError(ctl);
We usually use vshSaveLibvirtHelperError in such situations.
+ goto cleanup; + } + + if (!(keys = virStringSplit(buffer, "\n", -1))) + goto cleanup; + + nkeys = virStringListLength((const char **) keys); + } + + if (virDomainAuthorizedSSHKeysSet(dom, user, + (const char **) keys, nkeys, flags) < 0) { + goto cleanup; + } + + ret = true; + cleanup: + virshDomainFree(dom); + return ret; +}
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 Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_agent.c | 142 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 15 +++++ tests/qemuagenttest.c | 79 +++++++++++++++++++++++ 3 files changed, 236 insertions(+) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 7fbb4a9431..aeed74ec31 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2496,3 +2496,145 @@ qemuAgentSetResponseTimeout(qemuAgentPtr agent, { agent->timeout = timeout; } + +/** + * qemuAgentSSHGetAuthorizedKeys: + * @agent: agent object + * @user: user to get authorized keys for + * @keys: Array of authorized keys + * + * Fetch the public keys from @user's $HOME/.ssh/authorized_keys. + * + * Returns: number of keys returned on success, + * -1 otherwise (error is reported) + */ +int +qemuAgentSSHGetAuthorizedKeys(qemuAgentPtr agent, + const char *user, + char ***keys) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + virJSONValuePtr data = NULL; + size_t ndata; + size_t i; + char **keys_ret = NULL; + + if (!(cmd = qemuAgentMakeCommand("guest-ssh-get-authorized-keys", + "s:username", user, + NULL))) + return -1; + + if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) + return -1; + + if (!(data = virJSONValueObjectGetObject(reply, "return")) || + !(data = virJSONValueObjectGetArray(data, "keys"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu agent didn't return an array of keys")); + return -1; + } + + ndata = virJSONValueArraySize(data); + + keys_ret = g_new0(char *, 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 error; + } + + keys_ret[i] = g_strdup(virJSONValueGetString(entry)); + } + + *keys = g_steal_pointer(&keys_ret); + return ndata; + + error: + virStringListFreeCount(keys_ret, ndata); + return -1; +} + + +/** + * qemuAgentSSHAddAuthorizedKeys: + * @agent: agent object + * @user: user to add authorized keys for + * @keys: Array of authorized keys + * @nkeys: number of items in @keys array + * @reset: whether to truncate authorized keys file before writing + * + * Append SSH @keys into the @user's authorized keys file. If + * @reset is true then the file is truncated before write and + * thus contains only newly added @keys. + * + * Returns: 0 on success, + * -1 otherwise (error is reported) + */ +int +qemuAgentSSHAddAuthorizedKeys(qemuAgentPtr agent, + const char *user, + const char **keys, + size_t nkeys, + bool reset) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + g_autoptr(virJSONValue) jkeys = NULL; + + jkeys = qemuAgentMakeStringsArray(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; + + return qemuAgentCommand(agent, cmd, &reply, agent->timeout); +} + + +/** + * qemuAgentSSHRemoveAuthorizedKeys: + * @agent: agent object + * @user: user to remove authorized keys for + * @keys: Array of authorized keys + * @nkeys: number of items in @keys array + * + * Remove SSH @keys from the @user's authorized keys file. It's + * not considered an error when trying to remove a non-existent + * key. + * + * Returns: 0 on success, + * -1 otherwise (error is reported) + */ +int +qemuAgentSSHRemoveAuthorizedKeys(qemuAgentPtr agent, + const char *user, + const char **keys, + size_t nkeys) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + g_autoptr(virJSONValue) jkeys = NULL; + + jkeys = qemuAgentMakeStringsArray(keys, nkeys); + if (jkeys == NULL) + return -1; + + if (!(cmd = qemuAgentMakeCommand("guest-ssh-remove-authorized-keys", + "s:username", user, + "a:keys", &jkeys, + NULL))) + return -1; + + return qemuAgentCommand(agent, cmd, &reply, agent->timeout); +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 2eeb376a68..7cbab489ec 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -170,3 +170,18 @@ int qemuAgentGetTimezone(qemuAgentPtr mon, void qemuAgentSetResponseTimeout(qemuAgentPtr mon, int timeout); + +int qemuAgentSSHGetAuthorizedKeys(qemuAgentPtr agent, + const char *user, + char ***keys); + +int qemuAgentSSHAddAuthorizedKeys(qemuAgentPtr agent, + const char *user, + const char **keys, + size_t nkeys, + bool reset); + +int qemuAgentSSHRemoveAuthorizedKeys(qemuAgentPtr agent, + const char *user, + const char **keys, + size_t nkeys); diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 607bd97b5c..47ff92733a 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -35,6 +35,84 @@ virQEMUDriver driver; +static int +testQemuAgentSSHKeys(const void *data) +{ + virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt); + char **keys = NULL; + int nkeys = 0; + int ret = -1; + + if (!test) + return -1; + + if (qemuMonitorTestAddAgentSyncResponse(test) < 0) + goto cleanup; + + if (qemuMonitorTestAddItem(test, "guest-ssh-get-authorized-keys", + "{\"return\": {" + " \"keys\": [" + " \"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)) < 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], "algo2 key2 comments2")) { + virReportError(VIR_ERR_INTERNAL_ERROR, "Unexpected key returned: %s", keys[1]); + ret = -1; + goto cleanup; + } + + if ((ret = qemuAgentSSHAddAuthorizedKeys(qemuMonitorTestGetAgent(test), + "user", + (const char **) keys, + nkeys, + true)) < 0) + goto cleanup; + + if ((ret = qemuAgentSSHRemoveAuthorizedKeys(qemuMonitorTestGetAgent(test), + "user", + (const char **) keys, + nkeys)) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virStringListFreeCount(keys, nkeys); + qemuMonitorTestFree(test); + return ret; +} + + static int testQemuAgentFSFreeze(const void *data) { @@ -1315,6 +1393,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.26.2
On Mon, Nov 16, 2020 at 13:21:01 +0100, Michal Privoznik 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
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_agent.c | 142 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 15 +++++ tests/qemuagenttest.c | 79 +++++++++++++++++++++++ 3 files changed, 236 insertions(+)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 7fbb4a9431..aeed74ec31 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2496,3 +2496,145 @@ qemuAgentSetResponseTimeout(qemuAgentPtr agent, { agent->timeout = timeout; } + +/** + * qemuAgentSSHGetAuthorizedKeys: + * @agent: agent object + * @user: user to get authorized keys for + * @keys: Array of authorized keys + * + * Fetch the public keys from @user's $HOME/.ssh/authorized_keys. + * + * Returns: number of keys returned on success, + * -1 otherwise (error is reported) + */ +int +qemuAgentSSHGetAuthorizedKeys(qemuAgentPtr agent, + const char *user, + char ***keys) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + virJSONValuePtr data = NULL; + size_t ndata; + size_t i; + char **keys_ret = NULL; + + if (!(cmd = qemuAgentMakeCommand("guest-ssh-get-authorized-keys", + "s:username", user, + NULL))) + return -1; + + if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) + return -1; + + if (!(data = virJSONValueObjectGetObject(reply, "return")) || + !(data = virJSONValueObjectGetArray(data, "keys"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu agent didn't return an array of keys")); + return -1; + } + + ndata = virJSONValueArraySize(data); + + keys_ret = g_new0(char *, ndata);
+ 1
+ + 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"));
Whole error message should be on a single line.
+ goto error; + } + + keys_ret[i] = g_strdup(virJSONValueGetString(entry)); + } + + *keys = g_steal_pointer(&keys_ret); + return ndata; + + error: + virStringListFreeCount(keys_ret, ndata); + return -1; +}
[...] Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1888537 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 81 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ac38edf009..b69be1bedc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20094,6 +20094,85 @@ qemuDomainAgentSetResponseTimeout(virDomainPtr dom, } +static int +qemuDomainAuthorizedSSHKeysGet(virDomainPtr dom, + const char *user, + char ***keys, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + qemuAgentPtr agent; + int rv = -1; + + virCheckFlags(0, -1); + + if (!(vm = qemuDomainObjFromDomain(dom))) + return -1; + + if (virDomainAuthorizedSshKeysGetEnsureACL(dom->conn, vm->def) < 0) + return -1; + + if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0) + return -1; + + if (!qemuDomainAgentAvailable(vm, true)) + goto endagentjob; + + agent = qemuDomainObjEnterAgent(vm); + rv = qemuAgentSSHGetAuthorizedKeys(agent, user, keys); + qemuDomainObjExitAgent(vm, agent); + + endagentjob: + qemuDomainObjEndAgentJob(vm); + virDomainObjEndAPI(&vm); + return rv; +} + + +static int +qemuDomainAuthorizedSSHKeysSet(virDomainPtr dom, + const char *user, + const char **keys, + int nkeys, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + g_autoptr(virDomainObj) vm = NULL; + qemuAgentPtr agent; + const bool append = flags & VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_APPEND; + const bool remove = flags & VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_REMOVE; + int rv = -1; + + virCheckFlags(VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_APPEND | + VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_REMOVE, -1); + + if (!(vm = qemuDomainObjFromDomain(dom))) + return -1; + + if (virDomainAuthorizedSshKeysSetEnsureACL(dom->conn, vm->def) < 0) + return -1; + + if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0) + return -1; + + if (!qemuDomainAgentAvailable(vm, true)) + goto endagentjob; + + agent = qemuDomainObjEnterAgent(vm); + if (remove) + rv = qemuAgentSSHRemoveAuthorizedKeys(agent, user, keys, nkeys); + else + rv = qemuAgentSSHAddAuthorizedKeys(agent, user, keys, nkeys, !append); + qemuDomainObjExitAgent(vm, agent); + + endagentjob: + qemuDomainObjEndAgentJob(vm); + virDomainObjEndAPI(&vm); + return rv; +} + + static virHypervisorDriver qemuHypervisorDriver = { .name = QEMU_DRIVER_NAME, .connectURIProbe = qemuConnectURIProbe, @@ -20333,6 +20412,8 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainAgentSetResponseTimeout = qemuDomainAgentSetResponseTimeout, /* 5.10.0 */ .domainBackupBegin = qemuDomainBackupBegin, /* 6.0.0 */ .domainBackupGetXMLDesc = qemuDomainBackupGetXMLDesc, /* 6.0.0 */ + .domainAuthorizedSSHKeysGet = qemuDomainAuthorizedSSHKeysGet, /* 6.10.0 */ + .domainAuthorizedSSHKeysSet = qemuDomainAuthorizedSSHKeysSet, /* 6.10.0 */ }; -- 2.26.2
On Mon, Nov 16, 2020 at 13:21:02 +0100, Michal Privoznik wrote:
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1888537
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 81 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- NEWS.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 3fd3ce4cb9..3cdb6a4917 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -12,6 +12,12 @@ v6.10.0 (unreleased) ==================== * **New features** + * qemu: Implement OpenSSH authorized key file management APIs + + New APIs (``virDomainAuthorizedSSHKeysGet()`` and + ``virDomainAuthorizedSSHKeysSet()``) and virsh commands + (``get-user-sshkeys`` and ``set-user-sshkeys``) are added to manage + authorized_keys SSH file for user. * hyperv: implement new APIs -- 2.26.2
On Mon, Nov 16, 2020 at 13:21:03 +0100, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- NEWS.rst | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/NEWS.rst b/NEWS.rst index 3fd3ce4cb9..3cdb6a4917 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -12,6 +12,12 @@ v6.10.0 (unreleased) ====================
* **New features** + * qemu: Implement OpenSSH authorized key file management APIs + + New APIs (``virDomainAuthorizedSSHKeysGet()`` and + ``virDomainAuthorizedSSHKeysSet()``) and virsh commands + (``get-user-sshkeys`` and ``set-user-sshkeys``) are added to manage + authorized_keys SSH file for user.
This does not apply any more. Also please ensure that it conforms to ANF [1]. [1] ANF = Andrea's normal form https://www.redhat.com/archives/libvir-list/2020-November/msg00808.html
participants (2)
-
Michal Privoznik -
Peter Krempa