[libvirt] [PATCH v1 00/10] Keep original security label
I know I've sent several versions like ages ago, so this should not start with v1, but hey, this is completely new approach, so I'm gonna start from 1. Here, the virtlockd is misused to hold the original seclabels (although only DAC label is implemented so far). Even more, it does a reference counting, so that only the last label restore does the job, not the previous ones. Michal Privoznik (10): locking: Allow seclabel remembering locking: Implement seclabel stubs for NOP domain_lock: Introduce seclabel APIs locking: Add virLockSeclabelProtocol driver_lockd: Implement seclabel APIs lock_daemon: Implement server dispatch lock_daemon: Implement seclabel APIs security_dac: Cleanup virSecurityDACSetOwnershipInternal usage virSecurityManagerNew: Add virLockManagerPluginPtr security_dac: Keep original label .gitignore | 2 + src/Makefile.am | 34 ++- src/libvirt_private.syms | 4 + src/lock_seclabel_protocol-structs | 21 ++ src/locking/domain_lock.c | 65 ++++++ src/locking/domain_lock.h | 10 + src/locking/lock_daemon.c | 388 ++++++++++++++++++++++++++++++++++- src/locking/lock_daemon.h | 8 + src/locking/lock_daemon_dispatch.c | 77 +++++++ src/locking/lock_daemon_dispatch.h | 3 + src/locking/lock_driver.h | 43 ++++ src/locking/lock_driver_lockd.c | 118 ++++++++++- src/locking/lock_driver_nop.c | 22 ++ src/locking/lock_manager.c | 26 +++ src/locking/lock_manager.h | 9 + src/locking/lock_seclabel_protocol.x | 53 +++++ src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_driver.c | 3 +- src/qemu/qemu_driver.c | 7 +- src/security/security_dac.c | 145 ++++++++++--- 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 +- 27 files changed, 1028 insertions(+), 52 deletions(-) create mode 100644 src/lock_seclabel_protocol-structs create mode 100644 src/locking/lock_seclabel_protocol.x -- 1.8.5.5
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@redhat.com> --- 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:07PM +0200, Michal Privoznik wrote:
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@redhat.com> --- 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/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);
Can add ATTRIBUTE_NONNULL for all of the args in these methods. ACK if that's changed. 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 Wed, Sep 10, 2014 at 03:26:07PM +0200, Michal Privoznik wrote:
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.
+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);
I thin kwe should do *seclabel = NULL; to protect against drivers forgetting todo it
+ + return lock->driver->drvRecall(lock, path, model, seclabel); +}
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 :|
The NOP drivers needs implementation, although the function bodies are empty. This is because the virLockManager APIs check for driver implementation and returns an error if there's none. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/locking/lock_driver_nop.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/locking/lock_driver_nop.c b/src/locking/lock_driver_nop.c index 93a988e..edb450b 100644 --- a/src/locking/lock_driver_nop.c +++ b/src/locking/lock_driver_nop.c @@ -101,6 +101,25 @@ static void virLockManagerNopFree(virLockManagerPtr lock ATTRIBUTE_UNUSED) { } +static int +virLockManagerNopRememberSeclabel(virLockManagerPtr lock ATTRIBUTE_UNUSED, + const char *path ATTRIBUTE_UNUSED, + const char *model ATTRIBUTE_UNUSED, + const char *seclabel ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int +virLockManagerNopRecallSeclabel(virLockManagerPtr lock ATTRIBUTE_UNUSED, + const char *path ATTRIBUTE_UNUSED, + const char *model ATTRIBUTE_UNUSED, + char **seclabel ATTRIBUTE_UNUSED) +{ + /* A positive return value means no seclabel restoring. */ + return 1; +} + virLockDriver virLockDriverNop = { .version = VIR_LOCK_MANAGER_VERSION, @@ -118,4 +137,7 @@ virLockDriver virLockDriverNop = .drvRelease = virLockManagerNopRelease, .drvInquire = virLockManagerNopInquire, + + .drvRemember = virLockManagerNopRememberSeclabel, + .drvRecall = virLockManagerNopRecallSeclabel, }; -- 1.8.5.5
On Wed, Sep 10, 2014 at 03:26:08PM +0200, Michal Privoznik wrote:
The NOP drivers needs implementation, although the function bodies are empty. This is because the virLockManager APIs check for driver implementation and returns an error if there's none.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/locking/lock_driver_nop.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
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 :|
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 2 ++ src/locking/domain_lock.c | 65 +++++++++++++++++++++++++++++++++++++++++ src/locking/domain_lock.h | 10 +++++++ src/locking/lock_driver.h | 2 ++ src/locking/lock_driver_lockd.c | 4 +++ 5 files changed, 83 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cdc476a..db65aa5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -874,6 +874,8 @@ virDomainLockProcessInquire; virDomainLockProcessPause; virDomainLockProcessResume; virDomainLockProcessStart; +virDomainLockRecallSeclabel; +virDomainLockRememberSeclabel; # locking/lock_manager.h diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c index d7b681e..7de56b3 100644 --- a/src/locking/domain_lock.c +++ b/src/locking/domain_lock.c @@ -164,6 +164,30 @@ static virLockManagerPtr virDomainLockManagerNew(virLockManagerPluginPtr plugin, } +static virLockManagerPtr +virDomainLockManagerSeclabelNew(virLockManagerPluginPtr plugin) +{ + virLockManagerPtr lock; + virLockManagerParam params[] = { + /* nada */ + }; + VIR_DEBUG("plugin=%p", plugin); + + if (!(lock = virLockManagerNew(virLockManagerPluginGetDriver(plugin), + VIR_LOCK_MANAGER_OBJECT_TYPE_SECLABEL, + ARRAY_CARDINALITY(params), + params, + 0))) + goto error; + + return lock; + + error: + virLockManagerFree(lock); + return NULL; +} + + int virDomainLockProcessStart(virLockManagerPluginPtr plugin, const char *uri, virDomainObjPtr dom, @@ -378,3 +402,44 @@ int virDomainLockLeaseDetach(virLockManagerPluginPtr plugin, return ret; } + + +int virDomainLockRememberSeclabel(virLockManagerPluginPtr plugin, + const char *path, + const char *model, + const char *seclabel) +{ + virLockManagerPtr lock; + int ret = -1; + + VIR_DEBUG("plugin=%p path=%s model=%s seclabel=%s", + plugin, path, model, seclabel); + + if (!(lock = virDomainLockManagerSeclabelNew(plugin))) + return ret; + + ret = virLockManagerRememberSeclabel(lock, path, model, seclabel); + + virLockManagerFree(lock); + return ret; +} + +int virDomainLockRecallSeclabel(virLockManagerPluginPtr plugin, + const char *path, + const char *model, + char **seclabel) +{ + virLockManagerPtr lock; + int ret = -1; + + VIR_DEBUG("plugin=%p path=%s model=%s seclabel=%p", + plugin, path, model, seclabel); + + if (!(lock = virDomainLockManagerSeclabelNew(plugin))) + return ret; + + ret = virLockManagerRecallSeclabel(lock, path, model, seclabel); + + virLockManagerFree(lock); + return ret; +} diff --git a/src/locking/domain_lock.h b/src/locking/domain_lock.h index fb49102..7a4b119 100644 --- a/src/locking/domain_lock.h +++ b/src/locking/domain_lock.h @@ -66,4 +66,14 @@ int virDomainLockLeaseDetach(virLockManagerPluginPtr plugin, virDomainObjPtr dom, virDomainLeaseDefPtr lease); +int virDomainLockRememberSeclabel(virLockManagerPluginPtr plugin, + const char *path, + const char *model, + const char *seclabel); + +int virDomainLockRecallSeclabel(virLockManagerPluginPtr plugin, + const char *path, + const char *model, + char **seclabel); + #endif /* __VIR_DOMAIN_LOCK_H__ */ diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h index a82f240..0dde48c 100644 --- a/src/locking/lock_driver.h +++ b/src/locking/lock_driver.h @@ -42,6 +42,8 @@ typedef enum { typedef enum { /* The managed object is a virtual guest domain */ VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN = 0, + /* The managed object is a seclabel */ + VIR_LOCK_MANAGER_OBJECT_TYPE_SECLABEL = 1, } virLockManagerObjectType; typedef enum { diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 0a40e94..4bb5925 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -487,6 +487,10 @@ static int virLockManagerLockDaemonNew(virLockManagerPtr lock, } break; + case VIR_LOCK_MANAGER_OBJECT_TYPE_SECLABEL: + /* No parameters yet */ + break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown lock manager object type %d"), -- 1.8.5.5
On Wed, Sep 10, 2014 at 03:26:09PM +0200, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 2 ++ src/locking/domain_lock.c | 65 +++++++++++++++++++++++++++++++++++++++++ src/locking/domain_lock.h | 10 +++++++ src/locking/lock_driver.h | 2 ++ src/locking/lock_driver_lockd.c | 4 +++ 5 files changed, 83 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cdc476a..db65aa5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -874,6 +874,8 @@ virDomainLockProcessInquire; virDomainLockProcessPause; virDomainLockProcessResume; virDomainLockProcessStart; +virDomainLockRecallSeclabel; +virDomainLockRememberSeclabel;
# locking/lock_manager.h diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c index d7b681e..7de56b3 100644 --- a/src/locking/domain_lock.c +++ b/src/locking/domain_lock.c @@ -164,6 +164,30 @@ static virLockManagerPtr virDomainLockManagerNew(virLockManagerPluginPtr plugin, }
+static virLockManagerPtr +virDomainLockManagerSeclabelNew(virLockManagerPluginPtr plugin) +{ + virLockManagerPtr lock; + virLockManagerParam params[] = { + /* nada */ + };
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.
+ VIR_DEBUG("plugin=%p", plugin); + + if (!(lock = virLockManagerNew(virLockManagerPluginGetDriver(plugin), + VIR_LOCK_MANAGER_OBJECT_TYPE_SECLABEL, + ARRAY_CARDINALITY(params), + params, + 0))) + goto error; + + return lock; + + error: + virLockManagerFree(lock); + return NULL; +}
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 :|
So far no ConnectOpen() is introduced as it's not needed for such simple use case like this. It's crucial to separate this from virLockSpace program that already exists. Not only it requires virDomainObjPtr for its ConnectOpen() (subsequently all security drivers would need rework as they use virDomainDefPtr), but from nature of things it doesn't belong there either. virLockSpace handles disk locking, not labeling and it's not clean to pollute its namespace anyway. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- .gitignore | 2 ++ src/Makefile.am | 27 ++++++++++++++---- src/lock_seclabel_protocol-structs | 21 ++++++++++++++ src/locking/lock_seclabel_protocol.x | 53 ++++++++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 src/lock_seclabel_protocol-structs create mode 100644 src/locking/lock_seclabel_protocol.x diff --git a/.gitignore b/.gitignore index 9776ea1..f75ec19 100644 --- a/.gitignore +++ b/.gitignore @@ -123,7 +123,9 @@ /src/libvirt_*probes.h /src/libvirt_lxc /src/locking/lock_daemon_dispatch_stubs.h +/src/locking/lock_daemon_seclabel_dispatch_stubs.h /src/locking/lock_protocol.[ch] +/src/locking/lock_seclabel_protocol.[ch] /src/locking/qemu-lockd.conf /src/locking/qemu-sanlock.conf /src/locking/test_libvirt_sanlock.aug diff --git a/src/Makefile.am b/src/Makefile.am index fa741a8..7302abb 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -202,16 +202,21 @@ LOCK_DRIVER_SANLOCK_HELPER_SOURCES = \ LOCK_PROTOCOL_GENERATED = \ locking/lock_protocol.h \ locking/lock_protocol.c \ + locking/lock_seclabel_protocol.h \ + locking/lock_seclabel_protocol.c \ $(NULL) LOCK_PROTOCOL = $(srcdir)/locking/lock_protocol.x +LOCK_SECLABEL_PROTOCOL = $(srcdir)/locking/lock_seclabel_protocol.x EXTRA_DIST += $(LOCK_PROTOCOL) \ - $(LOCK_PROTOCOL_GENERATED) + $(LOCK_SECLABEL_PROTOCOL) \ + $(LOCK_PROTOCOL_GENERATED) BUILT_SOURCES += $(LOCK_PROTOCOL_GENERATED) MAINTAINERCLEANFILES += $(LOCK_PROTOCOL_GENERATED) LOCK_DAEMON_GENERATED = \ - locking/lock_daemon_dispatch_stubs.h + locking/lock_daemon_dispatch_stubs.h \ + locking/lock_daemon_seclabel_dispatch_stubs.h $(NULL) BUILT_SOURCES += $(LOCK_DAEMON_GENERATED) @@ -237,6 +242,11 @@ locking/lock_daemon_dispatch_stubs.h: $(LOCK_PROTOCOL) \ virLockSpaceProtocol VIR_LOCK_SPACE_PROTOCOL \ $(LOCK_PROTOCOL) > $(srcdir)/locking/lock_daemon_dispatch_stubs.h +locking/lock_daemon_seclabel_dispatch_stubs.h: $(LOCK_SECLABEL_PROTOCOL) \ + $(srcdir)/rpc/gendispatch.pl Makefile.am + $(AM_V_GEN)perl -w $(srcdir)/rpc/gendispatch.pl --mode=server \ + virLockSeclabelProtocol VIR_LOCK_SECLABEL_PROTOCOL \ + $(LOCK_SECLABEL_PROTOCOL) > $(srcdir)/$@ NETDEV_CONF_SOURCES = \ conf/netdev_bandwidth_conf.h conf/netdev_bandwidth_conf.c \ @@ -387,7 +397,8 @@ EXTRA_DIST += $(REMOTE_DRIVER_PROTOCOL) \ # The alternation of the following regexps matches both cases. r1 = /\* \d+ \*/ r2 = /\* <[[:xdigit:]]+> \S+:\d+ \*/ -struct_prefix = (remote_|qemu_|lxc_|keepalive|vir(Net|LockSpace|LXCMonitor)) +struct_prefix1 = (remote_|qemu_|lxc_|keepalive) +struct_prefix2 = vir(Net|LockSpace|LockSeclabel|LXCMonitor) # Depending on configure options, libtool creates one or both of # remote/{,.libs/}libvirt_driver_remote_la-remote_protocol.o. We want @@ -406,7 +417,8 @@ PDWTAGS = \ else \ $(PERL) -0777 -n \ -e 'foreach my $$p (split m!\n*(?:$(r1)|$(r2))\n!) {' \ - -e ' if ($$p =~ /^(struct|enum) $(struct_prefix)/ ||' \ + -e ' if ($$p =~ /^(struct|enum) $(struct_prefix1)/ ||' \ + -e ' $$P =~ /^(struct|enum) $(struct_prefix2)/ ||' \ -e ' $$p =~ /^enum {/) {' \ -e ' $$p =~ s!\t*/\*.*?\*/!!sg;' \ -e ' $$p =~ s!\s+\n!\n!sg;' \ @@ -459,6 +471,7 @@ PROTOCOL_STRUCTS = \ $(srcdir)/virkeepaliveprotocol-structs \ $(srcdir)/lxc_monitor_protocol-structs \ $(srcdir)/lock_protocol-structs \ + $(srcdir)/lock_seclabel_protocol-structs \ $(NULL) if WITH_REMOTE @@ -480,6 +493,9 @@ $(srcdir)/lxc_monitor_protocol-struct: \ $(srcdir)/lock_protocol-struct: \ $(srcdir)/%-struct: locking/lockd_la-%.lo $(PDWTAGS) +$(srcdir)/lock_seclabel_protocol-struct: \ + $(srcdir)/%-struct: locking/lockd_la-%.lo + $(PDWTAGS) else !WITH_REMOTE # The $(PROTOCOL_STRUCTS) files must live in git, because they cannot be @@ -2073,7 +2089,8 @@ RPC_PROBE_FILES = $(srcdir)/rpc/virnetprotocol.x \ $(srcdir)/remote/lxc_protocol.x \ $(srcdir)/remote/qemu_protocol.x \ $(srcdir)/lxc/lxc_monitor_protocol.x \ - $(srcdir)/locking/lock_protocol.x + $(srcdir)/locking/lock_protocol.x \ + $(srcdir)/locking/lock_seclabel_protocol.x libvirt_functions.stp: $(RPC_PROBE_FILES) $(srcdir)/rpc/gensystemtap.pl $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gensystemtap.pl $(RPC_PROBE_FILES) > $@ diff --git a/src/lock_seclabel_protocol-structs b/src/lock_seclabel_protocol-structs new file mode 100644 index 0000000..46f1eae --- /dev/null +++ b/src/lock_seclabel_protocol-structs @@ -0,0 +1,21 @@ +/* -*- c -*- */ +struct virLockSeclabelProtocolRememberSeclabelArgs { + virLockSeclabelProtocolNonNullString path; + virLockSeclabelProtocolNonNullString model; + virLockSeclabelProtocolNonNullString seclabel; +}; +struct virLockSeclabelProtocolRememberSeclabelRet { + int ret; +}; +struct virLockSeclabelProtocolRecallSeclabelArgs { + virLockSeclabelProtocolNonNullString path; + virLockSeclabelProtocolNonNullString model; +}; +struct virLockSeclabelProtocolRecallSeclabelRet { + virLockSeclabelProtocolNonNullString seclabel; + int ret; +}; +enum virLockSeclabelProtocolProcedure { + VIR_LOCK_SECLABEL_PROTOCOL_PROC_REMEMBER_SECLABEL = 1, + VIR_LOCK_SECLABEL_PROTOCOL_PROC_RECALL_SECLABEL = 2, +}; diff --git a/src/locking/lock_seclabel_protocol.x b/src/locking/lock_seclabel_protocol.x new file mode 100644 index 0000000..e769ebf --- /dev/null +++ b/src/locking/lock_seclabel_protocol.x @@ -0,0 +1,53 @@ +/* -*- c -*- + */ + +%#include "internal.h" + +/* Length of long, but not unbounded, strings. + * This is an arbitrary limit designed to stop the decoder from trying + * to allocate unbounded amounts of memory when fed with a bad message. + */ +const VIR_LOCK_SECLABEL_PROTOCOL_STRING_MAX = 65536; + +/* A long string, which may NOT be NULL. */ +typedef string virLockSeclabelProtocolNonNullString<VIR_LOCK_SECLABEL_PROTOCOL_STRING_MAX>; + +/* A long string, which may be NULL. */ +typedef virLockSeclabelProtocolNonNullString *virLockSeclabelProtocolString; + +struct virLockSeclabelProtocolRememberSeclabelArgs { + virLockSeclabelProtocolNonNullString path; + virLockSeclabelProtocolNonNullString model; + virLockSeclabelProtocolNonNullString seclabel; +}; + +struct virLockSeclabelProtocolRememberSeclabelRet { + int ret; +}; + +struct virLockSeclabelProtocolRecallSeclabelArgs { + virLockSeclabelProtocolNonNullString path; + virLockSeclabelProtocolNonNullString model; +}; + +struct virLockSeclabelProtocolRecallSeclabelRet { + virLockSeclabelProtocolNonNullString seclabel; + int ret; +}; + +const VIR_LOCK_SECLABEL_PROTOCOL_PROGRAM = 0x5EC1ABE1; +const VIR_LOCK_SECLABEL_PROTOCOL_PROGRAM_VERSION = 1; + +enum virLockSeclabelProtocolProcedure { + /** + * @generate: none + * @acl: none + */ + VIR_LOCK_SECLABEL_PROTOCOL_PROC_REMEMBER_SECLABEL = 1, + + /** + * @generate: none + * @acl: none + */ + VIR_LOCK_SECLABEL_PROTOCOL_PROC_RECALL_SECLABEL = 2 +}; -- 1.8.5.5
On 09/10/2014 03:26 PM, Michal Privoznik wrote:
So far no ConnectOpen() is introduced as it's not needed for such simple use case like this. It's crucial to separate this from virLockSpace program that already exists. Not only it requires virDomainObjPtr for its ConnectOpen() (subsequently all security drivers would need rework as they use virDomainDefPtr), but from nature of things it doesn't belong there either. virLockSpace handles disk locking, not labeling and it's not clean to pollute its namespace anyway.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- .gitignore | 2 ++ src/Makefile.am | 27 ++++++++++++++---- src/lock_seclabel_protocol-structs | 21 ++++++++++++++ src/locking/lock_seclabel_protocol.x | 53 ++++++++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 src/lock_seclabel_protocol-structs create mode 100644 src/locking/lock_seclabel_protocol.x
@@ -387,7 +397,8 @@ EXTRA_DIST += $(REMOTE_DRIVER_PROTOCOL) \ # The alternation of the following regexps matches both cases. r1 = /\* \d+ \*/ r2 = /\* <[[:xdigit:]]+> \S+:\d+ \*/ -struct_prefix = (remote_|qemu_|lxc_|keepalive|vir(Net|LockSpace|LXCMonitor)) +struct_prefix1 = (remote_|qemu_|lxc_|keepalive) +struct_prefix2 = vir(Net|LockSpace|LockSeclabel|LXCMonitor)
# Depending on configure options, libtool creates one or both of # remote/{,.libs/}libvirt_driver_remote_la-remote_protocol.o. We want @@ -406,7 +417,8 @@ PDWTAGS = \ else \ $(PERL) -0777 -n \ -e 'foreach my $$p (split m!\n*(?:$(r1)|$(r2))\n!) {' \ - -e ' if ($$p =~ /^(struct|enum) $(struct_prefix)/ ||' \ + -e ' if ($$p =~ /^(struct|enum) $(struct_prefix1)/ ||' \ + -e ' $$P =~ /^(struct|enum) $(struct_prefix2)/ ||' \
This p should be lowercase. Jan
On Wed, Sep 10, 2014 at 03:26:10PM +0200, Michal Privoznik wrote:
So far no ConnectOpen() is introduced as it's not needed for such simple use case like this. It's crucial to separate this from virLockSpace program that already exists. Not only it requires virDomainObjPtr for its ConnectOpen() (subsequently all security drivers would need rework as they use virDomainDefPtr), but from nature of things it doesn't belong there either. virLockSpace handles disk locking, not labeling and it's not clean to pollute its namespace anyway.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- .gitignore | 2 ++ src/Makefile.am | 27 ++++++++++++++---- src/lock_seclabel_protocol-structs | 21 ++++++++++++++ src/locking/lock_seclabel_protocol.x | 53 ++++++++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 src/lock_seclabel_protocol-structs create mode 100644 src/locking/lock_seclabel_protocol.x
diff --git a/src/lock_seclabel_protocol-structs b/src/lock_seclabel_protocol-structs new file mode 100644 index 0000000..46f1eae --- /dev/null +++ b/src/lock_seclabel_protocol-structs @@ -0,0 +1,21 @@ +/* -*- c -*- */ +struct virLockSeclabelProtocolRememberSeclabelArgs { + virLockSeclabelProtocolNonNullString path; + virLockSeclabelProtocolNonNullString model; + virLockSeclabelProtocolNonNullString seclabel; +}; +struct virLockSeclabelProtocolRememberSeclabelRet { + int ret;
What are the values of the 'ret' variable. Generally the RPC methods deal with error status at the protocol header level ?
+}; +struct virLockSeclabelProtocolRecallSeclabelArgs { + virLockSeclabelProtocolNonNullString path; + virLockSeclabelProtocolNonNullString model; +}; +struct virLockSeclabelProtocolRecallSeclabelRet { + virLockSeclabelProtocolNonNullString seclabel; + int ret; +}; +enum virLockSeclabelProtocolProcedure { + VIR_LOCK_SECLABEL_PROTOCOL_PROC_REMEMBER_SECLABEL = 1, + VIR_LOCK_SECLABEL_PROTOCOL_PROC_RECALL_SECLABEL = 2, +};
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 :|
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@redhat.com> --- 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
On Wed, Sep 10, 2014 at 03:26:11PM +0200, Michal Privoznik wrote:
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@redhat.com> --- src/locking/lock_driver_lockd.c | 114 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 109 insertions(+), 5 deletions(-)
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 :|
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@redhat.com> --- 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
On Wed, Sep 10, 2014 at 03:26:12PM +0200, Michal Privoznik wrote:
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@redhat.com> --- 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(+)
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 :|
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@redhat.com> --- 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
On Wed, Sep 10, 2014 at 03:26:13PM +0200, Michal Privoznik wrote:
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.
I wonder, since 'model' is mandatory and (path, model) should be unique, could we not avoid the linked list complexity by using a string of 'model:path' as the hash key, then we only have a single hash item too and O(1) access to values ?
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; }
What concerns me a little is that we don't have any association between the label, use count and the running VMs that triggered the need for the relabelling. Lets say that a virtualization host with VMs running crashes. The libvirtd and virtlocked on that host will loose all state. That's fine currently since you're not sharing the data across hosts. We're going to need to share the data though, so when we do that sharing, I think we're going to need to remember information about the VMs + hosts associated with the label, so that when a host crashes, we can have a chance of purging the now stale information. Now, as discussed on IRC it isn't critical to support multi-host mode right away, but this does impact on the RPC protocol, since the messages we are passing do not include any information about the VM or host. So I'm thinking that this series should record the VM name + uuid and the host name + uuid against the seclabels and plumb that through the RPC protocol at least. If we did that now, I think we might not even need to add new RPC protocol for lockd, and instead just re-use the existing protocol messages and at least some of the client/dispatcher code too. It would be more like a case of just setting up a new lockspace for recording info about labels, instead of a completely new set of infrastructure side-by-side. 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 19.09.2014 18:13, Daniel P. Berrange wrote:
On Wed, Sep 10, 2014 at 03:26:13PM +0200, Michal Privoznik wrote:
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.
I wonder, since 'model' is mandatory and (path, model) should be unique, could we not avoid the linked list complexity by using a string of 'model:path' as the hash key, then we only have a single hash item too and O(1) access to values ?
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; }
What concerns me a little is that we don't have any association between the label, use count and the running VMs that triggered the need for the relabelling.
Lets say that a virtualization host with VMs running crashes. The libvirtd and virtlocked on that host will loose all state. That's fine currently since you're not sharing the data across hosts. We're going to need to share the data though, so when we do that sharing, I think we're going to need to remember information about the VMs + hosts associated with the label, so that when a host crashes, we can have a chance of purging the now stale information.
I see.
Now, as discussed on IRC it isn't critical to support multi-host mode right away, but this does impact on the RPC protocol, since the messages we are passing do not include any information about the VM or host.
So I'm thinking that this series should record the VM name + uuid and the host name + uuid against the seclabels and plumb that through the RPC protocol at least.
Right, this would be great.
If we did that now, I think we might not even need to add new RPC protocol for lockd, and instead just re-use the existing protocol messages and at least some of the client/dispatcher code too. It would be more like a case of just setting up a new lockspace for recording info about labels, instead of a completely new set of infrastructure side-by-side.
I was thinking about this prior to starting new implementation. The problem I had found myself in was that in secdrivers we use pure virDomainDefPtr but the VM's pid is stored in virDomainObjPtr. And I don't think we want to refactor all the code just to tunnel the vm->pid into function actually calling chown() or fsetfilecon_raw(). Or do we? Michal
We have nice wrappers over internal function that eventually calls the Internal() function, but for future work it's better to call those wrappers instead of the internal function directly. This is due to fact that the wrappers differentiate between setting or restoring the label while the internal function does not. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/security_dac.c | 48 ++++++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index e398d2c..7f69d86 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -305,21 +305,29 @@ virSecurityDACSetOwnershipInternal(virSecurityDACDataPtr priv, static int -virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid) +virSecurityDACSetOwnership(virSecurityManagerPtr mgr, + virStorageSourcePtr src, + const char *path, + uid_t uid, + gid_t gid) { - return virSecurityDACSetOwnershipInternal(NULL, NULL, path, uid, gid); + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + /* XXX record previous ownership */ + return virSecurityDACSetOwnershipInternal(priv, src, path, uid, gid); } static int -virSecurityDACRestoreSecurityFileLabelInternal(virSecurityDACDataPtr priv, +virSecurityDACRestoreSecurityFileLabelInternal(virSecurityManagerPtr mgr, virStorageSourcePtr src, const char *path) { + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); VIR_INFO("Restoring DAC user and group on '%s'", NULLSTR(src ? src->path : path)); - /* XXX record previous ownership */ + /* XXX recall previous ownership */ return virSecurityDACSetOwnershipInternal(priv, src, path, 0, 0); } @@ -362,7 +370,7 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr, return -1; } - return virSecurityDACSetOwnershipInternal(priv, src, NULL, user, group); + return virSecurityDACSetOwnership(mgr, src, NULL, user, group); } @@ -434,7 +442,7 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, } } - return virSecurityDACRestoreSecurityFileLabelInternal(priv, src, NULL); + return virSecurityDACRestoreSecurityFileLabelInternal(mgr, src, NULL); } @@ -470,7 +478,7 @@ virSecurityDACSetSecurityHostdevLabelHelper(const char *file, if (virSecurityDACGetIds(secdef, priv, &user, &group, NULL, NULL)) return -1; - return virSecurityDACSetOwnership(file, user, group); + return virSecurityDACSetOwnership(mgr, NULL, file, user, group); } @@ -762,7 +770,8 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, switch ((virDomainChrType) dev_source->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: - ret = virSecurityDACSetOwnership(dev_source->data.file.path, + ret = virSecurityDACSetOwnership(mgr, NULL, + dev_source->data.file.path, user, group); break; @@ -771,11 +780,14 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, (virAsprintf(&out, "%s.out", dev_source->data.file.path) < 0)) goto done; if (virFileExists(in) && virFileExists(out)) { - if ((virSecurityDACSetOwnership(in, user, group) < 0) || - (virSecurityDACSetOwnership(out, user, group) < 0)) { + if ((virSecurityDACSetOwnership(mgr, NULL, + in, user, group) < 0) || + (virSecurityDACSetOwnership(mgr, NULL, + out, user, group) < 0)) { goto done; } - } else if (virSecurityDACSetOwnership(dev_source->data.file.path, + } else if (virSecurityDACSetOwnership(mgr, NULL, + dev_source->data.file.path, user, group) < 0) { goto done; } @@ -1041,19 +1053,23 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, return -1; if (def->os.loader && def->os.loader->nvram && - virSecurityDACSetOwnership(def->os.loader->nvram, user, group) < 0) + virSecurityDACSetOwnership(mgr, NULL, + def->os.loader->nvram, user, group) < 0) return -1; if (def->os.kernel && - virSecurityDACSetOwnership(def->os.kernel, user, group) < 0) + virSecurityDACSetOwnership(mgr, NULL, + def->os.kernel, user, group) < 0) return -1; if (def->os.initrd && - virSecurityDACSetOwnership(def->os.initrd, user, group) < 0) + virSecurityDACSetOwnership(mgr, NULL, + def->os.initrd, user, group) < 0) return -1; if (def->os.dtb && - virSecurityDACSetOwnership(def->os.dtb, user, group) < 0) + virSecurityDACSetOwnership(mgr, NULL, + def->os.dtb, user, group) < 0) return -1; return 0; @@ -1075,7 +1091,7 @@ virSecurityDACSetSavedStateLabel(virSecurityManagerPtr mgr, if (virSecurityDACGetImageIds(secdef, priv, &user, &group) < 0) return -1; - return virSecurityDACSetOwnership(savefile, user, group); + return virSecurityDACSetOwnership(mgr, NULL, savefile, user, group); } -- 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@redhat.com> --- 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
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/Makefile.am | 1 + src/security/security_dac.c | 103 ++++++++++++++++++++++++++++++++++++++------ 2 files changed, 92 insertions(+), 12 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 90a51f6..83c8fcb 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2631,6 +2631,7 @@ libvirt_lxc_LDADD = \ libvirt_security_manager.la \ libvirt_conf.la \ libvirt_util.la \ + libvirt_driver.la \ ../gnulib/lib/libgnu.la if WITH_DTRACE_PROBES libvirt_lxc_LDADD += libvirt_probes.lo diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 7f69d86..2af013e 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -23,6 +23,7 @@ #include <sys/stat.h> #include <fcntl.h> +#include "domain_lock.h" #include "security_dac.h" #include "virerror.h" #include "virfile.h" @@ -312,9 +313,56 @@ virSecurityDACSetOwnership(virSecurityManagerPtr mgr, gid_t gid) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virLockManagerPluginPtr lockPlugin = virSecurityManagerGetLockPlugin(mgr); + int ret = -1; + bool call_chown = true; + bool recall_label = false; + struct stat sb; + char *original_label; - /* XXX record previous ownership */ - return virSecurityDACSetOwnershipInternal(priv, src, path, uid, gid); + if (src && virStorageSourceIsLocalStorage(src)) + path = src->path; + + if (path) { + if (stat(path, &sb) < 0) { + virReportSystemError(errno, + _("unable to stat: %p"), + path); + goto cleanup; + } + + if (lockPlugin) { + if (virAsprintf(&original_label, "+%d:%+d", sb.st_uid, sb.st_gid) < 0) + goto cleanup; + + VIR_DEBUG("Remembering label '%s' for '%s' model on path '%s'", + original_label, SECURITY_DAC_NAME, path); + + if (virDomainLockRememberSeclabel(lockPlugin, path, + SECURITY_DAC_NAME, + original_label) < 0) { + VIR_FREE(original_label); + goto cleanup; + } + + recall_label = true; + } + call_chown = sb.st_uid != uid || sb.st_gid != gid; + } + + if (call_chown && + virSecurityDACSetOwnershipInternal(priv, src, path, uid, gid) < 0) + goto cleanup; + + ret = 0; + cleanup: + if (ret < 0 && recall_label) { + /* Setting label wen't wrong, but we've remembered the label. + * Recall it so internal refcount stays in sync. */ + virDomainLockRecallSeclabel(lockPlugin, path, + SECURITY_DAC_NAME, NULL); + } + return ret; } @@ -324,11 +372,50 @@ virSecurityDACRestoreSecurityFileLabelInternal(virSecurityManagerPtr mgr, const char *path) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virLockManagerPluginPtr lockPlugin = virSecurityManagerGetLockPlugin(mgr); + int ret = -1; + bool call_chown = true; + uid_t uid = 0; /* By default return to root:root */ + gid_t gid = 0; + VIR_INFO("Restoring DAC user and group on '%s'", NULLSTR(src ? src->path : path)); - /* XXX recall previous ownership */ - return virSecurityDACSetOwnershipInternal(priv, src, path, 0, 0); + if (src && virStorageSourceIsLocalStorage(src)) + path = src->path; + + if (lockPlugin && path) { + int rc; + char *original_label = NULL; + + if ((rc = virDomainLockRecallSeclabel(lockPlugin, path, SECURITY_DAC_NAME, &original_label)) < 0) { + /* Intentionally just WARN here - the label may not exist yet */ + VIR_WARN("Unable to get original label for %s", path); + } else if (rc > 0) { + /* Label exists, but refcount is still greater than zero. Don't restore label yet. */ + call_chown = false; + } else { + /* Hooray, label exists and we have the honour to restore the original label. */ + if (virParseOwnershipIds(original_label, &uid, &gid) < 0) { + /* Error already reported by helper */ + VIR_FREE(original_label); + goto cleanup; + } + VIR_FREE(original_label); + } + } + + if (call_chown) { + VIR_DEBUG("Restoring to %u:%u", + (unsigned int) uid, (unsigned int) gid); + + if (virSecurityDACSetOwnershipInternal(priv, src, path, uid, gid) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + return ret; } @@ -404,14 +491,6 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, if (!priv->dynamicOwnership) return 0; - /* Don't restore labels on readoly/shared disks, because other VMs may - * still be accessing these. Alternatively we could iterate over all - * running domains and try to figure out if it is in use, but this would - * not work for clustered filesystems, since we can't see running VMs using - * the file on other nodes. Safest bet is thus to skip the restore step. */ - if (src->readonly || src->shared) - return 0; - secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); if (secdef && !secdef->relabel) return 0; -- 1.8.5.5
On Wed, Sep 10, 2014 at 03:26:06PM +0200, Michal Privoznik wrote:
I know I've sent several versions like ages ago, so this should not start with v1, but hey, this is completely new approach, so I'm gonna start from 1.
Here, the virtlockd is misused to hold the original seclabels (although only DAC label is implemented so far). Even more, it does a reference counting, so that only the last label restore does the job, not the previous ones.
Ah interesting approach. Do you have a pointer to your most recent posting of the previous approach for comparison. I remember seeing it before, but I'm being unlucky finding it in the archives right now. 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 11.09.2014 13:13, Daniel P. Berrange wrote:
On Wed, Sep 10, 2014 at 03:26:06PM +0200, Michal Privoznik wrote:
I know I've sent several versions like ages ago, so this should not start with v1, but hey, this is completely new approach, so I'm gonna start from 1.
Here, the virtlockd is misused to hold the original seclabels (although only DAC label is implemented so far). Even more, it does a reference counting, so that only the last label restore does the job, not the previous ones.
Ah interesting approach. Do you have a pointer to your most recent posting of the previous approach for comparison. I remember seeing it before, but I'm being unlucky finding it in the archives right now.
I believe this was my last approach: http://www.redhat.com/archives/libvir-list/2014-March/msg00826.html The idea there was to have a file to keep original labels and use virtlockd to ensure mutual exclusion of multiple daemons. But I must say stripping the file and moving it into virtlockd (approach presented in this patch set) looks better to me. Michal
On 10.09.2014 15:26, Michal Privoznik wrote:
I know I've sent several versions like ages ago, so this should not start with v1, but hey, this is completely new approach, so I'm gonna start from 1.
Here, the virtlockd is misused to hold the original seclabels (although only DAC label is implemented so far). Even more, it does a reference counting, so that only the last label restore does the job, not the previous ones.
Michal Privoznik (10): locking: Allow seclabel remembering locking: Implement seclabel stubs for NOP domain_lock: Introduce seclabel APIs locking: Add virLockSeclabelProtocol driver_lockd: Implement seclabel APIs lock_daemon: Implement server dispatch lock_daemon: Implement seclabel APIs security_dac: Cleanup virSecurityDACSetOwnershipInternal usage virSecurityManagerNew: Add virLockManagerPluginPtr security_dac: Keep original label
.gitignore | 2 + src/Makefile.am | 34 ++- src/libvirt_private.syms | 4 + src/lock_seclabel_protocol-structs | 21 ++ src/locking/domain_lock.c | 65 ++++++ src/locking/domain_lock.h | 10 + src/locking/lock_daemon.c | 388 ++++++++++++++++++++++++++++++++++- src/locking/lock_daemon.h | 8 + src/locking/lock_daemon_dispatch.c | 77 +++++++ src/locking/lock_daemon_dispatch.h | 3 + src/locking/lock_driver.h | 43 ++++ src/locking/lock_driver_lockd.c | 118 ++++++++++- src/locking/lock_driver_nop.c | 22 ++ src/locking/lock_manager.c | 26 +++ src/locking/lock_manager.h | 9 + src/locking/lock_seclabel_protocol.x | 53 +++++ src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_driver.c | 3 +- src/qemu/qemu_driver.c | 7 +- src/security/security_dac.c | 145 ++++++++++--- 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 +- 27 files changed, 1028 insertions(+), 52 deletions(-) create mode 100644 src/lock_seclabel_protocol-structs create mode 100644 src/locking/lock_seclabel_protocol.x
Ping? I'd really like to see this one in the release. Michal
participants (3)
-
Daniel P. Berrange -
Ján Tomko -
Michal Privoznik