[libvirt] [PATCH 02/20] Secret manipulation step 2: Internal API

Adds a new driver type. Changes since the third submission: - Add "flags" parameter to virSecretDefineXML(), virSecretGetXMLDesc(), virSecretGetValue(), virSecretSetValue(), and all derived interfaces. - Fix a copy&pasted comment --- include/libvirt/virterror.h | 1 + src/datatypes.c | 155 +++++++++++++++++++++++++++++++++++++++++++ src/datatypes.h | 28 ++++++++ src/driver.h | 61 +++++++++++++++++ src/libvirt.c | 53 +++++++++++++++ src/libvirt_private.syms | 2 + src/virterror.c | 6 ++ 7 files changed, 306 insertions(+), 0 deletions(-) diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index e4d013f..5cbb120 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -166,6 +166,7 @@ typedef enum { VIR_ERR_NO_INTERFACE, /* interface driver not running */ VIR_ERR_INVALID_INTERFACE, /* invalid interface object */ VIR_ERR_MULTIPLE_INTERFACES, /* more than one matching interface found */ + VIR_WAR_NO_SECRET, /* failed to start secret storage */ } virErrorNumber; /** diff --git a/src/datatypes.c b/src/datatypes.c index d03a679..2e85196 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -109,6 +109,23 @@ virStorageVolFreeName(virStorageVolPtr vol, const char *name ATTRIBUTE_UNUSED) } /** + * virSecretFreeName: + * @secret_: a secret object + * + * Destroy the secret object, this is just used by the secret hash callback. + * + * Returns 0 in case of success and -1 in case of failure. + */ +static void +virSecretFreeName(void *secret_, const char *name ATTRIBUTE_UNUSED) +{ + virSecretPtr secret; + + secret = secret_; + virUnrefSecret(secret); +} + +/** * virGetConnect: * * Allocates a new hypervisor connection structure @@ -152,6 +169,9 @@ virGetConnect(void) { ret->nodeDevices = virHashCreate(256); if (ret->nodeDevices == NULL) goto failed; + ret->secrets = virHashCreate(20); + if (ret->secrets == NULL) + goto failed; ret->refs = 1; return(ret); @@ -170,6 +190,8 @@ failed: virHashFree(ret->storageVols, (virHashDeallocator) virStorageVolFreeName); if (ret->nodeDevices != NULL) virHashFree(ret->nodeDevices, (virHashDeallocator) virNodeDeviceFree); + if (ret->secrets != NULL) + virHashFree(ret->secrets, virSecretFreeName); virMutexDestroy(&ret->lock); VIR_FREE(ret); @@ -201,6 +223,8 @@ virReleaseConnect(virConnectPtr conn) { virHashFree(conn->storageVols, (virHashDeallocator) virStorageVolFreeName); if (conn->nodeDevices != NULL) virHashFree(conn->nodeDevices, (virHashDeallocator) virNodeDeviceFree); + if (conn->secrets != NULL) + virHashFree(conn->secrets, virSecretFreeName); virResetError(&conn->err); @@ -246,6 +270,8 @@ virUnrefConnect(virConnectPtr conn) { conn->storageDriver->close (conn); if (conn->deviceMonitor) conn->deviceMonitor->close (conn); + if (conn->secretDriver) + conn->secretDriver->close (conn); if (conn->driver) conn->driver->close (conn); @@ -1129,3 +1155,132 @@ virUnrefNodeDevice(virNodeDevicePtr dev) { virMutexUnlock(&dev->conn->lock); return (refs); } + +/** + * virGetSecret: + * @conn: the hypervisor connection + * @uuid: secret UUID + * + * Lookup if the secret is already registered for that connection, if so return + * a pointer to it, otherwise allocate a new structure, and register it in the + * table. In any case a corresponding call to virFreeSecret() is needed to not + * leak data. + * + * Returns a pointer to the secret, or NULL in case of failure + */ +virSecretPtr +virGetSecret(virConnectPtr conn, const char *uuid) +{ + virSecretPtr ret = NULL; + + if (!VIR_IS_CONNECT(conn) || uuid == NULL) { + virLibConnError(NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); + return NULL; + } + virMutexLock(&conn->lock); + + ret = virHashLookup(conn->secrets, uuid); + if (ret == NULL) { + if (VIR_ALLOC(ret) < 0) { + virMutexUnlock(&conn->lock); + virReportOOMError(conn); + goto error; + } + ret->magic = VIR_SECRET_MAGIC; + ret->conn = conn; + ret->uuid = strdup(uuid); + if (ret->uuid == NULL) { + virMutexUnlock(&conn->lock); + virReportOOMError(conn); + goto error; + } + + if (virHashAddEntry(conn->secrets, uuid, ret) < 0) { + virMutexUnlock(&conn->lock); + virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("failed to add secret to conn hash table")); + goto error; + } + conn->refs++; + } + ret->refs++; + virMutexUnlock(&conn->lock); + return ret; + +error: + if (ret != NULL) { + VIR_FREE(ret->uuid); + VIR_FREE(ret); + } + return NULL; +} + +/** + * virReleaseSecret: + * @secret: the secret to release + * + * Unconditionally release all memory associated with a secret. The conn.lock + * mutex must be held prior to calling this, and will be released prior to this + * returning. The secret obj must not be used once this method returns. + * + * It will also unreference the associated connection object, which may also be + * released if its ref count hits zero. + */ +static void +virReleaseSecret(virSecretPtr secret) { + virConnectPtr conn = secret->conn; + DEBUG("release secret %p %s", secret, secret->uuid); + + if (virHashRemoveEntry(conn->secrets, secret->uuid, NULL) < 0) { + virMutexUnlock(&conn->lock); + virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("secret missing from connection hash table")); + conn = NULL; + } + + secret->magic = -1; + VIR_FREE(secret->uuid); + VIR_FREE(secret); + + if (conn) { + DEBUG("unref connection %p %d", conn, conn->refs); + conn->refs--; + if (conn->refs == 0) { + virReleaseConnect(conn); + /* Already unlocked mutex */ + return; + } + virMutexUnlock(&conn->lock); + } +} + +/** + * virUnrefSecret: + * @secret: the secret to unreference + * + * Unreference the secret. If the use count drops to zero, the structure is + * actually freed. + * + * Returns the reference count or -1 in case of failure. + */ +int +virUnrefSecret(virSecretPtr secret) { + int refs; + + if (!VIR_IS_CONNECTED_SECRET(secret)) { + virLibConnError(NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); + return -1; + } + virMutexLock(&secret->conn->lock); + DEBUG("unref secret %p %s %d", secret, secret->uuid, secret->refs); + secret->refs--; + refs = secret->refs; + if (refs == 0) { + virReleaseSecret(secret); + /* Already unlocked mutex */ + return 0; + } + + virMutexUnlock(&secret->conn->lock); + return refs; +} diff --git a/src/datatypes.h b/src/datatypes.h index da83e02..56c3777 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -98,6 +98,16 @@ #define VIR_IS_NODE_DEVICE(obj) ((obj) && (obj)->magic==VIR_NODE_DEVICE_MAGIC) #define VIR_IS_CONNECTED_NODE_DEVICE(obj) (VIR_IS_NODE_DEVICE(obj) && VIR_IS_CONNECT((obj)->conn)) +/** + * VIR_SECRET_MAGIC: + * + * magic value used to protect the API when pointers to secret structures are + * passed down by the users. + */ +#define VIR_SECRET_MAGIC 0x5678DEAD +#define VIR_IS_SECRET(obj) ((obj) && (obj)->magic==VIR_SECRET_MAGIC) +#define VIR_IS_CONNECTED_SECRET(obj) (VIR_IS_SECRET(obj) && VIR_IS_CONNECT((obj)->conn)) + /** * _virConnect: @@ -119,6 +129,7 @@ struct _virConnect { virInterfaceDriverPtr interfaceDriver; virStorageDriverPtr storageDriver; virDeviceMonitorPtr deviceMonitor; + virSecretDriverPtr secretDriver; /* Private data pointer which can be used by driver and * network driver as they wish. @@ -149,6 +160,7 @@ struct _virConnect { virHashTablePtr storagePools;/* hash table for known storage pools */ virHashTablePtr storageVols;/* hash table for known storage vols */ virHashTablePtr nodeDevices; /* hash table for known node devices */ + virHashTablePtr secrets; /* hash taboe for known secrets */ int refs; /* reference count */ }; @@ -233,6 +245,18 @@ struct _virNodeDevice { char *parent; /* parent device name */ }; +/** + * _virSecret: + * + * Internal structure associated with a secret + */ +struct _virSecret { + unsigned int magic; /* specific value to check */ + int refs; /* reference count */ + virConnectPtr conn; /* pointer back to the connection */ + char *uuid; /* ID of the secret */ +}; + /************************************************************************ * * @@ -270,4 +294,8 @@ virNodeDevicePtr virGetNodeDevice(virConnectPtr conn, const char *name); int virUnrefNodeDevice(virNodeDevicePtr dev); +virSecretPtr virGetSecret(virConnectPtr conn, + const char *uuid); +int virUnrefSecret(virSecretPtr secret); + #endif diff --git a/src/driver.h b/src/driver.h index 79d46ff..a6b8e01 100644 --- a/src/driver.h +++ b/src/driver.h @@ -6,6 +6,9 @@ #ifndef __VIR_DRIVER_H__ #define __VIR_DRIVER_H__ +#include "config.h" +#include <stdbool.h> + #include <libxml/uri.h> #include "internal.h" @@ -799,6 +802,63 @@ struct _virDeviceMonitor { virDrvNodeDeviceDestroy deviceDestroy; }; +typedef virSecretPtr + (*virDrvSecretLookupByUUIDString) (virConnectPtr conn, + const char *uuid); +typedef virSecretPtr + (*virDrvSecretDefineXML) (virConnectPtr conn, + const char *xml, + unsigned int flags); +typedef char * + (*virDrvSecretGetXMLDesc) (virSecretPtr secret, + unsigned int flags); +typedef int + (*virDrvSecretSetValue) (virSecretPtr secret, + const unsigned char *value, + size_t value_size, + unsigned int flags); +typedef unsigned char * + (*virDrvSecretGetValue) (virSecretPtr secret, + size_t *value_size, + bool libvirt_internal_call, + unsigned int flags); +typedef int + (*virDrvSecretUndefine) (virSecretPtr secret); +typedef int + (*virDrvSecretNumOfSecrets) (virConnectPtr conn); +typedef int + (*virDrvSecretListSecrets) (virConnectPtr conn, + char **uuids, + int maxuuids); + +typedef struct _virSecretDriver virSecretDriver; +typedef virSecretDriver *virSecretDriverPtr; + +/** + * _virSecretDriver: + * + * Structure associated to a driver for storing secrets, defining the various + * entry points for it. + * + * All drivers must support the following fields/methods: + * - open + * - close + */ +struct _virSecretDriver { + const char *name; + virDrvOpen open; + virDrvClose close; + + virDrvSecretNumOfSecrets numOfSecrets; + virDrvSecretListSecrets listSecrets; + virDrvSecretLookupByUUIDString lookupByUUIDString; + virDrvSecretDefineXML defineXML; + virDrvSecretGetXMLDesc getXMLDesc; + virDrvSecretSetValue setValue; + virDrvSecretGetValue getValue; + virDrvSecretUndefine undefine; +}; + /* * Registration * TODO: also need ways to (des)activate a given driver @@ -809,6 +869,7 @@ int virRegisterNetworkDriver(virNetworkDriverPtr); int virRegisterInterfaceDriver(virInterfaceDriverPtr); int virRegisterStorageDriver(virStorageDriverPtr); int virRegisterDeviceMonitor(virDeviceMonitorPtr); +int virRegisterSecretDriver(virSecretDriverPtr); #ifdef WITH_LIBVIRTD int virRegisterStateDriver(virStateDriverPtr); #endif diff --git a/src/libvirt.c b/src/libvirt.c index ca8e003..bea60f4 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -86,6 +86,8 @@ static virStorageDriverPtr virStorageDriverTab[MAX_DRIVERS]; static int virStorageDriverTabCount = 0; static virDeviceMonitorPtr virDeviceMonitorTab[MAX_DRIVERS]; static int virDeviceMonitorTabCount = 0; +static virSecretDriverPtr virSecretDriverTab[MAX_DRIVERS]; +static int virSecretDriverTabCount = 0; #ifdef WITH_LIBVIRTD static virStateDriverPtr virStateDriverTab[MAX_DRIVERS]; static int virStateDriverTabCount = 0; @@ -684,6 +686,37 @@ virRegisterDeviceMonitor(virDeviceMonitorPtr driver) } /** + * virRegisterSecretDriver: + * @driver: pointer to a secret driver block + * + * Register a secret driver + * + * Returns the driver priority or -1 in case of error. + */ +int +virRegisterSecretDriver(virSecretDriverPtr driver) +{ + if (virInitialize() < 0) + return -1; + + if (driver == NULL) { + virLibConnError(NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); + return(-1); + } + + if (virSecretDriverTabCount >= MAX_DRIVERS) { + virLibConnError(NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); + return(-1); + } + + DEBUG ("registering %s as secret driver %d", + driver->name, virSecretDriverTabCount); + + virSecretDriverTab[virSecretDriverTabCount] = driver; + return virSecretDriverTabCount++; +} + +/** * virRegisterDriver: * @driver: pointer to a driver block * @@ -1096,6 +1129,26 @@ do_open (const char *name, } } + /* Secret manipulation driver. Optional */ + for (i = 0; i < virSecretDriverTabCount; i++) { + res = virSecretDriverTab[i]->open (ret, auth, flags); + DEBUG("secret driver %d %s returned %s", + i, virSecretDriverTab[i]->name, + res == VIR_DRV_OPEN_SUCCESS ? "SUCCESS" : + (res == VIR_DRV_OPEN_DECLINED ? "DECLINED" : + (res == VIR_DRV_OPEN_ERROR ? "ERROR" : "unknown status"))); + if (res == VIR_DRV_OPEN_ERROR) { + if (STREQ(virSecretDriverTab[i]->name, "remote")) { + virLibConnWarning (NULL, VIR_WAR_NO_SECRET, + "Is the daemon running ?"); + } + break; + } else if (res == VIR_DRV_OPEN_SUCCESS) { + ret->secretDriver = virSecretDriverTab[i]; + break; + } + } + return ret; failed: diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2bf4e15..f2c0736 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -52,10 +52,12 @@ virGetInterface; virGetNetwork; virGetStoragePool; virGetStorageVol; +virGetSecret; virUnrefStorageVol; virGetNodeDevice; virUnrefDomain; virUnrefConnect; +virUnrefSecret; # domain_conf.h diff --git a/src/virterror.c b/src/virterror.c index 362d8ef..83a0830 100644 --- a/src/virterror.c +++ b/src/virterror.c @@ -1068,6 +1068,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("multiple matching interfaces found: %s"); break; + case VIR_WAR_NO_SECRET: + if (info == NULL) + errmsg = _("Failed to find a secret storage driver"); + else + errmsg = _("Failed to find a secret storage driver: %s"); + break; } return (errmsg); } -- 1.6.2.5