[libvirt] [PATCH 0/8] Identity management APIs for RBAC

The patches in this series provide the identity management framework which is a pre-requisite for later role based access control patches.

From: "Daniel P. Berrange" <berrange@redhat.com> Add a virThreadCancel function. This functional is inherantly dangerous and not something we want to use in general, but integration with SELinux requires that we provide this stub. We leave out any Win32 impl to discourage further use and because obviously SELinux isn't enabled on Win32 Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virthread.h | 1 + src/util/virthreadpthread.c | 5 +++++ 3 files changed, 7 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ed46479..acaa4d7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1743,6 +1743,7 @@ virMutexInitRecursive; virMutexLock; virMutexUnlock; virOnce; +virThreadCancel; virThreadCreate; virThreadID; virThreadInitialize; diff --git a/src/util/virthread.h b/src/util/virthread.h index c59a2cf..3b11dca 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -54,6 +54,7 @@ int virThreadCreate(virThreadPtr thread, void virThreadSelf(virThreadPtr thread); bool virThreadIsSelf(virThreadPtr thread); void virThreadJoin(virThreadPtr thread); +void virThreadCancel(virThreadPtr thread); /* These next two functions are for debugging only, since they are not * guaranteed to give unique values for distinct threads on all diff --git a/src/util/virthreadpthread.c b/src/util/virthreadpthread.c index 9f02ba1..b42b333 100644 --- a/src/util/virthreadpthread.c +++ b/src/util/virthreadpthread.c @@ -236,6 +236,11 @@ void virThreadJoin(virThreadPtr thread) pthread_join(thread->thread, NULL); } +void virThreadCancel(virThreadPtr thread) +{ + pthread_cancel(thread->thread); +} + int virThreadLocalInit(virThreadLocalPtr l, virThreadLocalCleanup c) { -- 1.8.1.4

On 03/06/2013 05:49 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add a virThreadCancel function. This functional is inherantly
s/inherantly/inherently/
dangerous and not something we want to use in general, but integration with SELinux requires that we provide this stub. We leave out any Win32 impl to discourage further use and because obviously SELinux isn't enabled on Win32
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virthread.h | 1 + src/util/virthreadpthread.c | 5 +++++ 3 files changed, 7 insertions(+)
+++ b/src/util/virthreadpthread.c @@ -236,6 +236,11 @@ void virThreadJoin(virThreadPtr thread) pthread_join(thread->thread, NULL); }
+void virThreadCancel(virThreadPtr thread)
It would help to add documentation right before this implementation that mentions that the function exists SOLELY for use by threads created by third-party libraries which are prepared for cancellation, and that most libvirt threads do not qualify. Having the discouraging comment tucked away solely in the commit message won't help a future coder who doesn't realize the dangers. ACK with the comment added. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Thu, Mar 07, 2013 at 05:10:36PM -0700, Eric Blake wrote:
On 03/06/2013 05:49 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add a virThreadCancel function. This functional is inherantly
s/inherantly/inherently/
dangerous and not something we want to use in general, but integration with SELinux requires that we provide this stub. We leave out any Win32 impl to discourage further use and because obviously SELinux isn't enabled on Win32
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virthread.h | 1 + src/util/virthreadpthread.c | 5 +++++ 3 files changed, 7 insertions(+)
+++ b/src/util/virthreadpthread.c @@ -236,6 +236,11 @@ void virThreadJoin(virThreadPtr thread) pthread_join(thread->thread, NULL); }
+void virThreadCancel(virThreadPtr thread)
It would help to add documentation right before this implementation that mentions that the function exists SOLELY for use by threads created by third-party libraries which are prepared for cancellation, and that most libvirt threads do not qualify. Having the discouraging comment tucked away solely in the commit message won't help a future coder who doesn't realize the dangers.
ACK with the comment added.
I'll add it in the header file since that's where people will probably find the function first. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

