[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[or-cvs] r17200: {tor} Patch from Jacob Appelbaum and me to make User option more r (in tor/trunk: . doc src/common src/or)
Author: sjm217
Date: 2008-11-06 21:06:12 -0500 (Thu, 06 Nov 2008)
New Revision: 17200
Modified:
tor/trunk/ChangeLog
tor/trunk/configure.in
tor/trunk/doc/tor.1.in
tor/trunk/src/common/compat.c
tor/trunk/src/common/compat.h
tor/trunk/src/or/config.c
Log:
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
Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog 2008-11-07 01:50:37 UTC (rev 17199)
+++ tor/trunk/ChangeLog 2008-11-07 02:06:12 UTC (rev 17200)
@@ -5,6 +5,13 @@
exit policy doesn't allow it, we would remember what IP address
the relay said the destination address resolves to, even if it's
an internal IP address. Bugfix on 0.2.0.7-alpha; patch by rovv.
+ - The "User" and "Group" config options did not clear the
+ supplementary group entries for the process. The "User" option
+ has been made more robust, and also now also sets 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.
o Minor features:
- Now NodeFamily and MyFamily config options allow spaces in
Modified: tor/trunk/configure.in
===================================================================
--- tor/trunk/configure.in 2008-11-07 01:50:37 UTC (rev 17199)
+++ tor/trunk/configure.in 2008-11-07 02:06:12 UTC (rev 17200)
@@ -630,6 +630,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/trunk/doc/tor.1.in
===================================================================
--- tor/trunk/doc/tor.1.in 2008-11-07 01:50:37 UTC (rev 17199)
+++ tor/trunk/doc/tor.1.in 2008-11-07 02:06:12 UTC (rev 17200)
@@ -264,10 +264,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),
@@ -350,7 +346,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/trunk/src/common/compat.c
===================================================================
--- tor/trunk/src/common/compat.c 2008-11-07 01:50:37 UTC (rev 17199)
+++ tor/trunk/src/common/compat.c 2008-11-07 02:06:12 UTC (rev 17200)
@@ -920,61 +920,215 @@
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.
*/
int
-switch_id(const char *user, const char *group)
+log_credential_status()
{
+#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 only switch to their
+ * primary group. Return 0 on success. On failure, log and return -1.
+ */
+int
+switch_id(const char *user)
+{
+#ifndef MS_WINDOWS
struct passwd *pw = NULL;
- struct group *gr = NULL;
+ uid_t old_uid;
+ gid_t old_gid;
+
+ tor_assert(user);
+ /* Log the initial credential state */
if (user) {
- pw = getpwnam(user);
- if (pw == NULL) {
- log_warn(LD_CONFIG,"User '%s' not found.", user);
+ if (log_credential_status()) {
return -1;
}
}
+ log_fn(CREDENTIAL_LOG_LEVEL, LD_GENERAL, "Changing user and groups");
- /* 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);
+ /* 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. */
+ if (user) {
+ pw = getpwnam(user);
+ if (pw == NULL) {
+ log_warn(LD_CONFIG, "Error setting configured user: "
+ "'%s' not found.", user);
return -1;
}
+ } else {
+ /* We have no user supplied and so we'll bail out. */
+ log_warn(LD_CONFIG, "Error setting configured user: No user supplied.");
+ return -1;
+ }
- if (setgid(gr->gr_gid) != 0) {
- log_warn(LD_GENERAL,"Error setting to configured GID: %s",
- strerror(errno));
+ /* Properly switch egid,gid,euid,uid here or bail out */
+ if (setgroups(1, &pw->pw_gid)) {
+ log_warn(LD_GENERAL, "Error setting configured groups: %s",
+ strerror(errno));
+ return -1;
+ }
+
+ if (setegid(pw->pw_gid)) {
+ log_warn(LD_GENERAL, "Error setting configured egid: %s",
+ strerror(errno));
+ return -1;
+ }
+
+ if (setgid(pw->pw_gid)) {
+ log_warn(LD_GENERAL, "Error setting configured gid: %s",
+ strerror(errno));
+ return -1;
+ }
+
+ if (setuid(pw->pw_uid)) {
+ log_warn(LD_GENERAL, "Error setting configured uid: %s",
+ strerror(errno));
+ return -1;
+ }
+
+ if (seteuid(pw->pw_uid)){
+ log_warn(LD_GENERAL, "Error setting configured euid: %s",
+ 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");
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");
return -1;
}
}
+#endif
- /* now that the group is switched, we can switch users and lose
- privileges */
+ /* Check what really happened */
if (user) {
- if (setuid(pw->pw_uid) != 0) {
- log_warn(LD_GENERAL,"Error setting UID: %s", strerror(errno));
+ if (log_credential_status()) {
return -1;
}
}
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;
}
Modified: tor/trunk/src/common/compat.h
===================================================================
--- tor/trunk/src/common/compat.h 2008-11-07 01:50:37 UTC (rev 17199)
+++ tor/trunk/src/common/compat.h 2008-11-07 02:06:12 UTC (rev 17200)
@@ -447,7 +447,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/trunk/src/or/config.c
===================================================================
--- tor/trunk/src/or/config.c 2008-11-07 01:50:37 UTC (rev 17199)
+++ tor/trunk/src/or/config.c 2008-11-07 02:06:12 UTC (rev 17200)
@@ -219,7 +219,7 @@
V(GeoIPFile, FILENAME,
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, "1"),
@@ -434,7 +434,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 */
@@ -1084,13 +1083,12 @@
#endif
/* Setuid/setgid as appropriate */
- if (options->User || options->Group) {
+ if (options->User) {
/* XXXX021 We should only do this the first time through, not on
* every setconf. */
- if (switch_id(options->User, options->Group) != 0) {
+ 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;
}
}