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

[Michal Privoznik]

On 11/11/20 9:06 AM, Peter Krempa wrote:
> On Tue, Nov 10, 2020 at 16:11:41 +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
> 
> Doesn't management software already run something inside the VM which
> does all of this?

Not really. oVirt does, Kubevirt doesn't.

> 
>> 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.
> 
> To be fair, I surely hope that the qemu-guest-agent feature is disabled
> by default. This seems a bit scary to me.
> 
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>   include/libvirt/libvirt-domain.h |  17 +++++
>>   src/driver-hypervisor.h          |  15 ++++
>>   src/libvirt-domain.c             | 115 +++++++++++++++++++++++++++++++
>>   src/libvirt_public.syms          |   6 ++
>>   4 files changed, 153 insertions(+)
> 
> 
>> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
>> index 3c5f55176a..0a55a48952 100644
>> --- a/src/libvirt-domain.c
>> +++ b/src/libvirt-domain.c
>> @@ -12758,3 +12758,118 @@ 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.
> 
> Missing mention that hypervisor may require use of guest-agent.
> 
>> + *
>> + * 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);
>> +    virCheckNonNullArgReturn(user, -1);
>> +    virCheckNonNullArgReturn(keys, -1);
>> +    conn = domain->conn;
> 
> This API IMO _must_ use 'virCheckReadOnlyGoto(conn->flags, error);'
> 
> read-only users should not be able to access the guest agent for
> security reasons and also getting the list of authorized keys may
> actually leak somewhat sensitive data (allowing identification of the
> user)
> 

Ah, good point. I keep forgetting about RO connection.

Michal