From: "Daniel P. Berrange" <berrange@redhat.com> Currently the server determines whether authentication of clients is complete, by checking whether an identity is set. This patch removes that lame hack and replaces it with an explicit method for changing the client auth code * daemon/remote.c: Update for new APis * src/libvirt_private.syms, src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h: Remove virNetServerClientGetIdentity and virNetServerClientSetIdentity, adding a new method virNetServerClientSetAuth. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/remote.c | 14 +++++------- src/libvirt_private.syms | 3 +-- src/rpc/virnetserverclient.c | 52 +++++++------------------------------------- src/rpc/virnetserverclient.h | 5 +---- 4 files changed, 15 insertions(+), 59 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index c92223e..45c50f3 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2391,10 +2391,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 - auth = VIR_NET_SERVER_SERVICE_AUTH_NONE; + virNetServerClientSetAuth(client, 0); + auth = VIR_NET_SERVER_SERVICE_AUTH_NONE; VIR_FREE(ident); } } @@ -2535,9 +2533,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)); @@ -2869,7 +2865,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); @@ -3024,8 +3020,8 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, 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 acaa4d7..8604587 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -852,7 +852,6 @@ virNetServerClientClose; virNetServerClientDelayedClose; virNetServerClientGetAuth; virNetServerClientGetFD; -virNetServerClientGetIdentity; virNetServerClientGetPrivateData; virNetServerClientGetReadonly; virNetServerClientGetTLSKeySize; @@ -871,9 +870,9 @@ virNetServerClientPreExecRestart; virNetServerClientRemoteAddrString; virNetServerClientRemoveFilter; virNetServerClientSendMessage; +virNetServerClientSetAuth; virNetServerClientSetCloseHook; virNetServerClientSetDispatcher; -virNetServerClientSetIdentity; virNetServerClientStartKeepAlive; virNetServerClientWantClose; diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 446e1e9..9e519e6 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -64,7 +64,6 @@ struct _virNetServerClient virNetSocketPtr sock; int auth; bool readonly; - char *identity; #if WITH_GNUTLS virNetTLSContextPtr tlsCtxt; virNetTLSSessionPtr tls; @@ -442,7 +441,6 @@ virNetServerClientPtr virNetServerClientNewPostExecRestart(virJSONValuePtr objec virJSONValuePtr child; virNetServerClientPtr client = NULL; virNetSocketPtr sock; - const char *identity = NULL; int auth; bool readonly; unsigned int nrequests_max; @@ -463,12 +461,6 @@ virNetServerClientPtr virNetServerClientNewPostExecRestart(virJSONValuePtr objec _("Missing nrequests_client_max field in JSON state document")); return NULL; } - if (virJSONValueObjectHasKey(object, "identity") && - (!(identity = virJSONValueObjectGetString(object, "identity")))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing identity field in JSON state document")); - return NULL; - } if (!(child = virJSONValueObjectGet(object, "sock"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -493,10 +485,6 @@ virNetServerClientPtr virNetServerClientNewPostExecRestart(virJSONValuePtr objec } virObjectUnref(sock); - if (identity && - virNetServerClientSetIdentity(client, identity) < 0) - goto error; - if (privNew) { if (!(child = virJSONValueObjectGet(object, "privateData"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -536,10 +524,6 @@ virJSONValuePtr virNetServerClientPreExecRestart(virNetServerClientPtr client) if (virJSONValueObjectAppendNumberUint(object, "nrequests_max", client->nrequests_max) < 0) goto error; - if (client->identity && - virJSONValueObjectAppendString(object, "identity", client->identity) < 0) - goto error; - if (!(child = virNetSocketPreExecRestart(client->sock))) goto error; @@ -576,6 +560,13 @@ int virNetServerClientGetAuth(virNetServerClientPtr client) return auth; } +void virNetServerClientSetAuth(virNetServerClientPtr client, int auth) +{ + virObjectLock(client); + client->auth = auth; + virObjectUnlock(client); +} + bool virNetServerClientGetReadonly(virNetServerClientPtr client) { bool readonly; @@ -663,32 +654,6 @@ void virNetServerClientSetSASLSession(virNetServerClientPtr client, #endif -int virNetServerClientSetIdentity(virNetServerClientPtr client, - const char *identity) -{ - int ret = -1; - virObjectLock(client); - if (!(client->identity = strdup(identity))) { - virReportOOMError(); - goto error; - } - ret = 0; - -error: - virObjectUnlock(client); - return ret; -} - -const char *virNetServerClientGetIdentity(virNetServerClientPtr client) -{ - const char *identity; - virObjectLock(client); - identity = client->identity; - virObjectUnlock(client); - return identity; -} - - void *virNetServerClientGetPrivateData(virNetServerClientPtr client) { void *data; @@ -743,7 +708,6 @@ void virNetServerClientDispose(void *obj) client->privateDataFreeFunc) client->privateDataFreeFunc(client->privateData); - VIR_FREE(client->identity); #if WITH_SASL virObjectUnref(client->sasl); #endif @@ -1319,7 +1283,7 @@ bool virNetServerClientNeedAuth(virNetServerClientPtr client) { bool need = false; virObjectLock(client); - if (client->auth && !client->identity) + if (client->auth) need = true; virObjectUnlock(client); return need; diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 325f5d9..31414bc 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -76,6 +76,7 @@ void virNetServerClientRemoveFilter(virNetServerClientPtr client, int filterID); int virNetServerClientGetAuth(virNetServerClientPtr client); +void virNetServerClientSetAuth(virNetServerClientPtr client, int auth); bool virNetServerClientGetReadonly(virNetServerClientPtr client); # ifdef WITH_GNUTLS @@ -92,10 +93,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.8.1.4

