To keep original seclabel for files libvirt is touching we need a single point where the original seclabels can be stored. Instead of inventing a new one we can misuse virtlockd which already has nearly all the infrastructure we need. As nice feature, it keeps its internal state between virtlockd restarts. Again, it's something we are going to need, as we don't want to lose the original labels on the lock daemon restart. In this commit two functions are introduced: virLockManagerRememberSeclabel that takes three arguments: path, model and seclabel where @path is unique identifier for the file we are about to label, @model and @seclabel then represents original seclabel. virLockManagerRecallSeclabel then takes: path, model, *seclabel and returns number of references held on @path. If the return value is zero, *seclabel contains the original label stored by first call of RememberSeclabel(). If a positive value is returned, other domains are still using the @path and the original label shall not be restored. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/libvirt_private.syms | 2 ++ src/locking/lock_driver.h | 41 +++++++++++++++++++++++++++++++++++++++++ src/locking/lock_manager.c | 26 ++++++++++++++++++++++++++ src/locking/lock_manager.h | 9 +++++++++ 4 files changed, 78 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fdf4548..cdc476a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -887,7 +887,9 @@ virLockManagerPluginNew; virLockManagerPluginRef; virLockManagerPluginUnref; virLockManagerPluginUsesState; +virLockManagerRecallSeclabel; virLockManagerRelease; +virLockManagerRememberSeclabel; # nodeinfo.h diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h index a7d9782..a82f240 100644 --- a/src/locking/lock_driver.h +++ b/src/locking/lock_driver.h @@ -275,6 +275,44 @@ typedef int (*virLockDriverInquire)(virLockManagerPtr man, unsigned int flags); +/** + * virLockDriverRememberSeclabel: + * @manager: the lock manager context + * @path: which path we are remembering label for + * @model: security model (e.g. "DAC", "SELINUX", ...) + * @seclabel: security label to remember + * + * Remember security label for @path. Do that with reference + * counting, so only the first time something is actually + * remembered. + * + * Returns -1 on failure, or reference counter for @path. + */ +typedef int (*virLockDriverRememberSeclabel)(virLockManagerPtr man, + const char *path, + const char *model, + const char *seclabel); + +/** + * virLockDriverRecallSeclabel: + * @manager: the lock manager context + * @path: path to recall + * @model: security model (e.g. "DAC", "SELINUX", ...) + * @seclabel: previously stored seclabel + * + * Recall previously set seclabel. The @seclabel argument is + * required to be filled with original security label when zero + * is returned. It may, however, be filled if a positive value is + * returned too (depending on the lock driver used). + * + * Returns -1 on failure, or reference counter for @path (zero if + * @seclabel needs to be restored). + */ +typedef int (*virLockDriverRecallSeclabel)(virLockManagerPtr man, + const char *path, + const char *model, + char **seclabel); + struct _virLockManager { virLockDriverPtr driver; void *privateData; @@ -303,6 +341,9 @@ struct _virLockDriver { virLockDriverAcquire drvAcquire; virLockDriverRelease drvRelease; virLockDriverInquire drvInquire; + + virLockDriverRememberSeclabel drvRemember; + virLockDriverRecallSeclabel drvRecall; }; diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c index ec90d04..fb5cfe1 100644 --- a/src/locking/lock_manager.c +++ b/src/locking/lock_manager.c @@ -397,3 +397,29 @@ int virLockManagerFree(virLockManagerPtr lock) return 0; } + +int virLockManagerRememberSeclabel(virLockManagerPtr lock, + const char *path, + const char *model, + const char *seclabel) +{ + VIR_DEBUG("lock=%p path=%s model=%s seclabel=%s", + lock, path, model, seclabel); + + CHECK_MANAGER(drvRemember, -1); + + return lock->driver->drvRemember(lock, path, model, seclabel); +} + +int virLockManagerRecallSeclabel(virLockManagerPtr lock, + const char *path, + const char *model, + char **seclabel) +{ + VIR_DEBUG("lock=%p path=%s model=%s seclabel=%p", + lock, path, model, seclabel); + + CHECK_MANAGER(drvRecall, -1); + + return lock->driver->drvRecall(lock, path, model, seclabel); +} diff --git a/src/locking/lock_manager.h b/src/locking/lock_manager.h index 4189759..3fd5b9a 100644 --- a/src/locking/lock_manager.h +++ b/src/locking/lock_manager.h @@ -67,4 +67,13 @@ int virLockManagerInquire(virLockManagerPtr manager, int virLockManagerFree(virLockManagerPtr manager); +int virLockManagerRememberSeclabel(virLockManagerPtr lock, + const char *path, + const char *model, + const char *seclabel); +int virLockManagerRecallSeclabel(virLockManagerPtr lock, + const char *path, + const char *model, + char **seclabel); + #endif /* __VIR_LOCK_MANAGER_H__ */ -- 1.8.5.5

On Wed, Sep 10, 2014 at 03:26:09PM +0200, Michal Privoznik wrote: Thinking ahead to the time when we have to persist the lock information to disk, shared between multiple virtlockds, we might want to take some parameters here. Specifically a hostname and/or host uuid ? Though I guess we could possibly wait until that time - depends on impact on the RPC protocol in later patches, so will comment there. Tentative ACK. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 09/10/2014 03:26 PM, Michal Privoznik wrote: This p should be lowercase. Jan

