The most recent revision to lib/xtrans/Xtransutil.c causes significant problems for non-setuid Xservers such as XDarwin. Here is the cvs commit info: revision 1.4 date: 2004-07-30 21:00:20 +0000; author: eich; state: Exp; lines: +90 -19 2004-07-30 Egbert Eich <eich@freedesktop.org> * lib/xtrans/Xtransutil.c: (trans_mkdir): fail hard if socket directories cannot be chowned to root or chmod'ed to the requested mode if 'sticky' bit is requested for this directory instead of just print a warning that will remain unnoticed most of the times. And the comment from the file: /* * 'sticky' bit requested: assume application makes * certain security implications. If effective user ID * is != 0: fail as we may not be able to meet them. */ This seems draconian as non-setuid servers can never satisfy this requirement. What real security risks make it worth a hard failure? If nothing else this test should be dropped when the Imakefile setting InstallXserverSetUID is false.
xfs uses same same Xtransutil.c code and since it is not setuid root, it cannot chown to root and bails. Perhaps the code should check to make sure it's running as root before failing hard.
Ignore my previous comment -- too tired The relevant piece of info is that xfs uses this code and is bailing since it is not run setuid root. Perhaps this change should be backed out or solved in another way.
On today's release wrangler's call, it was suggested to either: 1. Create a helper program that is setuid root to create the dir, or 2. Revert the changes back to X11R6.7 version Any thoughts on which way is better?
Creating a setuid root help program might be the best solution for the long term. However, given the time constraints for this release, it seems preferable to revert to the X11R6.7 version. Clients have been using this library for a long time with its old behavior.
Comment from Alan Cox: #1 doesn't help on fundamentally non setuid using systems (or AFS) #2 doesn't fix the problem A combination of #1 and a .cf configured policy is needed. The suid helper itself is pretty trivial #define FIXED_PATH wherever int main(int argc, char *argv[]) { exit(chown(FIXED_PATH, 0, 0) ? errno: 0); } (Avoid things like error messages because then you get into all the close(1); close(2) type tricks with older systems) (http://freedesktop.org/pipermail/xorg/2004-August/002341.html)
Alan mentions in a followup that the simple setuid root helper is not as trivial a fix as first thought. This was discussed on today's release wranglers call but no decision was made. The general feeling was that either we revert the change or we rely on the vendors to create the directories in question at startup time. In addition, those file systems/platforms that cannot setuid root still need another solution if we do not revert this change. Other suggestions are welcome. We need to decide on a solution for Monday's release wranglers call.
I've metioned the problems for non-SUID programs several times and outlined possible solutions: Among them where: 1. Create the directories in question at install time with correct ownership/ permissions. This would be suitalbe for most Linux distros - at least as long as the tmp filesystem is persistent. 2. If the fs is not persistent run a boot script that creates those directories after the filesystem has been created. 3. Use PAM or whatever other login mechanism that is capable of running scripts with root wonership during the login process to create those dirs. It was suggested to describe possible solutions in the release notes. I thought about a suid helper program, too, as it would do the same as an SUID Xserver, but since the directory name is not known beforehand (X11, ICE, xfs use different names already), we would have to be able to pass the directory name as a command line argument, and I did not know how to do this in a fully unexploitable way.
At this point, the consensus appears to be to leave the test in. What should be done for operating and file systems that do not support setuid? One possibility would be to have a configuration option that would allow each OS to choose whether or not it should fail hard.
(In reply to comment #8) > At this point, the consensus appears to be to leave the test in. > > What should be done for operating and file systems that do not support setuid? > > One possibility would be to have a configuration option that would allow each OS > to choose whether or not it should fail hard. We talked about this solution during the release wranglers call today, and agreed that adding a configuration option would be best. Could someone who has a system that installs non-setuid servers please create a patch that would add the option? Thanks!
*** Bug 1174 has been marked as a duplicate of this bug. ***
I've marked my bug 1174 to this... I've also add mharris to the CC list (as installing Xorg from CVS on an FC2 will stop X working as the FC2 xfs startup script rm -rf /tmp/.font-unix at the start which clearly isn't a good idea anymore.. xfs also -droppriv so it never creates the directory again and the X server falls over...)
Assigning this to myself.
I'm still at loss here: What should be changed now? 1. On every system we have binaries that use this code which are non-suid. On some systems all binaries may be non-suid, however this problem exists anywhere. There are solutions for those cases that are non-suid so it is not that this code would never work on non-suid systems. They just should be documented. The security problem doesn't go away for non-suid binaries, it becomes more importand. 2. Another thing are platforms that don't support the sticky bit. There we have no choice but to accept a compromise. I don't think a compromise should be made if an individual fs doesn't support the sticky bit while the others on the same platform do. The answer here would probably be to not use the fs. So what should I do? a. Allow to accept that the owner and mode cannot be set to what is expected. b. Allow to accept that the mode cannot be set to what is expected.
For this release (to make it simple), how about a configuration option that allows people to revert to the 6.7 behavior. For example, have an option named something like XtransHardFail, which defaults to YES, but can be overridden in the host.def file. If YES, you get the current behavior, and if NO, you get the 6.7 behavior. We can work on optimizing the failure handling cases after the current release.
OK, I've added this. Set #define XtransFailSoft in host.def to set the old behavior. Please test and add to your OS if required. Please close the ticket when OK.
is this correct? #ifdef XtransFailSoft # define XtransFailDefine -DFAIL_HARD #else # define XtransFailDefine /**/ #endif At least the naming is mixed up. I'd suggest #ifndef XtransFailSoft # define XtransFailSoft NO #endif #if XtransFailSoft # define XtransFailDefine /**/ #else # define XtransFailDefine -DFAIL_HARD #endif
Thanks, Alexander. That's what I'm going to commit in a minute. The options where turned around. I shouldn't do these things late a night. I also agree that YES/NO settings are better than looking at what is defined - although we have examples of both in the config files.
Please add the appropriate setting if it differs from the default for your OS by Aug. 31, 2004.
On today's release wranglers call, we decided to set a deadline for patches of today at 6pm EDT. If patches are available by then, are small and self-contained, they can still be integreated into the release. Otherwise, we will move the remaining open bugs to the release notes bug 999 for documentation. For this bug, there is a solution available currently: Each platform that needs to disable the hard failure can add a line to their host.def file. However, if any of those platform maintainers would like to submit a patch to include the line in their default configuration file, please attach a patch to this bug.
Created attachment 805 [details] [review] Set XtransFailSoft YES for cygwin
Created attachment 807 [details] [review] Set XtransFailSoft for Darwin Default to old soft failure on Darwin so default build works out of the box. Distributors who provide additional mechanisms to create needed directories can change to hard failure as desired. Note: The error message still sounds ominous with soft failure even though the directory is actually created.
Thank you both for the patches. I have committed them (and changed the cygwin patch to put an #ifndef around it so that you can override this value in the host.def file if ever necessary). Closing.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.