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@xxxxxxxxxx> Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx> --- 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
