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.

4 files changed, 31 insertions, 14 deletions

diff --git a/src/common/compat.c b/src/common/compat.c
index 655193499e..217bc00315 100644
--- 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 b245d7d1bd..df95636200 100644
--- 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 5060b1b5be..0b95f95d25 100644
--- 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 b07130325f..54c5cb0985 100644
--- 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. */