This is the client side, so there's nothing more we need to do than call the RPC. Although, there's one interesting change: new virLockManagerLockSeclabelConnect() had to be implemented since the VIR_LOCK_SPACE_PROTOCOL_PROGRAM doesn't have any ConnectOpen() procedure. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/locking/lock_driver_lockd.c | 114 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 109 insertions(+), 5 deletions(-) diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 4bb5925..cce88ac 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -31,6 +31,7 @@ #include "virerror.h" #include "rpc/virnetclient.h" #include "lock_protocol.h" +#include "lock_seclabel_protocol.h" #include "configmake.h" #include "virstring.h" @@ -239,7 +240,9 @@ virLockManagerLockDaemonConnectionRestrict(virLockManagerPtr lock ATTRIBUTE_UNUS static virNetClientPtr virLockManagerLockDaemonConnectionNew(bool privileged, - virNetClientProgramPtr *prog) + virNetClientProgramPtr *prog, + unsigned int prog_magic, + unsigned int prog_version) { virNetClientPtr client = NULL; char *lockdpath; @@ -263,8 +266,8 @@ static virNetClientPtr virLockManagerLockDaemonConnectionNew(bool privileged, daemonPath))) goto error; - if (!(*prog = virNetClientProgramNew(VIR_LOCK_SPACE_PROTOCOL_PROGRAM, - VIR_LOCK_SPACE_PROTOCOL_PROGRAM_VERSION, + if (!(*prog = virNetClientProgramNew(prog_magic, + prog_version, NULL, 0, NULL))) @@ -295,7 +298,9 @@ virLockManagerLockDaemonConnect(virLockManagerPtr lock, { virNetClientPtr client; - if (!(client = virLockManagerLockDaemonConnectionNew(geteuid() == 0, program))) + if (!(client = virLockManagerLockDaemonConnectionNew(geteuid() == 0, program, + VIR_LOCK_SPACE_PROTOCOL_PROGRAM, + VIR_LOCK_SPACE_PROTOCOL_PROGRAM_VERSION))) return NULL; if (virLockManagerLockDaemonConnectionRegister(lock, @@ -313,6 +318,22 @@ virLockManagerLockDaemonConnect(virLockManagerPtr lock, } +static virNetClientPtr +virLockManagerLockSeclabelConnect(virLockManagerPtr lock ATTRIBUTE_UNUSED, + virNetClientProgramPtr *program, + int *counter ATTRIBUTE_UNUSED) +{ + virNetClientPtr client; + + if (!(client = virLockManagerLockDaemonConnectionNew(geteuid() == 0, program, + VIR_LOCK_SECLABEL_PROTOCOL_PROGRAM, + VIR_LOCK_SECLABEL_PROTOCOL_PROGRAM_VERSION))) + return NULL; + + return client; +} + + static int virLockManagerLockDaemonSetupLockspace(const char *path) { virNetClientPtr client; @@ -324,7 +345,9 @@ static int virLockManagerLockDaemonSetupLockspace(const char *path) memset(&args, 0, sizeof(args)); args.path = (char*)path; - if (!(client = virLockManagerLockDaemonConnectionNew(geteuid() == 0, &program))) + if (!(client = virLockManagerLockDaemonConnectionNew(geteuid() == 0, &program, + VIR_LOCK_SPACE_PROTOCOL_PROGRAM, + VIR_LOCK_SPACE_PROTOCOL_PROGRAM_VERSION))) return -1; if (virNetClientProgramCall(program, @@ -786,6 +809,84 @@ static int virLockManagerLockDaemonInquire(virLockManagerPtr lock ATTRIBUTE_UNUS return 0; } + +static int +virLockManagerLockRememberSeclabel(virLockManagerPtr lock, + const char *path, + const char *model, + const char *seclabel) +{ + virNetClientPtr client = NULL; + virNetClientProgramPtr program = NULL; + int counter = 0; + virLockSeclabelProtocolRememberSeclabelArgs args; + virLockSeclabelProtocolRememberSeclabelRet ret; + int rv = -1; + + args.path = (char *)path; + args.model = (char *)model; + args.seclabel = (char *)seclabel; + + if (!(client = virLockManagerLockSeclabelConnect(lock, &program, &counter))) + goto cleanup; + + if (virNetClientProgramCall(program, client, counter++, + VIR_LOCK_SECLABEL_PROTOCOL_PROC_REMEMBER_SECLABEL, + 0, NULL, NULL, NULL, + (xdrproc_t)xdr_virLockSeclabelProtocolRememberSeclabelArgs, &args, + (xdrproc_t)xdr_virLockSeclabelProtocolRememberSeclabelRet, &ret) < 0) + goto cleanup; + + rv = ret.ret; + + cleanup: + virNetClientClose(client); + virObjectUnref(client); + virObjectUnref(program); + return rv; +} + + +static int +virLockManagerLockRecallSeclabel(virLockManagerPtr lock, + const char *path, + const char *model, + char **seclabel) +{ + virNetClientPtr client = NULL; + virNetClientProgramPtr program = NULL; + int counter = 0; + virLockSeclabelProtocolRecallSeclabelArgs args; + virLockSeclabelProtocolRecallSeclabelRet ret; + int rv = -1; + + memset(&ret, 0, sizeof(ret)); + + args.path = (char *)path; + args.model = (char *)model; + + if (!(client = virLockManagerLockSeclabelConnect(lock, &program, &counter))) + goto cleanup; + + if (virNetClientProgramCall(program, client, counter++, + VIR_LOCK_SECLABEL_PROTOCOL_PROC_RECALL_SECLABEL, + 0, NULL, NULL, NULL, + (xdrproc_t)xdr_virLockSeclabelProtocolRecallSeclabelArgs, &args, + (xdrproc_t)xdr_virLockSeclabelProtocolRecallSeclabelRet, &ret) < 0) + goto cleanup; + + if (seclabel) + *seclabel = ret.seclabel; + rv = ret.ret; + + cleanup: + virNetClientClose(client); + virObjectUnref(client); + virObjectUnref(program); + return rv; +} + + virLockDriver virLockDriverImpl = { .version = VIR_LOCK_MANAGER_VERSION, @@ -803,4 +904,7 @@ virLockDriver virLockDriverImpl = .drvRelease = virLockManagerLockDaemonRelease, .drvInquire = virLockManagerLockDaemonInquire, + + .drvRemember = virLockManagerLockRememberSeclabel, + .drvRecall = virLockManagerLockRecallSeclabel, }; -- 1.8.5.5

