[or-cvs] r17255: {tor} Backport of changesets 17200, 17201, 17203-17206, 17228, 172 (in tor/branches/tor-0_2_0-patches: . contrib doc src/common src/or)

sjm217 at seul.org sjm217 at seul.org
Wed Nov 12 01:10:21 UTC 2008


Author: sjm217
Date: 2008-11-11 20:10:21 -0500 (Tue, 11 Nov 2008)
New Revision: 17255

Modified:
   tor/branches/tor-0_2_0-patches/ChangeLog
   tor/branches/tor-0_2_0-patches/configure.in
   tor/branches/tor-0_2_0-patches/contrib/linux-tor-prio.sh
   tor/branches/tor-0_2_0-patches/contrib/rc.subr
   tor/branches/tor-0_2_0-patches/contrib/tor.sh.in
   tor/branches/tor-0_2_0-patches/contrib/torctl.in
   tor/branches/tor-0_2_0-patches/doc/tor.1.in
   tor/branches/tor-0_2_0-patches/src/common/compat.c
   tor/branches/tor-0_2_0-patches/src/common/compat.h
   tor/branches/tor-0_2_0-patches/src/or/config.c
Log:
Backport of changesets 17200, 17201, 17203-17206, 17228, 17232, 17236: Patch from Jacob Appelbaum and me to make User option more robust, properly set supplementary groups, deprecated the Group option, and log more information on credential switching. Fixes bugs 848 and 857

Modified: tor/branches/tor-0_2_0-patches/ChangeLog
===================================================================
--- tor/branches/tor-0_2_0-patches/ChangeLog	2008-11-12 00:58:10 UTC (rev 17254)
+++ tor/branches/tor-0_2_0-patches/ChangeLog	2008-11-12 01:10:21 UTC (rev 17255)
@@ -1,4 +1,13 @@
 Changes in version 0.2.0.32 - 2008-??-??
+  o Security fixes:
+    - The "User" and "Group" config options did not clear the
+      supplementary group entries for the Tor process. The "User" option
+      is now more robust, and we now set the groups to the specified
+      user's primary group. The "Group" option is now ignored. For more
+      detailed logging on credential switching, set CREDENTIAL_LOG_LEVEL
+      in common/compat.c to LOG_NOTICE or higher. Patch by Jacob Appelbaum
+      and Steven Murdoch. Bugfix on 0.0.2pre14. Fixes bug 848 and 857.
+
   o Major bugfixes:
     - Fix a DOS opportunity during the voting signature collection process
       at directory authorities. Spotted by rovv. Bugfix on 0.2.0.x.

Modified: tor/branches/tor-0_2_0-patches/configure.in
===================================================================
--- tor/branches/tor-0_2_0-patches/configure.in	2008-11-12 00:58:10 UTC (rev 17254)
+++ tor/branches/tor-0_2_0-patches/configure.in	2008-11-12 01:10:21 UTC (rev 17255)
@@ -616,6 +616,9 @@
 AC_DEFINE_UNQUOTED(LOGFACILITY,$syslog_facility,[name of the syslog facility])
 AC_SUBST(LOGFACILITY)
 
