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

[Michel Dänzer]

On 11.12.2015 18:17, Martin Peres wrote:
> 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?

Because libgen.h and basename() aren't available on Windows AFAICT. I
figured this use case probably isn't common enough on Windows anyway to
bother making it work there as well.


> And why all clients of an xserver running on windows should be
> considered local?

You misread the patch. There's no change in behaviour on Windows,
because xtransLocalClient and by extension ComputeLocalClient returns
FALSE in all the same cases ComputeLocalClient did before the patch.


>> +    /* 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


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer