[libvirt] [PATCH 00/12] Fine grained access control for libvirt APIs

This is a repost of https://www.redhat.com/archives/libvir-list/2012-January/msg00907.html which got no comments last time out. This series of patch is the minimal required to get a working proof of concept implementation of fine grained access control in libvirt. This demonstrates - Obtaining a client identity from a socket - Ensuring RPC calls are executed with the correct identity sset - A policykit access driver that checks based on access vector alone - A SELinux access driver that checks based on access vector + object - A set of hooks in the QEMU driver to protect virDomainObjPtr access Things that are not done - APIs for changing the real/effective identity post-connect - A simple RBAC access driver for doing (Access vector, object) checks - SELinux policy for the SELinux driver - Access control hooks on all other QEMU driver methods - Access control hooks in LXC, UML, other libvirtd side drivers - Access control hooks in storage, network, interface, etc drivers - Document WTF todo to propagate SELinux contexts across TCP sockets using IPSec. Any hints welcome... - Lots more I can't think of right now I should note that the policykit driver is mostly useless because it is unable to let you do checks on anything other than permission name and UNIX process ID at this time. So what I've implemented with the polkit driver is really little more than a slightly more fine grained version of the VIR_CONNECT_RO flag. In theory it is supposed to be extendable to allow other types of identity information besides the process ID, and to include some kind of object identiers in the permission check, but no one seems to be attacking this. So I expect the simple RBAC driver to be the most used one in the common case usage of libvirt, and of course the SELinux driver.