And register the program, finally. Although, only stub implementation of the Remember() and Recall() functions is added. The real implementation will follow. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/locking/lock_daemon.c | 37 ++++++++++++++++++ src/locking/lock_daemon.h | 8 ++++ src/locking/lock_daemon_dispatch.c | 77 ++++++++++++++++++++++++++++++++++++++ src/locking/lock_daemon_dispatch.h | 3 ++ 4 files changed, 125 insertions(+) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 02d77e3..b39a63a 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -49,6 +49,7 @@ #include "locking/lock_daemon_dispatch.h" #include "locking/lock_protocol.h" +#include "locking/lock_seclabel_protocol.h" #include "configmake.h" @@ -277,6 +278,28 @@ virLockSpacePtr virLockDaemonFindLockSpace(virLockDaemonPtr lockd, } +int +virLockDaemonRememberSeclabel(virLockDaemonPtr lockd ATTRIBUTE_UNUSED, + const char *path ATTRIBUTE_UNUSED, + const char *model ATTRIBUTE_UNUSED, + const char *seclabel ATTRIBUTE_UNUSED) +{ + /* Implement me */ + return -1; +} + + +int +virLockDaemonRecallSeclabel(virLockDaemonPtr lockd ATTRIBUTE_UNUSED, + const char *path ATTRIBUTE_UNUSED, + const char *model ATTRIBUTE_UNUSED, + char **seclabel ATTRIBUTE_UNUSED) +{ + /* Implement me */ + return -1; +} + + static int virLockDaemonForkIntoBackground(const char *argv0) { @@ -1150,6 +1173,7 @@ virLockDaemonUsage(const char *argv0, bool privileged) #define MAX_LISTEN 5 int main(int argc, char **argv) { virNetServerProgramPtr lockProgram = NULL; + virNetServerProgramPtr seclabelProgram = NULL; char *remote_config_file = NULL; int statuswrite = -1; int ret = 1; @@ -1407,6 +1431,18 @@ int main(int argc, char **argv) { goto cleanup; } + if (!(seclabelProgram = virNetServerProgramNew(VIR_LOCK_SECLABEL_PROTOCOL_PROGRAM, + VIR_LOCK_SECLABEL_PROTOCOL_PROGRAM_VERSION, + virLockSeclabelProtocolProcs, + virLockSeclabelProtocolNProcs))) { + ret = VIR_LOCK_DAEMON_ERR_INIT; + goto cleanup; + } + if (virNetServerAddProgram(lockDaemon->srv, seclabelProgram) < 0) { + ret = VIR_LOCK_DAEMON_ERR_INIT; + goto cleanup; + } + /* Disable error func, now logging is setup */ virSetErrorFunc(NULL, virLockDaemonErrorHandler); @@ -1438,6 +1474,7 @@ int main(int argc, char **argv) { cleanup: virObjectUnref(lockProgram); + virObjectUnref(seclabelProgram); virLockDaemonFree(lockDaemon); if (statuswrite != -1) { if (ret != 0) { diff --git a/src/locking/lock_daemon.h b/src/locking/lock_daemon.h index da62edc..84c1fa3 100644 --- a/src/locking/lock_daemon.h +++ b/src/locking/lock_daemon.h @@ -53,4 +53,12 @@ int virLockDaemonAddLockSpace(virLockDaemonPtr lockd, virLockSpacePtr virLockDaemonFindLockSpace(virLockDaemonPtr lockd, const char *path); +int virLockDaemonRememberSeclabel(virLockDaemonPtr lockd, + const char *path, + const char *model, + const char *seclabel); +int virLockDaemonRecallSeclabel(virLockDaemonPtr lockd, + const char *path, + const char *model, + char **seclabel); #endif /* __VIR_LOCK_DAEMON_H__ */ diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c index 168a6af..28483f4 100644 --- a/src/locking/lock_daemon_dispatch.c +++ b/src/locking/lock_daemon_dispatch.c @@ -28,6 +28,7 @@ #include "virstring.h" #include "lock_daemon.h" #include "lock_protocol.h" +#include "lock_seclabel_protocol.h" #include "virerror.h" #define VIR_FROM_THIS VIR_FROM_RPC @@ -35,6 +36,7 @@ VIR_LOG_INIT("locking.lock_daemon_dispatch"); #include "lock_daemon_dispatch_stubs.h" +#include "lock_daemon_seclabel_dispatch_stubs.h" static int virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr server ATTRIBUTE_UNUSED, @@ -429,3 +431,78 @@ virLockSpaceProtocolDispatchCreateLockSpace(virNetServerPtr server ATTRIBUTE_UNU virMutexUnlock(&priv->lock); return rv; } + + +static int +virLockSeclabelProtocolDispatchRememberSeclabel(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + virLockSeclabelProtocolRememberSeclabelArgs *args, + virLockSeclabelProtocolRememberSeclabelRet *ret) +{ + int rc = -1; + int rv; + virLockDaemonClientPtr priv = + virNetServerClientGetPrivateData(client); + + virMutexLock(&priv->lock); + + if (priv->restricted) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("lock manager connection has been restricted")); + goto cleanup; + } + + if ((rv = virLockDaemonRememberSeclabel(lockDaemon, args->path, + args->model, args->seclabel)) < 0) + goto cleanup; + + ret->ret = rv; + rc = 0; + + cleanup: + if (rc < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return rc; +} + + +static int +virLockSeclabelProtocolDispatchRecallSeclabel(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + virLockSeclabelProtocolRecallSeclabelArgs *args, + virLockSeclabelProtocolRecallSeclabelRet *ret) +{ + int rc = -1; + int rv; + virLockDaemonClientPtr priv = + virNetServerClientGetPrivateData(client); + char *seclabel = NULL; + + virMutexLock(&priv->lock); + + if (priv->restricted) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("lock manager connection has been restricted")); + goto cleanup; + } + + if ((rv = virLockDaemonRecallSeclabel(lockDaemon, args->path, + args->model, &seclabel)) < 0) + goto cleanup; + + ret->seclabel = seclabel; + ret->ret = rv; + + rc = 0; + + cleanup: + if (rc < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return rc; +} diff --git a/src/locking/lock_daemon_dispatch.h b/src/locking/lock_daemon_dispatch.h index a193a58..e5d540b 100644 --- a/src/locking/lock_daemon_dispatch.h +++ b/src/locking/lock_daemon_dispatch.h @@ -28,4 +28,7 @@ extern virNetServerProgramProc virLockSpaceProtocolProcs[]; extern size_t virLockSpaceProtocolNProcs; +extern virNetServerProgramProc virLockSeclabelProtocolProcs[]; +extern size_t virLockSeclabelProtocolNProcs; + #endif /* __VIR_LOCK_DAEMON_DISPATCH_H__ */ -- 1.8.5.5

