Update KeepCapabilities based on comments from asn

* The option is now KeepBindCapabilities
* We now warn if the user specifically asked for KeepBindCapabilities
  and we can't deliver.
* The unit tests are willing to start.
* Fewer unused-variable warnings.
* More documentation, fewer misspellings.

diff --git a/changes/feature8195 b/changes/feature8195
index 0c366b595c119094551f8762c7405036e417122b..cb81f2e1d044819c3341480c91764879cd0f4368 100644 (file)
--- a/changes/feature8195
+++ b/changes/feature8195
@@ -3,4 +3,4 @@
       can now retain the capabilitity to bind to low ports.  By default,
       Tor will do this only when it's switching user ID and some low
       ports have been configured.  You can change this behavior with
-      the new option KeepCapabilities.  Closes ticket 8195.
+      the new option KeepBindCapabilities.  Closes ticket 8195.

diff --git a/doc/tor.1.txt b/doc/tor.1.txt
index a7bf28bbd5a326bb14702346fd327520db3aeb64..58313d8a492fc333b10ee8b1f8e2c8cd41ae97de 100644 (file)
--- a/doc/tor.1.txt
+++ b/doc/tor.1.txt
@@ -601,9 +601,9 @@ GENERAL OPTIONS
 [[User]] **User** __UID__::
     On startup, setuid to this user and setgid to their primary group.
 
-[[KeepCapabilities]] **KeepCapabilities** **0**|**1**|**auto**::
+[[KeepBindCapabilities]] **KeepBindCapabilities** **0**|**1**|**auto**::
     On Linux, when we are started as root and we switch our identity using
-    the **User** option, the **KeepCapabilities** option tells us whether to
+    the **User** option, the **KeepBindCapabilities** option tells us whether to
     try to retain our ability to bind to low ports.  If this value is 1, we
     try to keep the capability; if it is 0 we do not; and if it is **auto**,
     we keep the capability only if we are configured to listen on a low port.

diff --git a/src/common/compat.c b/src/common/compat.c
index 655193499ea21c073538db892cad67583afde17a..217bc0031571e2b5efadb44a57d546a2adb96953 100644 (file)
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -1997,8 +1997,11 @@ drop_capabilities(int pre_setuid)
 /** 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.
  *
- * If SWITCH_ID_KEEP_BINDLOW is set in 'flags', try to use the capabilitity
+ * If SWITCH_ID_KEEP_BINDLOW is set in 'flags', try to use the capability
  * system to retain the abilitity to bind low ports.
+ *
+ * If SWITCH_ID_WARN_IF_NO_CAPS is set in flags, also warn if we have
+ * don't have capability support.
  */
 int
 switch_id(const char *user, const unsigned flags)
@@ -2009,6 +2012,7 @@ switch_id(const char *user, const unsigned flags)
   gid_t old_gid;
   static int have_already_switched_id = 0;
   const int keep_bindlow = !!(flags & SWITCH_ID_KEEP_BINDLOW);
+  const int warn_if_no_caps = !!(flags & SWITCH_ID_WARN_IF_NO_CAPS);
 
   tor_assert(user);
 
@@ -2033,10 +2037,17 @@ switch_id(const char *user, const unsigned flags)
   }
 
 #ifdef HAVE_LINUX_CAPABILITIES
+  (void) warn_if_no_caps;
   if (keep_bindlow) {
     if (drop_capabilities(1))
       return -1;
   }
+#else
+  (void) keep_bindlow;
+  if (warn_if_no_caps) {
+    log_warn(LD_CONFIG, "KeepBindCapabilities set, but no capability support "
+             "on this system.");
+  }
 #endif
 
   /* Properly switch egid,gid,euid,uid here or bail out */

diff --git a/src/common/compat.h b/src/common/compat.h
index b245d7d1bdd3eb150124050e3463b85fd697cd57..df9563620072216cd7e2dbbf28656c793bed4b35 100644 (file)
--- a/src/common/compat.h
+++ b/src/common/compat.h
@@ -632,7 +632,10 @@ int tor_disable_debugger_attach(void);
 
 int have_capability_support(void);
 
-#define SWITCH_ID_KEEP_BINDLOW 1
+/** Flag for switch_id; see switch_id() for documentation */
+#define SWITCH_ID_KEEP_BINDLOW    (1<<0)
+/** Flag for switch_id; see switch_id() for documentation */
+#define SWITCH_ID_WARN_IF_NO_CAPS (1<<1)
 int switch_id(const char *user, unsigned flags);
 #ifdef HAVE_PWD_H
 char *get_user_homedir(const char *username);

