[libvirt] [PATCH 03/12] Remove hack using existance of an 'identity' string to disable auth

[Michal Privoznik]

On 02.05.2012 13:44, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at 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.