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@xxxxxxxxxx>
> >
> > 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list