+# Check if we have getresuid and getresgid
+AC_CHECK_FUNCS(getresuid getresgid)
+
 # Check for gethostbyname_r in all its glorious incompatible versions.
 #   (This logic is based on that in Python's configure.in)
 AH_TEMPLATE(HAVE_GETHOSTBYNAME_R,

Modified: tor/branches/tor-0_2_0-patches/contrib/linux-tor-prio.sh
===================================================================
--- tor/branches/tor-0_2_0-patches/contrib/linux-tor-prio.sh	2008-11-12 00:58:10 UTC (rev 17254)
+++ tor/branches/tor-0_2_0-patches/contrib/linux-tor-prio.sh	2008-11-12 01:10:21 UTC (rev 17255)
@@ -9,8 +9,8 @@
 # This script provides prioritization of Tor traffic below other
 # traffic on a Linux server. It has two modes of operation: UID based 
 # and IP based. The UID based method requires that Tor be launched from 
-# a specific user ID. The "User" and "Group" Tor config settings are 
-# insufficient, as they set the UID after the socket is created.
+# a specific user ID. The "User" Tor config setting is
+# insufficient, as it sets the UID after the socket is created.
 # Here is a three line C wrapper you can use to execute Tor and drop 
 # privs to UID 501 before it creates any sockets. Change the UID 
 # to the UID for your tor server user, and compile with 
@@ -49,7 +49,7 @@
 
 DEV=eth0
 
-# NOTE! You must START Tor under this UID. Using the Tor User/Group 
+# NOTE! You must START Tor under this UID. Using the Tor User
 # config setting is NOT sufficient.
 TOR_UID=$(id -u tor)
 

Modified: tor/branches/tor-0_2_0-patches/contrib/rc.subr
===================================================================
--- tor/branches/tor-0_2_0-patches/contrib/rc.subr	2008-11-12 00:58:10 UTC (rev 17254)
+++ tor/branches/tor-0_2_0-patches/contrib/rc.subr	2008-11-12 01:10:21 UTC (rev 17255)
@@ -14,7 +14,6 @@
 # tor_conf (str):       Points to your tor conf file
 #                       Default: /usr/local/etc/tor/torrc
 # tor_user (str):       Tor Daemon user. Default _tor
-# tor_groupr (str):     Tor Daemon group. Default _tor
 #
 
 . /etc/rc.subr
@@ -27,7 +26,6 @@
 : ${tor_enable="NO"}
 : ${tor_conf="/usr/local/etc/tor/torrc"}
 : ${tor_user="_tor"}
-: ${tor_group="_tor"}
 : ${tor_pidfile="/var/run/tor/tor.pid"}
 : ${tor_logfile="/var/log/tor"}
 : ${tor_datadir="/var/run/tor"}
@@ -35,7 +33,7 @@
 required_files=${tor_conf}
 required_dirs=${tor_datadir}
 command="/usr/local/bin/${name}"
-command_args="-f ${tor_conf} --pidfile ${tor_pidfile} --runasdaemon 1 --datadirectory ${tor_datadir} --user ${tor_user} --group ${tor_group}"
+command_args="-f ${tor_conf} --pidfile ${tor_pidfile} --runasdaemon 1 --datadirectory ${tor_datadir} --user ${tor_user}"
 extra_commands="log"
 log_cmd="${name}_log"
 

Modified: tor/branches/tor-0_2_0-patches/contrib/tor.sh.in
===================================================================
--- tor/branches/tor-0_2_0-patches/contrib/tor.sh.in	2008-11-12 00:58:10 UTC (rev 17254)
+++ tor/branches/tor-0_2_0-patches/contrib/tor.sh.in	2008-11-12 01:10:21 UTC (rev 17255)
@@ -31,8 +31,6 @@
 # torctl will use these environment variables
 TORUSER=@TORUSER@
 export TORUSER
-TORGROUP=@TORGROUP@
-export TORGROUP
 
 if [ -x /bin/su ] ; then
     SUPROG=/bin/su

Modified: tor/branches/tor-0_2_0-patches/contrib/torctl.in
===================================================================
--- tor/branches/tor-0_2_0-patches/contrib/torctl.in	2008-11-12 00:58:10 UTC (rev 17254)
+++ tor/branches/tor-0_2_0-patches/contrib/torctl.in	2008-11-12 01:10:21 UTC (rev 17255)
@@ -41,22 +41,18 @@
 TORARGS="--pidfile $PIDFILE --log \"notice file $LOGFILE\" --runasdaemon 1"
 TORARGS="$TORARGS --datadirectory $TORDATA"
 
-# If user and group names are set in the environment, then use them;
+# If user name is set in the environment, then use it;
 # otherwise run as the invoking user (or whatever user the config
 # file says)... unless the invoking user is root. The idea here is to
 # let an unprivileged user run tor for her own use using this script,
 # while still providing for it to be used as a system daemon.
 if [ "x`id -u`" = "x0" ]; then
     TORUSER=@TORUSER@
-    TORGROUP=@TORGROUP@
 fi
 
 if [ "x$TORUSER" != "x" ]; then
     TORARGS="$TORARGS --user $TORUSER"
 fi
-if [ "x$TORGROUP" != "x" ]; then
-    TORARGS="$TORARGS --group $TORGROUP"
-fi
 
 # We no longer wrap the Tor daemon startup in an su when running as
 # root, because it's too painful to make the use of su portable.

Modified: tor/branches/tor-0_2_0-patches/doc/tor.1.in
===================================================================
--- tor/branches/tor-0_2_0-patches/doc/tor.1.in	2008-11-12 00:58:10 UTC (rev 17254)
+++ tor/branches/tor-0_2_0-patches/doc/tor.1.in	2008-11-12 01:10:21 UTC (rev 17255)
@@ -259,10 +259,6 @@
 (Default: 0)
 .LP
 .TP
-\fBGroup \fR\fIGID\fP
-On startup, setgid to this group.
-.LP
-.TP
 \fBHttpProxy\fR \fIhost\fR[:\fIport\fR]\fP
 Tor will make all its directory requests through this host:port
 (or host:80 if port is not specified),
@@ -345,7 +341,7 @@
 .LP
 .TP
 \fBUser \fR\fIUID\fP
-On startup, setuid to this user.
+On startup, setuid to this user and setgid to their primary group.
 .LP
 .TP
 \fBHardwareAccel \fR\fB0\fR|\fB1\fP

Modified: tor/branches/tor-0_2_0-patches/src/common/compat.c
===================================================================
--- tor/branches/tor-0_2_0-patches/src/common/compat.c	2008-11-12 00:58:10 UTC (rev 17254)
+++ tor/branches/tor-0_2_0-patches/src/common/compat.c	2008-11-12 01:10:21 UTC (rev 17255)
@@ -871,62 +871,220 @@
   return 0;
 }
 