diff --git a/src/or/config.c b/src/or/config.c
index 5060b1b5be6a1b86febad439a0d277c9339de1c5..0b95f95d2556bb78b8d0084a2805f53aacdaa78b 100644 (file)
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -308,7 +308,7 @@ static config_var_t option_vars_[] = {
   V(Socks5ProxyUsername,         STRING,   NULL),
   V(Socks5ProxyPassword,         STRING,   NULL),
   V(KeepalivePeriod,             INTERVAL, "5 minutes"),
-  V(KeepCapabilities,            AUTOBOOL, "auto"),
+  V(KeepBindCapabilities,            AUTOBOOL, "auto"),
   VAR("Log",                     LINELIST, Logs,             NULL),
   V(LogMessageDomains,           BOOL,     "0"),
   V(LogTimeGranularity,          MSEC_INTERVAL, "1 second"),
@@ -1183,11 +1183,14 @@ options_act_reversible(const or_options_t *old_options, char **msg)
   }
 
   /* Setuid/setgid as appropriate */
-  tor_assert(have_low_ports != -1);
   if (options->User) {
+    tor_assert(have_low_ports != -1);
     unsigned switch_id_flags = 0;
-    if (options->KeepCapabilities == 1 ||
-        (options->KeepCapabilities == -1 && have_low_ports)) {
+    if (options->KeepBindCapabilities == 1) {
+      switch_id_flags |= SWITCH_ID_KEEP_BINDLOW;
+      switch_id_flags |= SWITCH_ID_WARN_IF_NO_CAPS;
+    }
+    if (options->KeepBindCapabilities == -1 && have_low_ports) {
       switch_id_flags |= SWITCH_ID_KEEP_BINDLOW;
     }
     if (switch_id(options->User, switch_id_flags) != 0) {
@@ -4008,8 +4011,8 @@ options_transition_allowed(const or_options_t *old,
     return -1;
   }
 
-  if (old->KeepCapabilities != new_val->KeepCapabilities) {
-    *msg = tor_strdup("While Tor is running, changing KeepCapabilities is "
+  if (old->KeepBindCapabilities != new_val->KeepBindCapabilities) {
+    *msg = tor_strdup("While Tor is running, changing KeepBindCapabilities is "
                       "not allowed.");
     return -1;
   }
@@ -6612,8 +6615,8 @@ parse_ports(or_options_t *options, int validate_only,
 }
 
 /** Given a list of <b>port_cfg_t</b> in <b>ports</b>, check them for internal
- * consistency and warn as appropriate.  Set *<b>n_low_port</b> to the number
- * of sub-1024 ports we will be binding. */
+ * consistency and warn as appropriate.  Set *<b>n_low_ports_out</b> to the
+ * number of sub-1024 ports we will be binding. */
 static int
 check_server_ports(const smartlist_t *ports,
                    const or_options_t *options,
@@ -6681,10 +6684,10 @@ check_server_ports(const smartlist_t *ports,
   }
 
   if (n_low_port && options->AccountingMax &&
-      (!have_capability_support() || options->KeepCapabilities == 0)) {
+      (!have_capability_support() || options->KeepBindCapabilities == 0)) {
     const char *extra = "";
-    if (options->KeepCapabilities == 0 && have_capability_support())
-      extra = ", and you have disabled KeepCapabilities.";
+    if (options->KeepBindCapabilities == 0 && have_capability_support())
+      extra = ", and you have disabled KeepBindCapabilities.";
     log_warn(LD_CONFIG,
           "You have set AccountingMax to use hibernation. You have also "
           "chosen a low DirPort or OrPort%s."

diff --git a/src/or/or.h b/src/or/or.h
index b07130325f5e1213d14135cd65f6ae1afb04c470..54c5cb09856fce72dd58e73a9e3987e79b21f383 100644 (file)
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -4319,7 +4319,7 @@ typedef struct {
   char *master_key_fname;
 
   /** Autobool: Do we try to retain capabilities if we can? */
-  int KeepCapabilities;
+  int KeepBindCapabilities;
 } or_options_t;
 
 /** Persistent state for an onion router, as saved to disk. */