From: "Daniel P. Berrange" <berrange@redhat.com> Allow the logging APIs to be called with a va_list for format args, instead of requiring var-args usage. * src/util/logging.h, src/util/logging.c: Add virLogVMessage --- src/util/logging.c | 29 ++++++++++++++++++++++++----- src/util/logging.h | 5 +++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/util/logging.c b/src/util/logging.c index 48a056d..0007226 100644 --- a/src/util/logging.c +++ b/src/util/logging.c @@ -683,6 +683,29 @@ virLogVersionString(char **msg) void virLogMessage(const char *category, int priority, const char *funcname, long long linenr, unsigned int flags, const char *fmt, ...) { + va_list ap; + va_start(ap, fmt); + virLogVMessage(category, priority, funcname, linenr, flags, fmt, ap); + va_end(ap); +} + +/** + * virLogMessage: + * @category: where is that message coming from + * @priority: the priority level + * @funcname: the function emitting the (debug) message + * @linenr: line where the message was emitted + * @flags: extra flags, 1 if coming from the error handler + * @fmt: the string format + * @vargs: format args + * + * Call the libvirt logger with some information. Based on the configuration + * the message may be stored, sent to output or just discarded + */ +void virLogVMessage(const char *category, int priority, const char *funcname, + long long linenr, unsigned int flags, const char *fmt, + va_list vargs) +{ static bool logVersionStderr = true; char *str = NULL; char *msg = NULL; @@ -690,7 +713,6 @@ void virLogMessage(const char *category, int priority, const char *funcname, int fprio, i, ret; int saved_errno = errno; int emit = 1; - va_list ap; if (!virLogInitialized) virLogStartup(); @@ -715,12 +737,9 @@ void virLogMessage(const char *category, int priority, const char *funcname, /* * serialize the error message, add level and timestamp */ - va_start(ap, fmt); - if (virVasprintf(&str, fmt, ap) < 0) { - va_end(ap); + if (virVasprintf(&str, fmt, vargs) < 0) { goto cleanup; } - va_end(ap); ret = virLogFormatString(&msg, funcname, linenr, priority, str); VIR_FREE(str); diff --git a/src/util/logging.h b/src/util/logging.h index 2343de0..e1a8116 100644 --- a/src/util/logging.h +++ b/src/util/logging.h @@ -128,6 +128,11 @@ extern void virLogMessage(const char *category, int priority, const char *funcname, long long linenr, unsigned int flags, const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(6, 7); +extern void virLogVMessage(const char *category, int priority, + const char *funcname, long long linenr, + unsigned int flags, + const char *fmt, + va_list vargs) ATTRIBUTE_FMT_PRINTF(6, 0); extern int virLogSetBufferSize(int size); extern void virLogEmergencyDumpAll(int signum); #endif -- 1.7.10

On 02.05.2012 13:44, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Allow the logging APIs to be called with a va_list for format args, instead of requiring var-args usage.
* src/util/logging.h, src/util/logging.c: Add virLogVMessage --- src/util/logging.c | 29 ++++++++++++++++++++++++----- src/util/logging.h | 5 +++++ 2 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/src/util/logging.c b/src/util/logging.c index 48a056d..0007226 100644 --- a/src/util/logging.c +++ b/src/util/logging.c @@ -683,6 +683,29 @@ virLogVersionString(char **msg) void virLogMessage(const char *category, int priority, const char *funcname, long long linenr, unsigned int flags, const char *fmt, ...) { + va_list ap; + va_start(ap, fmt); + virLogVMessage(category, priority, funcname, linenr, flags, fmt, ap); + va_end(ap); +} + +/** + * virLogMessage: + * @category: where is that message coming from + * @priority: the priority level + * @funcname: the function emitting the (debug) message + * @linenr: line where the message was emitted + * @flags: extra flags, 1 if coming from the error handler + * @fmt: the string format + * @vargs: format args + * + * Call the libvirt logger with some information. Based on the configuration + * the message may be stored, sent to output or just discarded + */ +void virLogVMessage(const char *category, int priority, const char *funcname, + long long linenr, unsigned int flags, const char *fmt, + va_list vargs) +{ static bool logVersionStderr = true; char *str = NULL; char *msg = NULL; @@ -690,7 +713,6 @@ void virLogMessage(const char *category, int priority, const char *funcname, int fprio, i, ret; int saved_errno = errno; int emit = 1; - va_list ap;
if (!virLogInitialized) virLogStartup(); @@ -715,12 +737,9 @@ void virLogMessage(const char *category, int priority, const char *funcname, /* * serialize the error message, add level and timestamp */ - va_start(ap, fmt); - if (virVasprintf(&str, fmt, ap) < 0) { - va_end(ap); + if (virVasprintf(&str, fmt, vargs) < 0) { goto cleanup; } - va_end(ap);
ret = virLogFormatString(&msg, funcname, linenr, priority, str); VIR_FREE(str); diff --git a/src/util/logging.h b/src/util/logging.h index 2343de0..e1a8116 100644 --- a/src/util/logging.h +++ b/src/util/logging.h @@ -128,6 +128,11 @@ extern void virLogMessage(const char *category, int priority, const char *funcname, long long linenr, unsigned int flags, const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(6, 7); +extern void virLogVMessage(const char *category, int priority, + const char *funcname, long long linenr, + unsigned int flags, + const char *fmt, + va_list vargs) ATTRIBUTE_FMT_PRINTF(6, 0); extern int virLogSetBufferSize(int size); extern void virLogEmergencyDumpAll(int signum); #endif
ACK Michal

On 05/02/2012 09:22 AM, Michal Privoznik wrote:
On 02.05.2012 13:44, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Allow the logging APIs to be called with a va_list for format args, instead of requiring var-args usage.
* src/util/logging.h, src/util/logging.c: Add virLogVMessage --- src/util/logging.c | 29 ++++++++++++++++++++++++----- src/util/logging.h | 5 +++++ 2 files changed, 29 insertions(+), 5 deletions(-)
+ +/** + * virLogMessage:
typo, should be virLogVMessage -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange@redhat.com> * src/util/threads-pthread.c, src/util/threads.h: Add virThreadCancel --- src/util/threads-pthread.c | 5 +++++ src/util/threads.h | 1 + 2 files changed, 6 insertions(+) diff --git a/src/util/threads-pthread.c b/src/util/threads-pthread.c index ea64887..cfd0b24 100644 --- a/src/util/threads-pthread.c +++ b/src/util/threads-pthread.c @@ -236,6 +236,11 @@ void virThreadJoin(virThreadPtr thread) pthread_join(thread->thread, NULL); } +void virThreadCancel(virThreadPtr thread) +{ + pthread_cancel(thread->thread); +} + int virThreadLocalInit(virThreadLocalPtr l, virThreadLocalCleanup c) { diff --git a/src/util/threads.h b/src/util/threads.h index e5000ea..9c1ef99 100644 --- a/src/util/threads.h +++ b/src/util/threads.h @@ -53,6 +53,7 @@ int virThreadCreate(virThreadPtr thread, void virThreadSelf(virThreadPtr thread); bool virThreadIsSelf(virThreadPtr thread); void virThreadJoin(virThreadPtr thread); +void virThreadCancel(virThreadPtr thread); /* These next two functions are for debugging only, since they are not * guaranteed to give unique values for distinct threads on all -- 1.7.10

On 02.05.2012 13:44, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
* src/util/threads-pthread.c, src/util/threads.h: Add virThreadCancel --- src/util/threads-pthread.c | 5 +++++ src/util/threads.h | 1 + 2 files changed, 6 insertions(+)
diff --git a/src/util/threads-pthread.c b/src/util/threads-pthread.c index ea64887..cfd0b24 100644 --- a/src/util/threads-pthread.c +++ b/src/util/threads-pthread.c @@ -236,6 +236,11 @@ void virThreadJoin(virThreadPtr thread) pthread_join(thread->thread, NULL); }
+void virThreadCancel(virThreadPtr thread) +{ + pthread_cancel(thread->thread); +} + int virThreadLocalInit(virThreadLocalPtr l, virThreadLocalCleanup c) { diff --git a/src/util/threads.h b/src/util/threads.h index e5000ea..9c1ef99 100644 --- a/src/util/threads.h +++ b/src/util/threads.h @@ -53,6 +53,7 @@ int virThreadCreate(virThreadPtr thread, void virThreadSelf(virThreadPtr thread); bool virThreadIsSelf(virThreadPtr thread); void virThreadJoin(virThreadPtr thread); +void virThreadCancel(virThreadPtr thread);
/* These next two functions are for debugging only, since they are not * guaranteed to give unique values for distinct threads on all
Okay, if we are brave enough. Although, win32 implementation is missing. Michal

On Wed, May 02, 2012 at 05:22:53PM +0200, Michal Privoznik wrote:
On 02.05.2012 13:44, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
* src/util/threads-pthread.c, src/util/threads.h: Add virThreadCancel --- src/util/threads-pthread.c | 5 +++++ src/util/threads.h | 1 + 2 files changed, 6 insertions(+)
diff --git a/src/util/threads-pthread.c b/src/util/threads-pthread.c index ea64887..cfd0b24 100644 --- a/src/util/threads-pthread.c +++ b/src/util/threads-pthread.c @@ -236,6 +236,11 @@ void virThreadJoin(virThreadPtr thread) pthread_join(thread->thread, NULL); }
+void virThreadCancel(virThreadPtr thread) +{ + pthread_cancel(thread->thread); +} + int virThreadLocalInit(virThreadLocalPtr l, virThreadLocalCleanup c) { diff --git a/src/util/threads.h b/src/util/threads.h index e5000ea..9c1ef99 100644 --- a/src/util/threads.h +++ b/src/util/threads.h @@ -53,6 +53,7 @@ int virThreadCreate(virThreadPtr thread, void virThreadSelf(virThreadPtr thread); bool virThreadIsSelf(virThreadPtr thread); void virThreadJoin(virThreadPtr thread); +void virThreadCancel(virThreadPtr thread);
/* These next two functions are for debugging only, since they are not * guaranteed to give unique values for distinct threads on all
Okay, if we are brave enough. Although, win32 implementation is missing.
Yeah I hate it, but this is actually required by libselinux to stop the AVC thread. I'm assuming libselinux is written to avoid leaking memory upon cancellation. 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 :|

From: "Daniel P. Berrange" <berrange@redhat.com> Currently the server determines whether authentication of clients is complete, by checking whether an identity is set. This patch removes that lame hack and replaces it with an explicit method for changing the client auth code * daemon/remote.c: Update for new APis * src/libvirt_private.syms, src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h: Remove virNetServerClientGetIdentity and virNetServerClientSetIdentity, adding a new method virNetServerClientSetAuth. --- daemon/remote.c | 14 +++++++------- src/libvirt_private.syms | 2 +- src/rpc/virnetserverclient.c | 36 ++++++++---------------------------- src/rpc/virnetserverclient.h | 5 +---- 4 files changed, 17 insertions(+), 40 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 16a8a05..0bf58d3 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2137,10 +2137,12 @@ remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; } VIR_INFO("Bypass polkit auth for privileged client %s", ident); - if (virNetServerClientSetIdentity(client, ident) < 0) + if (virNetServerClientSetIdentity(client, ident) < 0) { virResetLastError(); - else + } else { + virNetServerClientSetAuth(client, 0); auth = VIR_NET_SERVER_SERVICE_AUTH_NONE; + } VIR_FREE(ident); } } @@ -2279,9 +2281,7 @@ remoteSASLFinish(virNetServerClientPtr client) if (!virNetSASLContextCheckIdentity(saslCtxt, identity)) return -2; - if (virNetServerClientSetIdentity(client, identity) < 0) - goto error; - + virNetServerClientSetAuth(client, 0); virNetServerClientSetSASLSession(client, priv->sasl); VIR_DEBUG("Authentication successful %d", virNetServerClientGetFD(client)); @@ -2613,7 +2613,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, action, (long long) callerPid, callerUid); ret->complete = 1; - virNetServerClientSetIdentity(client, ident); + virNetServerClientSetAuth(client, 0); virMutexUnlock(&priv->lock); virCommandFree(cmd); VIR_FREE(pkout); @@ -2767,8 +2767,8 @@ remoteDispatchAuthPolkit(virNetServerPtr server, action, (long long) callerPid, callerUid, polkit_result_to_string_representation(pkresult)); ret->complete = 1; - virNetServerClientSetIdentity(client, ident); + virNetServerClientSetAuth(client, 0); virMutexUnlock(&priv->lock); VIR_FREE(ident); return 0; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d4038b2..391c977 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1384,8 +1384,8 @@ virNetServerClientRef; virNetServerClientRemoteAddrString; virNetServerClientRemoveFilter; virNetServerClientSendMessage; +virNetServerClientSetAuth; virNetServerClientSetCloseHook; -virNetServerClientSetIdentity; virNetServerClientSetPrivateData; virNetServerClientStartKeepAlive; diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 67600fd..81dbb32 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -67,7 +67,6 @@ struct _virNetServerClient virNetSocketPtr sock; int auth; bool readonly; - char *identity; virNetTLSContextPtr tlsCtxt; virNetTLSSessionPtr tls; #if HAVE_SASL @@ -408,6 +407,13 @@ int virNetServerClientGetAuth(virNetServerClientPtr client) return auth; } +void virNetServerClientSetAuth(virNetServerClientPtr client, int auth) +{ + virNetServerClientLock(client); + client->auth = auth; + virNetServerClientUnlock(client); +} + bool virNetServerClientGetReadonly(virNetServerClientPtr client) { bool readonly; @@ -492,31 +498,6 @@ void virNetServerClientSetSASLSession(virNetServerClientPtr client, #endif -int virNetServerClientSetIdentity(virNetServerClientPtr client, - const char *identity) -{ - int ret = -1; - virNetServerClientLock(client); - if (!(client->identity = strdup(identity))) { - virReportOOMError(); - goto error; - } - ret = 0; - -error: - virNetServerClientUnlock(client); - return ret; -} - -const char *virNetServerClientGetIdentity(virNetServerClientPtr client) -{ - const char *identity; - virNetServerClientLock(client); - identity = client->identity; - virNetServerClientLock(client); - return identity; -} - void virNetServerClientSetPrivateData(virNetServerClientPtr client, void *opaque, virNetServerClientFreeFunc ff) @@ -600,7 +581,6 @@ void virNetServerClientFree(virNetServerClientPtr client) client->privateDataFreeFunc) client->privateDataFreeFunc(client->privateData); - VIR_FREE(client->identity); #if HAVE_SASL virNetSASLSessionFree(client->sasl); #endif @@ -1130,7 +1110,7 @@ bool virNetServerClientNeedAuth(virNetServerClientPtr client) { bool need = false; virNetServerClientLock(client); - if (client->auth && !client->identity) + if (client->auth) need = true; virNetServerClientUnlock(client); return need; diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 154a160..633e9e1 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -52,6 +52,7 @@ void virNetServerClientRemoveFilter(virNetServerClientPtr client, int filterID); int virNetServerClientGetAuth(virNetServerClientPtr client); +void virNetServerClientSetAuth(virNetServerClientPtr client, int auth); bool virNetServerClientGetReadonly(virNetServerClientPtr client); bool virNetServerClientHasTLSSession(virNetServerClientPtr client); @@ -66,10 +67,6 @@ int virNetServerClientGetFD(virNetServerClientPtr client); bool virNetServerClientIsSecure(virNetServerClientPtr client); -int virNetServerClientSetIdentity(virNetServerClientPtr client, - const char *identity); -const char *virNetServerClientGetIdentity(virNetServerClientPtr client); - int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, uid_t *uid, gid_t *gid, pid_t *pid); -- 1.7.10

On 02.05.2012 13:44, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Currently the server determines whether authentication of clients is complete, by checking whether an identity is set. This patch removes that lame hack and replaces it with an explicit method for changing the client auth code
* daemon/remote.c: Update for new APis * src/libvirt_private.syms, src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h: Remove virNetServerClientGetIdentity and virNetServerClientSetIdentity, adding a new method virNetServerClientSetAuth. --- daemon/remote.c | 14 +++++++------- src/libvirt_private.syms | 2 +- src/rpc/virnetserverclient.c | 36 ++++++++---------------------------- src/rpc/virnetserverclient.h | 5 +---- 4 files changed, 17 insertions(+), 40 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 16a8a05..0bf58d3 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2137,10 +2137,12 @@ remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; } VIR_INFO("Bypass polkit auth for privileged client %s", ident); - if (virNetServerClientSetIdentity(client, ident) < 0) + if (virNetServerClientSetIdentity(client, ident) < 0) { virResetLastError(); - else + } else { + virNetServerClientSetAuth(client, 0); auth = VIR_NET_SERVER_SERVICE_AUTH_NONE; + } VIR_FREE(ident); } } @@ -2279,9 +2281,7 @@ remoteSASLFinish(virNetServerClientPtr client) if (!virNetSASLContextCheckIdentity(saslCtxt, identity)) return -2;
- if (virNetServerClientSetIdentity(client, identity) < 0) - goto error; - + virNetServerClientSetAuth(client, 0); virNetServerClientSetSASLSession(client, priv->sasl);
VIR_DEBUG("Authentication successful %d", virNetServerClientGetFD(client)); @@ -2613,7 +2613,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, action, (long long) callerPid, callerUid); ret->complete = 1;
- virNetServerClientSetIdentity(client, ident); + virNetServerClientSetAuth(client, 0); virMutexUnlock(&priv->lock); virCommandFree(cmd); VIR_FREE(pkout); @@ -2767,8 +2767,8 @@ remoteDispatchAuthPolkit(virNetServerPtr server, action, (long long) callerPid, callerUid, polkit_result_to_string_representation(pkresult)); ret->complete = 1; - virNetServerClientSetIdentity(client, ident);
+ virNetServerClientSetAuth(client, 0); virMutexUnlock(&priv->lock); VIR_FREE(ident); return 0; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d4038b2..391c977 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1384,8 +1384,8 @@ virNetServerClientRef; virNetServerClientRemoteAddrString; virNetServerClientRemoveFilter; virNetServerClientSendMessage; +virNetServerClientSetAuth; virNetServerClientSetCloseHook; -virNetServerClientSetIdentity; virNetServerClientSetPrivateData; virNetServerClientStartKeepAlive;
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 67600fd..81dbb32 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -67,7 +67,6 @@ struct _virNetServerClient virNetSocketPtr sock; int auth; bool readonly; - char *identity; virNetTLSContextPtr tlsCtxt; virNetTLSSessionPtr tls; #if HAVE_SASL @@ -408,6 +407,13 @@ int virNetServerClientGetAuth(virNetServerClientPtr client) return auth; }
+void virNetServerClientSetAuth(virNetServerClientPtr client, int auth) +{ + virNetServerClientLock(client); + client->auth = auth; + virNetServerClientUnlock(client); +} + bool virNetServerClientGetReadonly(virNetServerClientPtr client) { bool readonly; @@ -492,31 +498,6 @@ void virNetServerClientSetSASLSession(virNetServerClientPtr client, #endif
-int virNetServerClientSetIdentity(virNetServerClientPtr client, - const char *identity) -{ - int ret = -1; - virNetServerClientLock(client); - if (!(client->identity = strdup(identity))) { - virReportOOMError(); - goto error; - } - ret = 0; - -error: - virNetServerClientUnlock(client); - return ret; -} - -const char *virNetServerClientGetIdentity(virNetServerClientPtr client) -{ - const char *identity; - virNetServerClientLock(client); - identity = client->identity; - virNetServerClientLock(client); - return identity; -} - void virNetServerClientSetPrivateData(virNetServerClientPtr client, void *opaque, virNetServerClientFreeFunc ff) @@ -600,7 +581,6 @@ void virNetServerClientFree(virNetServerClientPtr client) client->privateDataFreeFunc) client->privateDataFreeFunc(client->privateData);
- VIR_FREE(client->identity); #if HAVE_SASL virNetSASLSessionFree(client->sasl); #endif @@ -1130,7 +1110,7 @@ bool virNetServerClientNeedAuth(virNetServerClientPtr client) { bool need = false; virNetServerClientLock(client); - if (client->auth && !client->identity) + if (client->auth) need = true; virNetServerClientUnlock(client); return need; diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 154a160..633e9e1 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -52,6 +52,7 @@ void virNetServerClientRemoveFilter(virNetServerClientPtr client, int filterID);
int virNetServerClientGetAuth(virNetServerClientPtr client); +void virNetServerClientSetAuth(virNetServerClientPtr client, int auth); bool virNetServerClientGetReadonly(virNetServerClientPtr client);
bool virNetServerClientHasTLSSession(virNetServerClientPtr client); @@ -66,10 +67,6 @@ int virNetServerClientGetFD(virNetServerClientPtr client);
bool virNetServerClientIsSecure(virNetServerClientPtr client);
-int virNetServerClientSetIdentity(virNetServerClientPtr client, - const char *identity); -const char *virNetServerClientGetIdentity(virNetServerClientPtr client); - int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, uid_t *uid, gid_t *gid, pid_t *pid);
Okay, I see your point. However why are you removing virNetServerClientSetIdentity() if we are still using it (it could be seen even from patch context)? ACK to the idea, though.

On Wed, May 02, 2012 at 05:23:01PM +0200, Michal Privoznik wrote:
On 02.05.2012 13:44, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Currently the server determines whether authentication of clients is complete, by checking whether an identity is set. This patch removes that lame hack and replaces it with an explicit method for changing the client auth code
* daemon/remote.c: Update for new APis * src/libvirt_private.syms, src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h: Remove virNetServerClientGetIdentity and virNetServerClientSetIdentity, adding a new method virNetServerClientSetAuth. --- daemon/remote.c | 14 +++++++------- src/libvirt_private.syms | 2 +- src/rpc/virnetserverclient.c | 36 ++++++++---------------------------- src/rpc/virnetserverclient.h | 5 +---- 4 files changed, 17 insertions(+), 40 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 16a8a05..0bf58d3 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2137,10 +2137,12 @@ remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; } VIR_INFO("Bypass polkit auth for privileged client %s", ident); - if (virNetServerClientSetIdentity(client, ident) < 0) + if (virNetServerClientSetIdentity(client, ident) < 0) { virResetLastError(); - else + } else { + virNetServerClientSetAuth(client, 0); auth = VIR_NET_SERVER_SERVICE_AUTH_NONE; + } VIR_FREE(ident); } } @@ -2279,9 +2281,7 @@ remoteSASLFinish(virNetServerClientPtr client) if (!virNetSASLContextCheckIdentity(saslCtxt, identity)) return -2;
- if (virNetServerClientSetIdentity(client, identity) < 0) - goto error; - + virNetServerClientSetAuth(client, 0); virNetServerClientSetSASLSession(client, priv->sasl);
VIR_DEBUG("Authentication successful %d", virNetServerClientGetFD(client)); @@ -2613,7 +2613,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, action, (long long) callerPid, callerUid); ret->complete = 1;
- virNetServerClientSetIdentity(client, ident); + virNetServerClientSetAuth(client, 0); virMutexUnlock(&priv->lock); virCommandFree(cmd); VIR_FREE(pkout); @@ -2767,8 +2767,8 @@ remoteDispatchAuthPolkit(virNetServerPtr server, action, (long long) callerPid, callerUid, polkit_result_to_string_representation(pkresult)); ret->complete = 1; - virNetServerClientSetIdentity(client, ident);
+ virNetServerClientSetAuth(client, 0); virMutexUnlock(&priv->lock); VIR_FREE(ident); return 0; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d4038b2..391c977 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1384,8 +1384,8 @@ virNetServerClientRef; virNetServerClientRemoteAddrString; virNetServerClientRemoveFilter; virNetServerClientSendMessage; +virNetServerClientSetAuth; virNetServerClientSetCloseHook; -virNetServerClientSetIdentity; virNetServerClientSetPrivateData; virNetServerClientStartKeepAlive;
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 67600fd..81dbb32 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -67,7 +67,6 @@ struct _virNetServerClient virNetSocketPtr sock; int auth; bool readonly; - char *identity; virNetTLSContextPtr tlsCtxt; virNetTLSSessionPtr tls; #if HAVE_SASL @@ -408,6 +407,13 @@ int virNetServerClientGetAuth(virNetServerClientPtr client) return auth; }
+void virNetServerClientSetAuth(virNetServerClientPtr client, int auth) +{ + virNetServerClientLock(client); + client->auth = auth; + virNetServerClientUnlock(client); +} + bool virNetServerClientGetReadonly(virNetServerClientPtr client) { bool readonly; @@ -492,31 +498,6 @@ void virNetServerClientSetSASLSession(virNetServerClientPtr client, #endif
-int virNetServerClientSetIdentity(virNetServerClientPtr client, - const char *identity) -{ - int ret = -1; - virNetServerClientLock(client); - if (!(client->identity = strdup(identity))) { - virReportOOMError(); - goto error; - } - ret = 0; - -error: - virNetServerClientUnlock(client); - return ret; -} - -const char *virNetServerClientGetIdentity(virNetServerClientPtr client) -{ - const char *identity; - virNetServerClientLock(client); - identity = client->identity; - virNetServerClientLock(client); - return identity; -} - void virNetServerClientSetPrivateData(virNetServerClientPtr client, void *opaque, virNetServerClientFreeFunc ff) @@ -600,7 +581,6 @@ void virNetServerClientFree(virNetServerClientPtr client) client->privateDataFreeFunc) client->privateDataFreeFunc(client->privateData);
- VIR_FREE(client->identity); #if HAVE_SASL virNetSASLSessionFree(client->sasl); #endif @@ -1130,7 +1110,7 @@ bool virNetServerClientNeedAuth(virNetServerClientPtr client) { bool need = false; virNetServerClientLock(client); - if (client->auth && !client->identity) + if (client->auth) need = true; virNetServerClientUnlock(client); return need; diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 154a160..633e9e1 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -52,6 +52,7 @@ void virNetServerClientRemoveFilter(virNetServerClientPtr client, int filterID);
int virNetServerClientGetAuth(virNetServerClientPtr client); +void virNetServerClientSetAuth(virNetServerClientPtr client, int auth); bool virNetServerClientGetReadonly(virNetServerClientPtr client);
bool virNetServerClientHasTLSSession(virNetServerClientPtr client); @@ -66,10 +67,6 @@ int virNetServerClientGetFD(virNetServerClientPtr client);
bool virNetServerClientIsSecure(virNetServerClientPtr client);
-int virNetServerClientSetIdentity(virNetServerClientPtr client, - const char *identity); -const char *virNetServerClientGetIdentity(virNetServerClientPtr client); - int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, uid_t *uid, gid_t *gid, pid_t *pid);
Okay, I see your point. However why are you removing virNetServerClientSetIdentity() if we are still using it (it could be seen even from patch context)?
ACK to the idea, though.
This was a rebase mistake. The following extra chunk was lost: diff --git a/daemon/remote.c b/daemon/remote.c index 0bf58d3..df9053b 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2137,12 +2137,8 @@ remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; } VIR_INFO("Bypass polkit auth for privileged client %s", ident); - if (virNetServerClientSetIdentity(client, ident) < 0) { - virResetLastError(); - } else { - virNetServerClientSetAuth(client, 0); - auth = VIR_NET_SERVER_SERVICE_AUTH_NONE; - } + virNetServerClientSetAuth(client, 0); + auth = VIR_NET_SERVER_SERVICE_AUTH_NONE; VIR_FREE(ident); } } which removes the last use of this API 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 05/02/2012 07:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Currently the server determines whether authentication of clients is complete, by checking whether an identity is set. This patch removes that lame hack and replaces it with an explicit method for changing the client auth code
* daemon/remote.c: Update for new APis * src/libvirt_private.syms, src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h: Remove virNetServerClientGetIdentity and virNetServerClientSetIdentity, adding a new method virNetServerClientSetAuth. --- daemon/remote.c | 14 +++++++------- src/libvirt_private.syms | 2 +- src/rpc/virnetserverclient.c | 36 ++++++++---------------------------- src/rpc/virnetserverclient.h | 5 +---- 4 files changed, 17 insertions(+), 40 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 16a8a05..0bf58d3 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2137,10 +2137,12 @@ remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; } VIR_INFO("Bypass polkit auth for privileged client %s", ident); - if (virNetServerClientSetIdentity(client, ident) < 0) + if (virNetServerClientSetIdentity(client, ident) < 0) {
It looks like this call needs to be removed. -- Regards, Corey

From: "Daniel P. Berrange" <berrange@redhat.com> Add new APIs virNetServerClientGetTLSSession, virNetServerClientIsLocal, virNetServerClientGetSecurityContext virNetServerClientGetSASLSession, virNetSocketGetSecurityContext and virNetTLSSessionGetX509DName --- src/rpc/virnetserverclient.c | 48 ++++++++++++++++++++++++++++++++++++++++++ src/rpc/virnetserverclient.h | 7 ++++++ src/rpc/virnetsocket.c | 44 ++++++++++++++++++++++++++++++++++++++ src/rpc/virnetsocket.h | 2 ++ src/rpc/virnettlscontext.c | 18 ++++++++++++++++ src/rpc/virnettlscontext.h | 2 ++ 6 files changed, 121 insertions(+) diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 81dbb32..1e9d3db 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -433,6 +433,16 @@ bool virNetServerClientHasTLSSession(virNetServerClientPtr client) return has; } + +virNetTLSSessionPtr virNetServerClientGetTLSSession(virNetServerClientPtr client) +{ + virNetTLSSessionPtr tls; + virNetServerClientLock(client); + tls = client->tls; + virNetServerClientUnlock(client); + return tls; +} + int virNetServerClientGetTLSKeySize(virNetServerClientPtr client) { int size = 0; @@ -453,6 +463,18 @@ int virNetServerClientGetFD(virNetServerClientPtr client) return fd; } + +bool virNetServerClientIsLocal(virNetServerClientPtr client) +{ + bool local = false; + virNetServerClientLock(client); + if (client->sock) + local = virNetSocketIsLocal(client->sock); + virNetServerClientUnlock(client); + return local; +} + + int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, uid_t *uid, gid_t *gid, pid_t *pid) { @@ -464,6 +486,22 @@ int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, return ret; } + +int virNetServerClientGetSecurityContext(virNetServerClientPtr client, + char **context) +{ + int ret; + *context = NULL; + virNetServerClientLock(client); + if (client->sock) + ret = virNetSocketGetSecurityContext(client->sock, context); + else + ret = 0; + virNetServerClientUnlock(client); + return ret; +} + + bool virNetServerClientIsSecure(virNetServerClientPtr client) { bool secure = false; @@ -495,6 +533,16 @@ void virNetServerClientSetSASLSession(virNetServerClientPtr client, virNetSASLSessionRef(sasl); virNetServerClientUnlock(client); } + + +virNetSASLSessionPtr virNetServerClientGetSASLSession(virNetServerClientPtr client) +{ + virNetSASLSessionPtr sasl; + virNetServerClientLock(client); + sasl = client->sasl; + virNetServerClientUnlock(client); + return sasl; +} #endif diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 633e9e1..a3b37a3 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -56,20 +56,27 @@ void virNetServerClientSetAuth(virNetServerClientPtr client, int auth); bool virNetServerClientGetReadonly(virNetServerClientPtr client); bool virNetServerClientHasTLSSession(virNetServerClientPtr client); +virNetTLSSessionPtr virNetServerClientGetTLSSession(virNetServerClientPtr client); int virNetServerClientGetTLSKeySize(virNetServerClientPtr client); # ifdef HAVE_SASL void virNetServerClientSetSASLSession(virNetServerClientPtr client, virNetSASLSessionPtr sasl); +virNetSASLSessionPtr virNetServerClientGetSASLSession(virNetServerClientPtr client); # endif int virNetServerClientGetFD(virNetServerClientPtr client); bool virNetServerClientIsSecure(virNetServerClientPtr client); +bool virNetServerClientIsLocal(virNetServerClientPtr client); + int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, uid_t *uid, gid_t *gid, pid_t *pid); +int virNetServerClientGetSecurityContext(virNetServerClientPtr client, + char **context); + void virNetServerClientRef(virNetServerClientPtr client); typedef void (*virNetServerClientFreeFunc)(void *data); diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index fa16d31..da2d961 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -35,6 +35,10 @@ # include <netinet/tcp.h> #endif +#ifdef HAVE_SELINUX +# include <selinux/selinux.h> +#endif + #include "virnetsocket.h" #include "util.h" #include "memory.h" @@ -860,6 +864,46 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock ATTRIBUTE_UNUSED, } #endif +#ifdef HAVE_SELINUX +int virNetSocketGetSecurityContext(virNetSocketPtr sock, + char **context) +{ + security_context_t seccon = NULL; + int ret = -1; + + *context = NULL; + + virMutexLock(&sock->lock); + if (getpeercon(sock->fd, &seccon) < 0) { + if (errno == ENOSYS) { + ret = 0; + goto cleanup; + } + virReportSystemError(errno, "%s", + _("Unable to query peer security context")); + goto cleanup; + } + + if (!(*context = strdup(seccon))) { + virReportOOMError(); + goto cleanup; + } + + ret = 0; +cleanup: + freecon(seccon); + virMutexUnlock(&sock->lock); + return ret; +} +#else +int virNetSocketGetSecurityContext(virNetSocketPtr sock, + char **context) +{ + *context = NULL; + return 0; +} +#endif + int virNetSocketSetBlocking(virNetSocketPtr sock, bool blocking) diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index 5ba7c8f..de42e5c 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -90,6 +90,8 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock, uid_t *uid, gid_t *gid, pid_t *pid); +int virNetSocketGetSecurityContext(virNetSocketPtr sock, + char **context); int virNetSocketSetBlocking(virNetSocketPtr sock, bool blocking); diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 7440c7a..b9970d9 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -77,6 +77,7 @@ struct _virNetTLSSession { virNetTLSSessionWriteFunc writeFunc; virNetTLSSessionReadFunc readFunc; void *opaque; + char *x509dname; }; @@ -1025,6 +1026,10 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, "[session]", gnutls_strerror(ret)); goto authfail; } + if (!(sess->x509dname = strdup(dname))) { + virReportOOMError(); + goto authfail; + } VIR_DEBUG("Peer DN is %s", dname); if (virNetTLSContextCheckCertDN(cert, "[session]", sess->hostname, dname, @@ -1395,6 +1400,18 @@ cleanup: return ssf; } +const char *virNetTLSSessionGetX509DName(virNetTLSSessionPtr sess) +{ + const char *ret = NULL; + + virMutexLock(&sess->lock); + + ret = sess->x509dname; + + virMutexUnlock(&sess->lock); + + return ret; +} void virNetTLSSessionFree(virNetTLSSessionPtr sess) { @@ -1411,6 +1428,7 @@ void virNetTLSSessionFree(virNetTLSSessionPtr sess) return; } + VIR_FREE(sess->x509dname); VIR_FREE(sess->hostname); gnutls_deinit(sess->session); virMutexUnlock(&sess->lock); diff --git a/src/rpc/virnettlscontext.h b/src/rpc/virnettlscontext.h index fdfce6d..0c45cb0 100644 --- a/src/rpc/virnettlscontext.h +++ b/src/rpc/virnettlscontext.h @@ -99,6 +99,8 @@ virNetTLSSessionGetHandshakeStatus(virNetTLSSessionPtr sess); int virNetTLSSessionGetKeySize(virNetTLSSessionPtr sess); +const char *virNetTLSSessionGetX509DName(virNetTLSSessionPtr sess); + void virNetTLSSessionFree(virNetTLSSessionPtr sess); -- 1.7.10

On 02.05.2012 13:44, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add new APIs virNetServerClientGetTLSSession, virNetServerClientIsLocal, virNetServerClientGetSecurityContext virNetServerClientGetSASLSession, virNetSocketGetSecurityContext and virNetTLSSessionGetX509DName --- src/rpc/virnetserverclient.c | 48 ++++++++++++++++++++++++++++++++++++++++++ src/rpc/virnetserverclient.h | 7 ++++++ src/rpc/virnetsocket.c | 44 ++++++++++++++++++++++++++++++++++++++ src/rpc/virnetsocket.h | 2 ++ src/rpc/virnettlscontext.c | 18 ++++++++++++++++ src/rpc/virnettlscontext.h | 2 ++ 6 files changed, 121 insertions(+)
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 81dbb32..1e9d3db 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -433,6 +433,16 @@ bool virNetServerClientHasTLSSession(virNetServerClientPtr client) return has; }
+ +virNetTLSSessionPtr virNetServerClientGetTLSSession(virNetServerClientPtr client) +{ + virNetTLSSessionPtr tls; + virNetServerClientLock(client); + tls = client->tls; + virNetServerClientUnlock(client); + return tls; +} + int virNetServerClientGetTLSKeySize(virNetServerClientPtr client) { int size = 0; @@ -453,6 +463,18 @@ int virNetServerClientGetFD(virNetServerClientPtr client) return fd; }
+ +bool virNetServerClientIsLocal(virNetServerClientPtr client) +{ + bool local = false; + virNetServerClientLock(client); + if (client->sock) + local = virNetSocketIsLocal(client->sock); + virNetServerClientUnlock(client); + return local; +} + + int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, uid_t *uid, gid_t *gid, pid_t *pid) { @@ -464,6 +486,22 @@ int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, return ret; }
+ +int virNetServerClientGetSecurityContext(virNetServerClientPtr client, + char **context) +{ + int ret;
= 0;
+ *context = NULL; + virNetServerClientLock(client); + if (client->sock) + ret = virNetSocketGetSecurityContext(client->sock, context); + else + ret = 0;
leave out this else branch.
+ virNetServerClientUnlock(client); + return ret; +} + + bool virNetServerClientIsSecure(virNetServerClientPtr client) { bool secure = false; @@ -495,6 +533,16 @@ void virNetServerClientSetSASLSession(virNetServerClientPtr client, virNetSASLSessionRef(sasl); virNetServerClientUnlock(client); } + + +virNetSASLSessionPtr virNetServerClientGetSASLSession(virNetServerClientPtr client) +{ + virNetSASLSessionPtr sasl; + virNetServerClientLock(client); + sasl = client->sasl; + virNetServerClientUnlock(client); + return sasl; +} #endif
diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 633e9e1..a3b37a3 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -56,20 +56,27 @@ void virNetServerClientSetAuth(virNetServerClientPtr client, int auth); bool virNetServerClientGetReadonly(virNetServerClientPtr client);
bool virNetServerClientHasTLSSession(virNetServerClientPtr client); +virNetTLSSessionPtr virNetServerClientGetTLSSession(virNetServerClientPtr client); int virNetServerClientGetTLSKeySize(virNetServerClientPtr client);
# ifdef HAVE_SASL void virNetServerClientSetSASLSession(virNetServerClientPtr client, virNetSASLSessionPtr sasl); +virNetSASLSessionPtr virNetServerClientGetSASLSession(virNetServerClientPtr client); # endif
int virNetServerClientGetFD(virNetServerClientPtr client);
bool virNetServerClientIsSecure(virNetServerClientPtr client);
+bool virNetServerClientIsLocal(virNetServerClientPtr client); + int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, uid_t *uid, gid_t *gid, pid_t *pid);
+int virNetServerClientGetSecurityContext(virNetServerClientPtr client, + char **context); + void virNetServerClientRef(virNetServerClientPtr client);
typedef void (*virNetServerClientFreeFunc)(void *data); diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index fa16d31..da2d961 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -35,6 +35,10 @@ # include <netinet/tcp.h> #endif
+#ifdef HAVE_SELINUX +# include <selinux/selinux.h> +#endif + #include "virnetsocket.h" #include "util.h" #include "memory.h" @@ -860,6 +864,46 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock ATTRIBUTE_UNUSED, } #endif
+#ifdef HAVE_SELINUX +int virNetSocketGetSecurityContext(virNetSocketPtr sock, + char **context) +{ + security_context_t seccon = NULL; + int ret = -1; + + *context = NULL; + + virMutexLock(&sock->lock); + if (getpeercon(sock->fd, &seccon) < 0) { + if (errno == ENOSYS) { + ret = 0; + goto cleanup; + } + virReportSystemError(errno, "%s", + _("Unable to query peer security context")); + goto cleanup; + } + + if (!(*context = strdup(seccon))) { + virReportOOMError(); + goto cleanup; + } + + ret = 0; +cleanup: + freecon(seccon); + virMutexUnlock(&sock->lock); + return ret; +} +#else +int virNetSocketGetSecurityContext(virNetSocketPtr sock, + char **context) +{ + *context = NULL; + return 0; +} +#endif +
sock needs to be ATTRIBUTE_UNUSED here.
int virNetSocketSetBlocking(virNetSocketPtr sock, bool blocking) diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index 5ba7c8f..de42e5c 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -90,6 +90,8 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock, uid_t *uid, gid_t *gid, pid_t *pid); +int virNetSocketGetSecurityContext(virNetSocketPtr sock, + char **context);
int virNetSocketSetBlocking(virNetSocketPtr sock, bool blocking); diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 7440c7a..b9970d9 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -77,6 +77,7 @@ struct _virNetTLSSession { virNetTLSSessionWriteFunc writeFunc; virNetTLSSessionReadFunc readFunc; void *opaque; + char *x509dname; };
@@ -1025,6 +1026,10 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, "[session]", gnutls_strerror(ret)); goto authfail; } + if (!(sess->x509dname = strdup(dname))) { + virReportOOMError(); + goto authfail; + } VIR_DEBUG("Peer DN is %s", dname);
if (virNetTLSContextCheckCertDN(cert, "[session]", sess->hostname, dname, @@ -1395,6 +1400,18 @@ cleanup: return ssf; }
+const char *virNetTLSSessionGetX509DName(virNetTLSSessionPtr sess) +{ + const char *ret = NULL; + + virMutexLock(&sess->lock); + + ret = sess->x509dname; + + virMutexUnlock(&sess->lock); + + return ret; +}
void virNetTLSSessionFree(virNetTLSSessionPtr sess) { @@ -1411,6 +1428,7 @@ void virNetTLSSessionFree(virNetTLSSessionPtr sess) return; }
+ VIR_FREE(sess->x509dname); VIR_FREE(sess->hostname); gnutls_deinit(sess->session); virMutexUnlock(&sess->lock); diff --git a/src/rpc/virnettlscontext.h b/src/rpc/virnettlscontext.h index fdfce6d..0c45cb0 100644 --- a/src/rpc/virnettlscontext.h +++ b/src/rpc/virnettlscontext.h @@ -99,6 +99,8 @@ virNetTLSSessionGetHandshakeStatus(virNetTLSSessionPtr sess);
int virNetTLSSessionGetKeySize(virNetTLSSessionPtr sess);
+const char *virNetTLSSessionGetX509DName(virNetTLSSessionPtr sess); + void virNetTLSSessionFree(virNetTLSSessionPtr sess);
ACK Michal (I have to stop here. Will continue tomorrow.)

From: "Daniel P. Berrange" <berrange@redhat.com> --- include/libvirt/libvirt.h.in | 30 ++++++ include/libvirt/virterror.h | 1 + src/datatypes.h | 20 ++++ src/libvirt.c | 246 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 + src/util/virterror.c | 6 ++ 6 files changed, 308 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ac5df95..86a9514 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1088,6 +1088,36 @@ typedef virConnectAuth *virConnectAuthPtr; VIR_EXPORT_VAR virConnectAuthPtr virConnectAuthPtrDefault; +typedef struct _virIdentity virIdentity; +typedef virIdentity *virIdentityPtr; + +typedef enum { + VIR_IDENTITY_ATTR_UNIX_USER_NAME, + VIR_IDENTITY_ATTR_UNIX_GROUP_NAME, + VIR_IDENTITY_ATTR_UNIX_PROCESS_ID, + VIR_IDENTITY_ATTR_SASL_USER_NAME, + VIR_IDENTITY_ATTR_X509_DISTINGUISHED_NAME, + VIR_IDENTITY_ATTR_SECURITY_CONTEXT, + + VIR_IDENTITY_ATTR_LAST, +} virIdentityAttrType; + + +virIdentityPtr virIdentityNew(void); +int virIdentityRef(virIdentityPtr ident); +int virIdentitySetAttr(virIdentityPtr ident, + unsigned int attr, + const char *value); + +int virIdentityGetAttr(virIdentityPtr ident, + unsigned int attr, + const char **value); + +int virIdentityIsEqual(virIdentityPtr identA, + virIdentityPtr identB); + +int virIdentityFree(virIdentityPtr ident); + /** * VIR_UUID_BUFLEN: * diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 7283207..e44390e 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -251,6 +251,7 @@ typedef enum { VIR_ERR_MIGRATE_UNSAFE = 81, /* Migration is not safe */ VIR_ERR_OVERFLOW = 82, /* integer overflow */ VIR_ERR_BLOCK_COPY_ACTIVE = 83, /* action prevented by block copy job */ + VIR_ERR_INVALID_IDENTITY = 84, /* Invalid identity pointer */ } virErrorNumber; /** diff --git a/src/datatypes.h b/src/datatypes.h index fc284d2..b8d8db2 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -38,6 +38,16 @@ /** + * VIR_IDENTITY_MAGIC: + * + * magic value used to protect the API when pointers to identity structures + * are passed down by the users. + */ +# define VIR_IDENTITY_MAGIC 0xB33FCAF3 +# define VIR_IS_IDENTITY(obj) ((obj) && (obj)->magic==VIR_IDENTITY_MAGIC) + + +/** * VIR_DOMAIN_MAGIC: * * magic value used to protect the API when pointers to domain structures @@ -190,6 +200,16 @@ struct _virConnect { int refs; /* reference count */ }; + +struct _virIdentity { + unsigned int magic; + virMutex lock; + int refs; + + char *attrs[VIR_IDENTITY_ATTR_LAST]; +}; + + /** * _virDomain: * diff --git a/src/libvirt.c b/src/libvirt.c index cfd7711..a206800 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1544,6 +1544,252 @@ virConnectRef(virConnectPtr conn) return 0; } + +/** + * virIdentityNew: + * + * Creates a new empty identity object. After creating, one or + * more identifying attributes should be set on the identity. + * + * Returns: a new empty identity + */ +virIdentityPtr virIdentityNew(void) +{ + virIdentityPtr ident; + + if (VIR_ALLOC(ident) < 0) { + virReportOOMError(); + return NULL; + } + + if (virMutexInit(&ident->lock) < 0) { + virReportSystemError(errno, "%s", + _("Unable to initialize mutex")); + VIR_FREE(ident); + return NULL; + } + ident->magic = VIR_IDENTITY_MAGIC; + ident->refs = 1; + + return ident; +} + +/** + * virIdentityRef: + * @ident: the identity to hold a reference on + * + * Increment the reference count on the identity. For each + * additional call to this method, there shall be a corresponding + * call to virIdentityFree to release the reference count, once + * the caller no longer needs the reference to this object. + * + * This method is typically useful for applications where multiple + * threads are using an identity object, and it is required that + * the identity remain around until all threads have finished using + * it. ie, each new thread using a identity would increment + * the reference count. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int virIdentityRef(virIdentityPtr ident) +{ + if ((!VIR_IS_IDENTITY(ident))) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + virMutexLock(&ident->lock); + VIR_DEBUG("ident=%p refs=%d", ident, ident->refs); + ident->refs++; + virMutexUnlock(&ident->lock); + return 0; +} + + +/** + * virIdentitySetAttr: + * @ident: the identity to modify + * @attr: the attribute type to set + * @value: the identifying value to associate with @attr + * + * Sets an identifying attribute @attr on @ident. Each + * @attr type can only be set once. + * + * Returns: 0 on success, or -1 on error + */ +int virIdentitySetAttr(virIdentityPtr ident, + unsigned int attr, + const char *value) +{ + VIR_DEBUG("ident=%p attribute=%u value=%s", ident, attr, NULLSTR(value)); + + if ((!VIR_IS_IDENTITY(ident))) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (attr >= VIR_IDENTITY_ATTR_LAST) { + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (!value) { + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + virMutexLock(&ident->lock); + + if (ident->attrs[attr]) { + virLibConnError(VIR_ERR_OPERATION_DENIED, "%s", + _("Identity attribute is already set")); + goto error; + } + + if (!(ident->attrs[attr] = strdup(value))) { + virReportOOMError(); + goto error; + } + + virMutexUnlock(&ident->lock); + return 0; + +error: + virDispatchError(NULL); + virMutexUnlock(&ident->lock); + return -1; +} + + +/** + * virIdentityGetAttr: + * @ident: the identity to query + * @attr: the attribute to read + * @value: filled with the attribute value + * + * Fills @value with a pointer to the value associated + * with the identifying attribute @attr in @ident. If + * @attr is not set, then it will simply be initialized + * to NULL and considered as a successful read + * + * Returns 0 on success, -1 on error + */ +int virIdentityGetAttr(virIdentityPtr ident, + unsigned int attr, + const char **value) +{ + VIR_DEBUG("ident=%p attribute=%d value=%p", ident, attr, value); + + if ((!VIR_IS_IDENTITY(ident))) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (attr >= VIR_IDENTITY_ATTR_LAST) { + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + virMutexLock(&ident->lock); + *value = ident->attrs[attr]; + virMutexUnlock(&ident->lock); + return 0; +} + + +/** + * virIdentityIsEqual: + * @identA: the first identity + * @identB: the second identity + * + * Compares every attribute in @identA and @identB + * to determine if they refer to the same identity + * + * Returns 1 if they are equal, 0 if not equal or -1 on error + */ +int virIdentityIsEqual(virIdentityPtr identA, + virIdentityPtr identB) +{ + VIR_DEBUG("identA=%p identB=%p", identA, identB); + int ret = 0; + size_t i; + + if ((!VIR_IS_IDENTITY(identA))) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if ((!VIR_IS_IDENTITY(identB))) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + virMutexLock(&identA->lock); + virMutexLock(&identB->lock); + + for (i = 0 ; i < VIR_IDENTITY_ATTR_LAST ; i++) { + if (identA->attrs[i] == NULL && + identB->attrs[i] != NULL) + goto cleanup; + if (identA->attrs[i] != NULL && + identB->attrs[i] == NULL) + goto cleanup; + if (STRNEQ(identA->attrs[i], + identB->attrs[i])) + goto cleanup; + } + + ret = 1; +cleanup: + virMutexUnlock(&identA->lock); + virMutexUnlock(&identB->lock); + return ret; +} + + +/** + * virIdentityFree: + * @ident: the identity to free + * + * Decrement the reference count on @ident. If the + * reference count hits zero, then the @ident object + * will be freed + * + * Returns number of references remaining, or -1 on error + */ +int virIdentityFree(virIdentityPtr ident) +{ + size_t i; + + if ((!VIR_IS_IDENTITY(ident))) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + virMutexLock(&ident->lock); + VIR_DEBUG("ident=%p refs=%d", ident, ident->refs); + ident->refs--; + if (ident->refs > 0) { + int ret = ident->refs; + virMutexUnlock(&ident->lock); + return ret; + } + for (i = 0 ; i < VIR_IDENTITY_ATTR_LAST ; i++) + VIR_FREE(ident->attrs[i]); + + virMutexUnlock(&ident->lock); + virMutexDestroy(&ident->lock); + VIR_FREE(ident); + return 0; +} + + + /* * Not for public use. This function is part of the internal * implementation of driver features in the remote case. diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 46c13fb..35f11b8 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -527,6 +527,11 @@ LIBVIRT_0.9.10 { virDomainShutdownFlags; virStorageVolResize; virStorageVolWipePattern; + virIdentityNew; + virIdentityIsEqual; + virIdentitySetAttr; + virIdentityGetAttr; + virIdentityFree; } LIBVIRT_0.9.9; LIBVIRT_0.9.11 { diff --git a/src/util/virterror.c b/src/util/virterror.c index b1a5d2b..33ef713 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -1259,6 +1259,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("block copy still active: %s"); break; + case VIR_ERR_INVALID_IDENTITY: + if (info == NULL) + errmsg = _("invalid identity pointer in"); + else + errmsg = _("invalid identity pointer in %s"); + break; } return errmsg; } -- 1.7.10

On 05/02/2012 05:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
A bit sparse on the commit message; even mentioning the term virIdentityPtr would help future crawls through 'git log' find this patch.
+++ b/include/libvirt/libvirt.h.in @@ -1088,6 +1088,36 @@ typedef virConnectAuth *virConnectAuthPtr;
VIR_EXPORT_VAR virConnectAuthPtr virConnectAuthPtrDefault;
+typedef struct _virIdentity virIdentity; +typedef virIdentity *virIdentityPtr; + +typedef enum { + VIR_IDENTITY_ATTR_UNIX_USER_NAME, + VIR_IDENTITY_ATTR_UNIX_GROUP_NAME, + VIR_IDENTITY_ATTR_UNIX_PROCESS_ID, + VIR_IDENTITY_ATTR_SASL_USER_NAME, + VIR_IDENTITY_ATTR_X509_DISTINGUISHED_NAME, + VIR_IDENTITY_ATTR_SECURITY_CONTEXT, + + VIR_IDENTITY_ATTR_LAST,
This last value should be guarded by VIR_ENUM_SENTINELS.
+ +virIdentityPtr virIdentityNew(void); +int virIdentityRef(virIdentityPtr ident); +int virIdentitySetAttr(virIdentityPtr ident, + unsigned int attr, + const char *value); + +int virIdentityGetAttr(virIdentityPtr ident, + unsigned int attr, + const char **value); + +int virIdentityIsEqual(virIdentityPtr identA, + virIdentityPtr identB); + +int virIdentityFree(virIdentityPtr ident);
Are there any other useful operations, such as knowing how many attributes in the identity are currently set?
/** + * VIR_IDENTITY_MAGIC: + * + * magic value used to protect the API when pointers to identity structures + * are passed down by the users. + */ +# define VIR_IDENTITY_MAGIC 0xB33FCAF3
How do we pick these magic numbers, anyway? :)
+ +struct _virIdentity { + unsigned int magic; + virMutex lock; + int refs; + + char *attrs[VIR_IDENTITY_ATTR_LAST]; +};
It looks like your intent is to store everything in this identity locally (contrast it to _virDomain, which stores only enough information locally to pass back over RPC to the remote side and have the remote side do a hash lookup for the rest of the domain information). It should be okay, though, especially since this identity does not include a name or uuid which could be used for a hash lookup for additional contents, anyway.
+ +/** + * virIdentityNew: + * + * Creates a new empty identity object. After creating, one or + * more identifying attributes should be set on the identity. + * + * Returns: a new empty identity
or NULL on error.
+/** + * virIdentityRef: + * @ident: the identity to hold a reference on + * + * Increment the reference count on the identity. For each + * additional call to this method, there shall be a corresponding + * call to virIdentityFree to release the reference count, once + * the caller no longer needs the reference to this object. + * + * This method is typically useful for applications where multiple + * threads are using an identity object, and it is required that + * the identity remain around until all threads have finished using + * it. ie, each new thread using a identity would increment
It looks weird to start a sentence with "ie,", but I don't have an alternative wording on the tip of my tongue.
+ * the reference count. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int virIdentityRef(virIdentityPtr ident) +{ + if ((!VIR_IS_IDENTITY(ident))) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
We really should be using a better error message than __FUNCTION__, especially since virLibConnError is already adding __FUNCTION__ into the list of arguments. (Throughout the patch).
+int virIdentitySetAttr(virIdentityPtr ident, + unsigned int attr, + const char *value) +{ + VIR_DEBUG("ident=%p attribute=%u value=%s", ident, attr, NULLSTR(value)); + + if ((!VIR_IS_IDENTITY(ident))) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (attr >= VIR_IDENTITY_ATTR_LAST) { + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (!value) { + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
Really? This means I can't clear out a value by passing in NULL. Should there be a counterpart API for clearing an attribute?
+ virDispatchError(NULL); + return -1; + } + + virMutexLock(&ident->lock); + + if (ident->attrs[attr]) { + virLibConnError(VIR_ERR_OPERATION_DENIED, "%s", + _("Identity attribute is already set"));
Then again, this makes it a write-once interface (once an identity attribute is set, you can't change it). Should we document that better?
+int virIdentityIsEqual(virIdentityPtr identA, + virIdentityPtr identB)
+ + for (i = 0 ; i < VIR_IDENTITY_ATTR_LAST ; i++) { + if (identA->attrs[i] == NULL && + identB->attrs[i] != NULL) + goto cleanup; + if (identA->attrs[i] != NULL && + identB->attrs[i] == NULL) + goto cleanup; + if (STRNEQ(identA->attrs[i], + identB->attrs[i])) + goto cleanup;
Use STRNEQ_NULLABLE to shorten this.
+++ b/src/libvirt_public.syms @@ -527,6 +527,11 @@ LIBVIRT_0.9.10 { virDomainShutdownFlags; virStorageVolResize; virStorageVolWipePattern; + virIdentityNew; + virIdentityIsEqual; + virIdentitySetAttr; + virIdentityGetAttr; + virIdentityFree; } LIBVIRT_0.9.9;
Wrong release. This should be in the LIBVIRT_0.9.12 section.
+++ b/src/util/virterror.c @@ -1259,6 +1259,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("block copy still active: %s"); break; + case VIR_ERR_INVALID_IDENTITY: + if (info == NULL) + errmsg = _("invalid identity pointer in");
That reads awkwardly. That says if I call: reportError(VIR_ERR_INVALID_IDENTITY, NULL); my error message will be "function: invalid identity pointer in".
+ else + errmsg = _("invalid identity pointer in %s");
Then again, if I call reportError(VIR_ERR_INVALID_IDENTITY, __FUNCTION__); the error message will be "function: invalid identity pointer in function" But it's copy and paste from existing practice, so it's no worse than before. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 05/02/2012 07:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
--- include/libvirt/libvirt.h.in | 30 ++++++ include/libvirt/virterror.h | 1 + src/datatypes.h | 20 ++++ src/libvirt.c | 246 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 + src/util/virterror.c | 6 ++ 6 files changed, 308 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ac5df95..86a9514 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1088,6 +1088,36 @@ typedef virConnectAuth *virConnectAuthPtr;
VIR_EXPORT_VAR virConnectAuthPtr virConnectAuthPtrDefault;
+typedef struct _virIdentity virIdentity; +typedef virIdentity *virIdentityPtr; + +typedef enum { + VIR_IDENTITY_ATTR_UNIX_USER_NAME, + VIR_IDENTITY_ATTR_UNIX_GROUP_NAME, + VIR_IDENTITY_ATTR_UNIX_PROCESS_ID, + VIR_IDENTITY_ATTR_SASL_USER_NAME, + VIR_IDENTITY_ATTR_X509_DISTINGUISHED_NAME, + VIR_IDENTITY_ATTR_SECURITY_CONTEXT, + + VIR_IDENTITY_ATTR_LAST, +} virIdentityAttrType; + + +virIdentityPtr virIdentityNew(void); +int virIdentityRef(virIdentityPtr ident); +int virIdentitySetAttr(virIdentityPtr ident, + unsigned int attr, + const char *value); + +int virIdentityGetAttr(virIdentityPtr ident, + unsigned int attr, + const char **value); + +int virIdentityIsEqual(virIdentityPtr identA, + virIdentityPtr identB); + +int virIdentityFree(virIdentityPtr ident); + /** * VIR_UUID_BUFLEN: * diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 7283207..e44390e 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -251,6 +251,7 @@ typedef enum { VIR_ERR_MIGRATE_UNSAFE = 81, /* Migration is not safe */ VIR_ERR_OVERFLOW = 82, /* integer overflow */ VIR_ERR_BLOCK_COPY_ACTIVE = 83, /* action prevented by block copy job */ + VIR_ERR_INVALID_IDENTITY = 84, /* Invalid identity pointer */ } virErrorNumber;
/** diff --git a/src/datatypes.h b/src/datatypes.h index fc284d2..b8d8db2 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -38,6 +38,16 @@
/** + * VIR_IDENTITY_MAGIC: + * + * magic value used to protect the API when pointers to identity structures + * are passed down by the users. + */ +# define VIR_IDENTITY_MAGIC 0xB33FCAF3 +# define VIR_IS_IDENTITY(obj) ((obj) && (obj)->magic==VIR_IDENTITY_MAGIC) + + +/** * VIR_DOMAIN_MAGIC: * * magic value used to protect the API when pointers to domain structures @@ -190,6 +200,16 @@ struct _virConnect { int refs; /* reference count */ };
+ +struct _virIdentity { + unsigned int magic; + virMutex lock; + int refs; + + char *attrs[VIR_IDENTITY_ATTR_LAST]; +}; + + /** * _virDomain: * diff --git a/src/libvirt.c b/src/libvirt.c index cfd7711..a206800 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1544,6 +1544,252 @@ virConnectRef(virConnectPtr conn) return 0; }
+ +/** + * virIdentityNew: + * + * Creates a new empty identity object. After creating, one or + * more identifying attributes should be set on the identity. + * + * Returns: a new empty identity + */ +virIdentityPtr virIdentityNew(void) +{ + virIdentityPtr ident; + + if (VIR_ALLOC(ident) < 0) { + virReportOOMError(); + return NULL; + } + + if (virMutexInit(&ident->lock) < 0) { + virReportSystemError(errno, "%s", + _("Unable to initialize mutex")); + VIR_FREE(ident); + return NULL; + } + ident->magic = VIR_IDENTITY_MAGIC; + ident->refs = 1; + + return ident; +} + +/** + * virIdentityRef: + * @ident: the identity to hold a reference on + * + * Increment the reference count on the identity. For each + * additional call to this method, there shall be a corresponding + * call to virIdentityFree to release the reference count, once + * the caller no longer needs the reference to this object. + * + * This method is typically useful for applications where multiple + * threads are using an identity object, and it is required that + * the identity remain around until all threads have finished using + * it. ie, each new thread using a identity would increment + * the reference count. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int virIdentityRef(virIdentityPtr ident) +{ + if ((!VIR_IS_IDENTITY(ident))) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + virMutexLock(&ident->lock); + VIR_DEBUG("ident=%p refs=%d", ident, ident->refs); + ident->refs++; + virMutexUnlock(&ident->lock); + return 0; +} + + +/** + * virIdentitySetAttr: + * @ident: the identity to modify + * @attr: the attribute type to set + * @value: the identifying value to associate with @attr + * + * Sets an identifying attribute @attr on @ident. Each + * @attr type can only be set once. + * + * Returns: 0 on success, or -1 on error + */ +int virIdentitySetAttr(virIdentityPtr ident, + unsigned int attr, + const char *value) +{ + VIR_DEBUG("ident=%p attribute=%u value=%s", ident, attr, NULLSTR(value)); + + if ((!VIR_IS_IDENTITY(ident))) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (attr >= VIR_IDENTITY_ATTR_LAST) { + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (!value) { + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + virMutexLock(&ident->lock); + + if (ident->attrs[attr]) { + virLibConnError(VIR_ERR_OPERATION_DENIED, "%s", + _("Identity attribute is already set")); + goto error; + } + + if (!(ident->attrs[attr] = strdup(value))) { + virReportOOMError(); + goto error; + } + + virMutexUnlock(&ident->lock); + return 0; + +error: + virDispatchError(NULL); + virMutexUnlock(&ident->lock); + return -1; +} + + +/** + * virIdentityGetAttr: + * @ident: the identity to query + * @attr: the attribute to read + * @value: filled with the attribute value + * + * Fills @value with a pointer to the value associated + * with the identifying attribute @attr in @ident. If + * @attr is not set, then it will simply be initialized + * to NULL and considered as a successful read + * + * Returns 0 on success, -1 on error + */ +int virIdentityGetAttr(virIdentityPtr ident, + unsigned int attr, + const char **value) +{ + VIR_DEBUG("ident=%p attribute=%d value=%p", ident, attr, value);
Do you want to use %u instead of %d here?
+ + if ((!VIR_IS_IDENTITY(ident))) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (attr >= VIR_IDENTITY_ATTR_LAST) { + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + virMutexLock(&ident->lock); + *value = ident->attrs[attr]; + virMutexUnlock(&ident->lock); + return 0; +} + + +/** + * virIdentityIsEqual: + * @identA: the first identity + * @identB: the second identity + * + * Compares every attribute in @identA and @identB + * to determine if they refer to the same identity + * + * Returns 1 if they are equal, 0 if not equal or -1 on error + */ +int virIdentityIsEqual(virIdentityPtr identA, + virIdentityPtr identB) +{ + VIR_DEBUG("identA=%p identB=%p", identA, identB); + int ret = 0; + size_t i; + + if ((!VIR_IS_IDENTITY(identA))) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if ((!VIR_IS_IDENTITY(identB))) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + virMutexLock(&identA->lock); + virMutexLock(&identB->lock); + + for (i = 0 ; i < VIR_IDENTITY_ATTR_LAST ; i++) { + if (identA->attrs[i] == NULL && + identB->attrs[i] != NULL) + goto cleanup; + if (identA->attrs[i] != NULL && + identB->attrs[i] == NULL) + goto cleanup; + if (STRNEQ(identA->attrs[i], + identB->attrs[i])) + goto cleanup; + } + + ret = 1; +cleanup: + virMutexUnlock(&identA->lock); + virMutexUnlock(&identB->lock); + return ret; +} + + +/** + * virIdentityFree: + * @ident: the identity to free + * + * Decrement the reference count on @ident. If the + * reference count hits zero, then the @ident object + * will be freed + * + * Returns number of references remaining, or -1 on error + */ +int virIdentityFree(virIdentityPtr ident) +{ + size_t i; + + if ((!VIR_IS_IDENTITY(ident))) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + virMutexLock(&ident->lock); + VIR_DEBUG("ident=%p refs=%d", ident, ident->refs); + ident->refs--; + if (ident->refs > 0) { + int ret = ident->refs; + virMutexUnlock(&ident->lock); + return ret; + } + for (i = 0 ; i < VIR_IDENTITY_ATTR_LAST ; i++) + VIR_FREE(ident->attrs[i]); + + virMutexUnlock(&ident->lock); + virMutexDestroy(&ident->lock); + VIR_FREE(ident); + return 0; +} + + + /* * Not for public use. This function is part of the internal * implementation of driver features in the remote case. diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 46c13fb..35f11b8 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -527,6 +527,11 @@ LIBVIRT_0.9.10 { virDomainShutdownFlags; virStorageVolResize; virStorageVolWipePattern; + virIdentityNew; + virIdentityIsEqual; + virIdentitySetAttr; + virIdentityGetAttr; + virIdentityFree;
Do you need to add virIdentityRef here too? -- Regards, Corey
} LIBVIRT_0.9.9;
LIBVIRT_0.9.11 { diff --git a/src/util/virterror.c b/src/util/virterror.c index b1a5d2b..33ef713 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -1259,6 +1259,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("block copy still active: %s"); break; + case VIR_ERR_INVALID_IDENTITY: + if (info == NULL) + errmsg = _("invalid identity pointer in"); + else + errmsg = _("invalid identity pointer in %s"); + break; } return errmsg; }

From: "Daniel P. Berrange" <berrange@redhat.com> --- include/libvirt/virterror.h | 3 + po/POTFILES.in | 1 + src/Makefile.am | 16 + src/access/apis.txt | 577 +++++++++++++++++++++++++++++++++++++ src/access/viraccessdriver.h | 51 ++++ src/access/viraccessdrivernop.c | 44 +++ src/access/viraccessdrivernop.h | 28 ++ src/access/viraccessdriverstack.c | 105 +++++++ src/access/viraccessdriverstack.h | 32 ++ src/access/viraccessmanager.c | 338 ++++++++++++++++++++++ src/access/viraccessmanager.h | 56 ++++ src/access/viraccessperm.c | 37 +++ src/access/viraccessperm.h | 73 +++++ src/libvirt_private.syms | 21 ++ src/util/virterror.c | 9 + 15 files changed, 1391 insertions(+) create mode 100644 src/access/apis.txt create mode 100644 src/access/viraccessdriver.h create mode 100644 src/access/viraccessdrivernop.c create mode 100644 src/access/viraccessdrivernop.h create mode 100644 src/access/viraccessdriverstack.c create mode 100644 src/access/viraccessdriverstack.h create mode 100644 src/access/viraccessmanager.c create mode 100644 src/access/viraccessmanager.h create mode 100644 src/access/viraccessperm.c create mode 100644 src/access/viraccessperm.h diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index e44390e..1daba04 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -88,6 +88,7 @@ typedef enum { VIR_FROM_URI = 45, /* Error from URI handling */ VIR_FROM_AUTH = 46, /* Error from auth handling */ VIR_FROM_DBUS = 47, /* Error from DBus */ + VIR_FROM_ACCESS = 48, /* Error from access control manager */ } virErrorDomain; @@ -252,6 +253,8 @@ typedef enum { VIR_ERR_OVERFLOW = 82, /* integer overflow */ VIR_ERR_BLOCK_COPY_ACTIVE = 83, /* action prevented by block copy job */ VIR_ERR_INVALID_IDENTITY = 84, /* Invalid identity pointer */ + VIR_ERR_ACCESS_DENIED = 85, /* operation on the object/resource + was denied */ } virErrorNumber; /** diff --git a/po/POTFILES.in b/po/POTFILES.in index 4ea544b..f898887 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -6,6 +6,7 @@ daemon/remote_dispatch.h daemon/stream.c gnulib/lib/gai_strerror.c gnulib/lib/regcomp.c +src/access/viraccessmanager.c src/conf/cpu_conf.c src/conf/domain_conf.c src/conf/domain_event.c diff --git a/src/Makefile.am b/src/Makefile.am index e48dfa5..0293562 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -531,6 +531,13 @@ SECURITY_DRIVER_APPARMOR_SOURCES = \ security/security_apparmor.h security/security_apparmor.c +ACCESS_DRIVER_SOURCES = \ + access/viraccessperm.h access/viraccessperm.c \ + access/viraccessmanager.h access/viraccessmanager.c \ + access/viraccessdriver.h \ + access/viraccessdrivernop.h access/viraccessdrivernop.c \ + access/viraccessdriverstack.h access/viraccessdriverstack.c + NODE_DEVICE_DRIVER_SOURCES = \ node_device/node_device_driver.c \ node_device/node_device_driver.h \ @@ -1115,6 +1122,15 @@ libvirt_driver_security_la_CFLAGS += $(APPARMOR_CFLAGS) libvirt_driver_security_la_LIBADD += $(APPARMOR_LIBS) endif +libvirt_driver_access_la_SOURCES = $(ACCESS_DRIVER_SOURCES) +noinst_LTLIBRARIES += libvirt_driver_access.la +libvirt_la_BUILT_LIBADD += libvirt_driver_access.la +libvirt_driver_access_la_CFLAGS = \ + -I@top_srcdir@/src/conf $(AM_CFLAGS) +libvirt_driver_access_la_LDFLAGS = $(AM_LDFLAGS) +libvirt_driver_access_la_LIBADD = + + # Add all conditional sources just in case... EXTRA_DIST += \ $(TEST_DRIVER_SOURCES) \ diff --git a/src/access/apis.txt b/src/access/apis.txt new file mode 100644 index 0000000..16cc49c --- /dev/null +++ b/src/access/apis.txt @@ -0,0 +1,577 @@ + +Non-driver based APIs + + +virConnCopyLastError: +virResetError: +virResetLastError: +virSaveLastError: +virSetErrorFunc: +virConnGetLastError: +virConnResetLastError: +virConnSetErrorFunc: +virCopyLastError: +virDefaultErrorFunc: +virFreeError: +virGetLastError: + +virInitialize: +virConnectClose: +virConnectGetLibVersion: +virGetVersion: +virConnectGetVersion: +virConnectGetType: +virConnectGetURI: + + +virConnectRef: +virDomainRef: +virInterfaceRef: +virNetworkRef: +virNodeDeviceRef: +virNWFilterRef: +virSecretRef: +virStoragePoolRef: +virStorageVolRef: +virStreamRef: + +virIdentityFree: +virDomainFree: +virDomainSnapshotFree: +virInterfaceFree: +virNetworkFree: +virNodeDeviceFree: +virNWFilterFree: +virSecretFree: +virStoragePoolFree: +virStorageVolFree: +virStreamFree: + +virDomainGetConnect: +virInterfaceGetConnect: +virDomainSnapshotGetConnect: +virNetworkGetConnect: +virSecretGetConnect: +virStoragePoolGetConnect: +virStorageVolGetConnect: +virDomainSnapshotGetDomain: + +virEventAddHandle: +virEventAddTimeout: +virEventRegisterDefaultImpl: +virEventRegisterImpl: +virEventRemoveHandle: +virEventRemoveTimeout: +virEventRunDefaultImpl: +virEventUpdateHandle: +virEventUpdateTimeout: + + +virConnectBaselineCPU: + + - No state access + +virConnectCompareCPU: + + - Access host CPU + +virConnectGetCapabilities: + + - Access host CPU, emulators, NUMA + +virConnectGetHostname: + + - hostname resolve + +virConnectGetIdentity: + + - No state + +virConnectGetMaxVcpus: + + - Hypercall + +virConnectGetSysinfo: + + - Sysfs / dmidecode ? (cached from capabilities) + +virConnectIsAlive: + + - Driver check + +virConnectIsEncrypted: +virConnectIsSecure: + + - Property lookup + +virConnectOpen: +virConnectOpenAuth: +virConnectOpenReadOnly: + + - RPC layer + +virConnectSetIdentity: + + - RPC layer + +virConnectSetKeepAlive: + + - RPC layer + +virNodeGetCellsFreeMemory: + + - NUMA props + +virNodeGetCPUStats: + + - Cgroups + +virNodeGetFreeMemory: + + - NUMA props + +virNodeGetInfo: + + - NUMA / sysfs + +virNodeGetMemoryStats: + + - CGroups + +virNodeGetSecurityModel: + + - Capabilities + +virNodeSuspendForDuration: + + - PM utils invoke + + + +virConnectNumOfDefinedDomains: +virConnectNumOfDomains: +virConnectListDefinedDomains: +virConnectListDomains: + + - 'search_domains' on libvirtd + - 'getattr' on each domain + + +virConnectDomainEventDeregister: +virConnectDomainEventDeregisterAny: +virConnectDomainEventRegister: +virConnectDomainEventRegisterAny: + + - 'monitor' on domain + +virConnectDomainXMLFromNative: +virConnectDomainXMLToNative: + + - 'domain_xml' on libvirtd + +virDomainAbortJob: + + - 'abort_job' + +virDomainBlockJobAbort: + + - 'abort_block_job' + +virDomainBlockJobSetSpeed: + + - 'setattr_block_job' + +virDomainBlockPeek: + + - 'block_peek' + +virDomainBlockPull: + + - 'block_pull' + - 'create_block_job' + +virDomainBlockResize: + + - 'block_resize' + +virDomainBlockStats: +virDomainBlockStatsFlags: + + - 'read' + +virDomainCoreDump: + + - 'coredump' + +virDomainCreate: +virDomainCreateLinux: +virDomainCreateWithFlags: + + - 'start' + +virDomainCreateXML: + + - 'start' + 'write' + +virDomainDefineXML: + + - 'save' + 'write' + +virDomainDestroy: +virDomainDestroyFlags: + + - 'stop' + + +virDomainGetID: +virDomainGetName: +virDomainGetUUID: +virDomainGetUUIDString: +virDomainLookupByUUIDString: + + - Outside driver + +virDomainLookupByID: +virDomainLookupByName: +virDomainLookupByUUID: + + - getattr + +virDomainGetAutostart: +virDomainGetBlkioParameters: +virDomainGetBlockInfo: +virDomainGetBlockIoTune: +virDomainGetBlockJobInfo: +virDomainGetControlInfo: +virDomainGetInfo: +virDomainGetInterfaceParameters: +virDomainGetJobInfo: +virDomainGetMaxMemory: +virDomainGetMaxVcpus: +virDomainGetMemoryParameters: +virDomainGetNumaParameters: +virDomainGetOSType: +virDomainGetSchedulerParameters: +virDomainGetSchedulerParametersFlags: +virDomainGetSchedulerType: +virDomainGetSecurityLabel: +virDomainGetState: +virDomainGetVcpuPinInfo: +virDomainGetVcpus: +virDomainGetVcpusFlags: +virDomainGetXMLDesc: +virDomainHasCurrentSnapshot: +virDomainHasManagedSaveImage: +virDomainInterfaceStats: +virDomainIsActive: +virDomainIsPersistent: +virDomainIsUpdated: +virDomainMemoryStats: + + - 'read' + +virDomainInjectNMI: + + - inject_nmi + + +virDomainManagedSave: + + - save_create + +virDomainManagedSaveRemove: + + - save_delete + +virDomainMemoryPeek: + + - memory_peek + + +virDomainMigrate: +virDomainMigrate2: +virDomainMigrateGetMaxSpeed: +virDomainMigrateSetMaxDowntime: +virDomainMigrateSetMaxSpeed: +virDomainMigrateToURI: +virDomainMigrateToURI2: + + - migrate + +virDomainOpenConsole: + + - open_console + +virDomainOpenGraphics: + + - open_graphics + +virDomainPinVcpu: +virDomainPinVcpuFlags: + + - write ? + +virDomainReboot: + + - reboot + +virDomainReset: + + - reset + +virDomainRestore: +virDomainRestoreFlags: + + - restore + - start + +virDomainResume: + + - resume + +virDomainRevertToSnapshot: + + - snapshot_revert + +virDomainSave: +virDomainSaveFlags: + + - stop + - save + +virDomainSaveImageDefineXML: + + - save_write (setattr ?) + +virDomainSaveImageGetXMLDesc: + + - save_getattr + + +virDomainScreenshot: + + - screenshot + +virDomainSendKey: + + - sendkey + +virDomainSetAutostart: +virDomainSetBlkioParameters: +virDomainSetBlockIoTune: +virDomainSetInterfaceParameters: +virDomainSetMaxMemory: +virDomainSetMemory: +virDomainSetMemoryFlags: +virDomainSetMemoryParameters: +virDomainSetNumaParameters: +virDomainSetSchedulerParameters: +virDomainSetSchedulerParametersFlags: +virDomainSetVcpus: +virDomainSetVcpusFlags: +virDomainAttachDevice: +virDomainAttachDeviceFlags: +virDomainUpdateDeviceFlags: +virDomainDetachDevice: +virDomainDetachDeviceFlags: + + + - write (+ possible save) + +virDomainShutdown: + + - shutdown + +virDomainSnapshotCreateXML: + + - snapshot_create + +virDomainSnapshotCurrent: + + - snapshot_getattr (or getattr ?) + +virDomainSnapshotDelete: + + - snapshot_delete + +virDomainSnapshotGetName: +virDomainSnapshotGetParent: +virDomainSnapshotGetXMLDesc: + + - snapshot_getattr + +virDomainSnapshotListChildrenNames: +virDomainSnapshotListNames: + + - snapshot_search + - Filter on snapshot_getattr + +virDomainSnapshotLookupByName: + + - snapshot_getattr + +virDomainSnapshotNum: +virDomainSnapshotNumChildren: + + - snapshot_search + - Filter on snapshot_getattr + +virDomainSuspend: + + - suspend + +virDomainUndefine: +virDomainUndefineFlags: + + - delete + + + +virConnectNumOfDefinedInterfaces: +virConnectNumOfInterfaces: +virConnectListDefinedInterfaces: +virConnectListInterfaces: +virInterfaceChangeBegin: +virInterfaceChangeCommit: +virInterfaceChangeRollback: +virInterfaceCreate: +virInterfaceDefineXML: +virInterfaceDestroy: +virInterfaceGetMACString: +virInterfaceGetName: +virInterfaceGetXMLDesc: +virInterfaceIsActive: +virInterfaceLookupByMACString: +virInterfaceLookupByName: +virInterfaceUndefine: + + +virConnectNumOfDefinedNetworks: +virConnectNumOfNetworks: +virConnectListDefinedNetworks: +virConnectListNetworks: +virNetworkCreate: +virNetworkCreateXML: +virNetworkDefineXML: +virNetworkDestroy: +virNetworkGetAutostart: +virNetworkGetBridgeName: +virNetworkGetName: +virNetworkGetUUID: +virNetworkGetUUIDString: +virNetworkGetXMLDesc: +virNetworkIsActive: +virNetworkIsPersistent: +virNetworkLookupByName: +virNetworkLookupByUUID: +virNetworkLookupByUUIDString: +virNetworkSetAutostart: +virNetworkUndefine: + + + +virNodeDeviceCreateXML: +virNodeDeviceDestroy: +virNodeDeviceDettach: +virNodeDeviceGetName: +virNodeDeviceGetParent: +virNodeDeviceGetXMLDesc: +virNodeDeviceListCaps: +virNodeDeviceLookupByName: +virNodeDeviceNumOfCaps: +virNodeDeviceReAttach: +virNodeDeviceReset: +virNodeListDevices: +virNodeNumOfDevices: + + + +virConnectNumOfNWFilters: +virConnectListNWFilters: +virNWFilterDefineXML: +virNWFilterGetName: +virNWFilterGetUUID: +virNWFilterGetUUIDString: +virNWFilterGetXMLDesc: +virNWFilterLookupByName: +virNWFilterLookupByUUID: +virNWFilterLookupByUUIDString: +virNWFilterUndefine: + + + +virConnectNumOfSecrets: +virConnectListSecrets: +virSecretDefineXML: +virSecretGetUsageID: +virSecretGetUsageType: +virSecretGetUUID: +virSecretGetUUIDString: +virSecretGetValue: +virSecretGetXMLDesc: +virSecretLookupByUsage: +virSecretLookupByUUID: +virSecretLookupByUUIDString: +virSecretSetValue: +virSecretUndefine: + + + +virConnectNumOfDefinedStoragePools: +virConnectNumOfStoragePools: +virConnectListDefinedStoragePools: +virConnectListStoragePools: +virConnectFindStoragePoolSources: +virStoragePoolBuild: +virStoragePoolCreate: +virStoragePoolCreateXML: +virStoragePoolDefineXML: +virStoragePoolDelete: +virStoragePoolDestroy: +virStoragePoolGetAutostart: +virStoragePoolGetInfo: +virStoragePoolGetName: +virStoragePoolGetUUID: +virStoragePoolGetUUIDString: +virStoragePoolGetXMLDesc: +virStoragePoolIsActive: +virStoragePoolIsPersistent: +virStoragePoolListVolumes: +virStoragePoolLookupByName: +virStoragePoolLookupByUUID: +virStoragePoolLookupByUUIDString: +virStoragePoolLookupByVolume: +virStoragePoolNumOfVolumes: +virStoragePoolRefresh: +virStoragePoolSetAutostart: +virStoragePoolUndefine: + + + +virStorageVolCreateXML: +virStorageVolCreateXMLFrom: +virStorageVolDelete: +virStorageVolDownload: +virStorageVolGetInfo: +virStorageVolGetKey: +virStorageVolGetName: +virStorageVolGetPath: +virStorageVolGetXMLDesc: +virStorageVolLookupByKey: +virStorageVolLookupByName: +virStorageVolLookupByPath: +virStorageVolUpload: +virStorageVolWipe: + + + +virStreamAbort: +virStreamEventAddCallback: +virStreamEventRemoveCallback: +virStreamEventUpdateCallback: +virStreamFinish: +virStreamNew: +virStreamRecv: +virStreamRecvAll: +virStreamSend: +virStreamSendAll: diff --git a/src/access/viraccessdriver.h b/src/access/viraccessdriver.h new file mode 100644 index 0000000..ae09162 --- /dev/null +++ b/src/access/viraccessdriver.h @@ -0,0 +1,51 @@ +/* + * viraccessdriver.h: access control driver + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __VIR_ACCESS_DRIVER_H__ +# define __VIR_ACCESS_DRIVER_H__ + +# include "conf/domain_conf.h" +# include "access/viraccessmanager.h" + +typedef bool (*virAccessDriverCheckConnectDrv)(virAccessManagerPtr manager, + virAccessPermConnect av); +typedef bool (*virAccessDriverCheckDomainDrv)(virAccessManagerPtr manager, + virDomainDefPtr def, + virAccessPermDomain av); + +typedef int (*virAccessDriverSetupDrv)(virAccessManagerPtr manager); +typedef void (*virAccessDriverCleanupDrv)(virAccessManagerPtr manager); + +typedef struct _virAccessDriver virAccessDriver; +typedef virAccessDriver *virAccessDriverPtr; + +struct _virAccessDriver { + size_t privateDataLen; + const char *name; + + virAccessDriverSetupDrv setup; + virAccessDriverCleanupDrv cleanup; + + virAccessDriverCheckConnectDrv checkConnect; + virAccessDriverCheckDomainDrv checkDomain; +}; + + +#endif /* __VIR_ACCESS_DRIVER_H__ */ diff --git a/src/access/viraccessdrivernop.c b/src/access/viraccessdrivernop.c new file mode 100644 index 0000000..7ba0719 --- /dev/null +++ b/src/access/viraccessdrivernop.c @@ -0,0 +1,44 @@ +/* + * viraccessdrivernop.c: no-op access control driver + * + * Copyright (C) 2011 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include <config.h> + +#include "access/viraccessdrivernop.h" + +static bool +virAccessDriverNopCheckConnect(virAccessManagerPtr manager ATTRIBUTE_UNUSED, + virAccessPermConnect av ATTRIBUTE_UNUSED) +{ + return true; +} + +static bool +virAccessDriverNopCheckDomain(virAccessManagerPtr manager ATTRIBUTE_UNUSED, + virDomainDefPtr def ATTRIBUTE_UNUSED, + virAccessPermDomain av ATTRIBUTE_UNUSED) +{ + return true; +} + +virAccessDriver accessDriverNop = { + .name = "none", + .checkConnect = virAccessDriverNopCheckConnect, + .checkDomain = virAccessDriverNopCheckDomain, +}; diff --git a/src/access/viraccessdrivernop.h b/src/access/viraccessdrivernop.h new file mode 100644 index 0000000..a3d9be3 --- /dev/null +++ b/src/access/viraccessdrivernop.h @@ -0,0 +1,28 @@ +/* + * viraccessdrivernop.h: no-op access control driver + * + * Copyright (C) 2011 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __VIR_ACCESS_DRIVER_NOP_H__ +# define __VIR_ACCESS_DRIVER_NOP_H__ + +# include "access/viraccessdriver.h" + +extern virAccessDriver accessDriverNop; + +#endif /* __VIR_ACCESS_DRIVER_NOP_H__ */ diff --git a/src/access/viraccessdriverstack.c b/src/access/viraccessdriverstack.c new file mode 100644 index 0000000..48aaafd --- /dev/null +++ b/src/access/viraccessdriverstack.c @@ -0,0 +1,105 @@ +/* + * viraccessdriverstack.c: stacked access control driver + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include <config.h> + +#include "access/viraccessdriverstack.h" +#include "memory.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_ACCESS + +typedef struct _virAccessDriverStackPrivate virAccessDriverStackPrivate; +typedef virAccessDriverStackPrivate *virAccessDriverStackPrivatePtr; + +struct _virAccessDriverStackPrivate { + virAccessManagerPtr *managers; + size_t managersLen; +}; + + +int virAccessDriverStackAppend(virAccessManagerPtr manager, + virAccessManagerPtr child) +{ + virAccessDriverStackPrivatePtr priv = virAccessManagerGetPrivateData(manager); + + if (VIR_EXPAND_N(priv->managers, priv->managersLen, 1) < 0) { + virReportOOMError(); + return -1; + } + + priv->managers[priv->managersLen-1] = child; + + return 0; +} + + +static void virAccessDriverStackCleanup(virAccessManagerPtr manager) +{ + virAccessDriverStackPrivatePtr priv = virAccessManagerGetPrivateData(manager); + size_t i; + + for (i = 0 ; i < priv->managersLen ; i++) { + virAccessManagerFree(priv->managers[i]); + } + VIR_FREE(priv->managers); +} + + +static bool +virAccessDriverStackCheckConnect(virAccessManagerPtr manager, + virAccessPermConnect av) +{ + virAccessDriverStackPrivatePtr priv = virAccessManagerGetPrivateData(manager); + bool ret = true; + size_t i; + + for (i = 0 ; i < priv->managersLen ; i++) { + /* We do not short-circuit on first denial - always check all drivers */ + if (!virAccessManagerCheckConnect(priv->managers[i], av)) + ret = false; + } + + return ret; +} + +static bool +virAccessDriverStackCheckDomain(virAccessManagerPtr manager, + virDomainDefPtr def, + virAccessPermDomain av) +{ + virAccessDriverStackPrivatePtr priv = virAccessManagerGetPrivateData(manager); + bool ret = true; + size_t i; + + for (i = 0 ; i < priv->managersLen ; i++) { + /* We do not short-circuit on first denial - always check all drivers */ + if (!virAccessManagerCheckDomain(priv->managers[i], def, av)) + ret = false; + } + + return ret; +} + +virAccessDriver accessDriverStack = { + .cleanup = virAccessDriverStackCleanup, + .checkConnect = virAccessDriverStackCheckConnect, + .checkDomain = virAccessDriverStackCheckDomain, +}; diff --git a/src/access/viraccessdriverstack.h b/src/access/viraccessdriverstack.h new file mode 100644 index 0000000..8ab3a0d --- /dev/null +++ b/src/access/viraccessdriverstack.h @@ -0,0 +1,32 @@ +/* + * viraccessdriverstack.h: stacked access control driver + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __VIR_ACCESS_DRIVER_STACK_H__ +# define __VIR_ACCESS_DRIVER_STACK_H__ + +# include "access/viraccessdriver.h" + + +int virAccessDriverStackAppend(virAccessManagerPtr manager, + virAccessManagerPtr child); + +extern virAccessDriver accessDriverStack; + +#endif /* __VIR_ACCESS_DRIVER_STACK_H__ */ diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c new file mode 100644 index 0000000..4e77bd6 --- /dev/null +++ b/src/access/viraccessmanager.c @@ -0,0 +1,338 @@ +/* + * viraccessmanager.c: access control manager + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include <config.h> + +#include "viraccessmanager.h" +#include "memory.h" +#include "virterror_internal.h" +#include "threads.h" +#if HAVE_SELINUX +# include <selinux/selinux.h> +#endif +#include "access/viraccessdrivernop.h" +#include "access/viraccessdriverstack.h" +#include "logging.h" + +#define VIR_FROM_THIS VIR_FROM_ACCESS +#define virAccessError(code, ...) \ + virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \ + __FUNCTION__, __LINE__, __VA_ARGS__) + +static volatile bool onceInitErr = false; +static virOnceControl onceInit = VIR_ONCE_CONTROL_INITIALIZER; +static virThreadLocal realIdentity; +static virThreadLocal effectiveIdentity; + +struct _virAccessManager { + virAccessDriverPtr drv; +}; + + +static void virAccessManagerOnceInit(void) +{ + if (virThreadLocalInit(&realIdentity, + (virThreadLocalCleanup)virIdentityFree) < 0) + onceInitErr = true; + if (virThreadLocalInit(&effectiveIdentity, + (virThreadLocalCleanup)virIdentityFree) < 0) + onceInitErr = true; +} + +static bool virAccessManagerInit(void) +{ + if (virOnce(&onceInit, virAccessManagerOnceInit) < 0 || + onceInitErr) { + virReportSystemError(errno, "%s", + _("Failed to initialize access manager")); + return false; + } + + return true; +} + +virIdentityPtr virAccessManagerGetSystemIdentity(void) +{ + char *username = NULL; + char *groupname = NULL; + char *seccontext = NULL; + virIdentityPtr ret = NULL; + gid_t gid = getgid(); + uid_t uid = getuid(); +#if HAVE_SELINUX + security_context_t con; +#endif + + if (!(username = virGetUserName(uid))) + goto cleanup; + if (!(groupname = virGetGroupName(gid))) + goto cleanup; + +#if HAVE_SELINUX + if (getcon(&con) < 0) { + virReportSystemError(errno, "%s", + _("Unable to lookup SELinux process context")); + goto cleanup; + } + seccontext = strdup(con); + freecon(con); + if (!seccontext) { + virReportOOMError(); + goto cleanup; + } +#endif + + if (!(ret = virIdentityNew())) + goto cleanup; + + if (username && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_USER_NAME, username) < 0) + goto error; + if (groupname && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_GROUP_NAME, groupname) < 0) + goto error; + if (seccontext && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_SECURITY_CONTEXT, seccontext) < 0) + goto error; + +cleanup: + VIR_FREE(username); + VIR_FREE(groupname); + VIR_FREE(seccontext); + return ret; + +error: + virIdentityFree(ret); + ret = NULL; + goto cleanup; +} + +virIdentityPtr virAccessManagerGetEffectiveIdentity(void) +{ + virIdentityPtr ret; + + if (!virAccessManagerInit()) + return NULL; + + ret = virThreadLocalGet(&effectiveIdentity); + virIdentityRef(ret); + return ret; +} + +int virAccessManagerSetEffectiveIdentity(virIdentityPtr identity) +{ + virIdentityPtr old; + + if (!virAccessManagerInit()) + return -1; + + old = virThreadLocalGet(&effectiveIdentity); + if (old) + virIdentityFree(old); + + if (virThreadLocalSet(&effectiveIdentity, identity) < 0) { + virReportSystemError(errno, "%s", + _("Unable to set thread local identity")); + return -1; + } + + if (identity) + virIdentityRef(identity); + return 0; +} + +virIdentityPtr virAccessManagerGetRealIdentity(void) +{ + virIdentityPtr ret; + + if (!virAccessManagerInit()) + return NULL; + + ret = virThreadLocalGet(&realIdentity); + virIdentityRef(ret); + return ret; +} + +int virAccessManagerSetRealIdentity(virIdentityPtr identity) +{ + virIdentityPtr old; + + if (!virAccessManagerInit()) + return -1; + + old = virThreadLocalGet(&realIdentity); + if (old) + virIdentityFree(old); + + if (virThreadLocalSet(&realIdentity, identity) < 0) { + virReportSystemError(errno, "%s", + _("Unable to set thread local identity")); + return -1; + } + + if (identity) + virIdentityRef(identity); + return 0; +} + + +static virAccessManagerPtr virAccessManagerNewDriver(virAccessDriverPtr drv) +{ + virAccessManagerPtr mgr; + + if (VIR_ALLOC_VAR(mgr, char, drv->privateDataLen) < 0) { + virReportOOMError(); + return NULL; + } + + mgr->drv = drv; + + if (mgr->drv->setup && + mgr->drv->setup(mgr) < 0) { + VIR_FREE(mgr); + return NULL; + } + + return mgr; +} + + +static virAccessDriverPtr accessDrivers[] = { + &accessDriverNop, +}; + + +static virAccessDriverPtr virAccessManagerFindDriver(const char *name) +{ + size_t i; + for (i = 0 ; i < ARRAY_CARDINALITY(accessDrivers) ; i++) { + if (STREQ(name, accessDrivers[i]->name)) + return accessDrivers[i]; + } + + return NULL; +} + + +virAccessManagerPtr virAccessManagerNew(const char *name) +{ + virAccessDriverPtr drv = virAccessManagerFindDriver(name); + if (!drv) + return NULL; + + return virAccessManagerNewDriver(drv); +} + + +virAccessManagerPtr virAccessManagerNewStack(const char **names, + size_t namesLen) +{ + virAccessManagerPtr manager = virAccessManagerNewDriver(&accessDriverStack); + size_t i; + + if (!manager) + return NULL; + + for (i = 0 ; i < namesLen ; i++) { + virAccessManagerPtr child = virAccessManagerNew(names[i]); + + if (!child) + goto error; + + if (virAccessDriverStackAppend(manager, child) < 0) { + virAccessManagerFree(child); + goto error; + } + } + + return manager; + +error: + virAccessManagerFree(manager); + return NULL; +} + + +void *virAccessManagerGetPrivateData(virAccessManagerPtr mgr) +{ + /* This accesses the memory just beyond mgr, which was allocated + * via VIR_ALLOC_VAR earlier. */ + return mgr + 1; +} + + +void virAccessManagerFree(virAccessManagerPtr mgr) +{ + if (!mgr) + return; + + if (mgr->drv->cleanup) + mgr->drv->cleanup(mgr); + + VIR_FREE(mgr); +} + + +/* Standard security practice is to not tell the caller *why* + * they were denied access. So this method takes the real + * libvirt errors & replaces it with a generic error. Fortunately + * the daemon logs will still contain the original error message + * should the admin need to debug things + */ +static bool +virAccessManagerSanitizeError(bool ret) +{ + if (!ret) { + virResetLastError(); + virAccessError(VIR_ERR_ACCESS_DENIED, NULL); + } + + return ret; +} + +bool virAccessManagerCheckConnect(virAccessManagerPtr manager, + virAccessPermConnect av) +{ + bool ret = true; + VIR_DEBUG("manager=%p driver=%s av=%d", + manager, manager->drv->name, av); + + if (manager->drv->checkConnect && + !manager->drv->checkConnect(manager, av)) + ret = false; + + return virAccessManagerSanitizeError(ret); +} + + +bool virAccessManagerCheckDomain(virAccessManagerPtr manager, + virDomainDefPtr def, + virAccessPermDomain av) +{ + bool ret = true; + VIR_DEBUG("manager=%p driver=%s def=%p av=%d", + manager, manager->drv->name, def, av); + + if (manager->drv->checkDomain && + !manager->drv->checkDomain(manager, def, av)) + ret = false; + + return virAccessManagerSanitizeError(ret); +} diff --git a/src/access/viraccessmanager.h b/src/access/viraccessmanager.h new file mode 100644 index 0000000..b7e19b5 --- /dev/null +++ b/src/access/viraccessmanager.h @@ -0,0 +1,56 @@ +/* + * viraccessmanager.h: access control manager + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __VIR_ACCESS_MANAGER_H__ +# define __VIR_ACCESS_MANAGER_H__ + +# include "rpc/virnetserverclient.h" +# include "conf/domain_conf.h" +# include "access/viraccessperm.h" + +virIdentityPtr virAccessManagerGetClientIdentity(virNetServerClientPtr client); +virIdentityPtr virAccessManagerGetSystemIdentity(void); + +virIdentityPtr virAccessManagerGetEffectiveIdentity(void); +int virAccessManagerSetEffectiveIdentity(virIdentityPtr identity); + +virIdentityPtr virAccessManagerGetRealIdentity(void); +int virAccessManagerSetRealIdentity(virIdentityPtr identity); + +typedef struct _virAccessManager virAccessManager; +typedef virAccessManager *virAccessManagerPtr; + +virAccessManagerPtr virAccessManagerNew(const char *name); +virAccessManagerPtr virAccessManagerNewStack(const char **names, + size_t namesLen); + + +void *virAccessManagerGetPrivateData(virAccessManagerPtr manager); +void virAccessManagerFree(virAccessManagerPtr manager); + + +bool virAccessManagerCheckConnect(virAccessManagerPtr manager, + virAccessPermConnect av); +bool virAccessManagerCheckDomain(virAccessManagerPtr manager, + virDomainDefPtr def, + virAccessPermDomain av); + + +#endif /* __VIR_ACCESS_MANAGER_H__ */ diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c new file mode 100644 index 0000000..7ccded5 --- /dev/null +++ b/src/access/viraccessperm.c @@ -0,0 +1,37 @@ +/* + * viraccessperm.c: access control permissions + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include <config.h> + +#include "viraccessperm.h" + + +VIR_ENUM_IMPL(virAccessPermConnect, + VIR_ACCESS_PERM_CONNECT_LAST, + "getattr", "search_domains"); + +VIR_ENUM_IMPL(virAccessPermDomain, + VIR_ACCESS_PERM_DOMAIN_LAST, + "getattr", "read", "write", "read_secure", + "start", "stop", "save", "delete", + "shutdown", "reboot", "reset", + "migrate", "snapshot", "suspend", "hibernate", "core_dump", + "inject_nmi", "send_key", "read_block", "read_mem", + "open_graphics", "open_console", "screenshot"); diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h new file mode 100644 index 0000000..83b17ad --- /dev/null +++ b/src/access/viraccessperm.h @@ -0,0 +1,73 @@ +/* + * viraccessperm.h: access control permissions + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __VIR_ACCESS_PERM_H__ +# define __VIR_ACCESS_PERM_H__ + +# include "internal.h" +# include "util.h" + +typedef enum { + VIR_ACCESS_PERM_CONNECT_GETATTR, + VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS, + + VIR_ACCESS_PERM_CONNECT_LAST, +} virAccessPermConnect; + +typedef enum { + VIR_ACCESS_PERM_DOMAIN_GETATTR, /* Name/ID/UUID access */ + VIR_ACCESS_PERM_DOMAIN_READ, /* Config access */ + VIR_ACCESS_PERM_DOMAIN_WRITE, /* Config change */ + VIR_ACCESS_PERM_DOMAIN_READ_SECURE, + + VIR_ACCESS_PERM_DOMAIN_START, + VIR_ACCESS_PERM_DOMAIN_STOP, + + VIR_ACCESS_PERM_DOMAIN_SAVE, + VIR_ACCESS_PERM_DOMAIN_DELETE, + + /* Merge these 3 into 1 ? */ + VIR_ACCESS_PERM_DOMAIN_SHUTDOWN, + VIR_ACCESS_PERM_DOMAIN_REBOOT, + VIR_ACCESS_PERM_DOMAIN_RESET, + + VIR_ACCESS_PERM_DOMAIN_MIGRATE, + VIR_ACCESS_PERM_DOMAIN_SNAPSHOT, + VIR_ACCESS_PERM_DOMAIN_SUSPEND, + VIR_ACCESS_PERM_DOMAIN_HIBERNATE, + VIR_ACCESS_PERM_DOMAIN_CORE_DUMP, + + VIR_ACCESS_PERM_DOMAIN_INJECT_NMI, + VIR_ACCESS_PERM_DOMAIN_SEND_KEY, + + VIR_ACCESS_PERM_DOMAIN_READ_BLOCK, + VIR_ACCESS_PERM_DOMAIN_READ_MEM, + + VIR_ACCESS_PERM_DOMAIN_OPEN_GRAPHICS, + VIR_ACCESS_PERM_DOMAIN_OPEN_CONSOLE, + VIR_ACCESS_PERM_DOMAIN_SCREENSHOT, + + VIR_ACCESS_PERM_DOMAIN_LAST, +} virAccessPermDomain; + +VIR_ENUM_DECL(virAccessPermConnect); +VIR_ENUM_DECL(virAccessPermDomain); + +#endif /* __VIR_ACCESS_PERM_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 391c977..ca60eb3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1181,6 +1181,27 @@ virAuthConfigNew; virAuthConfigNewData; +# viraccessmanager.h +virAccessManagerInit; +virAccessManagerGetSystemIdentity; +virAccessManagerGetRealIdentity; +virAccessManagerGetEffectiveIdentity; +virAccessManagerSetRealIdentity; +virAccessManagerSetEffectiveIdentity; +virAccessManagerNew; +virAccessManagerNewStack; +virAccessManagerFree; +virAccessManagerCheckConnect; +virAccessManagerCheckDomain; + + +# viraccessvector.h +virAccessVectorConnectTypeFromString; +virAccessVectorConnectTypeToString; +virAccessVectorDomainTypeFromString; +virAccessVectorDomainTypeToString; + + # viraudit.h virAuditClose; virAuditEncode; diff --git a/src/util/virterror.c b/src/util/virterror.c index 33ef713..e60a008 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -187,6 +187,9 @@ static const char *virErrorDomainName(virErrorDomain domain) { case VIR_FROM_DBUS: dom = "DBus "; break; + case VIR_FROM_ACCESS: + dom = "Access Manager "; + break; } return dom; } @@ -1265,6 +1268,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("invalid identity pointer in %s"); break; + case VIR_ERR_ACCESS_DENIED: + if (info == NULL) + errmsg = _("access denied"); + else + errmsg = _("access denied: %s"); + break; } return errmsg; } -- 1.7.10

On 05/02/2012 05:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
A bit sparse on the commit message, maybe even mentioning that src/access/apis.txt contains the details would help. You are adding a new directory; do any of the cfg.mk syntax check rules need to be updated to state which other drivers can include the headers from this directory?
+++ b/src/access/apis.txt @@ -0,0 +1,577 @@
Copyright header? Also, can you add a short description about what the rest of this file is describing?
+ +Non-driver based APIs + + +virConnCopyLastError: +virResetError: +virResetLastError: +virSaveLastError: +virSetErrorFunc: +virConnGetLastError: +virConnResetLastError: +virConnSetErrorFunc: +virCopyLastError: +virDefaultErrorFunc: +virFreeError: +virGetLastError:
You know, we really ought to divide libvirt.h into multiple headers, one per object type, to make this a bit easier to track.
+ +virConnectBaselineCPU: + + - No state access + +virConnectCompareCPU: + + - Access host CPU
If I understand this file correctly, you were basically enumerating which APIs need which access controls. Does that mean that adding a new API to libvirt.h will now also require updating this file? Is there any way to write a syntax-check rule that ensures that all public functions also have an entry in this file, to make sure we don't forget?
+++ b/src/access/viraccessdriver.h
+++ b/src/access/viraccessdrivernop.c @@ -0,0 +1,44 @@ +/* + * viraccessdrivernop.c: no-op access control driver + * + * Copyright (C) 2011 Red Hat, Inc.
2012 now (wow, you've been working on this for some time now...)
+++ b/src/access/viraccessdrivernop.h @@ -0,0 +1,28 @@ +/* + * viraccessdrivernop.h: no-op access control driver + * + * Copyright (C) 2011 Red Hat, Inc.
2012
+++ b/src/access/viraccessdriverstack.c
+static bool +virAccessDriverStackCheckConnect(virAccessManagerPtr manager, + virAccessPermConnect av) +{ + virAccessDriverStackPrivatePtr priv = virAccessManagerGetPrivateData(manager); + bool ret = true; + size_t i; + + for (i = 0 ; i < priv->managersLen ; i++) { + /* We do not short-circuit on first denial - always check all drivers */ + if (!virAccessManagerCheckConnect(priv->managers[i], av)) + ret = false;
So anyone can deny, but all are given a shot at it in case the access manager has side effects such as auditing their own decision (but note that an audited success may be countermanded by a different manager's denial). Seems reasonable.
+++ b/src/access/viraccessmanager.c
+ +virIdentityPtr virAccessManagerGetSystemIdentity(void) +{ + char *username = NULL; + char *groupname = NULL; + char *seccontext = NULL; + virIdentityPtr ret = NULL; + gid_t gid = getgid(); + uid_t uid = getuid(); +#if HAVE_SELINUX + security_context_t con; +#endif + + if (!(username = virGetUserName(uid))) + goto cleanup; + if (!(groupname = virGetGroupName(gid))) + goto cleanup; + +#if HAVE_SELINUX + if (getcon(&con) < 0) { + virReportSystemError(errno, "%s", + _("Unable to lookup SELinux process context")); + goto cleanup;
Does this fail even if SELinux is not enabled? And if so, what is the impact of failing here?
+ } + seccontext = strdup(con); + freecon(con); + if (!seccontext) { + virReportOOMError(); + goto cleanup; + } +#endif + + if (!(ret = virIdentityNew())) + goto cleanup; + + if (username && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_USER_NAME, username) < 0) + goto error; + if (groupname && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_GROUP_NAME, groupname) < 0) + goto error; + if (seccontext && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_SECURITY_CONTEXT, seccontext) < 0) + goto error;
Especially since here, you seem to be catering to the case of a NULL seccontext when SELinux was not compiled in.
+int virAccessManagerSetEffectiveIdentity(virIdentityPtr identity) +{ + virIdentityPtr old; + + if (!virAccessManagerInit()) + return -1; + + old = virThreadLocalGet(&effectiveIdentity); + if (old) + virIdentityFree(old);
This loses the old identity,
+ + if (virThreadLocalSet(&effectiveIdentity, identity) < 0) { + virReportSystemError(errno, "%s", + _("Unable to set thread local identity")); + return -1;
even if we fail to set the new one. I think you need to swap these two cases (and keep old in force on failure to set a new identity).
+int virAccessManagerSetRealIdentity(virIdentityPtr identity) +{ + virIdentityPtr old; + + if (!virAccessManagerInit()) + return -1; + + old = virThreadLocalGet(&realIdentity); + if (old) + virIdentityFree(old); + + if (virThreadLocalSet(&realIdentity, identity) < 0) { + virReportSystemError(errno, "%s", + _("Unable to set thread local identity")); + return -1; + }
Same comment.
+void *virAccessManagerGetPrivateData(virAccessManagerPtr mgr) +{ + /* This accesses the memory just beyond mgr, which was allocated + * via VIR_ALLOC_VAR earlier. */ + return mgr + 1;
Phew - we learned our lesson with the security manager.
+ +typedef enum { + VIR_ACCESS_PERM_CONNECT_GETATTR, + VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS, + + VIR_ACCESS_PERM_CONNECT_LAST, +} virAccessPermConnect; + +typedef enum { + VIR_ACCESS_PERM_DOMAIN_GETATTR, /* Name/ID/UUID access */ + VIR_ACCESS_PERM_DOMAIN_READ, /* Config access */ + VIR_ACCESS_PERM_DOMAIN_WRITE, /* Config change */ + VIR_ACCESS_PERM_DOMAIN_READ_SECURE, + + VIR_ACCESS_PERM_DOMAIN_START, + VIR_ACCESS_PERM_DOMAIN_STOP, + + VIR_ACCESS_PERM_DOMAIN_SAVE, + VIR_ACCESS_PERM_DOMAIN_DELETE, + + /* Merge these 3 into 1 ? */ + VIR_ACCESS_PERM_DOMAIN_SHUTDOWN, + VIR_ACCESS_PERM_DOMAIN_REBOOT, + VIR_ACCESS_PERM_DOMAIN_RESET,
Maybe not. While reboot can reuse an existing qemu process, shutdown/start will create a new qemu process, and there may be ramifications to that difference (such as whether a vnc session to qemu has to reconnect). Reboot and Reset might be mergeable, but I think I've made a case for keeping shutdown separate.
+++ b/src/libvirt_private.syms @@ -1181,6 +1181,27 @@ virAuthConfigNew; virAuthConfigNewData;
+# viraccessmanager.h
Sorting: viraccessmanager.h comes before virauth.h, and...
+virAccessManagerInit; +virAccessManagerGetSystemIdentity; +virAccessManagerGetRealIdentity; +virAccessManagerGetEffectiveIdentity; +virAccessManagerSetRealIdentity; +virAccessManagerSetEffectiveIdentity; +virAccessManagerNew; +virAccessManagerNewStack; +virAccessManagerFree; +virAccessManagerCheckConnect; +virAccessManagerCheckDomain;
within the file, sort the function names. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 05/02/2012 07:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
--- include/libvirt/virterror.h | 3 + po/POTFILES.in | 1 + src/Makefile.am | 16 + src/access/apis.txt | 577 +++++++++++++++++++++++++++++++++++++ src/access/viraccessdriver.h | 51 ++++ src/access/viraccessdrivernop.c | 44 +++ src/access/viraccessdrivernop.h | 28 ++ src/access/viraccessdriverstack.c | 105 +++++++ src/access/viraccessdriverstack.h | 32 ++
Does it make sense to split viraccessmanager code into a separate patch?
src/access/viraccessmanager.c | 338 ++++++++++++++++++++++ src/access/viraccessmanager.h | 56 ++++ src/access/viraccessperm.c | 37 +++ src/access/viraccessperm.h | 73 +++++ src/libvirt_private.syms | 21 ++ src/util/virterror.c | 9 + 15 files changed, 1391 insertions(+) create mode 100644 src/access/apis.txt create mode 100644 src/access/viraccessdriver.h create mode 100644 src/access/viraccessdrivernop.c create mode 100644 src/access/viraccessdrivernop.h create mode 100644 src/access/viraccessdriverstack.c create mode 100644 src/access/viraccessdriverstack.h create mode 100644 src/access/viraccessmanager.c create mode 100644 src/access/viraccessmanager.h create mode 100644 src/access/viraccessperm.c create mode 100644 src/access/viraccessperm.h
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index e44390e..1daba04 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -88,6 +88,7 @@ typedef enum { VIR_FROM_URI = 45, /* Error from URI handling */ VIR_FROM_AUTH = 46, /* Error from auth handling */ VIR_FROM_DBUS = 47, /* Error from DBus */ + VIR_FROM_ACCESS = 48, /* Error from access control manager */ } virErrorDomain;
@@ -252,6 +253,8 @@ typedef enum { VIR_ERR_OVERFLOW = 82, /* integer overflow */ VIR_ERR_BLOCK_COPY_ACTIVE = 83, /* action prevented by block copy job */ VIR_ERR_INVALID_IDENTITY = 84, /* Invalid identity pointer */ + VIR_ERR_ACCESS_DENIED = 85, /* operation on the object/resource + was denied */ } virErrorNumber;
/** diff --git a/po/POTFILES.in b/po/POTFILES.in index 4ea544b..f898887 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -6,6 +6,7 @@ daemon/remote_dispatch.h daemon/stream.c gnulib/lib/gai_strerror.c gnulib/lib/regcomp.c +src/access/viraccessmanager.c src/conf/cpu_conf.c src/conf/domain_conf.c src/conf/domain_event.c diff --git a/src/Makefile.am b/src/Makefile.am index e48dfa5..0293562 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -531,6 +531,13 @@ SECURITY_DRIVER_APPARMOR_SOURCES = \ security/security_apparmor.h security/security_apparmor.c
+ACCESS_DRIVER_SOURCES = \ + access/viraccessperm.h access/viraccessperm.c \ + access/viraccessmanager.h access/viraccessmanager.c \ + access/viraccessdriver.h \ + access/viraccessdrivernop.h access/viraccessdrivernop.c \ + access/viraccessdriverstack.h access/viraccessdriverstack.c + NODE_DEVICE_DRIVER_SOURCES = \ node_device/node_device_driver.c \ node_device/node_device_driver.h \ @@ -1115,6 +1122,15 @@ libvirt_driver_security_la_CFLAGS += $(APPARMOR_CFLAGS) libvirt_driver_security_la_LIBADD += $(APPARMOR_LIBS) endif
+libvirt_driver_access_la_SOURCES = $(ACCESS_DRIVER_SOURCES) +noinst_LTLIBRARIES += libvirt_driver_access.la +libvirt_la_BUILT_LIBADD += libvirt_driver_access.la +libvirt_driver_access_la_CFLAGS = \ + -I@top_srcdir@/src/conf $(AM_CFLAGS) +libvirt_driver_access_la_LDFLAGS = $(AM_LDFLAGS) +libvirt_driver_access_la_LIBADD = + + # Add all conditional sources just in case... EXTRA_DIST += \ $(TEST_DRIVER_SOURCES) \ diff --git a/src/access/apis.txt b/src/access/apis.txt new file mode 100644 index 0000000..16cc49c --- /dev/null +++ b/src/access/apis.txt @@ -0,0 +1,577 @@ + +Non-driver based APIs + + +virConnCopyLastError: +virResetError: +virResetLastError: +virSaveLastError: +virSetErrorFunc: +virConnGetLastError: +virConnResetLastError: +virConnSetErrorFunc: +virCopyLastError: +virDefaultErrorFunc: +virFreeError: +virGetLastError: + +virInitialize: +virConnectClose: +virConnectGetLibVersion: +virGetVersion: +virConnectGetVersion: +virConnectGetType: +virConnectGetURI: + + +virConnectRef: +virDomainRef: +virInterfaceRef: +virNetworkRef: +virNodeDeviceRef: +virNWFilterRef: +virSecretRef: +virStoragePoolRef: +virStorageVolRef: +virStreamRef: + +virIdentityFree: +virDomainFree: +virDomainSnapshotFree: +virInterfaceFree: +virNetworkFree: +virNodeDeviceFree: +virNWFilterFree: +virSecretFree: +virStoragePoolFree: +virStorageVolFree: +virStreamFree: + +virDomainGetConnect: +virInterfaceGetConnect: +virDomainSnapshotGetConnect: +virNetworkGetConnect: +virSecretGetConnect: +virStoragePoolGetConnect: +virStorageVolGetConnect: +virDomainSnapshotGetDomain: + +virEventAddHandle: +virEventAddTimeout: +virEventRegisterDefaultImpl: +virEventRegisterImpl: +virEventRemoveHandle: +virEventRemoveTimeout: +virEventRunDefaultImpl: +virEventUpdateHandle: +virEventUpdateTimeout: + + +virConnectBaselineCPU: + + - No state access + +virConnectCompareCPU: + + - Access host CPU + +virConnectGetCapabilities: + + - Access host CPU, emulators, NUMA + +virConnectGetHostname: + + - hostname resolve + +virConnectGetIdentity: + + - No state + +virConnectGetMaxVcpus: + + - Hypercall + +virConnectGetSysinfo: + + - Sysfs / dmidecode ? (cached from capabilities) + +virConnectIsAlive: + + - Driver check + +virConnectIsEncrypted: +virConnectIsSecure: + + - Property lookup + +virConnectOpen: +virConnectOpenAuth: +virConnectOpenReadOnly: + + - RPC layer + +virConnectSetIdentity: + + - RPC layer + +virConnectSetKeepAlive: + + - RPC layer + +virNodeGetCellsFreeMemory: + + - NUMA props + +virNodeGetCPUStats: + + - Cgroups + +virNodeGetFreeMemory: + + - NUMA props + +virNodeGetInfo: + + - NUMA / sysfs + +virNodeGetMemoryStats: + + - CGroups + +virNodeGetSecurityModel: + + - Capabilities + +virNodeSuspendForDuration: + + - PM utils invoke + + + +virConnectNumOfDefinedDomains: +virConnectNumOfDomains: +virConnectListDefinedDomains: +virConnectListDomains: + + - 'search_domains' on libvirtd + - 'getattr' on each domain + + +virConnectDomainEventDeregister: +virConnectDomainEventDeregisterAny: +virConnectDomainEventRegister: +virConnectDomainEventRegisterAny: + + - 'monitor' on domain + +virConnectDomainXMLFromNative: +virConnectDomainXMLToNative: + + - 'domain_xml' on libvirtd + +virDomainAbortJob: + + - 'abort_job' + +virDomainBlockJobAbort: + + - 'abort_block_job' + +virDomainBlockJobSetSpeed: + + - 'setattr_block_job' + +virDomainBlockPeek: + + - 'block_peek' + +virDomainBlockPull: + + - 'block_pull' + - 'create_block_job' + +virDomainBlockResize: + + - 'block_resize' + +virDomainBlockStats: +virDomainBlockStatsFlags: + + - 'read' + +virDomainCoreDump: + + - 'coredump' + +virDomainCreate: +virDomainCreateLinux: +virDomainCreateWithFlags: + + - 'start' + +virDomainCreateXML: + + - 'start' + 'write' + +virDomainDefineXML: + + - 'save' + 'write' + +virDomainDestroy: +virDomainDestroyFlags: + + - 'stop' + + +virDomainGetID: +virDomainGetName: +virDomainGetUUID: +virDomainGetUUIDString: +virDomainLookupByUUIDString: + + - Outside driver + +virDomainLookupByID: +virDomainLookupByName: +virDomainLookupByUUID: + + - getattr + +virDomainGetAutostart: +virDomainGetBlkioParameters: +virDomainGetBlockInfo: +virDomainGetBlockIoTune: +virDomainGetBlockJobInfo: +virDomainGetControlInfo: +virDomainGetInfo: +virDomainGetInterfaceParameters: +virDomainGetJobInfo: +virDomainGetMaxMemory: +virDomainGetMaxVcpus: +virDomainGetMemoryParameters: +virDomainGetNumaParameters: +virDomainGetOSType: +virDomainGetSchedulerParameters: +virDomainGetSchedulerParametersFlags: +virDomainGetSchedulerType: +virDomainGetSecurityLabel: +virDomainGetState: +virDomainGetVcpuPinInfo: +virDomainGetVcpus: +virDomainGetVcpusFlags: +virDomainGetXMLDesc: +virDomainHasCurrentSnapshot: +virDomainHasManagedSaveImage: +virDomainInterfaceStats: +virDomainIsActive: +virDomainIsPersistent: +virDomainIsUpdated: +virDomainMemoryStats: + + - 'read' + +virDomainInjectNMI: + + - inject_nmi + + +virDomainManagedSave: + + - save_create + +virDomainManagedSaveRemove: + + - save_delete + +virDomainMemoryPeek: + + - memory_peek + + +virDomainMigrate: +virDomainMigrate2: +virDomainMigrateGetMaxSpeed: +virDomainMigrateSetMaxDowntime: +virDomainMigrateSetMaxSpeed: +virDomainMigrateToURI: +virDomainMigrateToURI2: + + - migrate + +virDomainOpenConsole: + + - open_console + +virDomainOpenGraphics: + + - open_graphics + +virDomainPinVcpu: +virDomainPinVcpuFlags: + + - write ? + +virDomainReboot: + + - reboot + +virDomainReset: + + - reset + +virDomainRestore: +virDomainRestoreFlags: + + - restore + - start + +virDomainResume: + + - resume + +virDomainRevertToSnapshot: + + - snapshot_revert + +virDomainSave: +virDomainSaveFlags: + + - stop + - save + +virDomainSaveImageDefineXML: + + - save_write (setattr ?) + +virDomainSaveImageGetXMLDesc: + + - save_getattr + + +virDomainScreenshot: + + - screenshot + +virDomainSendKey: + + - sendkey + +virDomainSetAutostart: +virDomainSetBlkioParameters: +virDomainSetBlockIoTune: +virDomainSetInterfaceParameters: +virDomainSetMaxMemory: +virDomainSetMemory: +virDomainSetMemoryFlags: +virDomainSetMemoryParameters: +virDomainSetNumaParameters: +virDomainSetSchedulerParameters: +virDomainSetSchedulerParametersFlags: +virDomainSetVcpus: +virDomainSetVcpusFlags: +virDomainAttachDevice: +virDomainAttachDeviceFlags: +virDomainUpdateDeviceFlags: +virDomainDetachDevice: +virDomainDetachDeviceFlags: + + + - write (+ possible save) + +virDomainShutdown: + + - shutdown + +virDomainSnapshotCreateXML: + + - snapshot_create + +virDomainSnapshotCurrent: + + - snapshot_getattr (or getattr ?) + +virDomainSnapshotDelete: + + - snapshot_delete + +virDomainSnapshotGetName: +virDomainSnapshotGetParent: +virDomainSnapshotGetXMLDesc: + + - snapshot_getattr + +virDomainSnapshotListChildrenNames: +virDomainSnapshotListNames: + + - snapshot_search + - Filter on snapshot_getattr + +virDomainSnapshotLookupByName: + + - snapshot_getattr + +virDomainSnapshotNum: +virDomainSnapshotNumChildren: + + - snapshot_search + - Filter on snapshot_getattr + +virDomainSuspend: + + - suspend + +virDomainUndefine: +virDomainUndefineFlags: + + - delete + + + +virConnectNumOfDefinedInterfaces: +virConnectNumOfInterfaces: +virConnectListDefinedInterfaces: +virConnectListInterfaces: +virInterfaceChangeBegin: +virInterfaceChangeCommit: +virInterfaceChangeRollback: +virInterfaceCreate: +virInterfaceDefineXML: +virInterfaceDestroy: +virInterfaceGetMACString: +virInterfaceGetName: +virInterfaceGetXMLDesc: +virInterfaceIsActive: +virInterfaceLookupByMACString: +virInterfaceLookupByName: +virInterfaceUndefine: + + +virConnectNumOfDefinedNetworks: +virConnectNumOfNetworks: +virConnectListDefinedNetworks: +virConnectListNetworks: +virNetworkCreate: +virNetworkCreateXML: +virNetworkDefineXML: +virNetworkDestroy: +virNetworkGetAutostart: +virNetworkGetBridgeName: +virNetworkGetName: +virNetworkGetUUID: +virNetworkGetUUIDString: +virNetworkGetXMLDesc: +virNetworkIsActive: +virNetworkIsPersistent: +virNetworkLookupByName: +virNetworkLookupByUUID: +virNetworkLookupByUUIDString: +virNetworkSetAutostart: +virNetworkUndefine: + + + +virNodeDeviceCreateXML: +virNodeDeviceDestroy: +virNodeDeviceDettach: +virNodeDeviceGetName: +virNodeDeviceGetParent: +virNodeDeviceGetXMLDesc: +virNodeDeviceListCaps: +virNodeDeviceLookupByName: +virNodeDeviceNumOfCaps: +virNodeDeviceReAttach: +virNodeDeviceReset: +virNodeListDevices: +virNodeNumOfDevices: + + + +virConnectNumOfNWFilters: +virConnectListNWFilters: +virNWFilterDefineXML: +virNWFilterGetName: +virNWFilterGetUUID: +virNWFilterGetUUIDString: +virNWFilterGetXMLDesc: +virNWFilterLookupByName: +virNWFilterLookupByUUID: +virNWFilterLookupByUUIDString: +virNWFilterUndefine: + + + +virConnectNumOfSecrets: +virConnectListSecrets: +virSecretDefineXML: +virSecretGetUsageID: +virSecretGetUsageType: +virSecretGetUUID: +virSecretGetUUIDString: +virSecretGetValue: +virSecretGetXMLDesc: +virSecretLookupByUsage: +virSecretLookupByUUID: +virSecretLookupByUUIDString: +virSecretSetValue: +virSecretUndefine: + + + +virConnectNumOfDefinedStoragePools: +virConnectNumOfStoragePools: +virConnectListDefinedStoragePools: +virConnectListStoragePools: +virConnectFindStoragePoolSources: +virStoragePoolBuild: +virStoragePoolCreate: +virStoragePoolCreateXML: +virStoragePoolDefineXML: +virStoragePoolDelete: +virStoragePoolDestroy: +virStoragePoolGetAutostart: +virStoragePoolGetInfo: +virStoragePoolGetName: +virStoragePoolGetUUID: +virStoragePoolGetUUIDString: +virStoragePoolGetXMLDesc: +virStoragePoolIsActive: +virStoragePoolIsPersistent: +virStoragePoolListVolumes: +virStoragePoolLookupByName: +virStoragePoolLookupByUUID: +virStoragePoolLookupByUUIDString: +virStoragePoolLookupByVolume: +virStoragePoolNumOfVolumes: +virStoragePoolRefresh: +virStoragePoolSetAutostart: +virStoragePoolUndefine: + + + +virStorageVolCreateXML: +virStorageVolCreateXMLFrom: +virStorageVolDelete: +virStorageVolDownload: +virStorageVolGetInfo: +virStorageVolGetKey: +virStorageVolGetName: +virStorageVolGetPath: +virStorageVolGetXMLDesc: +virStorageVolLookupByKey: +virStorageVolLookupByName: +virStorageVolLookupByPath: +virStorageVolUpload: +virStorageVolWipe: + + + +virStreamAbort: +virStreamEventAddCallback: +virStreamEventRemoveCallback: +virStreamEventUpdateCallback: +virStreamFinish: +virStreamNew: +virStreamRecv: +virStreamRecvAll: +virStreamSend: +virStreamSendAll: diff --git a/src/access/viraccessdriver.h b/src/access/viraccessdriver.h new file mode 100644 index 0000000..ae09162 --- /dev/null +++ b/src/access/viraccessdriver.h @@ -0,0 +1,51 @@ +/* + * viraccessdriver.h: access control driver + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __VIR_ACCESS_DRIVER_H__ +# define __VIR_ACCESS_DRIVER_H__ + +# include "conf/domain_conf.h" +# include "access/viraccessmanager.h" + +typedef bool (*virAccessDriverCheckConnectDrv)(virAccessManagerPtr manager, + virAccessPermConnect av); +typedef bool (*virAccessDriverCheckDomainDrv)(virAccessManagerPtr manager, + virDomainDefPtr def, + virAccessPermDomain av); + +typedef int (*virAccessDriverSetupDrv)(virAccessManagerPtr manager); +typedef void (*virAccessDriverCleanupDrv)(virAccessManagerPtr manager); + +typedef struct _virAccessDriver virAccessDriver; +typedef virAccessDriver *virAccessDriverPtr; + +struct _virAccessDriver { + size_t privateDataLen; + const char *name; + + virAccessDriverSetupDrv setup; + virAccessDriverCleanupDrv cleanup; + + virAccessDriverCheckConnectDrv checkConnect; + virAccessDriverCheckDomainDrv checkDomain; +}; + + +#endif /* __VIR_ACCESS_DRIVER_H__ */ diff --git a/src/access/viraccessdrivernop.c b/src/access/viraccessdrivernop.c new file mode 100644 index 0000000..7ba0719 --- /dev/null +++ b/src/access/viraccessdrivernop.c @@ -0,0 +1,44 @@ +/* + * viraccessdrivernop.c: no-op access control driver + * + * Copyright (C) 2011 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include <config.h> + +#include "access/viraccessdrivernop.h" + +static bool +virAccessDriverNopCheckConnect(virAccessManagerPtr manager ATTRIBUTE_UNUSED, + virAccessPermConnect av ATTRIBUTE_UNUSED) +{ + return true; +} + +static bool +virAccessDriverNopCheckDomain(virAccessManagerPtr manager ATTRIBUTE_UNUSED, + virDomainDefPtr def ATTRIBUTE_UNUSED, + virAccessPermDomain av ATTRIBUTE_UNUSED) +{ + return true; +} + +virAccessDriver accessDriverNop = { + .name = "none", + .checkConnect = virAccessDriverNopCheckConnect, + .checkDomain = virAccessDriverNopCheckDomain, +}; diff --git a/src/access/viraccessdrivernop.h b/src/access/viraccessdrivernop.h new file mode 100644 index 0000000..a3d9be3 --- /dev/null +++ b/src/access/viraccessdrivernop.h @@ -0,0 +1,28 @@ +/* + * viraccessdrivernop.h: no-op access control driver + * + * Copyright (C) 2011 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __VIR_ACCESS_DRIVER_NOP_H__ +# define __VIR_ACCESS_DRIVER_NOP_H__ + +# include "access/viraccessdriver.h" + +extern virAccessDriver accessDriverNop; + +#endif /* __VIR_ACCESS_DRIVER_NOP_H__ */ diff --git a/src/access/viraccessdriverstack.c b/src/access/viraccessdriverstack.c new file mode 100644 index 0000000..48aaafd --- /dev/null +++ b/src/access/viraccessdriverstack.c @@ -0,0 +1,105 @@ +/* + * viraccessdriverstack.c: stacked access control driver + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include <config.h> + +#include "access/viraccessdriverstack.h" +#include "memory.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_ACCESS + +typedef struct _virAccessDriverStackPrivate virAccessDriverStackPrivate; +typedef virAccessDriverStackPrivate *virAccessDriverStackPrivatePtr; + +struct _virAccessDriverStackPrivate { + virAccessManagerPtr *managers; + size_t managersLen; +};
Could this be called _virAccessManagerStackPrivate? See my later comments and this may make more sense.
+ + +int virAccessDriverStackAppend(virAccessManagerPtr manager, + virAccessManagerPtr child) +{ + virAccessDriverStackPrivatePtr priv = virAccessManagerGetPrivateData(manager);
This is a driver that is calling access manager functions, which doesn't seem right. It seems like the access manager should be able to call down to drivers but not the other way around.
+ + if (VIR_EXPAND_N(priv->managers, priv->managersLen, 1) < 0) { + virReportOOMError(); + return -1; + } + + priv->managers[priv->managersLen-1] = child; + + return 0; +} + + +static void virAccessDriverStackCleanup(virAccessManagerPtr manager) +{ + virAccessDriverStackPrivatePtr priv = virAccessManagerGetPrivateData(manager); + size_t i; + + for (i = 0 ; i < priv->managersLen ; i++) { + virAccessManagerFree(priv->managers[i]); + } + VIR_FREE(priv->managers); +} + + +static bool +virAccessDriverStackCheckConnect(virAccessManagerPtr manager, + virAccessPermConnect av) +{ + virAccessDriverStackPrivatePtr priv = virAccessManagerGetPrivateData(manager); + bool ret = true; + size_t i; + + for (i = 0 ; i < priv->managersLen ; i++) { + /* We do not short-circuit on first denial - always check all drivers */ + if (!virAccessManagerCheckConnect(priv->managers[i], av)) + ret = false; + } + + return ret; +} + +static bool +virAccessDriverStackCheckDomain(virAccessManagerPtr manager, + virDomainDefPtr def, + virAccessPermDomain av) +{ + virAccessDriverStackPrivatePtr priv = virAccessManagerGetPrivateData(manager); + bool ret = true; + size_t i; + + for (i = 0 ; i < priv->managersLen ; i++) { + /* We do not short-circuit on first denial - always check all drivers */ + if (!virAccessManagerCheckDomain(priv->managers[i], def, av)) + ret = false; + } + + return ret; +} + +virAccessDriver accessDriverStack = {
I noticed that virAccessDriverStackAppend() is called from the access manager layer. At first this made me think there should be an assignment to '.append = virAccessDriverStackAppend' here instead of hard-coding the call in the access manager layer. Or even better, does the driver stack really need to be a driver? Would it make more sense to implement the driver stack entirely in the virAccessManager layer? I think virAccessManagerCheckConnect and virAccessManagerCheckDomain would then have to account for the stack loops.
+ .cleanup = virAccessDriverStackCleanup, + .checkConnect = virAccessDriverStackCheckConnect, + .checkDomain = virAccessDriverStackCheckDomain, +}; diff --git a/src/access/viraccessdriverstack.h b/src/access/viraccessdriverstack.h new file mode 100644 index 0000000..8ab3a0d --- /dev/null +++ b/src/access/viraccessdriverstack.h @@ -0,0 +1,32 @@ +/* + * viraccessdriverstack.h: stacked access control driver + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __VIR_ACCESS_DRIVER_STACK_H__ +# define __VIR_ACCESS_DRIVER_STACK_H__ + +# include "access/viraccessdriver.h" + + +int virAccessDriverStackAppend(virAccessManagerPtr manager, + virAccessManagerPtr child); + +extern virAccessDriver accessDriverStack; + +#endif /* __VIR_ACCESS_DRIVER_STACK_H__ */ diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c new file mode 100644 index 0000000..4e77bd6 --- /dev/null +++ b/src/access/viraccessmanager.c @@ -0,0 +1,338 @@ +/* + * viraccessmanager.c: access control manager + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include <config.h> + +#include "viraccessmanager.h" +#include "memory.h" +#include "virterror_internal.h" +#include "threads.h" +#if HAVE_SELINUX +# include <selinux/selinux.h> +#endif +#include "access/viraccessdrivernop.h" +#include "access/viraccessdriverstack.h" +#include "logging.h" + +#define VIR_FROM_THIS VIR_FROM_ACCESS +#define virAccessError(code, ...) \ + virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \ + __FUNCTION__, __LINE__, __VA_ARGS__) + +static volatile bool onceInitErr = false; +static virOnceControl onceInit = VIR_ONCE_CONTROL_INITIALIZER; +static virThreadLocal realIdentity; +static virThreadLocal effectiveIdentity; + +struct _virAccessManager { + virAccessDriverPtr drv; +}; + + +static void virAccessManagerOnceInit(void) +{ + if (virThreadLocalInit(&realIdentity, + (virThreadLocalCleanup)virIdentityFree) < 0) + onceInitErr = true; + if (virThreadLocalInit(&effectiveIdentity, + (virThreadLocalCleanup)virIdentityFree) < 0) + onceInitErr = true; +} + +static bool virAccessManagerInit(void) +{ + if (virOnce(&onceInit, virAccessManagerOnceInit) < 0 || + onceInitErr) { + virReportSystemError(errno, "%s", + _("Failed to initialize access manager")); + return false; + } + + return true; +} + +virIdentityPtr virAccessManagerGetSystemIdentity(void) +{ + char *username = NULL; + char *groupname = NULL; + char *seccontext = NULL; + virIdentityPtr ret = NULL; + gid_t gid = getgid(); + uid_t uid = getuid(); +#if HAVE_SELINUX + security_context_t con; +#endif + + if (!(username = virGetUserName(uid))) + goto cleanup; + if (!(groupname = virGetGroupName(gid))) + goto cleanup; + +#if HAVE_SELINUX + if (getcon(&con) < 0) { + virReportSystemError(errno, "%s", + _("Unable to lookup SELinux process context")); + goto cleanup; + } + seccontext = strdup(con); + freecon(con); + if (!seccontext) { + virReportOOMError(); + goto cleanup; + } +#endif + + if (!(ret = virIdentityNew())) + goto cleanup; + + if (username && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_USER_NAME, username) < 0) + goto error; + if (groupname && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_GROUP_NAME, groupname) < 0) + goto error; + if (seccontext && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_SECURITY_CONTEXT, seccontext) < 0) + goto error; + +cleanup: + VIR_FREE(username); + VIR_FREE(groupname); + VIR_FREE(seccontext); + return ret; + +error:
Is there a leak of username, groupname, or seccontext if you goto error?
+ virIdentityFree(ret); + ret = NULL; + goto cleanup; +} + +virIdentityPtr virAccessManagerGetEffectiveIdentity(void) +{ + virIdentityPtr ret; + + if (!virAccessManagerInit()) + return NULL; + + ret = virThreadLocalGet(&effectiveIdentity); + virIdentityRef(ret); + return ret; +} + +int virAccessManagerSetEffectiveIdentity(virIdentityPtr identity) +{ + virIdentityPtr old; + + if (!virAccessManagerInit()) + return -1; + + old = virThreadLocalGet(&effectiveIdentity); + if (old) + virIdentityFree(old); + + if (virThreadLocalSet(&effectiveIdentity, identity) < 0) { + virReportSystemError(errno, "%s", + _("Unable to set thread local identity")); + return -1; + } + + if (identity) + virIdentityRef(identity); + return 0; +} + +virIdentityPtr virAccessManagerGetRealIdentity(void) +{ + virIdentityPtr ret; + + if (!virAccessManagerInit()) + return NULL; + + ret = virThreadLocalGet(&realIdentity); + virIdentityRef(ret); + return ret; +} + +int virAccessManagerSetRealIdentity(virIdentityPtr identity) +{ + virIdentityPtr old; + + if (!virAccessManagerInit()) + return -1; + + old = virThreadLocalGet(&realIdentity); + if (old) + virIdentityFree(old); + + if (virThreadLocalSet(&realIdentity, identity) < 0) { + virReportSystemError(errno, "%s", + _("Unable to set thread local identity")); + return -1; + } + + if (identity) + virIdentityRef(identity); + return 0; +} + + +static virAccessManagerPtr virAccessManagerNewDriver(virAccessDriverPtr drv) +{ + virAccessManagerPtr mgr; + + if (VIR_ALLOC_VAR(mgr, char, drv->privateDataLen) < 0) { + virReportOOMError(); + return NULL; + } + + mgr->drv = drv; + + if (mgr->drv->setup &&
Is it safe to assume mgr->drv is valid and not a bad pointer? There are a few more places with similar code elsewhere in this patch.
+ mgr->drv->setup(mgr) < 0) { + VIR_FREE(mgr); + return NULL; + } + + return mgr; +} + + +static virAccessDriverPtr accessDrivers[] = { + &accessDriverNop, +}; + + +static virAccessDriverPtr virAccessManagerFindDriver(const char *name) +{ + size_t i; + for (i = 0 ; i < ARRAY_CARDINALITY(accessDrivers) ; i++) { + if (STREQ(name, accessDrivers[i]->name)) + return accessDrivers[i]; + } + + return NULL; +} + + +virAccessManagerPtr virAccessManagerNew(const char *name) +{ + virAccessDriverPtr drv = virAccessManagerFindDriver(name); + if (!drv) + return NULL; + + return virAccessManagerNewDriver(drv); +} + + +virAccessManagerPtr virAccessManagerNewStack(const char **names, + size_t namesLen) +{ + virAccessManagerPtr manager = virAccessManagerNewDriver(&accessDriverStack); + size_t i; + + if (!manager) + return NULL; + + for (i = 0 ; i < namesLen ; i++) { + virAccessManagerPtr child = virAccessManagerNew(names[i]); + + if (!child) + goto error; + + if (virAccessDriverStackAppend(manager, child) < 0) { + virAccessManagerFree(child); + goto error; + } + } + + return manager; + +error: + virAccessManagerFree(manager); + return NULL; +} + + +void *virAccessManagerGetPrivateData(virAccessManagerPtr mgr) +{ + /* This accesses the memory just beyond mgr, which was allocated + * via VIR_ALLOC_VAR earlier. */ + return mgr + 1; +} + + +void virAccessManagerFree(virAccessManagerPtr mgr) +{ + if (!mgr) + return; + + if (mgr->drv->cleanup) + mgr->drv->cleanup(mgr); + + VIR_FREE(mgr); +} + + +/* Standard security practice is to not tell the caller *why* + * they were denied access. So this method takes the real + * libvirt errors & replaces it with a generic error. Fortunately + * the daemon logs will still contain the original error message + * should the admin need to debug things + */ +static bool +virAccessManagerSanitizeError(bool ret) +{ + if (!ret) { + virResetLastError(); + virAccessError(VIR_ERR_ACCESS_DENIED, NULL); + } + + return ret; +} + +bool virAccessManagerCheckConnect(virAccessManagerPtr manager, + virAccessPermConnect av) +{ + bool ret = true; + VIR_DEBUG("manager=%p driver=%s av=%d", + manager, manager->drv->name, av); + + if (manager->drv->checkConnect && + !manager->drv->checkConnect(manager, av)) + ret = false; + + return virAccessManagerSanitizeError(ret); +} + + +bool virAccessManagerCheckDomain(virAccessManagerPtr manager, + virDomainDefPtr def, + virAccessPermDomain av) +{ + bool ret = true; + VIR_DEBUG("manager=%p driver=%s def=%p av=%d", + manager, manager->drv->name, def, av); + + if (manager->drv->checkDomain && + !manager->drv->checkDomain(manager, def, av)) + ret = false; + + return virAccessManagerSanitizeError(ret); +} diff --git a/src/access/viraccessmanager.h b/src/access/viraccessmanager.h new file mode 100644 index 0000000..b7e19b5 --- /dev/null +++ b/src/access/viraccessmanager.h @@ -0,0 +1,56 @@ +/* + * viraccessmanager.h: access control manager + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __VIR_ACCESS_MANAGER_H__ +# define __VIR_ACCESS_MANAGER_H__ + +# include "rpc/virnetserverclient.h" +# include "conf/domain_conf.h" +# include "access/viraccessperm.h" + +virIdentityPtr virAccessManagerGetClientIdentity(virNetServerClientPtr client); +virIdentityPtr virAccessManagerGetSystemIdentity(void); + +virIdentityPtr virAccessManagerGetEffectiveIdentity(void); +int virAccessManagerSetEffectiveIdentity(virIdentityPtr identity); + +virIdentityPtr virAccessManagerGetRealIdentity(void); +int virAccessManagerSetRealIdentity(virIdentityPtr identity); + +typedef struct _virAccessManager virAccessManager; +typedef virAccessManager *virAccessManagerPtr; + +virAccessManagerPtr virAccessManagerNew(const char *name); +virAccessManagerPtr virAccessManagerNewStack(const char **names, + size_t namesLen); + + +void *virAccessManagerGetPrivateData(virAccessManagerPtr manager); +void virAccessManagerFree(virAccessManagerPtr manager); + + +bool virAccessManagerCheckConnect(virAccessManagerPtr manager, + virAccessPermConnect av); +bool virAccessManagerCheckDomain(virAccessManagerPtr manager, + virDomainDefPtr def, + virAccessPermDomain av); + + +#endif /* __VIR_ACCESS_MANAGER_H__ */ diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c new file mode 100644 index 0000000..7ccded5 --- /dev/null +++ b/src/access/viraccessperm.c @@ -0,0 +1,37 @@ +/* + * viraccessperm.c: access control permissions + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include <config.h> + +#include "viraccessperm.h" + + +VIR_ENUM_IMPL(virAccessPermConnect, + VIR_ACCESS_PERM_CONNECT_LAST, + "getattr", "search_domains"); + +VIR_ENUM_IMPL(virAccessPermDomain, + VIR_ACCESS_PERM_DOMAIN_LAST, + "getattr", "read", "write", "read_secure", + "start", "stop", "save", "delete", + "shutdown", "reboot", "reset", + "migrate", "snapshot", "suspend", "hibernate", "core_dump", + "inject_nmi", "send_key", "read_block", "read_mem", + "open_graphics", "open_console", "screenshot"); diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h new file mode 100644 index 0000000..83b17ad --- /dev/null +++ b/src/access/viraccessperm.h @@ -0,0 +1,73 @@ +/* + * viraccessperm.h: access control permissions + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __VIR_ACCESS_PERM_H__ +# define __VIR_ACCESS_PERM_H__ + +# include "internal.h" +# include "util.h" + +typedef enum { + VIR_ACCESS_PERM_CONNECT_GETATTR, + VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS, + + VIR_ACCESS_PERM_CONNECT_LAST, +} virAccessPermConnect; + +typedef enum { + VIR_ACCESS_PERM_DOMAIN_GETATTR, /* Name/ID/UUID access */ + VIR_ACCESS_PERM_DOMAIN_READ, /* Config access */ + VIR_ACCESS_PERM_DOMAIN_WRITE, /* Config change */ + VIR_ACCESS_PERM_DOMAIN_READ_SECURE, + + VIR_ACCESS_PERM_DOMAIN_START, + VIR_ACCESS_PERM_DOMAIN_STOP, + + VIR_ACCESS_PERM_DOMAIN_SAVE, + VIR_ACCESS_PERM_DOMAIN_DELETE, + + /* Merge these 3 into 1 ? */ + VIR_ACCESS_PERM_DOMAIN_SHUTDOWN, + VIR_ACCESS_PERM_DOMAIN_REBOOT, + VIR_ACCESS_PERM_DOMAIN_RESET, + + VIR_ACCESS_PERM_DOMAIN_MIGRATE, + VIR_ACCESS_PERM_DOMAIN_SNAPSHOT, + VIR_ACCESS_PERM_DOMAIN_SUSPEND, + VIR_ACCESS_PERM_DOMAIN_HIBERNATE, + VIR_ACCESS_PERM_DOMAIN_CORE_DUMP, + + VIR_ACCESS_PERM_DOMAIN_INJECT_NMI, + VIR_ACCESS_PERM_DOMAIN_SEND_KEY, + + VIR_ACCESS_PERM_DOMAIN_READ_BLOCK, + VIR_ACCESS_PERM_DOMAIN_READ_MEM, + + VIR_ACCESS_PERM_DOMAIN_OPEN_GRAPHICS, + VIR_ACCESS_PERM_DOMAIN_OPEN_CONSOLE, + VIR_ACCESS_PERM_DOMAIN_SCREENSHOT, + + VIR_ACCESS_PERM_DOMAIN_LAST, +} virAccessPermDomain; + +VIR_ENUM_DECL(virAccessPermConnect); +VIR_ENUM_DECL(virAccessPermDomain); + +#endif /* __VIR_ACCESS_PERM_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 391c977..ca60eb3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1181,6 +1181,27 @@ virAuthConfigNew; virAuthConfigNewData;
+# viraccessmanager.h +virAccessManagerInit; +virAccessManagerGetSystemIdentity; +virAccessManagerGetRealIdentity; +virAccessManagerGetEffectiveIdentity; +virAccessManagerSetRealIdentity; +virAccessManagerSetEffectiveIdentity; +virAccessManagerNew; +virAccessManagerNewStack; +virAccessManagerFree; +virAccessManagerCheckConnect; +virAccessManagerCheckDomain; + + +# viraccessvector.h +virAccessVectorConnectTypeFromString; +virAccessVectorConnectTypeToString; +virAccessVectorDomainTypeFromString; +virAccessVectorDomainTypeToString; + + # viraudit.h virAuditClose; virAuditEncode; diff --git a/src/util/virterror.c b/src/util/virterror.c index 33ef713..e60a008 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -187,6 +187,9 @@ static const char *virErrorDomainName(virErrorDomain domain) { case VIR_FROM_DBUS: dom = "DBus "; break; + case VIR_FROM_ACCESS: + dom = "Access Manager "; + break; } return dom; } @@ -1265,6 +1268,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("invalid identity pointer in %s"); break; + case VIR_ERR_ACCESS_DENIED: + if (info == NULL) + errmsg = _("access denied"); + else + errmsg = _("access denied: %s"); + break; } return errmsg; }
-- Regards, Corey

From: "Daniel P. Berrange" <berrange@redhat.com> --- po/POTFILES.in | 1 + src/Makefile.am | 12 ++- src/access/org.libvirt.domain.policy | 37 ++++++++ src/access/viraccessdriverpolkit.c | 163 ++++++++++++++++++++++++++++++++++ src/access/viraccessdriverpolkit.h | 28 ++++++ src/access/viraccessmanager.c | 2 + 6 files changed, 241 insertions(+), 2 deletions(-) create mode 100644 src/access/org.libvirt.domain.policy create mode 100644 src/access/viraccessdriverpolkit.c create mode 100644 src/access/viraccessdriverpolkit.h diff --git a/po/POTFILES.in b/po/POTFILES.in index f898887..0c18fa0 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -6,6 +6,7 @@ daemon/remote_dispatch.h daemon/stream.c gnulib/lib/gai_strerror.c gnulib/lib/regcomp.c +src/access/viraccessdriverpolkit.c src/access/viraccessmanager.c src/conf/cpu_conf.c src/conf/domain_conf.c diff --git a/src/Makefile.am b/src/Makefile.am index 0293562..f9972ac 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -536,7 +536,12 @@ ACCESS_DRIVER_SOURCES = \ access/viraccessmanager.h access/viraccessmanager.c \ access/viraccessdriver.h \ access/viraccessdrivernop.h access/viraccessdrivernop.c \ - access/viraccessdriverstack.h access/viraccessdriverstack.c + access/viraccessdriverstack.h access/viraccessdriverstack.c \ + access/viraccessdriverpolkit.h access/viraccessdriverpolkit.c + +ACCESS_DRIVER_POLKIT_POLICY = \ + access/org.libvirt.domain.policy + NODE_DEVICE_DRIVER_SOURCES = \ node_device/node_device_driver.c \ @@ -1130,6 +1135,8 @@ libvirt_driver_access_la_CFLAGS = \ libvirt_driver_access_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_access_la_LIBADD = +polkitactiondir = $(datadir)/polkit-1/actions +polkitaction_DATA = $(ACCESS_DRIVER_POLKIT_POLICY) # Add all conditional sources just in case... EXTRA_DIST += \ @@ -1166,7 +1173,8 @@ EXTRA_DIST += \ $(SECRET_DRIVER_SOURCES) \ $(VBOX_DRIVER_EXTRA_DIST) \ $(VMWARE_DRIVER_SOURCES) \ - $(XENXS_SOURCES) + $(XENXS_SOURCES) \ + $(ACCESS_DRIVER_POLKIT_POLICY) check-local: augeas-check diff --git a/src/access/org.libvirt.domain.policy b/src/access/org.libvirt.domain.policy new file mode 100644 index 0000000..a8ebd84 --- /dev/null +++ b/src/access/org.libvirt.domain.policy @@ -0,0 +1,37 @@ +<!DOCTYPE policyconfig PUBLIC + "-//freedesktop//DTD PolicyKit Policy Configuration 1.0//EN" + "http://www.freedesktop.org/standards/PolicyKit/1.0/policyconfig.dtd"> + +<!-- +Policy definitions for libvirt daemon + +Copyright (c) 2007 Daniel P. Berrange <berrange redhat com> + +libvirt is licensed to you under the GNU Lesser General Public License +version 2. See COPYING for details. + +NOTE: If you make changes to this file, make sure to validate the file +using the polkit-policy-file-validate(1) tool. Changes made to this +file are instantly applied. +--> + +<policyconfig> + <action id="org.libvirt.domain.getattr"> + <description>Get virtual domain attributes</description> + <message>System policy prevents getattr on guest domains</message> + <defaults> + <allow_any>yes</allow_any> + <allow_inactive>yes</allow_inactive> + <allow_active>yes</allow_active> + </defaults> + </action> + <action id="org.libvirt.domain.read"> + <description>Get virtual domain attributes</description> + <message>System policy prevents getattr on guest domains</message> + <defaults> + <allow_any>yes</allow_any> + <allow_inactive>yes</allow_inactive> + <allow_active>yes</allow_active> + </defaults> + </action> +</policyconfig> diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdriverpolkit.c new file mode 100644 index 0000000..8364986 --- /dev/null +++ b/src/access/viraccessdriverpolkit.c @@ -0,0 +1,163 @@ +/* + * viraccessdriverpolkit.c: polkited access control driver + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include <config.h> + +#include "access/viraccessdriverpolkit.h" +#include "memory.h" +#include "command.h" +#include "logging.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_ACCESS +#define virAccessError(code, ...) \ + virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \ + __FUNCTION__, __LINE__, __VA_ARGS__) + +#define VIR_ACCESS_DRIVER_POLKIT_ACTION_PREFIX "org.libvirt" + +typedef struct _virAccessDriverPolkitPrivate virAccessDriverPolkitPrivate; +typedef virAccessDriverPolkitPrivate *virAccessDriverPolkitPrivatePtr; + +struct _virAccessDriverPolkitPrivate { + bool ignore; +}; + + +static void virAccessDriverPolkitCleanup(virAccessManagerPtr manager ATTRIBUTE_UNUSED) +{ +} + + +static char * +virAccessDriverPolkitFormatAction(const char *typename, + const char *avname) +{ + char *actionid = NULL; + + if (virAsprintf(&actionid, "%s.%s.%s", + VIR_ACCESS_DRIVER_POLKIT_ACTION_PREFIX, + typename, avname) < 0) { + virReportOOMError(); + return NULL; + } + + return actionid; +} + + +static char * +virAccessDriverPolkitFormatProcess(void) +{ + virIdentityPtr identity = virAccessManagerGetEffectiveIdentity(); + const char *process = NULL; + char *ret = NULL; + + if (!identity) { + virAccessError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No identity available")); + return NULL; + } + if (virIdentityGetAttr(identity, VIR_IDENTITY_ATTR_UNIX_PROCESS_ID, &process) < 0) + goto cleanup; + + if (!process) { + virAccessError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No UNIX process ID available")); + goto cleanup; + } + + if (!(ret = strdup(process))) { + virReportOOMError(); + goto cleanup; + } +cleanup: + virIdentityFree(identity); + return ret; +} + + +static bool +virAccessDriverPolkitCheck(virAccessManagerPtr manager ATTRIBUTE_UNUSED, + const char *typename, + const char *avname) +{ + char *actionid = virAccessDriverPolkitFormatAction(typename, avname); + char *process = virAccessDriverPolkitFormatProcess(); + virCommandPtr cmd; + int status; + int ret = false; + + if (!actionid || !process) + goto cleanup; + + cmd = virCommandNewArgList(PKCHECK_PATH, + "--action-id", actionid, + "--process", process, + NULL); + + if (virCommandRun(cmd, &status) < 0) + goto cleanup; + + if (status != 0) { + char *tmp = virCommandTranslateStatus(status); + virAccessError(VIR_ERR_ACCESS_DENIED, + _("Policy kit denied action %s from %s: %s"), + actionid, process, NULLSTR(tmp)); + VIR_FREE(tmp); + goto cleanup; + } + + ret = true; + +cleanup: + VIR_FREE(actionid); + VIR_FREE(process); + return ret; +} + + +static bool +virAccessDriverPolkitCheckConnect(virAccessManagerPtr manager, + virAccessPermConnect av) +{ + return virAccessDriverPolkitCheck(manager, + "connect", + virAccessPermConnectTypeToString(av)); +} + + +static bool +virAccessDriverPolkitCheckDomain(virAccessManagerPtr manager, + virDomainDefPtr def ATTRIBUTE_UNUSED, + virAccessPermDomain av) +{ + return virAccessDriverPolkitCheck(manager, + "domain", + virAccessPermDomainTypeToString(av)); +} + + +virAccessDriver accessDriverPolkit = { + .name = "polkit", + .cleanup = virAccessDriverPolkitCleanup, + .checkConnect = virAccessDriverPolkitCheckConnect, + .checkDomain = virAccessDriverPolkitCheckDomain, +}; diff --git a/src/access/viraccessdriverpolkit.h b/src/access/viraccessdriverpolkit.h new file mode 100644 index 0000000..ac71fa5 --- /dev/null +++ b/src/access/viraccessdriverpolkit.h @@ -0,0 +1,28 @@ +/* + * viraccessdriverpolkit.h: polkited access control driver + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __VIR_ACCESS_DRIVER_POLKIT_H__ +# define __VIR_ACCESS_DRIVER_POLKIT_H__ + +# include "access/viraccessdriver.h" + +extern virAccessDriver accessDriverPolkit; + +#endif /* __VIR_ACCESS_DRIVER_POLKIT_H__ */ diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c index 4e77bd6..e7444d5 100644 --- a/src/access/viraccessmanager.c +++ b/src/access/viraccessmanager.c @@ -29,6 +29,7 @@ #endif #include "access/viraccessdrivernop.h" #include "access/viraccessdriverstack.h" +#include "access/viraccessdriverpolkit.h" #include "logging.h" #define VIR_FROM_THIS VIR_FROM_ACCESS @@ -216,6 +217,7 @@ static virAccessManagerPtr virAccessManagerNewDriver(virAccessDriverPtr drv) static virAccessDriverPtr accessDrivers[] = { &accessDriverNop, + &accessDriverPolkit, }; -- 1.7.10

On 05/02/2012 05:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Sparse on the commit message.
--- po/POTFILES.in | 1 + src/Makefile.am | 12 ++- src/access/org.libvirt.domain.policy | 37 ++++++++ src/access/viraccessdriverpolkit.c | 163 ++++++++++++++++++++++++++++++++++ src/access/viraccessdriverpolkit.h | 28 ++++++ src/access/viraccessmanager.c | 2 + 6 files changed, 241 insertions(+), 2 deletions(-) create mode 100644 src/access/org.libvirt.domain.policy create mode 100644 src/access/viraccessdriverpolkit.c create mode 100644 src/access/viraccessdriverpolkit.h
@@ -536,7 +536,12 @@ ACCESS_DRIVER_SOURCES = \ access/viraccessmanager.h access/viraccessmanager.c \ access/viraccessdriver.h \ access/viraccessdrivernop.h access/viraccessdrivernop.c \ - access/viraccessdriverstack.h access/viraccessdriverstack.c + access/viraccessdriverstack.h access/viraccessdriverstack.c \ + access/viraccessdriverpolkit.h access/viraccessdriverpolkit.c
Sort these lines?
+++ b/src/access/org.libvirt.domain.policy @@ -0,0 +1,37 @@ +<!DOCTYPE policyconfig PUBLIC + "-//freedesktop//DTD PolicyKit Policy Configuration 1.0//EN" + "http://www.freedesktop.org/standards/PolicyKit/1.0/policyconfig.dtd"> + +<!-- +Policy definitions for libvirt daemon + +Copyright (c) 2007 Daniel P. Berrange <berrange redhat com>
2012
+ +libvirt is licensed to you under the GNU Lesser General Public License +version 2. See COPYING for details.
LGPLv2 _or later_
+ <action id="org.libvirt.domain.read"> + <description>Get virtual domain attributes</description> + <message>System policy prevents getattr on guest domains</message>
s/getattr/read/
+++ b/src/access/viraccessdriverpolkit.c
+ + if (virCommandRun(cmd, &status) < 0) + goto cleanup; + + if (status != 0) { + char *tmp = virCommandTranslateStatus(status); + virAccessError(VIR_ERR_ACCESS_DENIED, + _("Policy kit denied action %s from %s: %s"), + actionid, process, NULLSTR(tmp));
Given that all we do on failure is report it, should we just use virCommandRun(cmd, NULL)? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange@redhat.com> --- po/POTFILES.in | 1 + src/Makefile.am | 3 +- src/access/viraccessdriverselinux.c | 388 +++++++++++++++++++++++++++++++++++ src/access/viraccessdriverselinux.h | 28 +++ src/access/viraccessmanager.c | 2 + 5 files changed, 421 insertions(+), 1 deletion(-) create mode 100644 src/access/viraccessdriverselinux.c create mode 100644 src/access/viraccessdriverselinux.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 0c18fa0..7ab6dba 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -7,6 +7,7 @@ daemon/stream.c gnulib/lib/gai_strerror.c gnulib/lib/regcomp.c src/access/viraccessdriverpolkit.c +src/access/viraccessdriverselinux.c src/access/viraccessmanager.c src/conf/cpu_conf.c src/conf/domain_conf.c diff --git a/src/Makefile.am b/src/Makefile.am index f9972ac..64398f0 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -537,7 +537,8 @@ ACCESS_DRIVER_SOURCES = \ access/viraccessdriver.h \ access/viraccessdrivernop.h access/viraccessdrivernop.c \ access/viraccessdriverstack.h access/viraccessdriverstack.c \ - access/viraccessdriverpolkit.h access/viraccessdriverpolkit.c + access/viraccessdriverpolkit.h access/viraccessdriverpolkit.c \ + access/viraccessdriverselinux.h access/viraccessdriverselinux.c ACCESS_DRIVER_POLKIT_POLICY = \ access/org.libvirt.domain.policy diff --git a/src/access/viraccessdriverselinux.c b/src/access/viraccessdriverselinux.c new file mode 100644 index 0000000..2c64aff --- /dev/null +++ b/src/access/viraccessdriverselinux.c @@ -0,0 +1,388 @@ +/* + * viraccessdriverselinux.c: selinuxed access control driver + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include <config.h> + +#include "access/viraccessdriverselinux.h" +#include "memory.h" +#include "command.h" +#include "logging.h" +#include "threads.h" +#include "virterror_internal.h" + +#include <selinux/selinux.h> +#include <selinux/avc.h> +#include <selinux/av_permissions.h> +#include <selinux/flask.h> + + +#define VIR_FROM_THIS VIR_FROM_ACCESS +#define virAccessError(code, ...) \ + virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \ + __FUNCTION__, __LINE__, __VA_ARGS__) + +static void virAccessDriverSELinuxAVCLog(const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(1, 2); +static void virAccessDriverSELinuxAVCLogAudit(void *data, security_class_t class, char *buf, size_t bufleft); +static void *virAccessDriverSELinuxAVCCreateThread(void (*run) (void)); +static void virAccessDriverSELinuxAVCStopThread(void *thread); +static void *virAccessDriverSELinuxAVCAllocLock(void); +static void virAccessDriverSELinuxAVCGetLock(void *lock); +static void virAccessDriverSELinuxAVCReleaseLock(void *lock); +static void virAccessDriverSELinuxAVCFreeLock(void *lock); + + +/* AVC callback structures for use in avc_init. */ +static const struct avc_memory_callback virAccessDriverSELinuxAVCMemCallbacks = +{ + .func_malloc = malloc, + .func_free = free, +}; +static const struct avc_log_callback virAccessDriverSELinuxAVCLogCallbacks = +{ + .func_log = virAccessDriverSELinuxAVCLog, + .func_audit = virAccessDriverSELinuxAVCLogAudit, +}; +static const struct avc_thread_callback virAccessDriverSELinuxAVCThreadCallbacks = +{ + .func_create_thread = virAccessDriverSELinuxAVCCreateThread, + .func_stop_thread = virAccessDriverSELinuxAVCStopThread, +}; +static const struct avc_lock_callback virAccessDriverSELinuxAVCLockCallbacks = +{ + .func_alloc_lock = virAccessDriverSELinuxAVCAllocLock, + .func_get_lock = virAccessDriverSELinuxAVCGetLock, + .func_release_lock = virAccessDriverSELinuxAVCReleaseLock, + .func_free_lock = virAccessDriverSELinuxAVCFreeLock, +}; + + +typedef struct _virAccessDriverSELinuxPrivate virAccessDriverSELinuxPrivate; +typedef virAccessDriverSELinuxPrivate *virAccessDriverSELinuxPrivatePtr; + +struct _virAccessDriverSELinuxPrivate { + bool enabled; + + /* Cache for AVCs */ + struct avc_entry_ref aeref; + + /* SID of the daemon */ + security_id_t localSid; +}; + + +static int virAccessDriverSELinuxSetup(virAccessManagerPtr manager) +{ + virAccessDriverSELinuxPrivatePtr priv = virAccessManagerGetPrivateData(manager); + int r; + security_context_t localCon; + int ret = -1; + + if ((r = is_selinux_enabled()) < 0) { + virReportSystemError(errno, "%s", + _("Unable to determine if SELinux is enabled")); + return -1; + } + priv->enabled = r != 0; + priv->localSid = SECSID_WILD; + + avc_entry_ref_init(&priv->aeref); + + if (avc_init("avc", + &virAccessDriverSELinuxAVCMemCallbacks, + &virAccessDriverSELinuxAVCLogCallbacks, + &virAccessDriverSELinuxAVCThreadCallbacks, + &virAccessDriverSELinuxAVCLockCallbacks) < 0) { + virReportSystemError(errno, "%s", + _("Unable to initialize AVC system")); + goto cleanup; + } + + if (getcon(&localCon) < 0) { + virReportSystemError(errno, "%s", + _("Unable to get context of daemon")); + goto cleanup; + } + + if (avc_context_to_sid(localCon, &priv->localSid) < 0) { + virReportSystemError(errno, + _("Unable to convert context %s to SID"), + (char*)localCon); + goto cleanup; + } + VIR_FREE(localCon); + + ret = 0; +cleanup: + if (ret < 0) + priv->localSid = SECSID_WILD; + freecon(localCon); + return ret; +} + + +static void virAccessDriverSELinuxCleanup(virAccessManagerPtr manager) +{ + virAccessDriverSELinuxPrivatePtr priv = virAccessManagerGetPrivateData(manager); + + priv->localSid = SECSID_WILD; +} + + +static security_id_t +virAccessDriverSELinuxGetClientSID(void) +{ + virIdentityPtr identity = virAccessManagerGetEffectiveIdentity(); + const char *seccon = NULL; + security_id_t sid = SECSID_WILD; + + if (!identity) { + virAccessError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No identity available")); + return NULL; + } + if (virIdentityGetAttr(identity, VIR_IDENTITY_ATTR_SECURITY_CONTEXT, &seccon) < 0) + goto cleanup; + + if (!seccon) { + virAccessError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No security context available")); + goto cleanup; + } + + if (avc_context_to_sid((security_context_t)seccon, &sid) < 0) { + virReportSystemError(errno, + _("Unable to convert context %s to SID"), + seccon); + sid = SECSID_WILD; + goto cleanup; + } + +cleanup: + virIdentityFree(identity); + return sid; +} + + +static security_class_t +virAccessDriverSELinuxGetObjectClass(const char *typename) +{ + security_class_t ret; + + if ((ret = string_to_security_class(typename)) == 0) { + virReportSystemError(errno, + _("Unable to find security class '%s'"), typename); + return 0; + } + + return ret; +} + + +static access_vector_t +virAccessDriverSELinuxGetObjectPerm(const char *avname, const char *typename, security_class_t objectClass) +{ + access_vector_t ret; + + if (objectClass == 0) + return 0; + + if ((ret = string_to_av_perm(objectClass, avname)) == 0) { + virReportSystemError(errno, + _("Unable to find access vector '%s' for '%s'"), avname, typename); + return 0; + } + + return ret; +} + +static bool +virAccessDriverSELinuxCheck(virAccessManagerPtr manager, + security_id_t objectSid, + const char *typename, + const char *avname) +{ + virAccessDriverSELinuxPrivatePtr priv = virAccessManagerGetPrivateData(manager); + security_id_t clientSid = virAccessDriverSELinuxGetClientSID(); + security_class_t objectClass = virAccessDriverSELinuxGetObjectClass(typename); + access_vector_t objectVector = virAccessDriverSELinuxGetObjectPerm(avname, typename, objectClass); + int ret = false; + + if (clientSid == SECSID_WILD || + objectClass == 0 || + objectVector == 0) { + if (security_deny_unknown() == 0) { + VIR_WARN("Allow access, because policy does not deny unknown objects"); + ret = true; + } + goto cleanup; + } + + if (avc_has_perm(clientSid, objectSid, + objectClass, objectVector, + &priv->aeref, NULL) < 0) { + int save_errno = errno; + if (security_getenforce() == 0) { + char ebuf[1024]; + VIR_WARN("Ignoring denial in non-enforcing mode: %s", + virStrerror(save_errno, ebuf, sizeof(ebuf))); + ret = true; + goto cleanup; + } + switch (save_errno) { + case EACCES: + virAccessError(VIR_ERR_ACCESS_DENIED, "%s", + _("SELinux denying due to security policy")); + break; + case EINVAL: + virAccessError(VIR_ERR_ACCESS_DENIED, "%s", + _("SELinux denying due to invalid security context")); + break; + default: + virReportSystemError(errno, "%s", + _("SELinux denying")); + break; + } + goto cleanup; + } + + ret = true; + +cleanup: + return ret; +} + + +static bool +virAccessDriverSELinuxCheckConnect(virAccessManagerPtr manager, + virAccessPermConnect av) +{ + virAccessDriverSELinuxPrivatePtr priv = virAccessManagerGetPrivateData(manager); + + /* There's no object to use for targetSid here, so we + * instead use the daemon's context as the targetSid */ + return virAccessDriverSELinuxCheck(manager, + priv->localSid, + "connect", + virAccessPermConnectTypeToString(av)); +} + + +static bool +virAccessDriverSELinuxCheckDomain(virAccessManagerPtr manager, + virDomainDefPtr def ATTRIBUTE_UNUSED, + virAccessPermDomain av) +{ + security_id_t objectSid = 0; /* XXX get from 'def' */ + + return virAccessDriverSELinuxCheck(manager, + objectSid, + "domain", + virAccessPermDomainTypeToString(av)); +} + + + +static void virAccessDriverSELinuxAVCLog(const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + virLogVMessage("avc", VIR_LOG_WARN,__func__, __LINE__, 0, fmt, ap); + va_end(ap); +} + + +static void virAccessDriverSELinuxAVCLogAudit(void *data ATTRIBUTE_UNUSED, + security_class_t class ATTRIBUTE_UNUSED, + char *buf ATTRIBUTE_UNUSED, + size_t bufleft ATTRIBUTE_UNUSED) +{ +} + + +static void *virAccessDriverSELinuxAVCCreateThread(void (*run) (void)) +{ + virThreadPtr thread; + + if (VIR_ALLOC(thread) < 0) { + virReportOOMError(); + return NULL; + } + + if (virThreadCreate(thread, false, (virThreadFunc)run, NULL) < 0) { + virReportSystemError(errno, "%s", + _("Unable to create thread")); + VIR_FREE(thread); + } + + return thread; +} + + +static void virAccessDriverSELinuxAVCStopThread(void *thread) +{ + virThreadCancel(thread); + VIR_FREE(thread); +} + + +static void *virAccessDriverSELinuxAVCAllocLock(void) +{ + virMutexPtr lock; + if (VIR_ALLOC(lock) < 0) { + virReportOOMError(); + return NULL; + } + if (virMutexInit(lock) < 0) { + virReportSystemError(errno, "%s", + _("Unable to initialize mutex")); + VIR_FREE(lock); + return NULL; + } + return lock; +} + + +static void virAccessDriverSELinuxAVCGetLock(void *lock) +{ + virMutexLock(lock); +} + + +static void virAccessDriverSELinuxAVCReleaseLock(void *lock) +{ + virMutexUnlock(lock); +} + + +static void virAccessDriverSELinuxAVCFreeLock(void *lock) +{ + virMutexDestroy(lock); + VIR_FREE(lock); +} + + +virAccessDriver accessDriverSELinux = { + .privateDataLen = sizeof(virAccessDriverSELinuxPrivate), + .name = "selinux", + .setup = virAccessDriverSELinuxSetup, + .cleanup = virAccessDriverSELinuxCleanup, + .checkConnect = virAccessDriverSELinuxCheckConnect, + .checkDomain = virAccessDriverSELinuxCheckDomain, +}; diff --git a/src/access/viraccessdriverselinux.h b/src/access/viraccessdriverselinux.h new file mode 100644 index 0000000..3a53686 --- /dev/null +++ b/src/access/viraccessdriverselinux.h @@ -0,0 +1,28 @@ +/* + * viraccessdriverselinux.h: selinuxed access control driver + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __VIR_ACCESS_DRIVER_SELINUX_H__ +# define __VIR_ACCESS_DRIVER_SELINUX_H__ + +# include "access/viraccessdriver.h" + +extern virAccessDriver accessDriverSELinux; + +#endif /* __VIR_ACCESS_DRIVER_SELINUX_H__ */ diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c index e7444d5..4b215c4 100644 --- a/src/access/viraccessmanager.c +++ b/src/access/viraccessmanager.c @@ -30,6 +30,7 @@ #include "access/viraccessdrivernop.h" #include "access/viraccessdriverstack.h" #include "access/viraccessdriverpolkit.h" +#include "access/viraccessdriverselinux.h" #include "logging.h" #define VIR_FROM_THIS VIR_FROM_ACCESS @@ -218,6 +219,7 @@ static virAccessManagerPtr virAccessManagerNewDriver(virAccessDriverPtr drv) static virAccessDriverPtr accessDrivers[] = { &accessDriverNop, &accessDriverPolkit, + &accessDriverSELinux, }; -- 1.7.10

On 05/02/2012 05:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
sparse on the commit message (I sound like a broken record...)
+++ b/src/Makefile.am @@ -537,7 +537,8 @@ ACCESS_DRIVER_SOURCES = \ access/viraccessdriver.h \ access/viraccessdrivernop.h access/viraccessdrivernop.c \ access/viraccessdriverstack.h access/viraccessdriverstack.c \ - access/viraccessdriverpolkit.h access/viraccessdriverpolkit.c + access/viraccessdriverpolkit.h access/viraccessdriverpolkit.c \ + access/viraccessdriverselinux.h access/viraccessdriverselinux.c
selinux comes before stack, if you are sorting by name. Or, if you are listing generic files first (driver, driverstack) followed by implementations (drivernop, driverpolkit, driverselinux), then drivernop is too early.
+ +struct _virAccessDriverSELinuxPrivate { + bool enabled;
This looks like you are doing a one-shot setup, based on the enabled state at the time the struct is initialized. Normally, I can run 'setenforce [01]' on a live system, and it takes effect immediately, without restarting processes; but if you do one-shot initialization instead of live probing on every use, changing the permissions would require a libvirtd restart. Or am I mixing up enabled vs. enforcing (where enforcing can change on the fly, but only if enabled is true, where enabled is one-shot at boot).
+static int virAccessDriverSELinuxSetup(virAccessManagerPtr manager) +{ + virAccessDriverSELinuxPrivatePtr priv = virAccessManagerGetPrivateData(manager); + int r; + security_context_t localCon;
Uninit...
+ int ret = -1; + + if ((r = is_selinux_enabled()) < 0) { + virReportSystemError(errno, "%s", + _("Unable to determine if SELinux is enabled")); + return -1; + } + priv->enabled = r != 0; + priv->localSid = SECSID_WILD; + + avc_entry_ref_init(&priv->aeref); + + if (avc_init("avc", + &virAccessDriverSELinuxAVCMemCallbacks, + &virAccessDriverSELinuxAVCLogCallbacks, + &virAccessDriverSELinuxAVCThreadCallbacks, + &virAccessDriverSELinuxAVCLockCallbacks) < 0) { + virReportSystemError(errno, "%s", + _("Unable to initialize AVC system")); + goto cleanup;
still uninit,
+ } + + if (getcon(&localCon) < 0) { + virReportSystemError(errno, "%s", + _("Unable to get context of daemon")); + goto cleanup; + } + + if (avc_context_to_sid(localCon, &priv->localSid) < 0) { + virReportSystemError(errno, + _("Unable to convert context %s to SID"), + (char*)localCon); + goto cleanup; + } + VIR_FREE(localCon); + + ret = 0; +cleanup: + if (ret < 0) + priv->localSid = SECSID_WILD; + freecon(localCon);
and unconditionally freed on error. Oops.
+ + +static access_vector_t +virAccessDriverSELinuxGetObjectPerm(const char *avname, const char *typename, security_class_t objectClass)
Long lines; worth wrapping?
+ + +static void virAccessDriverSELinuxAVCLog(const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + virLogVMessage("avc", VIR_LOG_WARN,__func__, __LINE__, 0, fmt, ap);
This will always log with virAccessDriverSELinuxAVCLog as the function, which isn't quite as useful as stating which function really logged. For that to work, you'd need to maintain some sort of state stack, where you push the name and line of a function before calling into any selinux function that might trigger a callback, then this callback function inspects the state stack. But that's a lot of work, I can live with your current solution.
+ +static void *virAccessDriverSELinuxAVCCreateThread(void (*run) (void)) +{ + virThreadPtr thread; + + if (VIR_ALLOC(thread) < 0) { + virReportOOMError(); + return NULL; + } + + if (virThreadCreate(thread, false, (virThreadFunc)run, NULL) < 0) { + virReportSystemError(errno, "%s", + _("Unable to create thread")); + VIR_FREE(thread); + } + + return thread; +} + + +static void virAccessDriverSELinuxAVCStopThread(void *thread) +{ + virThreadCancel(thread); + VIR_FREE(thread);
Since thread cancellation introduces such a different paradigm in how you code (such as having to properly push and pop cancellation cleanup code), maybe it would make sense to alter your 2/12 patch to add a virThreadCreateFlags, and _only_ allow a thread to be cancelled if it passes in a flag that states it is cancellation-safe. This code would be the first client of that mode, where virThreadCancel() would fail if called on any other thread that has not already been audited and marked to be cancellation-safe. That is, I'd feel safer if the only threads that we allow libvirt to cancel are those that are known to be cancellation safe (such as the selinux thread, since it requires cancellation to work, so it must be safe insofar as all our callbacks are also cancellation safe). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 05/02/2012 07:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
--- po/POTFILES.in | 1 + src/Makefile.am | 3 +- src/access/viraccessdriverselinux.c | 388 +++++++++++++++++++++++++++++++++++ src/access/viraccessdriverselinux.h | 28 +++ src/access/viraccessmanager.c | 2 + 5 files changed, 421 insertions(+), 1 deletion(-) create mode 100644 src/access/viraccessdriverselinux.c create mode 100644 src/access/viraccessdriverselinux.h
diff --git a/po/POTFILES.in b/po/POTFILES.in index 0c18fa0..7ab6dba 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -7,6 +7,7 @@ daemon/stream.c gnulib/lib/gai_strerror.c gnulib/lib/regcomp.c src/access/viraccessdriverpolkit.c +src/access/viraccessdriverselinux.c src/access/viraccessmanager.c src/conf/cpu_conf.c src/conf/domain_conf.c diff --git a/src/Makefile.am b/src/Makefile.am index f9972ac..64398f0 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -537,7 +537,8 @@ ACCESS_DRIVER_SOURCES = \ access/viraccessdriver.h \ access/viraccessdrivernop.h access/viraccessdrivernop.c \ access/viraccessdriverstack.h access/viraccessdriverstack.c \ - access/viraccessdriverpolkit.h access/viraccessdriverpolkit.c + access/viraccessdriverpolkit.h access/viraccessdriverpolkit.c \ + access/viraccessdriverselinux.h access/viraccessdriverselinux.c
ACCESS_DRIVER_POLKIT_POLICY = \ access/org.libvirt.domain.policy diff --git a/src/access/viraccessdriverselinux.c b/src/access/viraccessdriverselinux.c new file mode 100644 index 0000000..2c64aff --- /dev/null +++ b/src/access/viraccessdriverselinux.c @@ -0,0 +1,388 @@ +/* + * viraccessdriverselinux.c: selinuxed access control driver + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include <config.h> + +#include "access/viraccessdriverselinux.h" +#include "memory.h" +#include "command.h" +#include "logging.h" +#include "threads.h" +#include "virterror_internal.h" + +#include <selinux/selinux.h> +#include <selinux/avc.h> +#include <selinux/av_permissions.h> +#include <selinux/flask.h> + + +#define VIR_FROM_THIS VIR_FROM_ACCESS +#define virAccessError(code, ...) \ + virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \ + __FUNCTION__, __LINE__, __VA_ARGS__) + +static void virAccessDriverSELinuxAVCLog(const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(1, 2); +static void virAccessDriverSELinuxAVCLogAudit(void *data, security_class_t class, char *buf, size_t bufleft); +static void *virAccessDriverSELinuxAVCCreateThread(void (*run) (void)); +static void virAccessDriverSELinuxAVCStopThread(void *thread); +static void *virAccessDriverSELinuxAVCAllocLock(void); +static void virAccessDriverSELinuxAVCGetLock(void *lock); +static void virAccessDriverSELinuxAVCReleaseLock(void *lock); +static void virAccessDriverSELinuxAVCFreeLock(void *lock); + + +/* AVC callback structures for use in avc_init. */ +static const struct avc_memory_callback virAccessDriverSELinuxAVCMemCallbacks = +{ + .func_malloc = malloc, + .func_free = free, +}; +static const struct avc_log_callback virAccessDriverSELinuxAVCLogCallbacks = +{ + .func_log = virAccessDriverSELinuxAVCLog, + .func_audit = virAccessDriverSELinuxAVCLogAudit, +}; +static const struct avc_thread_callback virAccessDriverSELinuxAVCThreadCallbacks = +{ + .func_create_thread = virAccessDriverSELinuxAVCCreateThread, + .func_stop_thread = virAccessDriverSELinuxAVCStopThread, +}; +static const struct avc_lock_callback virAccessDriverSELinuxAVCLockCallbacks = +{ + .func_alloc_lock = virAccessDriverSELinuxAVCAllocLock, + .func_get_lock = virAccessDriverSELinuxAVCGetLock, + .func_release_lock = virAccessDriverSELinuxAVCReleaseLock, + .func_free_lock = virAccessDriverSELinuxAVCFreeLock, +}; + + +typedef struct _virAccessDriverSELinuxPrivate virAccessDriverSELinuxPrivate; +typedef virAccessDriverSELinuxPrivate *virAccessDriverSELinuxPrivatePtr; + +struct _virAccessDriverSELinuxPrivate { + bool enabled; + + /* Cache for AVCs */ + struct avc_entry_ref aeref; + + /* SID of the daemon */ + security_id_t localSid; +}; + + +static int virAccessDriverSELinuxSetup(virAccessManagerPtr manager) +{ + virAccessDriverSELinuxPrivatePtr priv = virAccessManagerGetPrivateData(manager); + int r; + security_context_t localCon; + int ret = -1; + + if ((r = is_selinux_enabled()) < 0) { + virReportSystemError(errno, "%s", + _("Unable to determine if SELinux is enabled")); + return -1; + } + priv->enabled = r != 0; + priv->localSid = SECSID_WILD; + + avc_entry_ref_init(&priv->aeref); + + if (avc_init("avc", + &virAccessDriverSELinuxAVCMemCallbacks, + &virAccessDriverSELinuxAVCLogCallbacks, + &virAccessDriverSELinuxAVCThreadCallbacks, + &virAccessDriverSELinuxAVCLockCallbacks) < 0) {
It looks like avc_init is deprecated.
+ virReportSystemError(errno, "%s", + _("Unable to initialize AVC system")); + goto cleanup; + } + + if (getcon(&localCon) < 0) { + virReportSystemError(errno, "%s", + _("Unable to get context of daemon")); + goto cleanup; + } + + if (avc_context_to_sid(localCon, &priv->localSid) < 0) { + virReportSystemError(errno, + _("Unable to convert context %s to SID"), + (char*)localCon); + goto cleanup; + } + VIR_FREE(localCon); + + ret = 0; +cleanup: + if (ret < 0) + priv->localSid = SECSID_WILD; + freecon(localCon); + return ret; +} + + +static void virAccessDriverSELinuxCleanup(virAccessManagerPtr manager) +{ + virAccessDriverSELinuxPrivatePtr priv = virAccessManagerGetPrivateData(manager); + + priv->localSid = SECSID_WILD; +} + + +static security_id_t +virAccessDriverSELinuxGetClientSID(void) +{ + virIdentityPtr identity = virAccessManagerGetEffectiveIdentity(); + const char *seccon = NULL; + security_id_t sid = SECSID_WILD; + + if (!identity) { + virAccessError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No identity available")); + return NULL; + } + if (virIdentityGetAttr(identity, VIR_IDENTITY_ATTR_SECURITY_CONTEXT, &seccon) < 0) + goto cleanup; + + if (!seccon) { + virAccessError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No security context available")); + goto cleanup; + } + + if (avc_context_to_sid((security_context_t)seccon, &sid) < 0) { + virReportSystemError(errno, + _("Unable to convert context %s to SID"), + seccon); + sid = SECSID_WILD; + goto cleanup; + } + +cleanup: + virIdentityFree(identity); + return sid; +} + + +static security_class_t +virAccessDriverSELinuxGetObjectClass(const char *typename) +{ + security_class_t ret; + + if ((ret = string_to_security_class(typename)) == 0) { + virReportSystemError(errno, + _("Unable to find security class '%s'"), typename); + return 0; + } + + return ret; +} + + +static access_vector_t +virAccessDriverSELinuxGetObjectPerm(const char *avname, const char *typename, security_class_t objectClass) +{ + access_vector_t ret; + + if (objectClass == 0) + return 0; + + if ((ret = string_to_av_perm(objectClass, avname)) == 0) { + virReportSystemError(errno, + _("Unable to find access vector '%s' for '%s'"), avname, typename); + return 0; + } + + return ret; +} + +static bool +virAccessDriverSELinuxCheck(virAccessManagerPtr manager, + security_id_t objectSid, + const char *typename, + const char *avname) +{ + virAccessDriverSELinuxPrivatePtr priv = virAccessManagerGetPrivateData(manager); + security_id_t clientSid = virAccessDriverSELinuxGetClientSID(); + security_class_t objectClass = virAccessDriverSELinuxGetObjectClass(typename); + access_vector_t objectVector = virAccessDriverSELinuxGetObjectPerm(avname, typename, objectClass); + int ret = false; + + if (clientSid == SECSID_WILD || + objectClass == 0 || + objectVector == 0) {
So if the client has the wildcard sid it gets access. That makes sense. What about objectClass == 0 and objectVector == 0? I'm thinking these cover the case where the MAC admin has modified the enforcing policy and removed the corresponding class/perm definitions. It just makes me feel a little uncomfortable that we could accidentally fall through and grant access here, perhaps due to a coding error on a virAccessManagerCheckConnect or virAccessManagerCheckDomain call.
+ if (security_deny_unknown() == 0) { + VIR_WARN("Allow access, because policy does not deny unknown objects"); + ret = true; + } + goto cleanup; + } + + if (avc_has_perm(clientSid, objectSid, + objectClass, objectVector, + &priv->aeref, NULL) < 0) { + int save_errno = errno; + if (security_getenforce() == 0) { + char ebuf[1024]; + VIR_WARN("Ignoring denial in non-enforcing mode: %s", + virStrerror(save_errno, ebuf, sizeof(ebuf))); + ret = true; + goto cleanup; + } + switch (save_errno) { + case EACCES: + virAccessError(VIR_ERR_ACCESS_DENIED, "%s", + _("SELinux denying due to security policy")); + break; + case EINVAL: + virAccessError(VIR_ERR_ACCESS_DENIED, "%s", + _("SELinux denying due to invalid security context")); + break; + default: + virReportSystemError(errno, "%s", + _("SELinux denying")); + break; + } + goto cleanup; + } + + ret = true; + +cleanup: + return ret; +} + + +static bool +virAccessDriverSELinuxCheckConnect(virAccessManagerPtr manager, + virAccessPermConnect av) +{ + virAccessDriverSELinuxPrivatePtr priv = virAccessManagerGetPrivateData(manager); + + /* There's no object to use for targetSid here, so we + * instead use the daemon's context as the targetSid */ + return virAccessDriverSELinuxCheck(manager, + priv->localSid, + "connect", + virAccessPermConnectTypeToString(av)); +} + + +static bool +virAccessDriverSELinuxCheckDomain(virAccessManagerPtr manager, + virDomainDefPtr def ATTRIBUTE_UNUSED, + virAccessPermDomain av) +{ + security_id_t objectSid = 0; /* XXX get from 'def' */ + + return virAccessDriverSELinuxCheck(manager, + objectSid, + "domain", + virAccessPermDomainTypeToString(av)); +} + + + +static void virAccessDriverSELinuxAVCLog(const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + virLogVMessage("avc", VIR_LOG_WARN,__func__, __LINE__, 0, fmt, ap); + va_end(ap); +} + + +static void virAccessDriverSELinuxAVCLogAudit(void *data ATTRIBUTE_UNUSED, + security_class_t class ATTRIBUTE_UNUSED, + char *buf ATTRIBUTE_UNUSED, + size_t bufleft ATTRIBUTE_UNUSED) +{ +} + + +static void *virAccessDriverSELinuxAVCCreateThread(void (*run) (void)) +{ + virThreadPtr thread; + + if (VIR_ALLOC(thread) < 0) { + virReportOOMError(); + return NULL; + } + + if (virThreadCreate(thread, false, (virThreadFunc)run, NULL) < 0) { + virReportSystemError(errno, "%s", + _("Unable to create thread")); + VIR_FREE(thread); + } + + return thread; +} + + +static void virAccessDriverSELinuxAVCStopThread(void *thread) +{ + virThreadCancel(thread); + VIR_FREE(thread); +} + + +static void *virAccessDriverSELinuxAVCAllocLock(void) +{ + virMutexPtr lock; + if (VIR_ALLOC(lock) < 0) { + virReportOOMError(); + return NULL; + } + if (virMutexInit(lock) < 0) { + virReportSystemError(errno, "%s", + _("Unable to initialize mutex")); + VIR_FREE(lock); + return NULL; + } + return lock; +} + + +static void virAccessDriverSELinuxAVCGetLock(void *lock) +{ + virMutexLock(lock); +} + + +static void virAccessDriverSELinuxAVCReleaseLock(void *lock) +{ + virMutexUnlock(lock); +} + + +static void virAccessDriverSELinuxAVCFreeLock(void *lock) +{ + virMutexDestroy(lock); + VIR_FREE(lock); +} + + +virAccessDriver accessDriverSELinux = { + .privateDataLen = sizeof(virAccessDriverSELinuxPrivate), + .name = "selinux", + .setup = virAccessDriverSELinuxSetup, + .cleanup = virAccessDriverSELinuxCleanup, + .checkConnect = virAccessDriverSELinuxCheckConnect, + .checkDomain = virAccessDriverSELinuxCheckDomain, +}; diff --git a/src/access/viraccessdriverselinux.h b/src/access/viraccessdriverselinux.h new file mode 100644 index 0000000..3a53686 --- /dev/null +++ b/src/access/viraccessdriverselinux.h @@ -0,0 +1,28 @@ +/* + * viraccessdriverselinux.h: selinuxed access control driver + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __VIR_ACCESS_DRIVER_SELINUX_H__ +# define __VIR_ACCESS_DRIVER_SELINUX_H__ + +# include "access/viraccessdriver.h" + +extern virAccessDriver accessDriverSELinux; + +#endif /* __VIR_ACCESS_DRIVER_SELINUX_H__ */ diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c index e7444d5..4b215c4 100644 --- a/src/access/viraccessmanager.c +++ b/src/access/viraccessmanager.c @@ -30,6 +30,7 @@ #include "access/viraccessdrivernop.h" #include "access/viraccessdriverstack.h" #include "access/viraccessdriverpolkit.h" +#include "access/viraccessdriverselinux.h" #include "logging.h"
#define VIR_FROM_THIS VIR_FROM_ACCESS @@ -218,6 +219,7 @@ static virAccessManagerPtr virAccessManagerNewDriver(virAccessDriverPtr drv) static virAccessDriverPtr accessDrivers[] = { &accessDriverNop, &accessDriverPolkit, + &accessDriverSELinux, };
-- Regards, Corey

From: "Daniel P. Berrange" <berrange@redhat.com> Add APIs which allow storage of a real & effective identity on all server clients. Also add an API which allows creation of an initial identity based on the results of client authentication processes like TLS, x509, SASL, SO_PEERCRED --- src/rpc/virnetserverclient.c | 152 ++++++++++++++++++++++++++++++++++++++++++ src/rpc/virnetserverclient.h | 11 +++ 2 files changed, 163 insertions(+) diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 1e9d3db..9647ac3 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -75,6 +75,10 @@ struct _virNetServerClient int sockTimer; /* Timer to be fired upon cached data, * so we jump out from poll() immediately */ + + virIdentityPtr realIdentity; + virIdentityPtr effectiveIdentity; + /* Count of messages in the 'tx' queue, * and the server worker pool queue * ie RPC calls in progress. Does not count @@ -487,6 +491,149 @@ int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, } +virIdentityPtr virNetServerClientGetIdentity(virNetServerClientPtr client) +{ + char *processid = NULL; + char *username = NULL; + char *groupname = NULL; +#if HAVE_SASL + char *saslname = NULL; +#endif + char *x509dname = NULL; + char *seccontext = NULL; + virIdentityPtr ret = NULL; + virNetSASLSessionPtr sasl; + virNetTLSSessionPtr tls; + + if (virNetServerClientIsLocal(client)) { + gid_t gid; + uid_t uid; + pid_t pid; + if (virNetServerClientGetUNIXIdentity(client, &uid, &gid, &pid) < 0) + goto cleanup; + + if (!(username = virGetUserName(uid))) + goto cleanup; + if (!(groupname = virGetGroupName(gid))) + goto cleanup; + if (virAsprintf(&processid, "%d", (int)pid) < 0) + goto cleanup; + } + +#if HAVE_SASL + if ((sasl = virNetServerClientGetSASLSession(client))) { + const char *identity = virNetSASLSessionGetIdentity(sasl); + if (identity && + !(saslname = strdup(identity))) { + virReportOOMError(); + goto cleanup; + } + } +#endif + + if ((tls = virNetServerClientGetTLSSession(client))) { + const char *identity = virNetTLSSessionGetX509DName(tls); + if (identity && + !(x509dname = strdup(identity))) { + virReportOOMError(); + goto cleanup; + } + } + + if (virNetServerClientGetSecurityContext(client, &seccontext) < 0) + goto cleanup; + + if (!(ret = virIdentityNew())) + goto cleanup; + + if (username && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_USER_NAME, username) < 0) + goto error; + if (groupname && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_GROUP_NAME, groupname) < 0) + goto error; + if (processid && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_PROCESS_ID, processid) < 0) + goto error; +#if HAVE_SASL + if (saslname && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_SASL_USER_NAME, saslname) < 0) + goto error; +#endif + if (x509dname && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_X509_DISTINGUISHED_NAME, x509dname) < 0) + goto error; + if (seccontext && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_SECURITY_CONTEXT, seccontext) < 0) + goto error; + +cleanup: + VIR_FREE(username); + VIR_FREE(groupname); + VIR_FREE(processid); + VIR_FREE(seccontext); +#if HAVE_SASL + VIR_FREE(saslname); +#endif + VIR_FREE(x509dname); + return ret; + +error: + virIdentityFree(ret); + ret = NULL; + goto cleanup; +} + + +virIdentityPtr virNetServerClientGetRealIdentity(virNetServerClientPtr client) +{ + virIdentityPtr ret; + virNetServerClientLock(client); + ret = client->realIdentity; + if (ret) + virIdentityRef(ret); + virNetServerClientUnlock(client); + return ret; +} + + +virIdentityPtr virNetServerClientGetEffectiveIdentity(virNetServerClientPtr client) +{ + virIdentityPtr ret; + virNetServerClientLock(client); + ret = client->effectiveIdentity; + if (ret) + virIdentityRef(ret); + virNetServerClientUnlock(client); + return ret; +} + + +void virNetServerClientSetRealIdentity(virNetServerClientPtr client, + virIdentityPtr ident) +{ + virNetServerClientLock(client); + if (client->realIdentity) + virIdentityFree(client->realIdentity); + if (ident) + virIdentityRef(ident); + client->realIdentity = ident; + virNetServerClientUnlock(client); +} + +void virNetServerClientSetEffectiveIdentity(virNetServerClientPtr client, + virIdentityPtr ident) +{ + virNetServerClientLock(client); + if (client->effectiveIdentity) + virIdentityFree(client->effectiveIdentity); + if (ident) + virIdentityRef(ident); + client->effectiveIdentity = ident; + virNetServerClientUnlock(client); +} + + int virNetServerClientGetSecurityContext(virNetServerClientPtr client, char **context) { @@ -625,6 +772,11 @@ void virNetServerClientFree(virNetServerClientPtr client) return; } + if (client->realIdentity) + virIdentityFree(client->realIdentity); + if (client->effectiveIdentity) + virIdentityFree(client->effectiveIdentity); + if (client->privateData && client->privateDataFreeFunc) client->privateDataFreeFunc(client->privateData); diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index a3b37a3..7435eee 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -77,6 +77,17 @@ int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, int virNetServerClientGetSecurityContext(virNetServerClientPtr client, char **context); +virIdentityPtr virNetServerClientGetIdentity(virNetServerClientPtr client); + +virIdentityPtr virNetServerClientGetRealIdentity(virNetServerClientPtr client); +virIdentityPtr virNetServerClientGetEffectiveIdentity(virNetServerClientPtr client); + +void virNetServerClientSetRealIdentity(virNetServerClientPtr client, + virIdentityPtr ident); +void virNetServerClientSetEffectiveIdentity(virNetServerClientPtr client, + virIdentityPtr iden); + + void virNetServerClientRef(virNetServerClientPtr client); typedef void (*virNetServerClientFreeFunc)(void *data); -- 1.7.10

On 05/02/2012 05:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add APIs which allow storage of a real & effective identity on all server clients. Also add an API which allows creation of an initial identity based on the results of client authentication processes like TLS, x509, SASL, SO_PEERCRED --- src/rpc/virnetserverclient.c | 152 ++++++++++++++++++++++++++++++++++++++++++ src/rpc/virnetserverclient.h | 11 +++ 2 files changed, 163 insertions(+)
+ + if (!(username = virGetUserName(uid))) + goto cleanup; + if (!(groupname = virGetGroupName(gid))) + goto cleanup; + if (virAsprintf(&processid, "%d", (int)pid) < 0)
This truncates on mingw64; I'd prefer %lld, (long long)pid. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 05/02/2012 07:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add APIs which allow storage of a real & effective identity on all server clients. Also add an API which allows creation of an initial identity based on the results of client authentication processes like TLS, x509, SASL, SO_PEERCRED --- src/rpc/virnetserverclient.c | 152 ++++++++++++++++++++++++++++++++++++++++++ src/rpc/virnetserverclient.h | 11 +++ 2 files changed, 163 insertions(+)
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 1e9d3db..9647ac3 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -75,6 +75,10 @@ struct _virNetServerClient int sockTimer; /* Timer to be fired upon cached data, * so we jump out from poll() immediately */
+ + virIdentityPtr realIdentity; + virIdentityPtr effectiveIdentity; + /* Count of messages in the 'tx' queue, * and the server worker pool queue * ie RPC calls in progress. Does not count @@ -487,6 +491,149 @@ int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, }
+virIdentityPtr virNetServerClientGetIdentity(virNetServerClientPtr client) +{ + char *processid = NULL; + char *username = NULL; + char *groupname = NULL; +#if HAVE_SASL + char *saslname = NULL; +#endif + char *x509dname = NULL; + char *seccontext = NULL; + virIdentityPtr ret = NULL; + virNetSASLSessionPtr sasl; + virNetTLSSessionPtr tls; + + if (virNetServerClientIsLocal(client)) { + gid_t gid; + uid_t uid; + pid_t pid; + if (virNetServerClientGetUNIXIdentity(client, &uid, &gid, &pid) < 0) + goto cleanup; + + if (!(username = virGetUserName(uid))) + goto cleanup; + if (!(groupname = virGetGroupName(gid))) + goto cleanup; + if (virAsprintf(&processid, "%d", (int)pid) < 0) + goto cleanup; + } + +#if HAVE_SASL + if ((sasl = virNetServerClientGetSASLSession(client))) { + const char *identity = virNetSASLSessionGetIdentity(sasl); + if (identity && + !(saslname = strdup(identity))) { + virReportOOMError(); + goto cleanup; + } + } +#endif + + if ((tls = virNetServerClientGetTLSSession(client))) { + const char *identity = virNetTLSSessionGetX509DName(tls); + if (identity && + !(x509dname = strdup(identity))) { + virReportOOMError(); + goto cleanup; + } + } + + if (virNetServerClientGetSecurityContext(client, &seccontext) < 0) + goto cleanup; + + if (!(ret = virIdentityNew())) + goto cleanup; + + if (username && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_USER_NAME, username) < 0) + goto error; + if (groupname && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_GROUP_NAME, groupname) < 0) + goto error; + if (processid && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_PROCESS_ID, processid) < 0) + goto error; +#if HAVE_SASL + if (saslname && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_SASL_USER_NAME, saslname) < 0) + goto error; +#endif + if (x509dname && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_X509_DISTINGUISHED_NAME, x509dname) < 0) + goto error; + if (seccontext && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_SECURITY_CONTEXT, seccontext) < 0) + goto error; + +cleanup: + VIR_FREE(username); + VIR_FREE(groupname); + VIR_FREE(processid); + VIR_FREE(seccontext); +#if HAVE_SASL + VIR_FREE(saslname); +#endif + VIR_FREE(x509dname); + return ret; + +error:
Are there leaks here for username, groupname, processid, seccontext, etc? -- Regards, Corey

From: "Daniel P. Berrange" <berrange@redhat.com> When dispatching an RPC API call, setup the access manager to hold the real & effective identities of the current server client whose RPC is being dispatched. The setting is thread-local, so only affects the API call in this thread --- src/rpc/virnetserverclient.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/rpc/virnetserverclient.h | 3 +++ src/rpc/virnetserverprogram.c | 9 +++++++++ 3 files changed, 53 insertions(+) diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 9647ac3..043938e 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -34,6 +34,7 @@ #include "memory.h" #include "threads.h" #include "virkeepalive.h" +#include "access/viraccessmanager.h" #define VIR_FROM_THIS VIR_FROM_RPC #define virNetError(code, ...) \ @@ -634,6 +635,46 @@ void virNetServerClientSetEffectiveIdentity(virNetServerClientPtr client, } +int virNetServerClientActivateIdentity(virNetServerClientPtr client) +{ + virIdentityPtr real; + virIdentityPtr effective; + virIdentityPtr sys = NULL; + int ret = -1; + + real = virNetServerClientGetRealIdentity(client); + effective = virNetServerClientGetEffectiveIdentity(client); + if ((!real || !effective) && + !(sys = virAccessManagerGetSystemIdentity())) + goto cleanup; + + if (virAccessManagerSetRealIdentity(real ? real : sys) < 0) + goto cleanup; + if (virAccessManagerSetEffectiveIdentity(effective ? effective : sys) < 0) + goto cleanup; + + ret = 0; +cleanup: + if (real) + virIdentityFree(real); + if (effective) + virIdentityFree(effective); + if (sys) + virIdentityFree(sys); + return ret; +} + + +int virNetServerClientDeactivateIdentity(virNetServerClientPtr client ATTRIBUTE_UNUSED) +{ + if (virAccessManagerSetRealIdentity(NULL) < 0) + return -1; + if (virAccessManagerSetEffectiveIdentity(NULL) < 0) + return -1; + return 0; +} + + int virNetServerClientGetSecurityContext(virNetServerClientPtr client, char **context) { diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 7435eee..f3c95cc 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -87,6 +87,9 @@ void virNetServerClientSetRealIdentity(virNetServerClientPtr client, void virNetServerClientSetEffectiveIdentity(virNetServerClientPtr client, virIdentityPtr iden); +int virNetServerClientActivateIdentity(virNetServerClientPtr client); +int virNetServerClientDeactivateIdentity(virNetServerClientPtr client); + void virNetServerClientRef(virNetServerClientPtr client); diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c index 7f589c8..d141465 100644 --- a/src/rpc/virnetserverprogram.c +++ b/src/rpc/virnetserverprogram.c @@ -403,6 +403,9 @@ virNetServerProgramDispatchCall(virNetServerProgramPtr prog, if (virNetMessageDecodePayload(msg, dispatcher->arg_filter, arg) < 0) goto error; + if (virNetServerClientActivateIdentity(client) < 0) + goto error; + /* * When the RPC handler is called: * @@ -415,6 +418,12 @@ virNetServerProgramDispatchCall(virNetServerProgramPtr prog, */ rv = (dispatcher->func)(server, client, msg, &rerr, arg, ret); + if (virNetServerClientDeactivateIdentity(client) < 0) { + virErrorPtr err = virGetLastError(); + VIR_WARN("Failed to deactive identity %s", err ? err->message : "null"); + virResetLastError(); + } + /* * Clear out the FDs we got from the client, we don't * want to send them back ! -- 1.7.10

On 05/02/2012 05:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
When dispatching an RPC API call, setup the access manager to hold the real & effective identities of the current server client whose RPC is being dispatched. The setting is thread-local, so only affects the API call in this thread --- src/rpc/virnetserverclient.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/rpc/virnetserverclient.h | 3 +++ src/rpc/virnetserverprogram.c | 9 +++++++++ 3 files changed, 53 insertions(+)
* @@ -415,6 +418,12 @@ virNetServerProgramDispatchCall(virNetServerProgramPtr prog, */ rv = (dispatcher->func)(server, client, msg, &rerr, arg, ret);
+ if (virNetServerClientDeactivateIdentity(client) < 0) { + virErrorPtr err = virGetLastError(); + VIR_WARN("Failed to deactive identity %s", err ? err->message : "null");
s/deactive/deactivate/ -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange@redhat.com> Introduce a new 'access_driver' configuration parameter which specifies the name of the access control manager driver to activate. By default the 'no op' driver is active --- src/qemu/qemu.conf | 5 +++++ src/qemu/qemu_conf.c | 9 +++++++++ src/qemu/qemu_conf.h | 3 +++ src/qemu/qemu_driver.c | 24 ++++++++++++++++++++++++ 4 files changed, 41 insertions(+) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index cb87728..4ea4eb6 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -147,6 +147,11 @@ # guests will be blocked. Defaults to 0. # security_require_confined = 1 +# There is no default access control driver +# +# access_driver = "polkit" + + # The user ID for QEMU processes run by the system instance. #user = "root" diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 88a04bc..e4a4efc 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -210,6 +210,15 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, CHECK_TYPE ("security_require_confined", VIR_CONF_LONG); if (p) driver->securityRequireConfined = p->l; + p = virConfGetValue (conf, "access_driver"); + CHECK_TYPE ("access_driver", VIR_CONF_STRING); + if (p && p->str) { + if (!(driver->accessDriverName = strdup(p->str))) { + virReportOOMError(); + virConfFree(conf); + return -1; + } + } p = virConfGetValue (conf, "vnc_sasl"); CHECK_TYPE ("vnc_sasl", VIR_CONF_LONG); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 482e6d3..f3daa03 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -34,6 +34,7 @@ # include "domain_event.h" # include "threads.h" # include "security/security_manager.h" +# include "access/viraccessmanager.h" # include "cgroup.h" # include "pci.h" # include "hostusb.h" @@ -120,6 +121,8 @@ struct qemud_driver { bool securityDefaultConfined; bool securityRequireConfined; virSecurityManagerPtr securityManager; + char *accessDriverName; + virAccessManagerPtr accessManager; char *saveImageFormat; char *dumpImageFormat; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 86e82d6..751c3c7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -249,6 +249,26 @@ error: } +static int +qemuAccessInit(struct qemud_driver *driver) +{ + virAccessManagerPtr mgr = virAccessManagerNew(driver->accessDriverName ? + driver->accessDriverName : + "none"); + if (!mgr) + goto error; + + driver->accessManager = mgr; + + return 0; + +error: + VIR_ERROR(_("Failed to initialize access drivers")); + virAccessManagerFree(mgr); + return -1; +} + + static virCapsPtr qemuCreateCapabilities(virCapsPtr oldcaps, struct qemud_driver *driver) @@ -613,6 +633,9 @@ qemudStartup(int privileged) { if (qemuSecurityInit(qemu_driver) < 0) goto error; + if (qemuAccessInit(qemu_driver) < 0) + goto error; + if ((qemu_driver->caps = qemuCreateCapabilities(NULL, qemu_driver)) == NULL) goto error; @@ -857,6 +880,7 @@ qemudShutdown(void) { VIR_FREE(qemu_driver->dumpImageFormat); virSecurityManagerFree(qemu_driver->securityManager); + virAccessManagerFree(qemu_driver->accessManager); ebtablesContextFree(qemu_driver->ebtables); -- 1.7.10

On 05/02/2012 05:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Introduce a new 'access_driver' configuration parameter which specifies the name of the access control manager driver to activate. By default the 'no op' driver is active --- src/qemu/qemu.conf | 5 +++++ src/qemu/qemu_conf.c | 9 +++++++++ src/qemu/qemu_conf.h | 3 +++ src/qemu/qemu_driver.c | 24 ++++++++++++++++++++++++ 4 files changed, 41 insertions(+)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange@redhat.com> Inserts the minimal access control checks to the QEMU driver to protect usage of virDomainObjPtr objects. --- src/qemu/qemu_driver.c | 626 +++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_migration.c | 5 + 2 files changed, 605 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 751c3c7..ea090a6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1202,6 +1202,11 @@ static virDomainPtr qemudDomainLookupByID(virConnectPtr conn, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_GETATTR)) + goto cleanup; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) dom->id = vm->def->id; @@ -1229,6 +1234,11 @@ static virDomainPtr qemudDomainLookupByUUID(virConnectPtr conn, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_GETATTR)) + goto cleanup; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) dom->id = vm->def->id; @@ -1254,6 +1264,11 @@ static virDomainPtr qemudDomainLookupByName(virConnectPtr conn, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_GETATTR)) + goto cleanup; + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) dom->id = vm->def->id; @@ -1267,72 +1282,90 @@ cleanup: static int qemuDomainIsActive(virDomainPtr dom) { struct qemud_driver *driver = dom->conn->privateData; - virDomainObjPtr obj; + virDomainObjPtr vm; int ret = -1; qemuDriverLock(driver); - obj = virDomainFindByUUID(&driver->domains, dom->uuid); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); - if (!obj) { + if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } - ret = virDomainObjIsActive(obj); + + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + + ret = virDomainObjIsActive(vm); cleanup: - if (obj) - virDomainObjUnlock(obj); + if (vm) + virDomainObjUnlock(vm); return ret; } static int qemuDomainIsPersistent(virDomainPtr dom) { struct qemud_driver *driver = dom->conn->privateData; - virDomainObjPtr obj; + virDomainObjPtr vm; int ret = -1; qemuDriverLock(driver); - obj = virDomainFindByUUID(&driver->domains, dom->uuid); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); - if (!obj) { + if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } - ret = obj->persistent; + + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + + ret = vm->persistent; cleanup: - if (obj) - virDomainObjUnlock(obj); + if (vm) + virDomainObjUnlock(vm); return ret; } static int qemuDomainIsUpdated(virDomainPtr dom) { struct qemud_driver *driver = dom->conn->privateData; - virDomainObjPtr obj; + virDomainObjPtr vm; int ret = -1; qemuDriverLock(driver); - obj = virDomainFindByUUID(&driver->domains, dom->uuid); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); - if (!obj) { + if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } - ret = obj->updated; + + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + + ret = vm->updated; cleanup: - if (obj) - virDomainObjUnlock(obj); + if (vm) + virDomainObjUnlock(vm); return ret; } @@ -1354,10 +1387,12 @@ cleanup: static int qemudListDomains(virConnectPtr conn, int *ids, int nids) { struct qemud_driver *driver = conn->privateData; - int n; + int n = -1; qemuDriverLock(driver); - n = virDomainObjListGetActiveIDs(&driver->domains, ids, nids); + if (virAccessManagerCheckConnect(driver->accessManager, + VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)) + n = virDomainObjListGetActiveIDs(&driver->domains, ids, nids); qemuDriverUnlock(driver); return n; @@ -1365,10 +1400,12 @@ static int qemudListDomains(virConnectPtr conn, int *ids, int nids) { static int qemudNumDomains(virConnectPtr conn) { struct qemud_driver *driver = conn->privateData; - int n; + int n = -1; qemuDriverLock(driver); - n = virDomainObjListNumOfDomains(&driver->domains, 1); + if (virAccessManagerCheckConnect(driver->accessManager, + VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)) + n = virDomainObjListNumOfDomains(&driver->domains, 1); qemuDriverUnlock(driver); return n; @@ -1401,6 +1438,15 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, if (virSecurityManagerVerify(driver->securityManager, def) < 0) goto cleanup; + if (!virAccessManagerCheckDomain(driver->accessManager, + def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + if (!virAccessManagerCheckDomain(driver->accessManager, + def, + VIR_ACCESS_PERM_DOMAIN_START)) + goto cleanup; + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) goto cleanup; @@ -1491,6 +1537,11 @@ static int qemudDomainSuspend(virDomainPtr dom) { goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SUSPEND)) + goto cleanup; + priv = vm->privateData; if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) { @@ -1553,6 +1604,11 @@ static int qemudDomainResume(virDomainPtr dom) { goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SUSPEND)) + goto cleanup; + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -1613,6 +1669,11 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SHUTDOWN)) + goto cleanup; + priv = vm->privateData; if ((flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) || @@ -1694,6 +1755,11 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_REBOOT)) + goto cleanup; + priv = vm->privateData; if ((flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) || @@ -1785,6 +1851,11 @@ qemuDomainReset(virDomainPtr dom, unsigned int flags) goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_RESET)) + goto cleanup; + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -1847,6 +1918,11 @@ qemuDomainDestroyFlags(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_STOP)) + goto cleanup; + priv = vm->privateData; qemuDomainSetFakeReboot(driver, vm, false); @@ -1931,6 +2007,11 @@ static char *qemudDomainGetOSType(virDomainPtr dom) { goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (!(type = strdup(vm->def->os.type))) virReportOOMError(); @@ -1960,6 +2041,11 @@ qemuDomainGetMaxMemory(virDomainPtr dom) goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + ret = vm->def->mem.max_balloon; cleanup: @@ -1991,6 +2077,17 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && + !virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -2093,6 +2190,11 @@ static int qemuDomainInjectNMI(virDomainPtr domain, unsigned int flags) goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_INJECT_NMI)) + goto cleanup; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -2170,6 +2272,11 @@ static int qemuDomainSendKey(virDomainPtr domain, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SEND_KEY)) + goto cleanup; + priv = vm->privateData; if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) @@ -2216,6 +2323,11 @@ static int qemudDomainGetInfo(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + info->state = virDomainObjGetState(vm, NULL); if (!virDomainObjIsActive(vm)) { @@ -2303,6 +2415,11 @@ qemuDomainGetState(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + *state = virDomainObjGetState(vm, reason); ret = 0; @@ -2336,6 +2453,11 @@ qemuDomainGetControlInfo(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -2840,6 +2962,15 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_STOP)) + goto cleanup; + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_HIBERNATE)) + goto cleanup; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -2899,6 +3030,15 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_STOP)) + goto cleanup; + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_HIBERNATE)) + goto cleanup; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -2950,6 +3090,11 @@ qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + name = qemuDomainManagedSavePath(driver, vm); if (name == NULL) goto cleanup; @@ -2984,6 +3129,11 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + name = qemuDomainManagedSavePath(driver, vm); if (name == NULL) goto cleanup; @@ -3110,6 +3260,21 @@ static int qemudDomainCoreDump(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_CORE_DUMP)) + goto cleanup; + if (!(flags & VIR_DUMP_LIVE) && + !virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SUSPEND)) + goto cleanup; + if ((flags & VIR_DUMP_CRASH) && + !virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_STOP)) + goto cleanup; + if (qemuDomainObjBeginAsyncJobWithDriver(driver, vm, QEMU_ASYNC_JOB_DUMP) < 0) goto cleanup; @@ -3220,6 +3385,11 @@ qemuDomainScreenshot(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SCREENSHOT)) + goto cleanup; + priv = vm->privateData; if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) @@ -3446,6 +3616,16 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && + !virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SAVE)) + goto cleanup; + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -3564,6 +3744,16 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && + !virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SAVE)) + goto cleanup; + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) goto cleanup; @@ -3697,6 +3887,11 @@ qemudDomainGetVcpuPinInfo(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &targetDef) < 0) goto cleanup; @@ -3776,6 +3971,11 @@ qemudDomainGetVcpus(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -3867,6 +4067,11 @@ qemudDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &def) < 0) goto cleanup; @@ -3908,6 +4113,11 @@ static int qemudDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr sec goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (!virDomainVirtTypeToString(vm->def->virtType)) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("unknown virt type in domain definition '%d'"), @@ -4281,6 +4491,11 @@ qemuDomainRestoreFlags(virConnectPtr conn, if (fd < 0) goto cleanup; + if (!virAccessManagerCheckDomain(driver->accessManager, + def, + VIR_ACCESS_PERM_DOMAIN_START)) + goto cleanup; + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) goto cleanup; @@ -4345,6 +4560,11 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path, if (fd < 0) goto cleanup; + if (!virAccessManagerCheckDomain(driver->accessManager, + def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + ret = qemuDomainDefFormatXML(driver, def, flags); cleanup: @@ -4387,6 +4607,15 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + if (!virAccessManagerCheckDomain(driver->accessManager, + def, + VIR_ACCESS_PERM_DOMAIN_SAVE)) + goto cleanup; + xml = qemuDomainDefFormatXML(driver, def, (VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_SECURE)); if (!xml) @@ -4501,6 +4730,16 @@ static char *qemuDomainGetXMLDesc(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if ((flags & VIR_DOMAIN_XML_SECURE) && + !virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ_SECURE)) + goto cleanup; + /* Refresh current memory based on balloon info if supported */ if ((vm->def->memballoon != NULL) && (vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_NONE) && @@ -4717,20 +4956,24 @@ cleanup: static int qemudListDefinedDomains(virConnectPtr conn, char **const names, int nnames) { struct qemud_driver *driver = conn->privateData; - int n; + int n = -1; qemuDriverLock(driver); - n = virDomainObjListGetInactiveNames(&driver->domains, names, nnames); + if (virAccessManagerCheckConnect(driver->accessManager, + VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)) + n = virDomainObjListGetInactiveNames(&driver->domains, names, nnames); qemuDriverUnlock(driver); return n; } static int qemudNumDefinedDomains(virConnectPtr conn) { struct qemud_driver *driver = conn->privateData; - int n; + int n = -1; qemuDriverLock(driver); - n = virDomainObjListNumOfDomains(&driver->domains, 0); + if (virAccessManagerCheckConnect(driver->accessManager, + VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)) + n = virDomainObjListNumOfDomains(&driver->domains, 0); qemuDriverUnlock(driver); return n; @@ -4832,6 +5075,11 @@ qemuDomainStartWithFlags(virDomainPtr dom, unsigned int flags) goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_START)) + goto cleanup; + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -4987,6 +5235,15 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { if (virSecurityManagerVerify(driver->securityManager, def) < 0) goto cleanup; + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SAVE)) + goto cleanup; + if ((dupVM = virDomainObjIsDuplicate(&driver->domains, def, 0)) < 0) goto cleanup; @@ -5057,6 +5314,11 @@ qemuDomainUndefineFlags(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_DELETE)) + goto cleanup; + if (!vm->persistent) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot undefine transient domain")); @@ -5708,6 +5970,17 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + + if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && + !virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SAVE)) + goto cleanup; + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -5879,6 +6152,11 @@ static int qemudDomainGetAutostart(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + *autostart = vm->autostart; ret = 0; @@ -5906,6 +6184,15 @@ static int qemudDomainSetAutostart(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SAVE)) + goto cleanup; + if (!vm->persistent) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot set autostart for transient domain")); @@ -5996,8 +6283,22 @@ static char *qemuGetSchedulerType(virDomainPtr dom, struct qemud_driver *driver = dom->conn->privateData; char *ret = NULL; int rc; + virDomainObjPtr vm = NULL; qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (vm == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("No such domain %s"), dom->uuid); + goto cleanup; + } + + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup CPU controller is not mounted")); @@ -6019,6 +6320,8 @@ static char *qemuGetSchedulerType(virDomainPtr dom, virReportOOMError(); cleanup: + if (vm) + virDomainObjUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -6183,6 +6486,16 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && + !virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SAVE)) + goto cleanup; + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) goto cleanup; @@ -6337,6 +6650,11 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if ((*nparams) == 0) { /* Current number of blkio parameters supported by cgroups */ *nparams = QEMU_NB_BLKIO_PARAM; @@ -6525,6 +6843,16 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && + !virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SAVE)) + goto cleanup; + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) goto cleanup; @@ -6632,6 +6960,11 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) goto cleanup; @@ -6794,6 +7127,16 @@ qemuDomainSetNumaParameters(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && + !virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SAVE)) + goto cleanup; + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) goto cleanup; @@ -6957,6 +7300,11 @@ qemuDomainGetNumaParameters(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) goto cleanup; @@ -7178,6 +7526,16 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && + !virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SAVE)) + goto cleanup; + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &vmdef) < 0) goto cleanup; @@ -7392,6 +7750,11 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) goto cleanup; @@ -7524,6 +7887,11 @@ qemuDomainBlockResize(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + priv = vm->privateData; if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) @@ -7594,6 +7962,11 @@ qemuDomainBlockStats(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -7679,6 +8052,11 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -7848,6 +8226,11 @@ qemudDomainInterfaceStats (virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -7929,6 +8312,16 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && + !virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SAVE)) + goto cleanup; + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) goto cleanup; @@ -8092,6 +8485,11 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) goto cleanup; @@ -8206,6 +8604,11 @@ qemudDomainMemoryStats (virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) goto cleanup; @@ -8267,6 +8670,11 @@ qemudDomainBlockPeek (virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ_BLOCK)) + goto cleanup; + if (!path || path[0] == '\0') { qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("NULL or empty path")); @@ -8335,6 +8743,11 @@ qemudDomainMemoryPeek (virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ_MEM)) + goto cleanup; + if (flags != VIR_MEMORY_VIRTUAL && flags != VIR_MEMORY_PHYSICAL) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("flags parameter must be VIR_MEMORY_VIRTUAL or VIR_MEMORY_PHYSICAL")); @@ -8432,6 +8845,11 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (!path || path[0] == '\0') { qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("NULL or empty path")); @@ -8795,6 +9213,11 @@ qemudDomainMigratePerform (virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_MIGRATE)) + goto cleanup; + if (flags & VIR_MIGRATE_PEER2PEER) { dconnuri = uri; uri = NULL; @@ -8841,6 +9264,11 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_MIGRATE)) + goto cleanup; + /* Do not use cookies in v2 protocol, since the cookie * length was not sufficiently large, causing failures * migrating between old & new libvirtd @@ -8885,6 +9313,11 @@ qemuDomainMigrateBegin3(virDomainPtr domain, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_MIGRATE)) + goto cleanup; + if ((flags & VIR_MIGRATE_CHANGE_PROTECTION)) { if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) goto cleanup; @@ -9072,6 +9505,11 @@ qemuDomainMigratePerform3(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_MIGRATE)) + goto cleanup; + ret = qemuMigrationPerform(driver, dom->conn, vm, xmlin, dconnuri, uri, cookiein, cookieinlen, cookieout, cookieoutlen, @@ -9109,6 +9547,11 @@ qemuDomainMigrateFinish3(virConnectPtr dconn, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_MIGRATE)) + goto cleanup; + dom = qemuMigrationFinish(driver, dconn, vm, cookiein, cookieinlen, cookieout, cookieoutlen, @@ -9143,6 +9586,11 @@ qemuDomainMigrateConfirm3(virDomainPtr domain, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_MIGRATE)) + goto cleanup; + if (!qemuMigrationJobIsActive(vm, QEMU_ASYNC_JOB_MIGRATION_OUT)) goto cleanup; @@ -9384,6 +9832,11 @@ static int qemuDomainGetJobInfo(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + priv = vm->privateData; if (virDomainObjIsActive(vm)) { @@ -9434,6 +9887,11 @@ static int qemuDomainAbortJob(virDomainPtr dom) { goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_ABORT) < 0) goto cleanup; @@ -9496,6 +9954,11 @@ qemuDomainMigrateSetMaxDowntime(virDomainPtr dom, return -1; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_MIGRATE)) + goto cleanup; + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MIGRATION_OP) < 0) goto cleanup; @@ -9552,6 +10015,11 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, return -1; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_MIGRATE)) + goto cleanup; + priv = vm->privateData; if (virDomainObjIsActive(vm)) { if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MIGRATION_OP) < 0) @@ -9609,6 +10077,11 @@ qemuDomainMigrateGetMaxSpeed(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_MIGRATE)) + goto cleanup; + priv = vm->privateData; *bandwidth = priv->migMaxBandwidth; ret = 0; @@ -10318,6 +10791,11 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SNAPSHOT)) + goto cleanup; + if (qemuProcessAutoDestroyActive(driver, vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is marked for auto destroy")); @@ -10567,6 +11045,11 @@ static int qemuDomainSnapshotListNames(virDomainPtr domain, char **names, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + n = virDomainSnapshotObjListGetNames(&vm->snapshots, names, nameslen, flags); @@ -10598,6 +11081,11 @@ static int qemuDomainSnapshotNum(virDomainPtr domain, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + /* All qemu snapshots have libvirt metadata, so * VIR_DOMAIN_SNAPSHOT_LIST_METADATA makes no difference to our * answer. */ @@ -10636,6 +11124,11 @@ qemuDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name); if (!snap) { qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, @@ -10676,6 +11169,11 @@ qemuDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name); if (!snap) { qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, @@ -10718,6 +11216,11 @@ static virDomainSnapshotPtr qemuDomainSnapshotLookupByName(virDomainPtr domain, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + snap = virDomainSnapshotFindByName(&vm->snapshots, name); if (!snap) { qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, @@ -10753,6 +11256,11 @@ static int qemuDomainHasCurrentSnapshot(virDomainPtr domain, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + ret = (vm->current_snapshot != NULL); cleanup: @@ -10783,6 +11291,11 @@ qemuDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name); if (!snap) { qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, @@ -10826,6 +11339,11 @@ static virDomainSnapshotPtr qemuDomainSnapshotCurrent(virDomainPtr domain, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if (!vm->current_snapshot) { qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, "%s", _("the domain does not have a current snapshot")); @@ -10861,6 +11379,11 @@ static char *qemuDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name); if (!snap) { qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, @@ -10931,6 +11454,11 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SNAPSHOT)) + goto cleanup; + snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name); if (!snap) { qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, @@ -11297,6 +11825,11 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SNAPSHOT)) + goto cleanup; + snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name); if (!snap) { qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, @@ -11414,6 +11947,11 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -11473,6 +12011,11 @@ static virDomainPtr qemuDomainAttach(virConnectPtr conn, &pidfile, &monConfig, &monJSON))) goto cleanup; + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_START)) + goto cleanup; + if (!monConfig) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("No monitor connection for pid %u"), pid_value); @@ -11567,6 +12110,11 @@ qemuDomainOpenConsole(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_OPEN_CONSOLE)) + goto cleanup; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -11686,6 +12234,12 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } + + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -11862,6 +12416,11 @@ qemuDomainOpenGraphics(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_OPEN_GRAPHICS)) + goto cleanup; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -11957,6 +12516,16 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_WRITE)) + goto cleanup; + if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && + !virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_SAVE)) + goto cleanup; + device = qemuDiskPathToAlias(vm, disk, &idx); if (!device) { goto cleanup; @@ -12106,6 +12675,11 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, goto cleanup; } + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; + if ((*nparams) == 0) { /* Current number of parameters supported by QEMU Block I/O Throttling */ *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3a420be..3fab80d 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1219,6 +1219,11 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_MIGRATE)) + goto cleanup; + if (!qemuMigrationIsAllowed(driver, NULL, def)) goto cleanup; -- 1.7.10

