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