-/** Call setuid and setgid to run as <b>user</b>:<b>group</b>.  Return 0 on
- * success.  On failure, log and return -1.
+/** Log details of current user and group credentials. Return 0 on
+ * success. Logs and return -1 on failure.
  */
+static int
+log_credential_status(void)
+{
+#define CREDENTIAL_LOG_LEVEL LOG_INFO
+#ifndef MS_WINDOWS
+  /* Real, effective and saved UIDs */
+  uid_t ruid, euid, suid;
+  /* Read, effective and saved GIDs */
+  gid_t rgid, egid, sgid;
+  /* Supplementary groups */
+  gid_t sup_gids[NGROUPS_MAX + 1];
+  /* Number of supplementary groups */
+  int ngids;
+
+  /* log UIDs */
+#ifdef HAVE_GETRESUID
+  if (getresuid(&ruid, &euid, &suid) != 0 ) {
+    log_warn(LD_GENERAL, "Error getting changed UIDs: %s", strerror(errno));
+    return -1;
+  } else {
+    log_fn(CREDENTIAL_LOG_LEVEL, LD_GENERAL,
+           "UID is %u (real), %u (effective), %u (saved)", ruid, euid, suid);
+  }
+#else
+  /* getresuid is not present on MacOS X, so we can't get the saved (E)UID */
+  ruid = getuid();
+  euid = geteuid();
+  (void)suid;
+
+  log_fn(CREDENTIAL_LOG_LEVEL, LD_GENERAL,
+         "UID is %u (real), %u (effective), unknown (saved)", ruid, euid);
+#endif
+
+  /* log GIDs */
+#ifdef HAVE_GETRESGID
+  if (getresgid(&rgid, &egid, &sgid) != 0 ) {
+    log_warn(LD_GENERAL, "Error getting changed GIDs: %s", strerror(errno));
+    return -1;
+  } else {
+    log_fn(CREDENTIAL_LOG_LEVEL, LD_GENERAL,
+           "GID is %u (real), %u (effective), %u (saved)", rgid, egid, sgid);
+  }
+#else
+  /* getresgid is not present on MacOS X, so we can't get the saved (E)GID */
+  rgid = getgid();
+  egid = getegid();
+  (void)sgid;
+  log_fn(CREDENTIAL_LOG_LEVEL, LD_GENERAL,
+         "GID is %u (real), %u (effective), unknown (saved)", rgid, egid);
+#endif
+
+  /* log supplementary groups */
+  if ((ngids = getgroups(NGROUPS_MAX + 1, sup_gids)) < 0) {
+    log_warn(LD_GENERAL, "Error getting supplementary GIDs: %s",
+             strerror(errno));
+    return -1;
+  } else {
+    int i;
+    char *strgid;
+    char *s = NULL;
+    int formatting_error = 0;
+    smartlist_t *elts = smartlist_create();
+
+    for (i = 0; i<ngids; i++) {
+      strgid = tor_malloc(11);
+      if (tor_snprintf(strgid, 11, "%u", (unsigned)sup_gids[i]) == -1) {
+        log_warn(LD_GENERAL, "Error printing supplementary GIDs");
+        formatting_error = 1;
+        goto error;
+      }
+      smartlist_add(elts, strgid);
+    }
+
+    s = smartlist_join_strings(elts, " ", 0, NULL);
+
+    log_fn(CREDENTIAL_LOG_LEVEL, LD_GENERAL, "Supplementary groups are: %s",s);
+
+   error:
+    tor_free(s);
+    SMARTLIST_FOREACH(elts, char *, cp,
+    {
+      tor_free(cp);
+    });
+    smartlist_free(elts);
+
+    if (formatting_error)
+      return -1;
+  }
+#endif
+
+  return 0;
+}
+
+/** Call setuid and setgid to run as <b>user</b> and switch to their
+ * primary group.  Return 0 on success.  On failure, log and return -1.
+ */
 int
