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: [email protected], http://das-netzwerkteam.de freeBusy: https://mail.das-netzwerkteam.de/freebusy/m.gabriel%40das-netzwerkteam.de.xfb
pgpkg6U5b4RdK.pgp
Description: Digitale PGP-Unterschrift
_______________________________________________ X2go-Dev mailing list [email protected] https://lists.berlios.de/mailman/listinfo/x2go-dev
