Re: [PATCH v3 1/6] Introduce OpenSSH authorized key file mgmt APIs
On 11/23/20 12:50 PM, Daniel P. Berrangé wrote:
On Wed, Nov 18, 2020 at 02:34:19PM +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(a)redhat.com>
> Reviewed-by: Peter Krempa <pkrempa(a)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);
Is there a reason you used "int nkeys" here rather than "unsigned int
nkeys".
No, you're right - it should have been uint.
The code clearly requires a non-negative value, but nothing ever checks that,
so a caller passing -1 would have undefined behaviour AFAICT.
Technically, passing a negative value would fail at RPC because it would
hit the limit for keys stored in one RPC message, but that's not good
either.
So can we change this to unsigned int ?
Yep, will post a patch.
Michal