[X2go-Dev] x2goserver package with setuidwrapper

Mike Gabriel mike.gabriel at das-netzwerkteam.de
Sun Jul 17 13:41:05 CEST 2011


Hi Reinhard,

thanks for taking a first closer look!!!

On So 17 Jul 2011 12:18:49 CEST Reinhard Tartler wrote:

> Well, the changes in git are somewhat convoluted and take me a
> considerable time to review properly. I don't know if and when I have
> time to do a proper review in the near future, but here are my first
> observations:
>
>  - the clean rule needs work:
>    - x2goserver/x2gosqlitewrapper is not deleted

Ok.

>    - x2goserver-extensions/.build_man2html and
>      x2goserver/.build_man2html should be deleted from the branch, as
>      the content is autogenerated

Ok. Will add that. I commit the html man pages to Git as they are  
linked from the X2go wiki, so that we have man pages available TTW.  
There might be another way of doing this easily, but this was my  
spontaneous approach to make man pages (incl. regular updates)  
available TTW.

>  - /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.

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.

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.

>  - the wrapper itself uses a static buffer to copy the link of
>    /proc/self/exe there. this makes sense, but where does BUFSIZ come
>    from and how big is it? Do we have enough control over the
>    buildsystem so that it doesn't for some reason gets too small? TBH,
>    I'd rather go with dynamic memory allocation here.

I am not a C coder at all, so I basically copy+pasted snippets from  
the internet. I wil be happy to learn how this could be changed into a  
more secure bit of code. I will be glad, if you could provide a change  
for this and commit it to Git.

>  - remark for the future, maybe the whole sqlitewrapper.pl can/should be
>    implemented in C?

Might it be an idea to extend that on the complete server code? C code  
is just a PITA to debug compared to interpreter languages...

Greets+Thanks,
Mike

-- 

DAS-NETZWERKTEAM
mike gabriel, dorfstr. 27, 24245 barmissen
fon: +49 (4302) 281418, fax: +49 (4302) 281419

GnuPG Key ID 0xB588399B
mail: mike.gabriel at das-netzwerkteam.de, http://das-netzwerkteam.de

freeBusy:
https://mail.das-netzwerkteam.de/freebusy/m.gabriel%40das-netzwerkteam.de.xfb
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digitale PGP-Unterschrift
URL: <http://lists.x2go.org/pipermail/x2go-dev/attachments/20110717/f8a36779/attachment.pgp>


More information about the x2go-dev mailing list