[PATCH xserver] check for elevated privileges not uid=0 (V3)

Antoine Martin antoine at nagafix.co.uk
Mon Oct 10 09:27:29 PDT 2011


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 <antoine at nagafix.co.uk>
---
 configure.ac                   |    7 ++++
 hw/xfree86/common/xf86Config.c |   28 +++++++++---------
 hw/xfree86/common/xf86Init.c   |   63 ++++++++++++++++++++++++++++++++++------
 hw/xfree86/common/xf86Priv.h   |    1 +
 include/xorg-config.h.in       |    6 ++++
 5 files changed, 82 insertions(+), 23 deletions(-)

diff --git a/configure.ac b/configure.ac
index 67a6836..fe836cc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -211,6 +211,13 @@ AC_FUNC_VPRINTF
 AC_CHECK_FUNCS([geteuid getuid link memmove memset mkstemp strchr strrchr \
 		strtol getopt getopt_long vsnprintf walkcontext backtrace \
 		getisax getzoneid shmctl64 strcasestr ffs vasprintf])
+
+dnl Checks for uid/gid functions used for testing elevated privileges
+AC_CHECK_FUNC([issetugid],
+        AC_DEFINE(HASSETUGID,1,[Has issetugid() function]))
+AC_CHECK_FUNC([getresuid],
+        AC_DEFINE(HASGETRESUID,1,[Has getresuid() & getresgid() functions]))
+
 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..29f7638 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -237,6 +237,50 @@ xf86PrintMarkers(void)
   LogPrintMarkers();
 }
 
+static Bool privsTested = FALSE;
+static Bool privsElevated = FALSE;
+Bool xf86PrivsElevated(void)
+{
+  if (!privsTested) {
+#if !defined(WIN32)
+    if ((getuid() != geteuid()) || (getgid() != getegid())) {
+      privsElevated = TRUE;
+    } else {
+#if defined(HASSETUGID)
+      privsElevated = issetugid();
+#elif defined(HASGETRESUID)
+      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
+      /*
+       * If there are saved ID's the process might still be priviledged
+       * 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) == -1) {
+	  /* XXX ouch, coudn't get back to original uid
+	   what can we do ??? */
+	   _exit(127);
+        }
+	privsElevated = 1;
+      }
+#endif
+    }
+#endif
+    privsTested = TRUE;
+  }
+  return privsElevated;
+}
+
 static Bool
 xf86CreateRootWindow(WindowPtr pWin)
 {
@@ -896,7 +940,7 @@ OsVendorInit(void)
 
 #ifdef O_NONBLOCK
   if (!beenHere) {
-    if (geteuid() == 0 && getuid() != geteuid())
+    if (xf86PrivsElevated())
     {
       int status;
 
@@ -1067,10 +1111,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 +1143,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 +1156,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 +1442,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..18e5fb7 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 HASSETUGID
+
+/* Have getresuid */
+#undef HASGETRESUID
+
 #endif /* _XORG_CONFIG_H_ */
-- 
1.7.6.4


--------------010705090803000401040105--


More information about the xorg-devel mailing list