[PATCH v3 1/6] Introduce OpenSSH authorized key file mgmt APIs

[Daniel P. Berrangé]

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 at redhat.com>
> Reviewed-by: Peter Krempa <pkrempa at 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".

The code clearly requires a non-negative value, but nothing ever checks that,
so a caller passing -1 would have undefined behaviour AFAICT.

So can we change this to unsigned int ?


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 :|