On Sun, Jul 17, 2011 at 13:41:05 (CEST), Mike Gabriel wrote:
[...]
- /usr/lib/x2go/x2gosqlitewrapper.pl is still installed suid, while /usr/bin/x2gosqlitewrapper is not. Is this really intended? If it is, then the whole design is somewhat mysterious to me.
o /usr/bin/x2gosqlitewrapper is the setuidwrapper and is started as the user, who runs the X2go session (e.g. user foo), this C binary has to be set root:root:0755. o /usr/lib/x2go/x2gosqlitewrapper.pl is the actual Perl script that has to run as user ,,x2guser''. This file has to be set x2gouser:x2gousers:6755.
I'm surprised that this works at all. Calling /usr/lib/x2go/x2gosqlitewrapper.pl now directly leads to this error:
/usr/lib/x2go/x2gosqlitewrapper.pl Can't do setuid (cannot exec sperl) zsh: exit 2 /usr/lib/x2go/x2gosqlitewrapper.pl
Which is expected, because perl-suid is not installed.
As user foo I execute /usr/bin/x2gosqlitewrapper, this wraps an execvpe call around the Perl script /usr/lib/x2go/x2gosqlitewrapper.pl. The execvpe call evokes a change of the effective UID/GID as /usr/lib/x2go/x2gosqlitewrapper.pl has its setuid bit set.
So this trick effectively disables perls internal suid check somehow, and I currently don't see what this non-suid wrapper does differently than the shell that tries to execute the script.
What you can try out is: do a chmod 6755 /usr/bin/X2gosqlitewrapper and a chown x2gouser:x2gousers /usr/bin/x2gosqlitewrapper and then call x2golistsessions (on a server that uses sqlite as X2go db backend). You will get a kernel complaint (or libc) that this is not allowed with the current kernel configuration.
I just did: -rwsr-sr-x 1 x2gouser x2gousers 5388 2011-07-18 00:12 /usr/bin/x2gosqlitewrapper* -rwxr-xr-x 1 x2gouser x2gousers 10094 2011-07-18 00:02 /usr/lib/x2go/x2gosqlitewrapper.pl*
And everything works as expected. You need to become more specific what you observed.
Now after reading the perlsec manpage, let me quote this part:
,---- | Security Bugs | | Beyond the obvious problems that stem from giving special | privileges to systems as flexible as scripts, on many versions of | Unix, set-id scripts are inherently insecure right from the | start. The problem is a race condition in the kernel. Between | the time the kernel opens the file to see which interpreter to | run and when the (now-set-id) interpreter turns around and | reopens the file to interpret it, the file in question may have | changed, especially if you have symbolic links on your system. | [...] | | The use of suidperl is considered deprecated, and will be | removed in Perl 5.12.0. It is strongly recommended that all | code uses the simplier and more secure C-wrappers described | below. | | If the kernel set-id script feature isn't disabled, Perl will | complain loudly that your set-id script is insecure. You'll | need to either disable the kernel set-id script feature, or put | a C wrapper around the script. A C wrapper is just a compiled | program that does nothing except call your Perl program. | Compiled programs are not subject to the kernel bug that | plagues set-id scripts. `----
Followed is a small example with the recommendation to make the wrapper suid and not the script.
In any case, I guess no file should be made actually writeable by the x2gouser. Moreover, is the script run in tainted mode this way? AFAIUI no, but we can (and should) set it forcefully in the perl script.
-- Gruesse/greetings, Reinhard Tartler, KeyID 945348A4