Bugzilla – Bug 1029
Hard failure if socket directories cannot be chowned to root is bad
Last modified: 2004-09-01 01:24:43 UTC
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:
date: 2004-07-30 21:00:20 +0000; author: eich; state: Exp; lines: +90 -19
2004-07-30 Egbert Eich <firstname.lastname@example.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
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)
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
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
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
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
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
We can work on optimizing the failure handling cases after the current release.
OK, I've added this. Set
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?
# define XtransFailDefine -DFAIL_HARD
# define XtransFailDefine /**/
At least the naming is mixed up.
# define XtransFailSoft NO
# define XtransFailDefine /**/
# define XtransFailDefine -DFAIL_HARD
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
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).