[libvirt] [PATCH v2 0/6] Identity management APIs for RBAC

An update of https://www.redhat.com/archives/libvir-list/2013-March/msg00186.html The first two patches are merged and fixes for the other patches applied.

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_gnutls.syms | 2 ++ src/libvirt_private.syms | 3 +++ src/libvirt_sasl.syms | 1 + 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 ++ 9 files changed, 125 insertions(+) diff --git a/src/libvirt_gnutls.syms b/src/libvirt_gnutls.syms index bd4f950..6eb6741 100644 --- a/src/libvirt_gnutls.syms +++ b/src/libvirt_gnutls.syms @@ -13,6 +13,7 @@ virNetServerSetTLSContext; # rpc/virnetserverclient.h virNetServerClientGetTLSKeySize; +virNetServerClientGetTLSSession; virNetServerClientHasTLSSession; @@ -33,6 +34,7 @@ virNetTLSContextNewServerPath; virNetTLSInit; virNetTLSSessionGetHandshakeStatus; virNetTLSSessionGetKeySize; +virNetTLSSessionGetX509DName; virNetTLSSessionHandshake; virNetTLSSessionNew; virNetTLSSessionRead; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fbd540a..3243416 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -853,11 +853,13 @@ virNetServerClientGetAuth; virNetServerClientGetFD; virNetServerClientGetPrivateData; virNetServerClientGetReadonly; +virNetServerClientGetSecurityContext; virNetServerClientGetUNIXIdentity; virNetServerClientImmediateClose; virNetServerClientInit; virNetServerClientInitKeepAlive; virNetServerClientIsClosed; +virNetServerClientIsLocal; virNetServerClientIsSecure; virNetServerClientLocalAddrString; virNetServerClientNeedAuth; @@ -922,6 +924,7 @@ virNetSocketClose; virNetSocketDupFD; virNetSocketGetFD; virNetSocketGetPort; +virNetSocketGetSecurityContext; virNetSocketGetUNIXIdentity; virNetSocketHasCachedData; virNetSocketHasPassFD; diff --git a/src/libvirt_sasl.syms b/src/libvirt_sasl.syms index beb8825..1241884 100644 --- a/src/libvirt_sasl.syms +++ b/src/libvirt_sasl.syms @@ -26,6 +26,7 @@ virNetSASLSessionServerStep; # rpc/virnetserverclient.h +virNetServerClientGetSASLSession; virNetServerClientSetSASLSession; 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/13/2013 09:24 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_gnutls.syms | 2 ++ src/libvirt_private.syms | 3 +++ src/libvirt_sasl.syms | 1 + 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 ++ 9 files changed, 125 insertions(+)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

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 | 60 +++++++++++++++ tests/Makefile.am | 5 ++ tests/viridentitytest.c | 176 +++++++++++++++++++++++++++++++++++++++++++ 10 files changed, 438 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 5b5c3c2..07e241f 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -151,6 +151,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 a6cc839..6ba00e8 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 3243416..e636e75 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1287,6 +1287,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..7ebf851 --- /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, 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..5992b1a --- /dev/null +++ b/src/util/viridentity.h @@ -0,0 +1,60 @@ +/* + * 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) + ATTRIBUTE_NONNULL(1) + ATTRIBUTE_NONNULL(3); + +int virIdentityGetAttr(virIdentityPtr ident, + unsigned int attr, + const char **value) + ATTRIBUTE_NONNULL(1) + ATTRIBUTE_NONNULL(3); + +int virIdentityIsEqual(virIdentityPtr identA, + virIdentityPtr identB) + ATTRIBUTE_NONNULL(1) + ATTRIBUTE_NONNULL(2); + +#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 Wed, Mar 13, 2013 at 15:24:01 +0000, 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
...
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;
This doesn't seem to be used anywhere yet but would "invalid identity: %s" result in better error messages?
} return errmsg; } diff --git a/src/util/viridentity.c b/src/util/viridentity.c new file mode 100644 index 0000000..7ebf851 --- /dev/null +++ b/src/util/viridentity.c
...
+/** + * 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; +}
This API never fails, so why do we need to document unreachable -1 on error and use int rather than bool? Especially when you use this API as if it was just returning true/false.
diff --git a/tests/viridentitytest.c b/tests/viridentitytest.c new file mode 100644 index 0000000..adb4f7d --- /dev/null +++ b/tests/viridentitytest.c ... +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)) {
Hmm, could be easier to just check if val is 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");
s/no/not/
+ goto cleanup; + } ...
ACK with the comments addressed. Jirka

On Tue, Mar 19, 2013 at 12:20:51PM +0100, Jiri Denemark wrote:
On Wed, Mar 13, 2013 at 15:24:01 +0000, 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
...
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;
This doesn't seem to be used anywhere yet but would "invalid identity: %s" result in better error messages?
Oh this is left over from earlier versions fo the patch. I'll kill it now.
} return errmsg; } diff --git a/src/util/viridentity.c b/src/util/viridentity.c new file mode 100644 index 0000000..7ebf851 --- /dev/null +++ b/src/util/viridentity.c
...
+/** + * 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; +}
This API never fails, so why do we need to document unreachable -1 on error and use int rather than bool? Especially when you use this API as if it was just returning true/false.
Again, left over cruft from earlier versions. 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> 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 7ebf851..acb0cb9 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 5992b1a..a13f5ea 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

On Wed, Mar 13, 2013 at 15:24:02 +0000, Daniel P. Berrange wrote:
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(+)
ACK Jirka

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 acb0cb9..1c43081 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 a13f5ea..b337031 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

On Wed, Mar 13, 2013 at 15:24:03 +0000, Daniel P. Berrange wrote:
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 acb0cb9..1c43081 100644 --- a/src/util/viridentity.c +++ b/src/util/viridentity.c ... @@ -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;
Quite cosmetic, but is there any reason why we use uid/gid variables rather than calling getuid/getgid directly here?
+ +#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;
All three lines with virIdentitySetAttr() calls are too long.
+ +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
ACK Jirka

On Tue, Mar 19, 2013 at 12:21:31PM +0100, Jiri Denemark wrote:
On Wed, Mar 13, 2013 at 15:24:03 +0000, Daniel P. Berrange wrote:
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 acb0cb9..1c43081 100644 --- a/src/util/viridentity.c +++ b/src/util/viridentity.c ... @@ -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;
Quite cosmetic, but is there any reason why we use uid/gid variables rather than calling getuid/getgid directly here?
No particular reason, I'll change that. 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> 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 | 112 +++++++++++++++++++++++++++++++++++++++++++ src/rpc/virnetserverclient.h | 3 ++ 3 files changed, 116 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e636e75..6aae909 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -851,6 +851,7 @@ virNetServerClientClose; virNetServerClientDelayedClose; virNetServerClientGetAuth; virNetServerClientGetFD; +virNetServerClientGetIdentity; virNetServerClientGetPrivateData; virNetServerClientGetReadonly; virNetServerClientGetSecurityContext; diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 40c8173..850f388 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,113 @@ int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, } +static virIdentityPtr +virNetServerClientCreateIdentity(virNetServerClientPtr client) +{ + char *processid = NULL; + char *username = NULL; + char *groupname = NULL; +#if WITH_SASL + char *saslname = NULL; +#endif + char *x509dname = NULL; + char *seccontext = NULL; + virIdentityPtr ret = NULL; + + if (client->sock && virNetSocketIsLocal(client->sock)) { + gid_t gid; + uid_t uid; + pid_t pid; + if (virNetSocketGetUNIXIdentity(client->sock, &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 (client->sasl) { + const char *identity = virNetSASLSessionGetIdentity(client->sasl); + if (identity && + !(saslname = strdup(identity))) { + virReportOOMError(); + goto cleanup; + } + } +#endif + + if (client->tls) { + const char *identity = virNetTLSSessionGetX509DName(client->tls); + if (identity && + !(x509dname = strdup(identity))) { + virReportOOMError(); + goto cleanup; + } + } + + if (client->sock && + virNetSocketGetSecurityContext(client->sock, &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 +860,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

On Wed, Mar 13, 2013 at 15:24:04 +0000, Daniel P. Berrange wrote:
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
...
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 40c8173..850f388 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c ... @@ -642,6 +645,113 @@ int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, }
+static virIdentityPtr +virNetServerClientCreateIdentity(virNetServerClientPtr client) +{ + char *processid = NULL; + char *username = NULL; + char *groupname = NULL; +#if WITH_SASL + char *saslname = NULL; +#endif + char *x509dname = NULL; + char *seccontext = NULL; + virIdentityPtr ret = NULL; + + if (client->sock && virNetSocketIsLocal(client->sock)) { + gid_t gid; + uid_t uid; + pid_t pid; + if (virNetSocketGetUNIXIdentity(client->sock, &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)
This should use "%lld" and (long long)pid to be consistent with the way we format PIDs in libvirt. Also you sould call virReportOOMError() here since virAsprintf() won't do it for you.
+ goto cleanup; + } + +#if WITH_SASL + if (client->sasl) { + const char *identity = virNetSASLSessionGetIdentity(client->sasl); + if (identity && + !(saslname = strdup(identity))) { + virReportOOMError(); + goto cleanup; + } + } +#endif + + if (client->tls) { + const char *identity = virNetTLSSessionGetX509DName(client->tls); + if (identity && + !(x509dname = strdup(identity))) { + virReportOOMError(); + goto cleanup; + } + } + + if (client->sock && + virNetSocketGetSecurityContext(client->sock, &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;
Long lines again.
+ +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; +} ...
ACK Jirka

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

On Wed, Mar 13, 2013 at 15:24:05 +0000, Daniel P. Berrange wrote:
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(+)
ACK Jirka

Ping, any more reviews welcome... Daniel On Wed, Mar 13, 2013 at 03:23:59PM +0000, Daniel P. Berrange wrote:
An update of
https://www.redhat.com/archives/libvir-list/2013-March/msg00186.html
The first two patches are merged and fixes for the other patches applied.
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- |: 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 :|
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Jiri Denemark