On 03/06/2013 05:49 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Currently the server determines whether authentication of clients is complete, by checking whether an identity is set. This patch removes that lame hack and replaces it with an explicit method for changing the client auth code
* daemon/remote.c: Update for new APis * src/libvirt_private.syms, src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h: Remove virNetServerClientGetIdentity and virNetServerClientSetIdentity, adding a new method virNetServerClientSetAuth.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/remote.c | 14 +++++------- src/libvirt_private.syms | 3 +-- src/rpc/virnetserverclient.c | 52 +++++++------------------------------------- src/rpc/virnetserverclient.h | 5 +---- 4 files changed, 15 insertions(+), 59 deletions(-)
ACK, and nice reduction in size along the way. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange@redhat.com> A socket object has various pieces of security data associated with it, such as the SELinux context, the SASL username and the x509 distinguished name. Add new APIs to virNetServerClient and related modules to access this data. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 6 ++++++ src/rpc/virnetserverclient.c | 46 ++++++++++++++++++++++++++++++++++++++++++++ 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 ++ 7 files changed, 125 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8604587..9bae350 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -854,13 +854,17 @@ virNetServerClientGetAuth; virNetServerClientGetFD; virNetServerClientGetPrivateData; virNetServerClientGetReadonly; +virNetServerClientGetSASLSession; +virNetServerClientGetSecurityContext; virNetServerClientGetTLSKeySize; +virNetServerClientGetTLSSession; virNetServerClientGetUNIXIdentity; virNetServerClientHasTLSSession; virNetServerClientImmediateClose; virNetServerClientInit; virNetServerClientInitKeepAlive; virNetServerClientIsClosed; +virNetServerClientIsLocal; virNetServerClientIsSecure; virNetServerClientLocalAddrString; virNetServerClientNeedAuth; @@ -926,6 +930,7 @@ virNetSocketClose; virNetSocketDupFD; virNetSocketGetFD; virNetSocketGetPort; +virNetSocketGetSecurityContext; virNetSocketGetUNIXIdentity; virNetSocketHasCachedData; virNetSocketHasPassFD; @@ -964,6 +969,7 @@ virNetTLSContextNewServerPath; virNetTLSInit; virNetTLSSessionGetHandshakeStatus; virNetTLSSessionGetKeySize; +virNetTLSSessionGetX509DName; virNetTLSSessionHandshake; virNetTLSSessionNew; virNetTLSSessionRead; diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 9e519e6..40c8173 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -587,6 +587,16 @@ bool virNetServerClientHasTLSSession(virNetServerClientPtr client) return has; } + +virNetTLSSessionPtr virNetServerClientGetTLSSession(virNetServerClientPtr client) +{ + virNetTLSSessionPtr tls; + virObjectLock(client); + tls = client->tls; + virObjectUnlock(client); + return tls; +} + int virNetServerClientGetTLSKeySize(virNetServerClientPtr client) { int size = 0; @@ -608,6 +618,18 @@ int virNetServerClientGetFD(virNetServerClientPtr client) return fd; } + +bool virNetServerClientIsLocal(virNetServerClientPtr client) +{ + bool local = false; + virObjectLock(client); + if (client->sock) + local = virNetSocketIsLocal(client->sock); + virObjectUnlock(client); + return local; +} + + int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, uid_t *uid, gid_t *gid, pid_t *pid) { @@ -619,6 +641,20 @@ int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, return ret; } + +int virNetServerClientGetSecurityContext(virNetServerClientPtr client, + char **context) +{ + int ret = 0; + *context = NULL; + virObjectLock(client); + if (client->sock) + ret = virNetSocketGetSecurityContext(client->sock, context); + virObjectUnlock(client); + return ret; +} + + bool virNetServerClientIsSecure(virNetServerClientPtr client) { bool secure = false; @@ -651,6 +687,16 @@ void virNetServerClientSetSASLSession(virNetServerClientPtr client, client->sasl = virObjectRef(sasl); virObjectUnlock(client); } + + +virNetSASLSessionPtr virNetServerClientGetSASLSession(virNetServerClientPtr client) +{ + virNetSASLSessionPtr sasl; + virObjectLock(client); + sasl = client->sasl; + virObjectUnlock(client); + return sasl; +} #endif diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 31414bc..f8643f5 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -81,21 +81,28 @@ bool virNetServerClientGetReadonly(virNetServerClientPtr client); # ifdef WITH_GNUTLS bool virNetServerClientHasTLSSession(virNetServerClientPtr client); +virNetTLSSessionPtr virNetServerClientGetTLSSession(virNetServerClientPtr client); int virNetServerClientGetTLSKeySize(virNetServerClientPtr client); # endif # ifdef WITH_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 *virNetServerClientGetPrivateData(virNetServerClientPtr client); typedef void (*virNetServerClientCloseFunc)(virNetServerClientPtr client); diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 93980d6..af63975 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -40,6 +40,10 @@ #endif #include "c-ctype.h" +#ifdef HAVE_SELINUX +# include <selinux/selinux.h> +#endif + #include "virnetsocket.h" #include "virutil.h" #include "viralloc.h" @@ -1154,6 +1158,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 ATTRIBUTE_UNUSED, + 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 13583f8..7392c72 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -114,6 +114,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 6665268..aa7ba49 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -71,6 +71,7 @@ struct _virNetTLSSession { virNetTLSSessionWriteFunc writeFunc; virNetTLSSessionReadFunc readFunc; void *opaque; + char *x509dname; }; static virClassPtr virNetTLSContextClass; @@ -1026,6 +1027,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, @@ -1361,11 +1366,24 @@ cleanup: return ssf; } +const char *virNetTLSSessionGetX509DName(virNetTLSSessionPtr sess) +{ + const char *ret = NULL; + + virObjectLock(sess); + + ret = sess->x509dname; + + virObjectUnlock(sess); + + return ret; +} void virNetTLSSessionDispose(void *obj) { virNetTLSSessionPtr sess = obj; + VIR_FREE(sess->x509dname); VIR_FREE(sess->hostname); gnutls_deinit(sess->session); } diff --git a/src/rpc/virnettlscontext.h b/src/rpc/virnettlscontext.h index 5910ceb..21539ad 100644 --- a/src/rpc/virnettlscontext.h +++ b/src/rpc/virnettlscontext.h @@ -94,4 +94,6 @@ virNetTLSSessionGetHandshakeStatus(virNetTLSSessionPtr sess); int virNetTLSSessionGetKeySize(virNetTLSSessionPtr sess); +const char *virNetTLSSessionGetX509DName(virNetTLSSessionPtr sess); + #endif -- 1.8.1.4

On 03/06/2013 05:49 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
A socket object has various pieces of security data associated with it, such as the SELinux context, the SASL username and the x509 distinguished name. Add new APIs to virNetServerClient and related modules to access this data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 6 ++++++ src/rpc/virnetserverclient.c | 46 ++++++++++++++++++++++++++++++++++++++++++++ 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 ++ 7 files changed, 125 insertions(+)
+++ b/src/rpc/virnetserverclient.c @@ -587,6 +587,16 @@ bool virNetServerClientHasTLSSession(virNetServerClientPtr client) return has; }
+ +virNetTLSSessionPtr virNetServerClientGetTLSSession(virNetServerClientPtr client) +{ + virNetTLSSessionPtr tls; + virObjectLock(client); + tls = client->tls; + virObjectUnlock(client); + return tls; +}
This needs to be guarded by WITH_GNUTLS (since client->tls doesn't exist otherwise). Which in turn may affect your libvirt_private.syms if you don't create a counterpart stub function.
+ + +virNetSASLSessionPtr virNetServerClientGetSASLSession(virNetServerClientPtr client) +{ + virNetSASLSessionPtr sasl; + virObjectLock(client); + sasl = client->sasl; + virObjectUnlock(client); + return sasl; +} #endif
This function was inside an #ifdef, but you declared it in libvirt_private.syms, so you'd need a counterpart stub function. Shoot, ran out of review time halfway through. Overall the idea looks sound, though. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Thu, Mar 07, 2013 at 05:33:07PM -0700, Eric Blake wrote:
On 03/06/2013 05:49 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
A socket object has various pieces of security data associated with it, such as the SELinux context, the SASL username and the x509 distinguished name. Add new APIs to virNetServerClient and related modules to access this data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 6 ++++++ src/rpc/virnetserverclient.c | 46 ++++++++++++++++++++++++++++++++++++++++++++ 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 ++ 7 files changed, 125 insertions(+)
+++ b/src/rpc/virnetserverclient.c @@ -587,6 +587,16 @@ bool virNetServerClientHasTLSSession(virNetServerClientPtr client) return has; }
+ +virNetTLSSessionPtr virNetServerClientGetTLSSession(virNetServerClientPtr client) +{ + virNetTLSSessionPtr tls; + virObjectLock(client); + tls = client->tls; + virObjectUnlock(client); + return tls; +}
This needs to be guarded by WITH_GNUTLS (since client->tls doesn't exist otherwise). Which in turn may affect your libvirt_private.syms if you don't create a counterpart stub function.
You can't see it from the context, but this is already inside a WITH_GNUTLS block that is started earlier, and finished later. Seems we already have a few problems with the symbol file if WITH_GNUTLS is not defined, so we'll need to fix that up globally.
+ + +virNetSASLSessionPtr virNetServerClientGetSASLSession(virNetServerClientPtr client) +{ + virNetSASLSessionPtr sasl; + virObjectLock(client); + sasl = client->sasl; + virObjectUnlock(client); + return sasl; +} #endif
This function was inside an #ifdef, but you declared it in libvirt_private.syms, so you'd need a counterpart stub function.
I'll move it to the existing libvirt_sasl.syms files
Shoot, ran out of review time halfway through. Overall the idea looks sound, though.
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

From: "Daniel P. Berrange" <berrange@redhat.com> Introduce a local object virIdentity for managing security attributes used to form a client application's identity. Instances of this object are intended to be used as if they were immutable, once created & populated with attributes Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .gitignore | 1 + include/libvirt/virterror.h | 2 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 7 ++ src/util/virerror.c | 7 ++ src/util/viridentity.c | 178 ++++++++++++++++++++++++++++++++++++++++++++ src/util/viridentity.h | 54 ++++++++++++++ tests/Makefile.am | 5 ++ tests/viridentitytest.c | 176 +++++++++++++++++++++++++++++++++++++++++++ 10 files changed, 432 insertions(+) create mode 100644 src/util/viridentity.c create mode 100644 src/util/viridentity.h create mode 100644 tests/viridentitytest.c diff --git a/.gitignore b/.gitignore index c918372..68030d5 100644 --- a/.gitignore +++ b/.gitignore @@ -180,6 +180,7 @@ /tests/virdrivermoduletest /tests/virendiantest /tests/virhashtest +/tests/viridentitytest /tests/virkeyfiletest /tests/virlockspacetest /tests/virnet*test diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 4d79620..13b0abf 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -115,6 +115,7 @@ typedef enum { VIR_FROM_SSH = 50, /* Error from libssh2 connection transport */ VIR_FROM_LOCKSPACE = 51, /* Error from lockspace */ VIR_FROM_INITCTL = 52, /* Error from initctl device communication */ + VIR_FROM_IDENTITY = 53, /* Error from identity code */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_DOMAIN_LAST @@ -288,6 +289,7 @@ typedef enum { VIR_ERR_AGENT_UNRESPONSIVE = 86, /* guest agent is unresponsive, not running or not usable */ VIR_ERR_RESOURCE_BUSY = 87, /* resource is already in use */ + VIR_ERR_INVALID_IDENTITY = 88, /* Invalid identity pointer */ } virErrorNumber; /** diff --git a/po/POTFILES.in b/po/POTFILES.in index bd2c02e..502b2ac 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -150,6 +150,7 @@ src/util/vireventpoll.c src/util/virfile.c src/util/virhash.c src/util/virhook.c +src/util/viridentity.c src/util/virinitctl.c src/util/viriptables.c src/util/virjson.c diff --git a/src/Makefile.am b/src/Makefile.am index c1659a4..78dea13 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -83,6 +83,7 @@ UTIL_SOURCES = \ util/virhash.c util/virhash.h \ util/virhashcode.c util/virhashcode.h \ util/virhook.c util/virhook.h \ + util/viridentity.c util/viridentity.h \ util/virinitctl.c util/virinitctl.h \ util/viriptables.c util/viriptables.h \ util/virjson.c util/virjson.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9bae350..d4657d1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1311,6 +1311,13 @@ virHookInitialize; virHookPresent; +# util/viridentity.h +virIdentityGetAttr; +virIdentityIsEqual; +virIdentityNew; +virIdentitySetAttr; + + # util/virinitctl.h virInitctlSetRunLevel; diff --git a/src/util/virerror.c b/src/util/virerror.c index 40c3b25..8cb8548 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -118,6 +118,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST, "SSH transport layer", /* 50 */ "Lock Space", "Init control", + "Identity", ) @@ -1213,6 +1214,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("resource busy %s"); break; + case VIR_ERR_INVALID_IDENTITY: + if (info == NULL) + errmsg = _("invalid identity"); + else + errmsg = _("invalid identity %s"); + break; } return errmsg; } diff --git a/src/util/viridentity.c b/src/util/viridentity.c new file mode 100644 index 0000000..fb94b36 --- /dev/null +++ b/src/util/viridentity.c @@ -0,0 +1,178 @@ +/* + * viridentity.c: helper APIs for managing user identities + * + * Copyright (C) 2012-2013 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, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include <config.h> + +#include "internal.h" +#include "viralloc.h" +#include "virerror.h" +#include "viridentity.h" +#include "virlog.h" +#include "virobject.h" +#include "virthread.h" + +#define VIR_FROM_THIS VIR_FROM_IDENTITY + + +struct _virIdentity { + virObject parent; + + char *attrs[VIR_IDENTITY_ATTR_LAST]; +}; + +static virClassPtr virIdentityClass; + +static void virIdentityDispose(void *obj); + +static int virIdentityOnceInit(void) +{ + if (!(virIdentityClass = virClassNew(virClassForObject(), + "virIdentity", + sizeof(virIdentity), + virIdentityDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virIdentity) + + +/** + * 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 (virIdentityInitialize() < 0) + return NULL; + + if (!(ident = virObjectNew(virIdentityClass))) + return NULL; + + return ident; +} + + +static void virIdentityDispose(void *object) +{ + virIdentityPtr ident = object; + size_t i; + + for (i = 0 ; i < VIR_IDENTITY_ATTR_LAST ; i++) + VIR_FREE(ident->attrs[i]); +} + + +/** + * 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) +{ + int ret = -1; + VIR_DEBUG("ident=%p attribute=%u value=%s", ident, attr, NULLSTR(value)); + + if (ident->attrs[attr]) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("Identity attribute is already set")); + goto cleanup; + } + + if (!(ident->attrs[attr] = strdup(value))) { + virReportOOMError(); + goto cleanup; + } + + ret = 0; + +cleanup: + return ret; +} + + +/** + * 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); + + *value = ident->attrs[attr]; + + 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) +{ + int ret = 0; + size_t i; + VIR_DEBUG("identA=%p identB=%p", identA, identB); + + for (i = 0 ; i < VIR_IDENTITY_ATTR_LAST ; i++) { + if (STRNEQ_NULLABLE(identA->attrs[i], + identB->attrs[i])) + goto cleanup; + } + + ret = 1; +cleanup: + return ret; +} diff --git a/src/util/viridentity.h b/src/util/viridentity.h new file mode 100644 index 0000000..11a4ba1 --- /dev/null +++ b/src/util/viridentity.h @@ -0,0 +1,54 @@ +/* + * viridentity.h: helper APIs for managing user identities + * + * Copyright (C) 2012-2013 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, see + * <http://www.gnu.org/licenses/>. + * + */ + +#ifndef __VIR_IDENTITY_H__ +# define __VIR_IDENTITY_H__ + +# include "virobject.h" + +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 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); + +#endif /* __VIR_IDENTITY_H__ */ diff --git a/tests/Makefile.am b/tests/Makefile.am index d3a7868..6bb946a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -98,6 +98,7 @@ test_programs = virshtest sockettest \ virtimetest viruritest virkeyfiletest \ virauthconfigtest \ virbitmaptest virendiantest \ + viridentitytest \ virlockspacetest \ virstringtest \ virportallocatortest \ @@ -568,6 +569,10 @@ virstoragetest_SOURCES = \ virstoragetest.c testutils.h testutils.c virstoragetest_LDADD = $(LDADDS) +viridentitytest_SOURCES = \ + viridentitytest.c testutils.h testutils.c +viridentitytest_LDADD = $(LDADDS) + virlockspacetest_SOURCES = \ virlockspacetest.c testutils.h testutils.c virlockspacetest_LDADD = $(LDADDS) diff --git a/tests/viridentitytest.c b/tests/viridentitytest.c new file mode 100644 index 0000000..adb4f7d --- /dev/null +++ b/tests/viridentitytest.c @@ -0,0 +1,176 @@ +/* + * Copyright (C) 2013 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, see + * <http://www.gnu.org/licenses/>. + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> + +#include <stdlib.h> + +#include "testutils.h" + +#include "viridentity.h" +#include "virutil.h" +#include "virerror.h" +#include "viralloc.h" +#include "virlog.h" + +#include "virlockspace.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + + +static int testIdentityAttrs(const void *data ATTRIBUTE_UNUSED) +{ + int ret = -1; + virIdentityPtr ident; + const char *val; + + if (!(ident = virIdentityNew())) + goto cleanup; + + if (virIdentitySetAttr(ident, + VIR_IDENTITY_ATTR_UNIX_USER_NAME, + "fred") < 0) + goto cleanup; + + if (virIdentityGetAttr(ident, + VIR_IDENTITY_ATTR_UNIX_USER_NAME, + &val) < 0) + goto cleanup; + + if (STRNEQ_NULLABLE(val, "fred")) { + VIR_DEBUG("Expected 'fred' got '%s'", NULLSTR(val)); + goto cleanup; + } + + if (virIdentityGetAttr(ident, + VIR_IDENTITY_ATTR_UNIX_GROUP_NAME, + &val) < 0) + goto cleanup; + + if (STRNEQ_NULLABLE(val, NULL)) { + VIR_DEBUG("Unexpected groupname attribute"); + goto cleanup; + } + + if (virIdentitySetAttr(ident, + VIR_IDENTITY_ATTR_UNIX_USER_NAME, + "joe") != -1) { + VIR_DEBUG("Unexpectedly overwrote attribute"); + goto cleanup; + } + + if (virIdentityGetAttr(ident, + VIR_IDENTITY_ATTR_UNIX_USER_NAME, + &val) < 0) + goto cleanup; + + if (STRNEQ_NULLABLE(val, "fred")) { + VIR_DEBUG("Expected 'fred' got '%s'", NULLSTR(val)); + goto cleanup; + } + + ret = 0; +cleanup: + virObjectUnref(ident); + return ret; +} + + +static int testIdentityEqual(const void *data ATTRIBUTE_UNUSED) +{ + int ret = -1; + virIdentityPtr identa = NULL; + virIdentityPtr identb = NULL; + + if (!(identa = virIdentityNew())) + goto cleanup; + if (!(identb = virIdentityNew())) + goto cleanup; + + if (!virIdentityIsEqual(identa, identb)) { + VIR_DEBUG("Empty identities were no equal"); + goto cleanup; + } + + if (virIdentitySetAttr(identa, + VIR_IDENTITY_ATTR_UNIX_USER_NAME, + "fred") < 0) + goto cleanup; + + if (virIdentityIsEqual(identa, identb)) { + VIR_DEBUG("Mis-matched identities should not be equal"); + goto cleanup; + } + + if (virIdentitySetAttr(identb, + VIR_IDENTITY_ATTR_UNIX_USER_NAME, + "fred") < 0) + goto cleanup; + + if (!virIdentityIsEqual(identa, identb)) { + VIR_DEBUG("Matched identities were not equal"); + goto cleanup; + } + + if (virIdentitySetAttr(identa, + VIR_IDENTITY_ATTR_UNIX_GROUP_NAME, + "flintstone") < 0) + goto cleanup; + if (virIdentitySetAttr(identb, + VIR_IDENTITY_ATTR_UNIX_GROUP_NAME, + "flintstone") < 0) + goto cleanup; + + if (!virIdentityIsEqual(identa, identb)) { + VIR_DEBUG("Matched identities were not equal"); + goto cleanup; + } + + if (virIdentitySetAttr(identb, + VIR_IDENTITY_ATTR_SASL_USER_NAME, + "fred@FLINTSTONE.COM") < 0) + goto cleanup; + + if (virIdentityIsEqual(identa, identb)) { + VIR_DEBUG("Mis-atched identities should not be equal"); + goto cleanup; + } + + ret = 0; +cleanup: + virObjectUnref(identa); + virObjectUnref(identb); + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + + if (virtTestRun("Identity attributes ", 1, testIdentityAttrs, NULL) < 0) + ret = -1; + if (virtTestRun("Identity equality ", 1, testIdentityEqual, NULL) < 0) + ret = -1; + + return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN(mymain) -- 1.8.1.4

On 03/06/2013 05:49 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Introduce a local object virIdentity for managing security attributes used to form a client application's identity. Instances of this object are intended to be used as if they were immutable, once created & populated with attributes
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> ---
+/** + * 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) +{ + int ret = -1; + VIR_DEBUG("ident=%p attribute=%u value=%s", ident, attr, NULLSTR(value));
This says value can be NULL...
+ + if (ident->attrs[attr]) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("Identity attribute is already set")); + goto cleanup; + } + + if (!(ident->attrs[attr] = strdup(value))) {
...but this would crash. Isn't it easier to just require that value be non-NULL if you are going to call this function?
+ + if (!virIdentityIsEqual(identa, identb)) { + VIR_DEBUG("Empty identities were no equal");
s/no/not/
+ + if (virIdentityIsEqual(identa, identb)) { + VIR_DEBUG("Mis-atched identities should not be equal");
s/Mis-atched/Mismatched/ -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange@redhat.com> To allow any internal API to get the current identity, add APIs to associate a virIdentityPtr with the current thread, via a thread local Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/viridentity.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/util/viridentity.h | 3 +++ 2 files changed, 62 insertions(+) diff --git a/src/util/viridentity.c b/src/util/viridentity.c index fb94b36..2b4198b 100644 --- a/src/util/viridentity.c +++ b/src/util/viridentity.c @@ -39,6 +39,7 @@ struct _virIdentity { }; static virClassPtr virIdentityClass; +static virThreadLocal virIdentityCurrent; static void virIdentityDispose(void *obj); @@ -50,11 +51,69 @@ static int virIdentityOnceInit(void) virIdentityDispose))) return -1; + if (virThreadLocalInit(&virIdentityCurrent, + (virThreadLocalCleanup)virObjectUnref) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot initialize thread local for current identity")); + return -1; + } + return 0; } VIR_ONCE_GLOBAL_INIT(virIdentity) +/** + * virIdentityGetCurrent: + * + * Get the current identity associated with this thread. The + * caller will own a reference to the returned identity, but + * must not modify the object in any way, other than to + * release the reference when done with virObjectUnref + * + * Returns: a reference to the current identity, or NULL + */ +virIdentityPtr virIdentityGetCurrent(void) +{ + virIdentityPtr ident; + + if (virIdentityOnceInit() < 0) + return NULL; + + ident = virThreadLocalGet(&virIdentityCurrent); + return virObjectRef(ident); +} + + +/** + * virIdentitySetCurrent: + * + * Set the new identity to be associated with this thread. + * The caller should not modify the passed identity after + * it has been set, other than to release its own reference. + * + * Returns 0 on success, or -1 on error + */ +int virIdentitySetCurrent(virIdentityPtr ident) +{ + virIdentityPtr old; + + if (virIdentityOnceInit() < 0) + return -1; + + old = virThreadLocalGet(&virIdentityCurrent); + virObjectUnref(old); + + if (virThreadLocalSet(&virIdentityCurrent, + virObjectRef(ident)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to set thread local identity")); + return -1; + } + + return 0; +} + /** * virIdentityNew: diff --git a/src/util/viridentity.h b/src/util/viridentity.h index 11a4ba1..0825c90 100644 --- a/src/util/viridentity.h +++ b/src/util/viridentity.h @@ -38,6 +38,9 @@ typedef enum { VIR_IDENTITY_ATTR_LAST, } virIdentityAttrType; +virIdentityPtr virIdentityGetCurrent(void); +int virIdentitySetCurrent(virIdentityPtr ident); + virIdentityPtr virIdentityNew(void); int virIdentitySetAttr(virIdentityPtr ident, -- 1.8.1.4

From: "Daniel P. Berrange" <berrange@redhat.com> If no user identity is available, some operations may wish to use the system identity. ie the identity of the current process itself. Add an API to get such an identity. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/viridentity.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/util/viridentity.h | 2 ++ 2 files changed, 73 insertions(+) diff --git a/src/util/viridentity.c b/src/util/viridentity.c index 2b4198b..004b8dc 100644 --- a/src/util/viridentity.c +++ b/src/util/viridentity.c @@ -21,6 +21,11 @@ #include <config.h> +#include <unistd.h> +#if HAVE_SELINUX +# include <selinux/selinux.h> +#endif + #include "internal.h" #include "viralloc.h" #include "virerror.h" @@ -28,6 +33,7 @@ #include "virlog.h" #include "virobject.h" #include "virthread.h" +#include "virutil.h" #define VIR_FROM_THIS VIR_FROM_IDENTITY @@ -116,6 +122,71 @@ int virIdentitySetCurrent(virIdentityPtr ident) /** + * virIdentityGetSystem: + * + * Returns an identity that represents the system itself. + * This is the identity that the process is running as + * + * Returns a reference to the system identity, or NULL + */ +virIdentityPtr virIdentityGetSystem(void) +{ + char *username = NULL; + char *groupname = NULL; + char *seccontext = NULL; + virIdentityPtr ret = NULL; + gid_t gid = getgid(); + uid_t uid = getuid(); +#if HAVE_SELINUX + security_context_t con; +#endif + + if (!(username = virGetUserName(uid))) + goto cleanup; + if (!(groupname = virGetGroupName(gid))) + goto cleanup; + +#if HAVE_SELINUX + if (getcon(&con) < 0) { + virReportSystemError(errno, "%s", + _("Unable to lookup SELinux process context")); + goto cleanup; + } + seccontext = strdup(con); + freecon(con); + if (!seccontext) { + virReportOOMError(); + goto cleanup; + } +#endif + + if (!(ret = virIdentityNew())) + goto cleanup; + + if (username && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_USER_NAME, username) < 0) + goto error; + if (groupname && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_GROUP_NAME, groupname) < 0) + goto error; + if (seccontext && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_SECURITY_CONTEXT, seccontext) < 0) + goto error; + +cleanup: + VIR_FREE(username); + VIR_FREE(groupname); + VIR_FREE(seccontext); + return ret; + +error: + virObjectUnref(ret); + ret = NULL; + goto cleanup; +} + + +/** * virIdentityNew: * * Creates a new empty identity object. After creating, one or diff --git a/src/util/viridentity.h b/src/util/viridentity.h index 0825c90..ceaddf7 100644 --- a/src/util/viridentity.h +++ b/src/util/viridentity.h @@ -41,6 +41,8 @@ typedef enum { virIdentityPtr virIdentityGetCurrent(void); int virIdentitySetCurrent(virIdentityPtr ident); +virIdentityPtr virIdentityGetSystem(void); + virIdentityPtr virIdentityNew(void); int virIdentitySetAttr(virIdentityPtr ident, -- 1.8.1.4

From: "Daniel P. Berrange" <berrange@redhat.com> Add APIs which allow creation of a virIdentity from the info associated with a virNetServerClientPtr instance. This is done based on the results of client authentication processes like TLS, x509, SASL, SO_PEERCRED Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 1 + src/rpc/virnetserverclient.c | 113 +++++++++++++++++++++++++++++++++++++++++++ src/rpc/virnetserverclient.h | 3 ++ 3 files changed, 117 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d4657d1..d725f9c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -852,6 +852,7 @@ virNetServerClientClose; virNetServerClientDelayedClose; virNetServerClientGetAuth; virNetServerClientGetFD; +virNetServerClientGetIdentity; virNetServerClientGetPrivateData; virNetServerClientGetReadonly; virNetServerClientGetSASLSession; diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 40c8173..6ca5a41 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -74,6 +74,9 @@ struct _virNetServerClient int sockTimer; /* Timer to be fired upon cached data, * so we jump out from poll() immediately */ + + virIdentityPtr identity; + /* Count of messages in the 'tx' queue, * and the server worker pool queue * ie RPC calls in progress. Does not count @@ -642,6 +645,114 @@ int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, } +static virIdentityPtr +virNetServerClientCreateIdentity(virNetServerClientPtr client) +{ + char *processid = NULL; + char *username = NULL; + char *groupname = NULL; +#if WITH_SASL + char *saslname = NULL; + virNetSASLSessionPtr sasl; +#endif + char *x509dname = NULL; + char *seccontext = NULL; + virIdentityPtr ret = NULL; + virNetTLSSessionPtr tls; + + if (virNetServerClientIsLocal(client)) { + gid_t gid; + uid_t uid; + pid_t pid; + if (virNetServerClientGetUNIXIdentity(client, &uid, &gid, &pid) < 0) + goto cleanup; + + if (!(username = virGetUserName(uid))) + goto cleanup; + if (!(groupname = virGetGroupName(gid))) + goto cleanup; + if (virAsprintf(&processid, "%d", (int)pid) < 0) + goto cleanup; + } + +#if WITH_SASL + if ((sasl = virNetServerClientGetSASLSession(client))) { + const char *identity = virNetSASLSessionGetIdentity(sasl); + if (identity && + !(saslname = strdup(identity))) { + virReportOOMError(); + goto cleanup; + } + } +#endif + + if ((tls = virNetServerClientGetTLSSession(client))) { + const char *identity = virNetTLSSessionGetX509DName(tls); + if (identity && + !(x509dname = strdup(identity))) { + virReportOOMError(); + goto cleanup; + } + } + + if (virNetServerClientGetSecurityContext(client, &seccontext) < 0) + goto cleanup; + + if (!(ret = virIdentityNew())) + goto cleanup; + + if (username && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_USER_NAME, username) < 0) + goto error; + if (groupname && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_GROUP_NAME, groupname) < 0) + goto error; + if (processid && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_UNIX_PROCESS_ID, processid) < 0) + goto error; +#if HAVE_SASL + if (saslname && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_SASL_USER_NAME, saslname) < 0) + goto error; +#endif + if (x509dname && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_X509_DISTINGUISHED_NAME, x509dname) < 0) + goto error; + if (seccontext && + virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_SECURITY_CONTEXT, seccontext) < 0) + goto error; + +cleanup: + VIR_FREE(username); + VIR_FREE(groupname); + VIR_FREE(processid); + VIR_FREE(seccontext); +#if HAVE_SASL + VIR_FREE(saslname); +#endif + VIR_FREE(x509dname); + return ret; + +error: + virObjectUnref(ret); + ret = NULL; + goto cleanup; +} + + +virIdentityPtr virNetServerClientGetIdentity(virNetServerClientPtr client) +{ + virIdentityPtr ret = NULL; + virObjectLock(client); + if (!client->identity) + client->identity = virNetServerClientCreateIdentity(client); + if (client->identity) + ret = virObjectRef(client->identity); + virObjectUnlock(client); + return ret; +} + + int virNetServerClientGetSecurityContext(virNetServerClientPtr client, char **context) { @@ -750,6 +861,8 @@ void virNetServerClientDispose(void *obj) { virNetServerClientPtr client = obj; + virObjectUnref(client->identity); + if (client->privateData && client->privateDataFreeFunc) client->privateDataFreeFunc(client->privateData); diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index f8643f5..b3b8df0 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -24,6 +24,7 @@ #ifndef __VIR_NET_SERVER_CLIENT_H__ # define __VIR_NET_SERVER_CLIENT_H__ +# include "viridentity.h" # include "virnetsocket.h" # include "virnetmessage.h" # include "virobject.h" @@ -103,6 +104,8 @@ int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, int virNetServerClientGetSecurityContext(virNetServerClientPtr client, char **context); +virIdentityPtr virNetServerClientGetIdentity(virNetServerClientPtr client); + void *virNetServerClientGetPrivateData(virNetServerClientPtr client); typedef void (*virNetServerClientCloseFunc)(virNetServerClientPtr client); -- 1.8.1.4

From: "Daniel P. Berrange" <berrange@redhat.com> When dispatching an RPC API call, setup the current identity to hold the identity of the network client associated with the RPC message being dispatched. The setting is thread-local, so only affects the API call in this thread Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetserverprogram.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c index 414b978..b80923d 100644 --- a/src/rpc/virnetserverprogram.c +++ b/src/rpc/virnetserverprogram.c @@ -375,6 +375,7 @@ virNetServerProgramDispatchCall(virNetServerProgramPtr prog, virNetServerProgramProcPtr dispatcher; virNetMessageError rerr; size_t i; + virIdentityPtr identity = NULL; memset(&rerr, 0, sizeof(rerr)); @@ -419,6 +420,12 @@ virNetServerProgramDispatchCall(virNetServerProgramPtr prog, if (virNetMessageDecodePayload(msg, dispatcher->arg_filter, arg) < 0) goto error; + if (!(identity = virNetServerClientGetIdentity(client))) + goto error; + + if (virIdentitySetCurrent(identity) < 0) + goto error; + /* * When the RPC handler is called: * @@ -431,6 +438,9 @@ virNetServerProgramDispatchCall(virNetServerProgramPtr prog, */ rv = (dispatcher->func)(server, client, msg, &rerr, arg, ret); + if (virIdentitySetCurrent(NULL) < 0) + goto error; + /* * If rv == 1, this indicates the dispatch func has * populated 'msg' with a list of FDs to return to @@ -481,6 +491,7 @@ virNetServerProgramDispatchCall(virNetServerProgramPtr prog, VIR_FREE(arg); VIR_FREE(ret); + virObjectUnref(identity); /* Put reply on end of tx queue to send out */ return virNetServerClientSendMessage(client, msg); @@ -491,6 +502,7 @@ error: VIR_FREE(arg); VIR_FREE(ret); + virObjectUnref(identity); return rv; } -- 1.8.1.4
participants (2)
-
Daniel P. Berrange
-
Eric Blake