Subject: Bug#784: x2gobroker-agent.pl commands break if user shell environment has login settings it doesn't like

From: Jason Alavaliant <[email-masked]>
Date: Tue, 10 Feb 2015 18:13:52 +1300

Package: x2gobroker-agent
Version: 0.0.2.3

One of my users started to get their connections to the x2go broker 
failing with 'Error downloading http://x2go-broker-url/plain/inifile - 
server replied: Internal Server Error'

Tracking back through the commands the broker was logging the issue 
turned out be

/usr/lib/x2go/x2gobroker-agent $USER listsessions
(and other commands that x2gobroker-agent.pl runs by su - username -c 
command) returning things like

OK
[1] 32542
klipper: Command not found.
[1] + exit 1       klipper

which was a result of how the command is run;     since the 
x2gobroker-agent $USER FOO   commands all internally su to the named 
user to run the command it results in the user's environment being 
sourced which puts them at the risk of being broken by the user's 
environment.    My user in this case had embedded a command that didn't 
exist on the broker host to run as soon as their shell environment was 
created which caused the broker to error out.   (possibly compounded by 
in this case the shell tcsh being used as the user's default shell - 
tcsh in my experience being very temperamental with it's startup files - 
a single failure will stop the environment file sourcing to error our 
instantly and not process further lines.  I've not had time to cross 
compare with bash etc and see if this problem only exists for tcsh or if 
it effects most shells).

I managed to work around the issue in my case by fixing the su commands 
in x2gobroker-agent.pl to use /bin/sh (which links to /bin/dash) on my 
system to avoid user shell startup file environment issues breaking 
logging in via the broker.    It feels more like a workaround that a 
proper fix to me so I'm not suggesting the attached patch for merging,   
rather just as an example of how I got around it in my case.     My 
feeling is that there must be a better way to ensure these 
x2gobroker-agent commands can't be broken by the user env but I'm not 
managing to come up with a better solution currently.

Thanks
Jason

--- old/x2go/x2gobroker-agent.pl	2015-02-10 16:21:42.844859824 +1300
+++ new/x2go/x2gobroker-agent.pl	2015-02-10 16:24:09.745595707 +1300
@@ -109,7 +109,7 @@
 {
 	InitX2GoUser($uid, $uidNumber, $gidNumber, $home);
 	print "OK\n";
-	exec ("/bin/su - $uid -c \"x2golistsessions --all-servers\"");
+	exec ("/bin/su - $uid -s /bin/sh -c \"x2golistsessions --all-servers\"");
 }
 
 if( ($mode eq 'findbusyservers_by_sessionstats') || ($mode eq 'findbusyservers'))
@@ -135,7 +135,7 @@
 
 	InitX2GoUser($uid, $uidNumber, $gidNumber, $home);
 	print "OK\n";
-	my $busy_servers = `/bin/su - $uid -c \"x2gogetservers\"`;
+	my $busy_servers = `/bin/su - $uid -s /bin/sh -c \"x2gogetservers\"`;
 
 	my %server_load = ();
 	my $num_sessions = 0;
@@ -161,7 +161,7 @@
 {
 	InitX2GoUser($uid, $uidNumber, $gidNumber, $home);
 	print "OK\n";
-	exec ("/bin/su - $uid -c \"x2gogetservers\"");
+	exec ("/bin/su - $uid -s /bin/sh -c \"x2gogetservers\"");
 }
 
 if($mode eq 'addauthkey')
@@ -187,7 +187,7 @@
 	InitX2GoUser($uid, $uidNumber, $gidNumber, $home);
 	print "OK\n";
 	my $sid=shift;
-	exec ("/bin/su - $uid -c \"\$(x2gopath lib)/x2gochangestatus S $sid\"");
+	exec ("/bin/su - $uid -s /bin/sh -c \"\$(x2gopath lib)/x2gochangestatus S $sid\"");
 }
 
 if($mode eq 'terminatesession')
@@ -195,7 +195,7 @@
 	InitX2GoUser($uid, $uidNumber, $gidNumber, $home);
 	print "OK\n";
 	my $sid=shift;
-	exec ("/bin/su - $uid -c \"\$(x2gopath lib)/x2gochangestatus T $sid\"");
+	exec ("/bin/su - $uid -s /bin/sh -c \"\$(x2gopath lib)/x2gochangestatus T $sid\"");
 }
 
 if($mode eq 'ping')