[PATCH xserver v2.1] os: Treat ssh as a non-local client (v2)

[Martin Peres]

On 11/12/15 04:34, Michel Dänzer wrote:
> From: Adam Jackson <ajax at redhat.com>
>
> By the time we get to ComputeLocalClient, we've already done
> NextAvailableClient → ReserveClientIds → DetermineClientCmd (assuming
> we're built with #define CLIENTIDS), so we can look up the name of the
> client process and refuse to treat ssh's X forwarding as if it were
> local.
>
> v2: (Michel Dänzer)
>      * Only match "ssh" itself, not other executable names starting with
>        that prefix.
>      * Ignore executable path for the match.
>
> Signed-off-by: Adam Jackson <ajax at redhat.com>
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
> ---
>
> v2.1: Slightly extended code comment
>
>   os/access.c | 29 ++++++++++++++++++++++++++---
>   1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/os/access.c b/os/access.c
> index 10a48c3..3e32128 100644
> --- a/os/access.c
> +++ b/os/access.c
> @@ -101,6 +101,7 @@ SOFTWARE.
>   #include <sys/socket.h>
>   #include <sys/ioctl.h>
>   #include <ctype.h>
> +#include <libgen.h>
>   
>   #ifndef NO_LOCAL_CLIENT_CRED
>   #include <pwd.h>
> @@ -1081,9 +1082,8 @@ ResetHosts(const char *display)
>       }
>   }
>   
> -/* Is client on the local host */
> -Bool
> -ComputeLocalClient(ClientPtr client)
> +static Bool
> +xtransLocalClient(ClientPtr client)
>   {
>       int alen, family, notused;
>       Xtransaddr *from = NULL;
> @@ -1116,6 +1116,29 @@ ComputeLocalClient(ClientPtr client)
>       return FALSE;
>   }
>   
> +/* Is client on the local host */
> +Bool
> +ComputeLocalClient(ClientPtr client)
> +{
> +    if (!xtransLocalClient(client))
> +        return FALSE;
> +
> +#ifndef WIN32
Why add this ifndef? And why all clients of an xserver running on 
windows should be considered local?

Seems like this function is implementing the logic "Can we safely use 
the features of posix-provided extensions?" instead of sticking to what 
the name suggests. If it is a hack, it would be nice to document it at 
least :)

Thanks for spending time on this!
> +    /* If the executable name is "ssh", assume that this client connection
> +     * is forwarded from another host via SSH
> +     */
> +    if (client->clientIds->cmdname) {
> +        char *cmdname = strdup(client->clientIds->cmdname);
> +        Bool ret = strcmp(basename(cmdname), "ssh") != 0;
> +
> +        free(cmdname);
> +        return ret;
> +    }
> +#endif
> +
> +    return TRUE;
> +}
> +
>   /*
>    * Return the uid and all gids of a connected local client
>    * Allocates a LocalClientCredRec - caller must call FreeLocalClientCreds