commit 405a8d3fb4884d5e5c5f32881a1a810b733a5aad Author: Nick Mathewson <nickm@torproject.org> Date: Thu Nov 26 11:03:35 2015 -0500 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 | 2 +- doc/tor.1.txt | 4 ++-- src/common/compat.c | 13 ++++++++++++- src/common/compat.h | 5 ++++- src/or/config.c | 25 ++++++++++++++----------- src/or/or.h | 2 +- 6 files changed, 34 insertions(+), 17 deletions(-) diff --git a/changes/feature8195 b/changes/feature8195 index 0c366b5..cb81f2e 100644 --- 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 a7bf28b..58313d8 100644 --- 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 6551934..217bc00 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 b245d7d..df95636 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 5060b1b..0b95f95 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 b071303..54c5cb0 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. */