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.

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.

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.

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 */

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);

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 ||
+    if (options->KeepBindCapabilities == 1) {
-        (options->KeepCapabilities == -1 && have_low_ports)) {
+      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) {
+  if (old->KeepBindCapabilities != new_val->KeepBindCapabilities) {
-    *msg = tor_strdup("While Tor is running, changing KeepCapabilities is "
+    *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
+ * consistency and warn as appropriate.  Set *<b>n_low_ports_out</b> to the
- * of sub-1024 ports we will be binding. */
+ * 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())
+    if (options->KeepBindCapabilities == 0 && have_capability_support())
-      extra = ", and you have disabled KeepCapabilities.";
+      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."

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. */