From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; 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: Okay, if we are brave enough. Although, win32 implementation is missing. Michal

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; 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 Wed, May 02, 2012 at 05:23:01PM +0200, Michal Privoznik wrote: 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 :|

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; 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

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; --- 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 07:44 AM, Daniel P. Berrange wrote: Do you want to use %u instead of %d here? Do you need to add virIdentityRef here too? -- Regards, Corey

On 05/02/2012 05:44 AM, Daniel P. Berrange wrote: 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? Copyright header? Also, can you add a short description about what the rest of this file is describing? You know, we really ought to divide libvirt.h into multiple headers, one per object type, to make this a bit easier to track. 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? 2012 now (wow, you've been working on this for some time now...) 2012 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. Does this fail even if SELinux is not enabled? And if so, what is the impact of failing here? Especially since here, you seem to be catering to the case of a NULL seccontext when SELinux was not compiled in. This loses the old identity, 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). Same comment. Phew - we learned our lesson with the security manager. 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. Sorting: viraccessmanager.h comes before virauth.h, and... within the file, sort the function names. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; --- 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

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; --- 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 07:44 AM, Daniel P. Berrange wrote: It looks like avc_init is deprecated. 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. -- Regards, Corey

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; 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

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; 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

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; 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 Thu, May 03, 2012 at 05:32:05PM -0600, Eric Blake wrote: 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 :|