-switch_id(const char *user, const char *group)
+switch_id(const char *user)
 {
 #ifndef MS_WINDOWS
   struct passwd *pw = NULL;
-  struct group *gr = NULL;
+  uid_t old_uid;
+  gid_t old_gid;
+  static int have_already_switched_id = 0;
 
-  if (user) {
-    pw = getpwnam(user);
-    if (pw == NULL) {
-      log_warn(LD_CONFIG,"User '%s' not found.", user);
-      return -1;
-    }
+  tor_assert(user);
+
+  if (have_already_switched_id)
+    return 0;
+
+  /* Log the initial credential state */
+  if (log_credential_status())
+    return -1;
+
+  log_fn(CREDENTIAL_LOG_LEVEL, LD_GENERAL, "Changing user and groups");
+
+  /* Get old UID/GID to check if we changed correctly */
+  old_uid = getuid();
+  old_gid = getgid();
+
+  /* Lookup the user and group information, if we have a problem, bail out. */
+  pw = getpwnam(user);
+  if (pw == NULL) {
+    log_warn(LD_CONFIG, "Error setting configured user: %s not found", user);
+    return -1;
   }
 
-  /* switch the group first, while we still have the privileges to do so */
-  if (group) {
-    gr = getgrnam(group);
-    if (gr == NULL) {
-      log_warn(LD_CONFIG,"Group '%s' not found.", group);
+  /* Properly switch egid,gid,euid,uid here or bail out */
+  if (setgroups(1, &pw->pw_gid)) {
+    log_warn(LD_GENERAL, "Error setting groups to gid %d: \"%s\". "
+             "If you set the \"User\" option, you must start Tor as root.",
+             (int)pw->pw_gid, strerror(errno));
+    return -1;
+  }
+
+  if (setegid(pw->pw_gid)) {
+    log_warn(LD_GENERAL, "Error setting egid to %d: %s",
+             (int)pw->pw_gid, strerror(errno));
+    return -1;
+  }
+
+  if (setgid(pw->pw_gid)) {
+    log_warn(LD_GENERAL, "Error setting gid to %d: %s",
+             (int)pw->pw_gid, strerror(errno));
+    return -1;
+  }
+
+  if (setuid(pw->pw_uid)) {
+    log_warn(LD_GENERAL, "Error setting configured uid to %s (%d): %s",
+             user, (int)pw->pw_uid, strerror(errno));
+    return -1;
+  }
+
+  if (seteuid(pw->pw_uid)) {
+    log_warn(LD_GENERAL, "Error setting configured euid to %s (%d): %s",
+             user, (int)pw->pw_uid, strerror(errno));
+    return -1;
+  }
+
+  /* This is how OpenBSD rolls:
+  if (setgroups(1, &pw->pw_gid) || setegid(pw->pw_gid) ||
+      setgid(pw->pw_gid) || setuid(pw->pw_uid) || seteuid(pw->pw_uid)) {
+      setgid(pw->pw_gid) || seteuid(pw->pw_uid) || setuid(pw->pw_uid)) {
+    log_warn(LD_GENERAL, "Error setting configured UID/GID: %s",
+    strerror(errno));
+    return -1;
+  }
+  */
+
+  /* We've properly switched egid, gid, euid, uid, and supplementary groups if
+   * we're here. */
+
+#if !defined(CYGWIN) && !defined(__CYGWIN__)
+  /* If we tried to drop privilege to a group/user other than root, attempt to
+   * restore root (E)(U|G)ID, and abort if the operation succeeds */
+
+  /* Only check for privilege dropping if we were asked to be non-root */
+  if (pw->pw_uid) {
+    /* Try changing GID/EGID */
+    if (pw->pw_gid != old_gid &&
+        (setgid(old_gid) != -1 || setegid(old_gid) != -1)) {
+      log_warn(LD_GENERAL, "Was able to restore group credentials even after "
+               "switching GID: this means that the setgid code didn't work.");
       return -1;
     }
 
-    if (setgid(gr->gr_gid) != 0) {
-      log_warn(LD_GENERAL,"Error setting to configured GID: %s",
-               strerror(errno));
+    /* Try changing UID/EUID */
+    if (pw->pw_uid != old_uid &&
+        (setuid(old_uid) != -1 || seteuid(old_uid) != -1)) {
+      log_warn(LD_GENERAL, "Was able to restore user credentials even after "
+               "switching UID: this means that the setuid code didn't work.");
       return -1;
     }
-  } else if (user) {
-    if (setgid(pw->pw_gid) != 0) {
-      log_warn(LD_GENERAL,"Error setting to user GID: %s", strerror(errno));
-      return -1;
-    }
   }
+#endif
 
-  /* now that the group is switched, we can switch users and lose
-     privileges */
-  if (user) {
-    if (setuid(pw->pw_uid) != 0) {
-      log_warn(LD_GENERAL,"Error setting UID: %s", strerror(errno));
-      return -1;
-    }
+  /* Check what really happened */
+  if (log_credential_status()) {
+    return -1;
   }
 
+  have_already_switched_id = 1; /* mark success so we never try again */
   return 0;
+
 #else
   (void)user;
-  (void)group;
-#endif
 
   log_warn(LD_CONFIG,
-           "User or group specified, but switching users is not supported.");
+           "User specified but switching users is unsupported on your OS.");
   return -1;
+#endif
 }
 
 #ifdef HAVE_PWD_H

Modified: tor/branches/tor-0_2_0-patches/src/common/compat.h
===================================================================
--- tor/branches/tor-0_2_0-patches/src/common/compat.h	2008-11-12 00:58:10 UTC (rev 17254)
+++ tor/branches/tor-0_2_0-patches/src/common/compat.h	2008-11-12 01:10:21 UTC (rev 17255)
@@ -463,7 +463,7 @@
 typedef unsigned long rlim_t;
 #endif
 int set_max_file_descriptors(rlim_t limit, int *max);
-int switch_id(const char *user, const char *group);
+int switch_id(const char *user);
 #ifdef HAVE_PWD_H
 char *get_user_homedir(const char *username);
 #endif

Modified: tor/branches/tor-0_2_0-patches/src/or/config.c
===================================================================
--- tor/branches/tor-0_2_0-patches/src/or/config.c	2008-11-12 00:58:10 UTC (rev 17254)
+++ tor/branches/tor-0_2_0-patches/src/or/config.c	2008-11-12 01:10:21 UTC (rev 17255)
@@ -204,7 +204,7 @@
   V(GeoIPFile,                   STRING,
     SHARE_DATADIR PATH_SEPARATOR "tor" PATH_SEPARATOR "geoip"),
 #endif
-  V(Group,                       STRING,   NULL),
+  OBSOLETE("Group"),
   V(HardwareAccel,               BOOL,     "0"),
   V(HashedControlPassword,       LINELIST, NULL),
   V(HidServDirectoryV2,          BOOL,     "0"),
@@ -391,7 +391,6 @@
   /* { "FastFirstHopPK", "" }, */
   /* FetchServerDescriptors, FetchHidServDescriptors,
    * FetchUselessDescriptors */
-  { "Group", "On startup, setgid to this group." },
   { "HardwareAccel", "If set, Tor tries to use hardware crypto accelerators "
     "when it can." },
   /* HashedControlPassword */
@@ -1031,13 +1030,10 @@
 #endif
 
   /* Setuid/setgid as appropriate */
-  if (options->User || options->Group) {
-    /* XXXX021 We should only do this the first time through, not on
-     * every setconf. */
-    if (switch_id(options->User, options->Group) != 0) {
+  if (options->User) {
+    if (switch_id(options->User) != 0) {
       /* No need to roll back, since you can't change the value. */
-      *msg = tor_strdup("Problem with User or Group value. "
-                        "See logs for details.");
+      *msg = tor_strdup("Problem with User value. See logs for details.");
       goto done;
     }
   }
@@ -1868,9 +1864,9 @@
         result->value = tor_strdup("");
       break;
     case CONFIG_TYPE_OBSOLETE:
-      log_warn(LD_CONFIG,
-               "You asked me for the value of an obsolete config option '%s'.",
-               key);
+      log_fn(LOG_PROTOCOL_WARN, LD_CONFIG,
+             "You asked me for the value of an obsolete config option '%s'.",
+             key);
       tor_free(result->key);
       tor_free(result);
       return NULL;



More information about the tor-commits mailing list