The most interesting part where all the hard work takes place. For storing triplets of strings a two dimensional table would be the most intuitive approach. Or, if the triplet is rearranged into pair of one string and a pair, a hash table can be used. But hey, we already have those! Excellent. In other words, <path, model, seclabel> is turned into: <path, <model, seclabel>> The @path is then the key that the hash table is filled with. The inner pair is turned into linked list (hash table in a hash table would be awful really). Moreover, we need to store yet another item: reference counter. While virtlockd holds internal state in a JSON file between, this feature is honored too. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/locking/lock_daemon.c | 375 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 361 insertions(+), 14 deletions(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index b39a63a..66fbe03 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -58,12 +58,30 @@ VIR_LOG_INIT("locking.lock_daemon"); #define VIR_LOCK_DAEMON_NUM_LOCKSPACES 3 +#define VIR_LOCK_DAEMON_NUM_SECLABELS 8 struct _virLockDaemon { virMutex lock; virNetServerPtr srv; virHashTablePtr lockspaces; virLockSpacePtr defaultLockspace; + virHashTablePtr seclabels; +}; + +typedef struct _virSeclabel virSeclabel; +typedef virSeclabel *virSeclabelPtr; +struct _virSeclabel { + char *model; + char *seclabel; + unsigned int refcount; +}; + +typedef struct _virSeclabelList virSeclabelList; +typedef virSeclabelList *virSeclabelListPtr; +struct _virSeclabelList { + virSeclabelPtr item; + virSeclabelListPtr prev; + virSeclabelListPtr next; }; virLockDaemonPtr lockDaemon = NULL; @@ -121,6 +139,7 @@ virLockDaemonFree(virLockDaemonPtr lockd) virObjectUnref(lockd->srv); virHashFree(lockd->lockspaces); virLockSpaceFree(lockd->defaultLockspace); + virHashFree(lockd->seclabels); VIR_FREE(lockd); } @@ -132,6 +151,178 @@ static void virLockDaemonLockSpaceDataFree(void *data, virLockSpaceFree(data); } + +static void +virSeclabelFree(virSeclabelPtr label) +{ + if (!label) + return; + + VIR_FREE(label->model); + VIR_FREE(label->seclabel); + VIR_FREE(label); +} + + +static void +virSeclabelListFree(virSeclabelListPtr list) +{ + while (list) { + virSeclabelListPtr tmp = list->next; + virSeclabelFree(list->item); + VIR_FREE(list); + list = tmp; + } +} + + +static virSeclabelListPtr +virSeclabelListFind(virSeclabelListPtr list, + const char *model) +{ + while (list && list->item) { + if (STREQ(list->item->model, model)) + return list; + list = list->next; + } + return NULL; +} + + +static virSeclabelListPtr +virSeclabelListAdd(virSeclabelListPtr list, + const char *model, + const char *seclabel) +{ + virSeclabelPtr tmp = NULL; + virSeclabelListPtr item = NULL; + virSeclabelListPtr ret = NULL; + + if (!list) + return NULL; + + while (list && list->next) { + if (STREQ(list->item->model, model)) + return list; + list = list->next; + } + + /* The model does not exist yet. Append at the end of the list */ + if (VIR_ALLOC(tmp) < 0 || + VIR_STRDUP(tmp->model, model) < 0 || + VIR_STRDUP(tmp->seclabel, seclabel) < 0) + goto cleanup; + + if (!list->item) { + /* Handle special case */ + list->item = tmp; + item = list; + } else { + if (VIR_ALLOC(item) < 0) + goto cleanup; + + item->item = tmp; + list->next = item; + item->prev = list; + } + + ret = item; + item = NULL; + tmp = NULL; + + cleanup: + virSeclabelListFree(item); + virSeclabelFree(tmp); + return ret; +} + + +static void +virLockDaemonSeclabelDataFree(void *data, + const void *key ATTRIBUTE_UNUSED) +{ + virSeclabelListFree(data); +} + + +static virJSONValuePtr +virSeclabelPreExecRestart(virSeclabelListPtr list) +{ + virJSONValuePtr object = virJSONValueNewArray(); + + if (!object) + return NULL; + + while (list) { + virSeclabelPtr seclabel = list->item; + virJSONValuePtr child = virJSONValueNewObject(); + + if (!child || + virJSONValueArrayAppend(object, child) < 0) { + virJSONValueFree(child); + goto error; + } + + if (virJSONValueObjectAppendString(child, "model", seclabel->model) < 0 || + virJSONValueObjectAppendString(child, "seclabel", seclabel->seclabel) < 0 || + virJSONValueObjectAppendNumberUint(child, "refcount", seclabel->refcount) < 0) + goto error; + + list = list->next; + } + + return object; + + error: + virJSONValueFree(object); + return NULL; +} + + +static virSeclabelListPtr +virSeclabelPostExecRestart(virJSONValuePtr object) +{ + virSeclabelListPtr list; + ssize_t i, n; + + if (VIR_ALLOC(list) < 0) + goto error; + + if ((n = virJSONValueArraySize(object)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed seclabel list")); + goto error; + } + + for (i = 0; i < n; i++) { + virJSONValuePtr child = virJSONValueArrayGet(object, i); + const char *model; + const char *seclabel; + unsigned int refcount; + virSeclabelListPtr item; + + if (!(model = virJSONValueObjectGetString(child, "model")) || + !(seclabel = virJSONValueObjectGetString(child, "seclabel")) || + virJSONValueObjectGetNumberUint(child, "refcount", &refcount) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed seclabel list")); + goto error; + } + + if (!(item = virSeclabelListAdd(list, model, seclabel))) + goto error; + + item->item->refcount = refcount; + } + + return list; + + error: + virSeclabelListFree(list); + return NULL; +} + + static virLockDaemonPtr virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) { @@ -163,6 +354,10 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) if (!(lockd->defaultLockspace = virLockSpaceNew(NULL))) goto error; + if (!(lockd->seclabels = virHashCreate(VIR_LOCK_DAEMON_NUM_SECLABELS, + virLockDaemonSeclabelDataFree))) + goto error; + return lockd; error: @@ -176,7 +371,7 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) { virLockDaemonPtr lockd; virJSONValuePtr child; - virJSONValuePtr lockspaces; + virJSONValuePtr lockspaces, seclabels; size_t i; int n; @@ -194,6 +389,10 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) virLockDaemonLockSpaceDataFree))) goto error; + if (!(lockd->seclabels = virHashCreate(VIR_LOCK_DAEMON_NUM_SECLABELS, + virLockDaemonSeclabelDataFree))) + goto error; + if (!(child = virJSONValueObjectGet(object, "defaultLockspace"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing defaultLockspace data from JSON file")); @@ -231,6 +430,38 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) } } + if ((seclabels = virJSONValueObjectGet(object, "seclabels"))) { + /* Parse iff seclabels attribute is present */ + if ((n = virJSONValueArraySize(seclabels)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed seclabels data from JSON file")); + goto error; + } + + for (i = 0; i < n; i++) { + virSeclabelListPtr list; + virJSONValuePtr seclabel_list; + const char *path; + + child = virJSONValueArrayGet(seclabels, i); + + if (!(path = virJSONValueObjectGetString(child, "path")) || + !(seclabel_list = virJSONValueObjectGet(child, "seclabel"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed seclabels data from JSON file")); + goto error; + } + + if (!(list = virSeclabelPostExecRestart(seclabel_list))) + goto error; + + if (virHashAddEntry(lockd->seclabels, path, list) < 0) { + virSeclabelListFree(list); + goto error; + } + } + } + if (!(child = virJSONValueObjectGet(object, "server"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing server data from JSON file")); @@ -279,24 +510,109 @@ virLockSpacePtr virLockDaemonFindLockSpace(virLockDaemonPtr lockd, int -virLockDaemonRememberSeclabel(virLockDaemonPtr lockd ATTRIBUTE_UNUSED, - const char *path ATTRIBUTE_UNUSED, - const char *model ATTRIBUTE_UNUSED, - const char *seclabel ATTRIBUTE_UNUSED) +virLockDaemonRememberSeclabel(virLockDaemonPtr lockd, + const char *path, + const char *model, + const char *seclabel) { - /* Implement me */ - return -1; + int ret = -1; + virSeclabelListPtr list = NULL; + virSeclabelListPtr tmp = NULL; + bool list_created = false; + + virMutexLock(&lockd->lock); + + if (!(list = virHashLookup(lockd->seclabels, path))) { + if (VIR_ALLOC(list) < 0) + goto cleanup; + + if (virHashAddEntry(lockd->seclabels, path, list) < 0) + goto cleanup; + + list_created = true; + } + + tmp = list; + if (!(tmp = virSeclabelListFind(list, model))) { + /* seclabel for @model doesn't exit yet */ + if (!(tmp = virSeclabelListAdd(list, model, seclabel))) + goto cleanup; + } + + /* Update refcount */ + ret = ++tmp->item->refcount; + + cleanup: + virMutexUnlock(&lockd->lock); + if (ret < 0 && list_created) { + virHashRemoveEntry(lockd->seclabels, path); + virSeclabelListFree(list); + } + return ret; } int -virLockDaemonRecallSeclabel(virLockDaemonPtr lockd ATTRIBUTE_UNUSED, - const char *path ATTRIBUTE_UNUSED, - const char *model ATTRIBUTE_UNUSED, - char **seclabel ATTRIBUTE_UNUSED) +virLockDaemonRecallSeclabel(virLockDaemonPtr lockd, + const char *path, + const char *model, + char **seclabel) { - /* Implement me */ - return -1; + int ret = -1; + virSeclabelListPtr list = NULL; + virSeclabelPtr label = NULL; + + virMutexLock(&lockd->lock); + + if (!(list = virHashLookup(lockd->seclabels, path))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to find path '%s' in seclabels hashtable"), + path); + goto cleanup; + } + + if (!(list = virSeclabelListFind(list, model))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to find entry for path " + "'%s' and security model '%s'"), + path, model); + goto cleanup; + } + + label = list->item; + + if (seclabel && VIR_STRDUP(*seclabel, label->seclabel) < 0) + goto cleanup; + + /* Now decrement refcount */ + if (!label->refcount) { + /* Huh funny. This should never happen (TM) */ + VIR_WARN("path '%s' had no refcounts", path); + } else { + label->refcount--; + } + + if (!(ret = label->refcount)) { + /* Okay, this is the last reference, adjust the linked list of + * seclabels */ + if (list->prev) + list->prev->next = list->next; + if (list->next) + list->next->prev = list->prev; + + if (!list->next && !list->prev) { + /* The last seclabel for @path, remove it. */ + virHashRemoveEntry(lockd->seclabels, path); + } else { + /* There's another seclabel for @path, remove only this item. */ + list->next = list->prev = NULL; + virSeclabelListFree(list); + } + } + + cleanup: + virMutexUnlock(&lockd->lock); + return ret; } @@ -1034,7 +1350,7 @@ virLockDaemonPreExecRestart(const char *state_file, virJSONValuePtr object; char *magic; virHashKeyValuePairPtr pairs = NULL, tmp; - virJSONValuePtr lockspaces; + virJSONValuePtr lockspaces, seclabels; VIR_DEBUG("Running pre-restart exec"); @@ -1080,6 +1396,37 @@ virLockDaemonPreExecRestart(const char *state_file, tmp++; } + if (!(seclabels = virJSONValueNewArray()) || + virJSONValueObjectAppend(object, "seclabels", seclabels) < 0) { + virJSONValueFree(seclabels); + goto cleanup; + } + + VIR_FREE(pairs); + tmp = pairs = virHashGetItems(lockDaemon->seclabels, NULL); + while (tmp && tmp->key) { + virSeclabelListPtr list = (virSeclabelListPtr)tmp->value; + virJSONValuePtr seclabel_list; + + if (!(seclabel_list = virSeclabelPreExecRestart(list))) + goto cleanup; + + if (!(child = virJSONValueNewObject()) || + virJSONValueObjectAppendString(child, "path", tmp->key) < 0 || + virJSONValueObjectAppend(child, "seclabel", seclabel_list) < 0) { + virJSONValueFree(seclabel_list); + virJSONValueFree(child); + goto cleanup; + } + + if (virJSONValueArrayAppend(seclabels, child) < 0) { + virJSONValueFree(child); + goto cleanup; + } + + tmp++; + } + if (!(magic = virLockDaemonGetExecRestartMagic())) goto cleanup; -- 1.8.5.5

For future work it's crucial to have virLockManagerPluginPtr stored in virSecurityDriver. Therefore, we must pass it when creating the security driver. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/Makefile.am | 6 +++++- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_driver.c | 3 ++- src/qemu/qemu_driver.c | 7 +++++-- src/security/security_manager.c | 25 ++++++++++++++++++++----- src/security/security_manager.h | 6 +++++- tests/Makefile.am | 1 + tests/qemuhotplugtest.c | 2 +- tests/seclabeltest.c | 2 +- tests/securityselinuxlabeltest.c | 2 +- tests/securityselinuxtest.c | 2 +- 11 files changed, 43 insertions(+), 15 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 7302abb..90a51f6 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1264,6 +1264,7 @@ libvirt_driver_qemu_impl_la_CFLAGS = \ $(LIBNL_CFLAGS) \ -I$(top_srcdir)/src/access \ -I$(top_srcdir)/src/conf \ + -I$(top_srcdir)/src/locking \ $(AM_CFLAGS) libvirt_driver_qemu_impl_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_qemu_impl_la_LIBADD = $(CAPNG_LIBS) \ @@ -1303,6 +1304,7 @@ libvirt_driver_lxc_impl_la_CFLAGS = \ $(FUSE_CFLAGS) \ -I$(top_srcdir)/src/access \ -I$(top_srcdir)/src/conf \ + -I$(top_srcdir)/src/locking \ $(AM_CFLAGS) libvirt_driver_lxc_impl_la_LIBADD = $(CAPNG_LIBS) $(LIBNL_LIBS) $(FUSE_LIBS) if WITH_BLKID @@ -1650,7 +1652,7 @@ libvirt_security_manager_la_SOURCES = $(SECURITY_DRIVER_SOURCES) noinst_LTLIBRARIES += libvirt_security_manager.la libvirt_la_BUILT_LIBADD += libvirt_security_manager.la libvirt_security_manager_la_CFLAGS = \ - -I$(top_srcdir)/src/conf $(AM_CFLAGS) + -I$(top_srcdir)/src/conf -I$(top_srcdir)/src/locking $(AM_CFLAGS) libvirt_security_manager_la_LDFLAGS = $(AM_LDFLAGS) libvirt_security_manager_la_LIBADD = $(SECDRIVER_LIBS) if WITH_SECDRIVER_SELINUX @@ -2636,6 +2638,7 @@ endif WITH_DTRACE_PROBES libvirt_lxc_LDADD += $(SECDRIVER_LIBS) libvirt_lxc_CFLAGS = \ -I$(top_srcdir)/src/conf \ + -I$(top_srcdir)/src/locking \ $(AM_CFLAGS) \ $(PIE_CFLAGS) \ $(LIBNL_CFLAGS) \ @@ -2672,6 +2675,7 @@ virt_aa_helper_LDADD += libvirt_probes.lo endif WITH_DTRACE_PROBES virt_aa_helper_CFLAGS = \ -I$(top_srcdir)/src/conf \ + -I$(top_srcdir)/src/locking \ -I$(top_srcdir)/src/security \ $(AM_CFLAGS) \ $(PIE_CFLAGS) \ diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 1861dd6..eb9dea0 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -2414,7 +2414,7 @@ int main(int argc, char *argv[]) if (!(ctrl->securityManager = virSecurityManagerNew(securityDriver, LXC_DRIVER_NAME, - false, false, false))) + false, false, false, NULL))) goto cleanup; if (ctrl->def->seclabels) { diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index f93360f..ccb4de9 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1539,7 +1539,8 @@ lxcSecurityInit(virLXCDriverConfigPtr cfg) LXC_DRIVER_NAME, false, cfg->securityDefaultConfined, - cfg->securityRequireConfined); + cfg->securityRequireConfined, + NULL); if (!mgr) goto error; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a8cda43..eecdb7b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -392,7 +392,8 @@ qemuSecurityInit(virQEMUDriverPtr driver) QEMU_DRIVER_NAME, cfg->allowDiskFormatProbing, cfg->securityDefaultConfined, - cfg->securityRequireConfined))) + cfg->securityRequireConfined, + driver->lockManager))) goto error; if (!stack) { if (!(stack = virSecurityManagerNewStack(mgr))) @@ -409,7 +410,8 @@ qemuSecurityInit(virQEMUDriverPtr driver) QEMU_DRIVER_NAME, cfg->allowDiskFormatProbing, cfg->securityDefaultConfined, - cfg->securityRequireConfined))) + cfg->securityRequireConfined, + driver->lockManager))) goto error; if (!(stack = virSecurityManagerNewStack(mgr))) goto error; @@ -424,6 +426,7 @@ qemuSecurityInit(virQEMUDriverPtr driver) cfg->securityDefaultConfined, cfg->securityRequireConfined, cfg->dynamicOwnership, + driver->lockManager, qemuSecurityChownCallback))) goto error; if (!stack) { diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 8671620..bbfbfef 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -41,6 +41,7 @@ struct _virSecurityManager { bool defaultConfined; bool requireConfined; const char *virtDriver; + virLockManagerPluginPtr lockPlugin; void *privateData; }; @@ -78,7 +79,8 @@ virSecurityManagerNewDriver(virSecurityDriverPtr drv, const char *virtDriver, bool allowDiskFormatProbing, bool defaultConfined, - bool requireConfined) + bool requireConfined, + virLockManagerPluginPtr lockPlugin) { virSecurityManagerPtr mgr; char *privateData; @@ -105,6 +107,7 @@ virSecurityManagerNewDriver(virSecurityDriverPtr drv, mgr->defaultConfined = defaultConfined; mgr->requireConfined = requireConfined; mgr->virtDriver = virtDriver; + mgr->lockPlugin = lockPlugin; mgr->privateData = privateData; if (drv->open(mgr) < 0) { @@ -124,7 +127,8 @@ virSecurityManagerNewStack(virSecurityManagerPtr primary) virSecurityManagerGetDriver(primary), virSecurityManagerGetAllowDiskFormatProbing(primary), virSecurityManagerGetDefaultConfined(primary), - virSecurityManagerGetRequireConfined(primary)); + virSecurityManagerGetRequireConfined(primary), + virSecurityManagerGetLockPlugin(primary)); if (!mgr) return NULL; @@ -153,6 +157,7 @@ virSecurityManagerNewDAC(const char *virtDriver, bool defaultConfined, bool requireConfined, bool dynamicOwnership, + virLockManagerPluginPtr lockPlugin, virSecurityManagerDACChownCallback chownCallback) { virSecurityManagerPtr mgr = @@ -160,7 +165,8 @@ virSecurityManagerNewDAC(const char *virtDriver, virtDriver, allowDiskFormatProbing, defaultConfined, - requireConfined); + requireConfined, + lockPlugin); if (!mgr) return NULL; @@ -182,7 +188,8 @@ virSecurityManagerNew(const char *name, const char *virtDriver, bool allowDiskFormatProbing, bool defaultConfined, - bool requireConfined) + bool requireConfined, + virLockManagerPluginPtr lockPlugin) { virSecurityDriverPtr drv = virSecurityDriverLookup(name, virtDriver); if (!drv) @@ -212,7 +219,8 @@ virSecurityManagerNew(const char *name, virtDriver, allowDiskFormatProbing, defaultConfined, - requireConfined); + requireConfined, + lockPlugin); } @@ -333,6 +341,13 @@ virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr) } +virLockManagerPluginPtr +virSecurityManagerGetLockPlugin(virSecurityManagerPtr mgr) +{ + return mgr->lockPlugin; +} + + /** * virSecurityManagerRestoreDiskLabel: * @mgr: security manager object diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 156f882..0605996 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -26,6 +26,7 @@ # include "domain_conf.h" # include "vircommand.h" # include "virstoragefile.h" +# include "lock_manager.h" typedef struct _virSecurityManager virSecurityManager; typedef virSecurityManager *virSecurityManagerPtr; @@ -34,7 +35,8 @@ virSecurityManagerPtr virSecurityManagerNew(const char *name, const char *virtDriver, bool allowDiskFormatProbing, bool defaultConfined, - bool requireConfined); + bool requireConfined, + virLockManagerPluginPtr lockPlugin); virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary); int virSecurityManagerStackAddNested(virSecurityManagerPtr stack, @@ -62,6 +64,7 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, bool defaultConfined, bool requireConfined, bool dynamicOwnership, + virLockManagerPluginPtr lockPlugin, virSecurityManagerDACChownCallback chownCallback); int virSecurityManagerPreFork(virSecurityManagerPtr mgr); @@ -77,6 +80,7 @@ const char *virSecurityManagerGetBaseLabel(virSecurityManagerPtr mgr, int virtTy bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr); bool virSecurityManagerGetDefaultConfined(virSecurityManagerPtr mgr); bool virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr); +virLockManagerPluginPtr virSecurityManagerGetLockPlugin(virSecurityManagerPtr mgr); int virSecurityManagerRestoreDiskLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, diff --git a/tests/Makefile.am b/tests/Makefile.am index d6c3cfb..a11b164 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -29,6 +29,7 @@ INCLUDES = \ -I$(top_builddir)/src -I$(top_srcdir)/src \ -I$(top_srcdir)/src/util \ -I$(top_srcdir)/src/conf \ + -I$(top_srcdir)/src/locking \ $(GETTEXT_CPPFLAGS) AM_CFLAGS = \ diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 9d39968..56ff3ba 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -358,7 +358,7 @@ mymain(void) if (!driver.lockManager) return EXIT_FAILURE; - if (!(mgr = virSecurityManagerNew("none", "qemu", false, false, false))) + if (!(mgr = virSecurityManagerNew("none", "qemu", false, false, false, NULL))) return EXIT_FAILURE; if (!(driver.securityManager = virSecurityManagerNewStack(mgr))) return EXIT_FAILURE; diff --git a/tests/seclabeltest.c b/tests/seclabeltest.c index 51765c9..3a6e7a2 100644 --- a/tests/seclabeltest.c +++ b/tests/seclabeltest.c @@ -17,7 +17,7 @@ main(int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) if (virThreadInitialize() < 0) return EXIT_FAILURE; - mgr = virSecurityManagerNew(NULL, "QEMU", false, true, false); + mgr = virSecurityManagerNew(NULL, "QEMU", false, true, false, NULL); if (mgr == NULL) { fprintf(stderr, "Failed to start security driver"); return EXIT_FAILURE; diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index 455eb74..dfc3bed 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -360,7 +360,7 @@ mymain(void) if (!rc) return EXIT_AM_SKIP; - if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false))) { + if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false, NULL))) { virErrorPtr err = virGetLastError(); fprintf(stderr, "Unable to initialize security driver: %s\n", err->message); diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c index 3b5c3e5..d0810d8 100644 --- a/tests/securityselinuxtest.c +++ b/tests/securityselinuxtest.c @@ -272,7 +272,7 @@ mymain(void) int ret = 0; virSecurityManagerPtr mgr; - if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false))) { + if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false, NULL))) { virErrorPtr err = virGetLastError(); fprintf(stderr, "Unable to initialize security driver: %s\n", err->message); -- 1.8.5.5