Bug 1029

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.