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