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

[Michal Prívozník]

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 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".

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