On 11/03/2011 11:16 AM, Keith Packard wrote: > On Sat, 29 Oct 2011 20:47:16 +0700, Antoine Martin <[email protected]> > wrote: > >> Keith, is there anything else I should do to get this merged? > > This kind of patch could really use some careful review by someone with > a good knowledge of Posix security, given the sensitive nature of any > kind of privilege checking code. I know there are many pitfalls in the > various APIs, I just don't know what they are. For what it's worth, this code is identical to the libX11 code already merged, almost verbatim.. http://cgit.freedesktop.org/xorg/lib/libX11/tree/src/xlibi18n/lcFile.c#n221
> It's useful to have things tested, but in this case, I don't feel like > it's sufficient to know whether there are corner cases which aren't > correctly handled. OK, I can't help with the review beyond what I have already done, but what I can do is make it easier for others to verify/look at it. Attached is a simple shell script which builds the xf86PrivsElevated() function completely standalone, using all the locally available combinations of: * issetugid * getresuid * seteuid-test fallback And then tests each resulting binary with: * owned by root or not * suid/non-suid * as a regular user / as root I have tested this on a number of systems/distros, some with SELinux, apparmor, etc.. Thanks Antoine PS: the script uses sudo to chmod/chown the binaries, here's a sample output: $ ./test_privs.sh ************************************************** building all variants (some may fail - that's OK): ************************************************** /tmp/ccyEXRCj.o: In function `xf86PrivsElevated': test_privs.c:(.text+0x59): undefined reference to `issetugid' collect2: ld returned 1 exit status failed to build issetugid built getresuid variant built fallbackseteuidtest variant Testing as current user: 500: -rwxrwxr-x. 1 500 500 7719 Dec 14 17:00 ./test_privs_getresuid.bin xf86PrivsElevated()=0 -rwxrwxr-x. 1 500 500 7719 Dec 14 17:00 ./test_privs_fallbackseteuidtest.bin xf86PrivsElevated()=0 Testing via sudo: -rwxrwxr-x. 1 500 500 7719 Dec 14 17:00 ./test_privs_getresuid.bin xf86PrivsElevated()=0 -rwxrwxr-x. 1 500 500 7719 Dec 14 17:00 ./test_privs_fallbackseteuidtest.bin xf86PrivsElevated()=0 Testing suid non root: -rwsrwsr-x. 1 500 500 7719 Dec 14 17:00 ./test_privs_getresuid.bin xf86PrivsElevated()=0 -rwsrwsr-x. 1 500 500 7719 Dec 14 17:00 ./test_privs_fallbackseteuidtest.bin xf86PrivsElevated()=0 Testing suid non root via sudo: -rwsrwsr-x. 1 500 500 7719 Dec 14 17:00 ./test_privs_getresuid.bin xf86PrivsElevated()=1 -rwsrwsr-x. 1 500 500 7719 Dec 14 17:00 ./test_privs_fallbackseteuidtest.bin xf86PrivsElevated()=1 Testing suid binary as root: -rwsrwsr-x. 1 0 0 7719 Dec 14 17:00 ./test_privs_getresuid.bin xf86PrivsElevated()=0 -rwsrwsr-x. 1 0 0 7719 Dec 14 17:00 ./test_privs_fallbackseteuidtest.bin xf86PrivsElevated()=0 Testing suid binary as current user: -rwsrwsr-x. 1 0 0 7719 Dec 14 17:00 ./test_privs_getresuid.bin xf86PrivsElevated()=1 -rwsrwsr-x. 1 0 0 7719 Dec 14 17:00 ./test_privs_fallbackseteuidtest.bin xf86PrivsElevated()=1 Cleaning up
test_privs.sh
Description: application/shellscript
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
