]> git.ipfire.org Git - thirdparty/tor.git/commitdiff
Backport of changesets 17200, 17201, 17203-17206, 17228, 17232, 17236: Patch from...
authorSteven Murdoch <Steven.Murdoch@cl.cam.ac.uk>
Wed, 12 Nov 2008 01:10:21 +0000 (01:10 +0000)
committerSteven Murdoch <Steven.Murdoch@cl.cam.ac.uk>
Wed, 12 Nov 2008 01:10:21 +0000 (01:10 +0000)
svn:r17255

ChangeLog
configure.in
contrib/linux-tor-prio.sh
contrib/rc.subr
contrib/tor.sh.in
contrib/torctl.in
doc/tor.1.in
src/common/compat.c
src/common/compat.h
src/or/config.c

index 41b4432afa50dcb297174cfe85034f62275bd567..8fb38da6fe4bba1cf60f9298811cef3a70a5ab1c 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -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.
index ccf1005483d20934c30012bfc29344b890b9e019..3e556252b49064430d0f9fe78010082bdae2ced6 100644 (file)
@@ -616,6 +616,9 @@ syslog_facility="$withval", syslog_facility="LOG_DAEMON")
 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,
index 0ebb47564a1b45536bd88c2688af823b4a65540c..d7481668cafe0a4629bac72a085dab1d31515e88 100644 (file)
@@ -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)
 
index 8852b60466de838765ff5933bfa517b3998b21f2..117ae71d472fa76ffe9fa047052a24dc4a9d7c5f 100644 (file)
@@ -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 @@ load_rc_config ${name}
 : ${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 @@ load_rc_config ${name}
 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"
 
index 362a455732add844524e013a4544a0d691902f0b..e169761a628bdd5638d55ecc569b9f05900f13e0 100644 (file)
@@ -31,8 +31,6 @@ TORCTL=@BINDIR@/torctl
 # torctl will use these environment variables
 TORUSER=@TORUSER@
 export TORUSER
-TORGROUP=@TORGROUP@
-export TORGROUP
 
 if [ -x /bin/su ] ; then
     SUPROG=/bin/su
index 4136bd9434e16e4e592fb2f98ba0f9df1710d91c..4cc137da46d6c724ed352adad1a40642cfaedecf 100644 (file)
@@ -41,22 +41,18 @@ TORDATA="@LOCALSTATEDIR@/lib/tor"
 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.
index 54b48558421507738ba0edbc20fa71016ee22dab..14996b418b00c00c0cc06df37026879dbd160531 100644 (file)
@@ -259,10 +259,6 @@ script to enumerate Tor nodes that exit to certain addresses.
 (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 @@ about what sites a user might have visited. (Default: 1)
 .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
index c7bd0fb56461374cde7c47ddc1d3e64eb2ba5532..cfc73ff3956011fd2ddea43203822df9787dbad5 100644 (file)
@@ -871,62 +871,220 @@ set_max_file_descriptors(rlim_t limit, int *max_out)
   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);
-      return -1;
-    }
+  /* 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 (setgid(gr->gr_gid) != 0) {
-      log_warn(LD_GENERAL,"Error setting to configured GID: %s",
-               strerror(errno));
+  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;
     }
-  } else if (user) {
-    if (setgid(pw->pw_gid) != 0) {
-      log_warn(LD_GENERAL,"Error setting to user 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;
     }
   }
+#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
index ea66093568703ef9724c80300fdf59eca43bd2c3..8cf7302595249ed8ee217b6b56686a516f743697 100644 (file)
@@ -463,7 +463,7 @@ void set_uint32(char *cp, uint32_t v) ATTR_NONNULL((1));
 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
index 5dc5f53aa22866e54ef9779a4ed08535448bfac5..71bab432e23be27d1aea2666b926b973d460fb61 100644 (file)
@@ -204,7 +204,7 @@ static config_var_t _option_vars[] = {
   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 @@ static config_var_description_t options_description[] = {
   /* { "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 @@ options_act_reversible(or_options_t *old_options, char **msg)
 #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 @@ get_assigned_option(config_format_t *fmt, or_options_t *options,
         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;