This allows us to run the server as a normal user whilst still
being able to use the -modulepath, -logfile and -config switches
We define a xf86PrivsElevated which will do the checks and cache
the result in case it is called more than once.
Also renamed the paths #defines to match their new meaning.
Original discussion which led to this patch can be found here:
http://lists.freedesktop.org/archives/xorg-devel/2011-September/025853.html
Signed-off-by: Antoine Martin <[email protected]>
Changes from the previous version of this patch:
* use FatalError() rather than exit() (thx to Julien Cristau)
* remove comment superseded by FatalError message (thx to Tormod Volden)
* more defensive code: print warning and assume privs are elevated if
getresuid or getresgid fail, ensure all codepaths set privsElevated
explicitly and make it default to TRUE (in case we ever miss one)
* email title includes patch version in the right location
(btw, there are changes in xserver's autoconf, not just hw/xfree86)
Thanks
Antoine
---
configure.ac | 4 ++-
hw/xfree86/common/xf86Config.c | 28 +++++++-------
hw/xfree86/common/xf86Init.c | 76 +++++++++++++++++++++++++++++++++++-----
hw/xfree86/common/xf86Priv.h | 1 +
include/xorg-config.h.in | 6 +++
5 files changed, 91 insertions(+), 24 deletions(-)
diff --git a/configure.ac b/configure.ac
index 67a6836..a79e2be 100644
--- a/configure.ac
+++ b/configure.ac
@@ -208,9 +208,11 @@ AC_SUBST(DLOPEN_LIBS)
dnl Checks for library functions.
AC_FUNC_VPRINTF
-AC_CHECK_FUNCS([geteuid getuid link memmove memset mkstemp strchr strrchr \
+AC_CHECK_FUNCS([geteuid getuid issetugid getresuid \
+ link memmove memset mkstemp strchr strrchr \
strtol getopt getopt_long vsnprintf walkcontext backtrace \
getisax getzoneid shmctl64 strcasestr ffs vasprintf])
+
AC_FUNC_ALLOCA
dnl Old HAS_* names used in os/*.c.
AC_CHECK_FUNC([getdtablesize],
diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c
index f8c1b65..8ccc52a 100644
--- a/hw/xfree86/common/xf86Config.c
+++ b/hw/xfree86/common/xf86Config.c
@@ -72,8 +72,8 @@
* These paths define the way the config file search is done. The escape
* sequences are documented in parser/scan.c.
*/
-#ifndef ROOT_CONFIGPATH
-#define ROOT_CONFIGPATH "%A," "%R," \
+#ifndef ALL_CONFIGPATH
+#define ALL_CONFIGPATH "%A," "%R," \
"/etc/X11/%R," "%P/etc/X11/%R," \
"%E," "%F," \
"/etc/X11/%F," "%P/etc/X11/%F," \
@@ -83,8 +83,8 @@
"%P/lib/X11/%X.%H," \
"%P/lib/X11/%X"
#endif
-#ifndef USER_CONFIGPATH
-#define USER_CONFIGPATH "/etc/X11/%S," "%P/etc/X11/%S," \
+#ifndef RESTRICTED_CONFIGPATH
+#define RESTRICTED_CONFIGPATH "/etc/X11/%S," "%P/etc/X11/%S," \
"/etc/X11/%G," "%P/etc/X11/%G," \
"/etc/X11/%X," "/etc/%X," \
"%P/etc/X11/%X.%H," \
@@ -92,14 +92,14 @@
"%P/lib/X11/%X.%H," \
"%P/lib/X11/%X"
#endif
-#ifndef ROOT_CONFIGDIRPATH
-#define ROOT_CONFIGDIRPATH "%A," "%R," \
+#ifndef ALL_CONFIGDIRPATH
+#define ALL_CONFIGDIRPATH "%A," "%R," \
"/etc/X11/%R," "%C/X11/%R," \
"/etc/X11/%X," "%C/X11/%X"
#endif
-#ifndef USER_CONFIGDIRPATH
-#define USER_CONFIGDIRPATH "/etc/X11/%R," "%C/X11/%R," \
- "/etc/X11/%X," "%C/X11/%X"
+#ifndef RESTRICTED_CONFIGDIRPATH
+#define RESTRICTED_CONFIGDIRPATH "/etc/X11/%R," "%C/X11/%R," \
+ "/etc/X11/%X," "%C/X11/%X"
#endif
#ifndef SYS_CONFIGDIRPATH
#define SYS_CONFIGDIRPATH "/usr/share/X11/%X," "%D/X11/%X"
@@ -2302,12 +2302,12 @@ xf86HandleConfigFile(Bool autoconfig)
Bool implicit_layout = FALSE;
if (!autoconfig) {
- if (getuid() == 0) {
- filesearch = ROOT_CONFIGPATH;
- dirsearch = ROOT_CONFIGDIRPATH;
+ if (!xf86PrivsElevated()) {
+ filesearch = ALL_CONFIGPATH;
+ dirsearch = ALL_CONFIGDIRPATH;
} else {
- filesearch = USER_CONFIGPATH;
- dirsearch = USER_CONFIGDIRPATH;
+ filesearch = RESTRICTED_CONFIGPATH;
+ dirsearch = RESTRICTED_CONFIGDIRPATH;
}
if (xf86ConfigFile)
diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index 350918d..3fead6f 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -237,6 +237,63 @@ xf86PrintMarkers(void)
LogPrintMarkers();
}
+Bool xf86PrivsElevated(void)
+{
+ static Bool privsTested = FALSE;
+ static Bool privsElevated = TRUE;
+
+ if (!privsTested) {
+#if !defined(WIN32)
+ if ((getuid() != geteuid()) || (getgid() != getegid())) {
+ privsElevated = TRUE;
+ } else {
+#if defined(HAVE_ISSETUGID)
+ privsElevated = issetugid();
+#elif defined(HAVE_GETRESUID)
+ uid_t ruid, euid, suid;
+ gid_t rgid, egid, sgid;
+
+ if ((getresuid(&ruid, &euid, &suid) == 0) &&
+ (getresgid(&rgid, &egid, &sgid) == 0)) {
+ privsElevated = (euid != suid) || (egid != sgid);
+ }
+ else {
+ printf("Failed getresuid or getresgid");
+ /* Something went wrong, make defensive assumption */
+ privsElevated = TRUE;
+ }
+#else
+ if (getuid()==0) {
+ /* running as root: uid==euid==0 */
+ privsElevated = FALSE;
+ }
+ else {
+ /*
+ * If there are saved ID's the process might still be privileged
+ * even though the above test succeeded. If issetugid() and
+ * getresgid() aren't available, test this by trying to set
+ * euid to 0.
+ */
+ unsigned int oldeuid;
+ oldeuid = geteuid();
+
+ if (seteuid(0) != 0) {
+ privsElevated = FALSE;
+ } else {
+ if (seteuid(oldeuid) != 0) {
+ FatalError("Failed to drop privileges. Exiting\n");
+ }
+ privsElevated = TRUE;
+ }
+ }
+#endif
+ }
+#endif
+ privsTested = TRUE;
+ }
+ return privsElevated;
+}
+
static Bool
xf86CreateRootWindow(WindowPtr pWin)
{
@@ -896,7 +953,7 @@ OsVendorInit(void)
#ifdef O_NONBLOCK
if (!beenHere) {
- if (geteuid() == 0 && getuid() != geteuid())
+ if (xf86PrivsElevated())
{
int status;
@@ -1067,10 +1124,11 @@ ddxProcessArgument(int argc, char **argv, int i)
FatalError("Required argument to %s not specified\n", argv[i]); \
}
- /* First the options that are only allowed for root */
+ /* First the options that are not allowed with elevated privileges */
if (!strcmp(argv[i], "-modulepath") || !strcmp(argv[i], "-logfile")) {
- if ( (geteuid() == 0) && (getuid() != 0) ) {
- FatalError("The '%s' option can only be used by root.\n", argv[i]);
+ if (xf86PrivsElevated()) {
+ FatalError("The '%s' option cannot be used with "
+ "elevated privileges.\n", argv[i]);
}
else if (!strcmp(argv[i], "-modulepath"))
{
@@ -1098,9 +1156,9 @@ ddxProcessArgument(int argc, char **argv, int i)
if (!strcmp(argv[i], "-config") || !strcmp(argv[i], "-xf86config"))
{
CHECK_FOR_REQUIRED_ARGUMENT();
- if (getuid() != 0 && !xf86PathIsSafe(argv[i + 1])) {
+ if (xf86PrivsElevated() && !xf86PathIsSafe(argv[i + 1])) {
FatalError("\nInvalid argument for %s\n"
- "\tFor non-root users, the file specified with %s must be\n"
+ "\tWith elevated privileges, the file specified with %s must be\n"
"\ta relative path and must not contain any \"..\" elements.\n"
"\tUsing default "__XCONFIGFILE__" search path.\n\n",
argv[i], argv[i]);
@@ -1111,9 +1169,9 @@ ddxProcessArgument(int argc, char **argv, int i)
if (!strcmp(argv[i], "-configdir"))
{
CHECK_FOR_REQUIRED_ARGUMENT();
- if (getuid() != 0 && !xf86PathIsSafe(argv[i + 1])) {
+ if (xf86PrivsElevated() && !xf86PathIsSafe(argv[i + 1])) {
FatalError("\nInvalid argument for %s\n"
- "\tFor non-root users, the file specified with %s must be\n"
+ "\tWith elevated privileges, the file specified with %s must be\n"
"\ta relative path and must not contain any \"..\" elements.\n"
"\tUsing default "__XCONFIGDIR__" search path.\n\n",
argv[i], argv[i]);
@@ -1397,7 +1455,7 @@ ddxUseMsg(void)
ErrorF("\n");
ErrorF("\n");
ErrorF("Device Dependent Usage\n");
- if (getuid() == 0 || geteuid() != 0)
+ if (!xf86PrivsElevated())
{
ErrorF("-modulepath paths specify the module search path\n");
ErrorF("-logfile file specify a log file name\n");
diff --git a/hw/xfree86/common/xf86Priv.h b/hw/xfree86/common/xf86Priv.h
index 1fe3d7e..49cae87 100644
--- a/hw/xfree86/common/xf86Priv.h
+++ b/hw/xfree86/common/xf86Priv.h
@@ -147,6 +147,7 @@ extern _X_EXPORT Bool xf86LoadModules(char **list, pointer
*optlist);
extern _X_EXPORT int xf86SetVerbosity(int verb);
extern _X_EXPORT int xf86SetLogVerbosity(int verb);
extern _X_EXPORT Bool xf86CallDriverProbe( struct _DriverRec * drv, Bool
detect_only );
+extern _X_EXPORT Bool xf86PrivsElevated(void);
#endif /* _NO_XF86_PROTOTYPES */
diff --git a/include/xorg-config.h.in b/include/xorg-config.h.in
index 0d1ea91..ae0c8ec 100644
--- a/include/xorg-config.h.in
+++ b/include/xorg-config.h.in
@@ -139,4 +139,10 @@
/* Build with libdrm support */
#undef WITH_LIBDRM
+/* Have setugid */
+#undef HAVE_ISSETUGID
+
+/* Have getresuid */
+#undef HAVE_GETRESUID
+
#endif /* _XORG_CONFIG_H_ */
--
1.7.6.4
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