On 05/02/2012 05:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Inserts the minimal access control checks to the QEMU driver to protect usage of virDomainObjPtr objects. --- src/qemu/qemu_driver.c | 626 +++++++++++++++++++++++++++++++++++++++++++--
600 lines qualifies as minimal - wow, we've got a lot of new code to add for the other checks. I'm wondering if you can factor this for a smaller addition:
@@ -1267,72 +1282,90 @@ cleanup: static int qemuDomainIsActive(virDomainPtr dom) { struct qemud_driver *driver = dom->conn->privateData; - virDomainObjPtr obj; + virDomainObjPtr vm; int ret = -1;
qemuDriverLock(driver); - obj = virDomainFindByUUID(&driver->domains, dom->uuid); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); - if (!obj) { + if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } - ret = virDomainObjIsActive(obj); + + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; +
The pattern of getting a VM, followed by an access check, is pretty common. What if we introduce a helper function: vm = qemuDomainFindByUUIDAccess(driver, dom->uuid, VIR_ACCESS_PERM_DOMAIN_READ); which does both the virDOmainFindByUUID(&driver->domains, dom->uuid) and the virAccessManagerCheckDomain(driver->accessManager, vm->def, access_perm).
@@ -2899,6 +3030,15 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) goto cleanup; }
+ if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_STOP)) + goto cleanup; + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_HIBERNATE)) + goto cleanup;
Hmm, code like this, where you check two separate permissions, makes me wonder if we should allow virAccessManagerCheckDomain to take a bitmap argument for all checks to run, as well as a simple way of generating a bitmap for one or more access bits in one go. Or even some glue magic to allow checking an unbounded list of permissions in what appears to be one call: virAccessManagerCheckDomain(driver->accessManager, vm->def, VIR_ACCESS_PERM_DOMAIN_STOP, VIR_ACCESS_PERM_DOMAIN_HIBERNATE) #define virAccessManagerCheck(...) \ virAccessManagerCheckAll(__VA_ARGS__, 0) virAccessManagerCheckAll(manager, def, ...) { va_list list; int perm; va_start(list, def); while ((perm = va_arg(list, int)) virAccessManagerCheckDomainOne(manager, def, perm); va_end(list); } The bulk of the patch looks mechanical, and I didn't closely check it for inaccuracies. But the overall idea seems worthwhile. I'm afraid that this series won't make it into 0.9.12, though, as it is rather invasive at this point when we already have rc1 out. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Thu, May 03, 2012 at 05:32:05PM -0600, Eric Blake wrote:
On 05/02/2012 05:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Inserts the minimal access control checks to the QEMU driver to protect usage of virDomainObjPtr objects. --- src/qemu/qemu_driver.c | 626 +++++++++++++++++++++++++++++++++++++++++++--
600 lines qualifies as minimal - wow, we've got a lot of new code to add for the other checks. I'm wondering if you can factor this for a smaller addition:
@@ -1267,72 +1282,90 @@ cleanup: static int qemuDomainIsActive(virDomainPtr dom) { struct qemud_driver *driver = dom->conn->privateData; - virDomainObjPtr obj; + virDomainObjPtr vm; int ret = -1;
qemuDriverLock(driver); - obj = virDomainFindByUUID(&driver->domains, dom->uuid); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); - if (!obj) { + if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } - ret = virDomainObjIsActive(obj); + + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_READ)) + goto cleanup; +
The pattern of getting a VM, followed by an access check, is pretty common. What if we introduce a helper function:
vm = qemuDomainFindByUUIDAccess(driver, dom->uuid, VIR_ACCESS_PERM_DOMAIN_READ);
which does both the virDOmainFindByUUID(&driver->domains, dom->uuid) and the virAccessManagerCheckDomain(driver->accessManager, vm->def, access_perm).
@@ -2899,6 +3030,15 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) goto cleanup; }
+ if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_STOP)) + goto cleanup; + if (!virAccessManagerCheckDomain(driver->accessManager, + vm->def, + VIR_ACCESS_PERM_DOMAIN_HIBERNATE)) + goto cleanup;
Hmm, code like this, where you check two separate permissions, makes me wonder if we should allow virAccessManagerCheckDomain to take a bitmap argument for all checks to run, as well as a simple way of generating a bitmap for one or more access bits in one go. Or even some glue magic to allow checking an unbounded list of permissions in what appears to be one call:
virAccessManagerCheckDomain(driver->accessManager, vm->def, VIR_ACCESS_PERM_DOMAIN_STOP, VIR_ACCESS_PERM_DOMAIN_HIBERNATE)
#define virAccessManagerCheck(...) \ virAccessManagerCheckAll(__VA_ARGS__, 0) virAccessManagerCheckAll(manager, def, ...) { va_list list; int perm;
va_start(list, def); while ((perm = va_arg(list, int)) virAccessManagerCheckDomainOne(manager, def, perm); va_end(list); }
The bulk of the patch looks mechanical, and I didn't closely check it for inaccuracies. But the overall idea seems worthwhile.
I'm afraid that this series won't make it into 0.9.12, though, as it is rather invasive at this point when we already have rc1 out.
Oh I wasn't even intending it for that. This is more of an RFC to see if people like the direction this is going, before doing more work on it. There are still all the other drivers to add access checks to :-) 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 05/02/2012 07:44 AM, Daniel P. Berrange wrote:
This is a repost of
https://www.redhat.com/archives/libvir-list/2012-January/msg00907.html
which got no comments last time out.
This series of patch is the minimal required to get a working proof of concept implementation of fine grained access control in libvirt.
This demonstrates
- Obtaining a client identity from a socket - Ensuring RPC calls are executed with the correct identity sset - A policykit access driver that checks based on access vector alone - A SELinux access driver that checks based on access vector + object - A set of hooks in the QEMU driver to protect virDomainObjPtr access
Things that are not done
- APIs for changing the real/effective identity post-connect - A simple RBAC access driver for doing (Access vector, object) checks - SELinux policy for the SELinux driver - Access control hooks on all other QEMU driver methods - Access control hooks in LXC, UML, other libvirtd side drivers - Access control hooks in storage, network, interface, etc drivers - Document WTF todo to propagate SELinux contexts across TCP sockets using IPSec. Any hints welcome... - Lots more I can't think of right now
Does it make sense to have an AppArmor driver too? -- Regards, Corey
I should note that the policykit driver is mostly useless because it is unable to let you do checks on anything other than permission name and UNIX process ID at this time. So what I've implemented with the polkit driver is really little more than a slightly more fine grained version of the VIR_CONNECT_RO flag. In theory it is supposed to be extendable to allow other types of identity information besides the process ID, and to include some kind of object identiers in the permission check, but no one seems to be attacking this.
So I expect the simple RBAC driver to be the most used one in the common case usage of libvirt, and of course the SELinux driver.
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (4)
-
Corey Bryant
-
Daniel P. Berrange
-
Eric Blake
-
Michal Privoznik