[tor-commits] [tor/master] Get all default flags from port_cfg_new()
nickm at torproject.org
nickm at torproject.org
Mon Mar 23 13:07:01 UTC 2020
commit 1a9cbc5bb4c85132d54e92d1ac583a2a0a9447ae
Author: MrSquanchee <usuraj35 at gmail.com>
Date: Thu Mar 12 12:56:49 2020 +0530
Get all default flags from port_cfg_new()
Now port_cfg_new() returns all default flags and
port_parse_config() acts on defaults returned by port_cfg_new()
that is uses the default port_cfg_t object returned by port_cfg_new()
and modifies them later according to the port specifications in
configuration files
Might close tor#32994.
---
changes/ticket32994 | 8 +++
src/app/config/config.c | 181 +++++++++++++++++++++++-------------------------
2 files changed, 94 insertions(+), 95 deletions(-)
diff --git a/changes/ticket32994 b/changes/ticket32994
new file mode 100644
index 000000000..7d3eb6cf8
--- /dev/null
+++ b/changes/ticket32994
@@ -0,0 +1,8 @@
+ o Code simplification and refactoring:
+ - port_new_cfg() returns default object with reasonable values.
+ - port_parse_config() uses this default object instead of
+ temporary variables.
+ - Files changed -> src/app/config/config.c
+ - Functions changed -> port_new_cfg() and port_parse_config()
+ - Closes ticket 32994.
+ - Patch by MrSquanchee
diff --git a/src/app/config/config.c b/src/app/config/config.c
index ad664873e..96ce215c0 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -5886,12 +5886,18 @@ port_cfg_new(size_t namelen)
{
tor_assert(namelen <= SIZE_T_CEILING - sizeof(port_cfg_t) - 1);
port_cfg_t *cfg = tor_malloc_zero(sizeof(port_cfg_t) + namelen + 1);
+
+ /* entry_cfg flags */
cfg->entry_cfg.ipv4_traffic = 1;
cfg->entry_cfg.ipv6_traffic = 1;
cfg->entry_cfg.prefer_ipv6 = 1;
cfg->entry_cfg.dns_request = 1;
cfg->entry_cfg.onion_traffic = 1;
cfg->entry_cfg.prefer_ipv6_virtaddr = 1;
+ cfg->entry_cfg.session_group = SESSION_GROUP_UNSET;
+ cfg->entry_cfg.isolation_flags = ISO_DEFAULT;
+
+ /* Other flags default to 0 due to tor_malloc_zero */
return cfg;
}
@@ -6095,11 +6101,12 @@ port_parse_config(smartlist_t *out,
const unsigned is_unix_socket = flags & CL_PORT_IS_UNIXSOCKET;
int got_zero_port=0, got_nonzero_port=0;
char *unix_socket_path = NULL;
+ port_cfg_t *cfg = NULL;
/* If there's no FooPort, then maybe make a default one. */
if (! ports) {
if (defaultport && defaultaddr && out) {
- port_cfg_t *cfg = port_cfg_new(is_unix_socket ? strlen(defaultaddr) : 0);
+ cfg = port_cfg_new(is_unix_socket ? strlen(defaultaddr) : 0);
cfg->type = listener_type;
if (is_unix_socket) {
tor_addr_make_unspec(&cfg->addr);
@@ -6109,8 +6116,6 @@ port_parse_config(smartlist_t *out,
cfg->port = defaultport;
tor_addr_parse(&cfg->addr, defaultaddr);
}
- cfg->entry_cfg.session_group = SESSION_GROUP_UNSET;
- cfg->entry_cfg.isolation_flags = ISO_DEFAULT;
smartlist_add(out, cfg);
}
return 0;
@@ -6124,28 +6129,12 @@ port_parse_config(smartlist_t *out,
for (; ports; ports = ports->next) {
tor_addr_t addr;
tor_addr_make_unspec(&addr);
-
- int port;
- int sessiongroup = SESSION_GROUP_UNSET;
- unsigned isolation = ISO_DEFAULT;
- int prefer_no_auth = 0;
- int socks_iso_keep_alive = 0;
-
+ int port, ok,
+ has_used_unix_socket_only_option = 0,
+ is_unix_tagged_addr = 0;
uint16_t ptmp=0;
- int ok;
- /* This must be kept in sync with port_cfg_new's defaults */
- int no_listen = 0, no_advertise = 0, all_addrs = 0,
- bind_ipv4_only = 0, bind_ipv6_only = 0,
- ipv4_traffic = 1, ipv6_traffic = 1, prefer_ipv6 = 1, dns_request = 1,
- onion_traffic = 1,
- cache_ipv4 = 0, use_cached_ipv4 = 0,
- cache_ipv6 = 0, use_cached_ipv6 = 0,
- prefer_ipv6_automap = 1, world_writable = 0, group_writable = 0,
- relax_dirmode_check = 0,
- has_used_unix_socket_only_option = 0, extended_errors = 0;
-
- int is_unix_tagged_addr = 0;
const char *rest_of_line = NULL;
+
if (port_cfg_line_extract_addrport(ports->value,
&addrport, &is_unix_tagged_addr, &rest_of_line)<0) {
log_warn(LD_CONFIG, "Invalid %sPort line with unparsable address",
@@ -6221,17 +6210,20 @@ port_parse_config(smartlist_t *out,
}
}
+ /* Default port_cfg_t object initialization */
+ cfg = port_cfg_new(unix_socket_path ? strlen(unix_socket_path) : 0);
+
if (unix_socket_path && default_to_group_writable)
- group_writable = 1;
+ cfg->is_group_writable = 1;
/* Now parse the rest of the options, if any. */
if (use_server_options) {
/* This is a server port; parse advertising options */
SMARTLIST_FOREACH_BEGIN(elts, char *, elt) {
if (!strcasecmp(elt, "NoAdvertise")) {
- no_advertise = 1;
+ cfg->server_cfg.no_advertise = 1;
} else if (!strcasecmp(elt, "NoListen")) {
- no_listen = 1;
+ cfg->server_cfg.no_listen = 1;
#if 0
/* not implemented yet. */
} else if (!strcasecmp(elt, "AllAddrs")) {
@@ -6239,33 +6231,36 @@ port_parse_config(smartlist_t *out,
all_addrs = 1;
#endif /* 0 */
} else if (!strcasecmp(elt, "IPv4Only")) {
- bind_ipv4_only = 1;
+ cfg->server_cfg.bind_ipv4_only = 1;
} else if (!strcasecmp(elt, "IPv6Only")) {
- bind_ipv6_only = 1;
+ cfg->server_cfg.bind_ipv6_only = 1;
} else {
log_warn(LD_CONFIG, "Unrecognized %sPort option '%s'",
portname, escaped(elt));
}
} SMARTLIST_FOREACH_END(elt);
- if (no_advertise && no_listen) {
+ if (cfg->server_cfg.no_advertise && cfg->server_cfg.no_listen) {
log_warn(LD_CONFIG, "Tried to set both NoListen and NoAdvertise "
"on %sPort line '%s'",
portname, escaped(ports->value));
goto err;
}
- if (bind_ipv4_only && bind_ipv6_only) {
+ if (cfg->server_cfg.bind_ipv4_only &&
+ cfg->server_cfg.bind_ipv6_only) {
log_warn(LD_CONFIG, "Tried to set both IPv4Only and IPv6Only "
"on %sPort line '%s'",
portname, escaped(ports->value));
goto err;
}
- if (bind_ipv4_only && tor_addr_family(&addr) != AF_INET) {
+ if (cfg->server_cfg.bind_ipv4_only &&
+ tor_addr_family(&addr) != AF_INET) {
log_warn(LD_CONFIG, "Could not interpret %sPort address as IPv4",
portname);
goto err;
}
- if (bind_ipv6_only && tor_addr_family(&addr) != AF_INET6) {
+ if (cfg->server_cfg.bind_ipv6_only &&
+ tor_addr_family(&addr) != AF_INET6) {
log_warn(LD_CONFIG, "Could not interpret %sPort address as IPv6",
portname);
goto err;
@@ -6284,12 +6279,12 @@ port_parse_config(smartlist_t *out,
portname, escaped(elt));
goto err;
}
- if (sessiongroup >= 0) {
+ if (cfg->entry_cfg.session_group >= 0) {
log_warn(LD_CONFIG, "Multiple SessionGroup options on %sPort",
portname);
goto err;
}
- sessiongroup = group;
+ cfg->entry_cfg.session_group = group;
continue;
}
@@ -6299,15 +6294,15 @@ port_parse_config(smartlist_t *out,
}
if (!strcasecmp(elt, "GroupWritable")) {
- group_writable = !no;
+ cfg->is_group_writable = !no;
has_used_unix_socket_only_option = 1;
continue;
} else if (!strcasecmp(elt, "WorldWritable")) {
- world_writable = !no;
+ cfg->is_world_writable = !no;
has_used_unix_socket_only_option = 1;
continue;
} else if (!strcasecmp(elt, "RelaxDirModeCheck")) {
- relax_dirmode_check = !no;
+ cfg->relax_dirmode_check = !no;
has_used_unix_socket_only_option = 1;
continue;
}
@@ -6320,19 +6315,19 @@ port_parse_config(smartlist_t *out,
if (takes_hostnames) {
if (!strcasecmp(elt, "IPv4Traffic")) {
- ipv4_traffic = ! no;
+ cfg->entry_cfg.ipv4_traffic = ! no;
continue;
} else if (!strcasecmp(elt, "IPv6Traffic")) {
- ipv6_traffic = ! no;
+ cfg->entry_cfg.ipv6_traffic = ! no;
continue;
} else if (!strcasecmp(elt, "PreferIPv6")) {
- prefer_ipv6 = ! no;
+ cfg->entry_cfg.prefer_ipv6 = ! no;
continue;
} else if (!strcasecmp(elt, "DNSRequest")) {
- dns_request = ! no;
+ cfg->entry_cfg.dns_request = ! no;
continue;
} else if (!strcasecmp(elt, "OnionTraffic")) {
- onion_traffic = ! no;
+ cfg->entry_cfg.onion_traffic = ! no;
continue;
} else if (!strcasecmp(elt, "OnionTrafficOnly")) {
/* Only connect to .onion addresses. Equivalent to
@@ -6343,46 +6338,50 @@ port_parse_config(smartlist_t *out,
"DNSRequest, IPv4Traffic, and/or IPv6Traffic instead.",
portname, escaped(elt));
} else {
- ipv4_traffic = ipv6_traffic = dns_request = 0;
+ cfg->entry_cfg.ipv4_traffic = 0;
+ cfg->entry_cfg.ipv6_traffic = 0;
+ cfg->entry_cfg.dns_request = 0;
}
continue;
}
}
if (!strcasecmp(elt, "CacheIPv4DNS")) {
warn_client_dns_cache(elt, no); // since 0.2.9.2-alpha
- cache_ipv4 = ! no;
+ cfg->entry_cfg.cache_ipv4_answers = ! no;
continue;
} else if (!strcasecmp(elt, "CacheIPv6DNS")) {
warn_client_dns_cache(elt, no); // since 0.2.9.2-alpha
- cache_ipv6 = ! no;
+ cfg->entry_cfg.cache_ipv6_answers = ! no;
continue;
} else if (!strcasecmp(elt, "CacheDNS")) {
warn_client_dns_cache(elt, no); // since 0.2.9.2-alpha
- cache_ipv4 = cache_ipv6 = ! no;
+ cfg->entry_cfg.cache_ipv4_answers = ! no;
+ cfg->entry_cfg.cache_ipv6_answers = ! no;
continue;
} else if (!strcasecmp(elt, "UseIPv4Cache")) {
warn_client_dns_cache(elt, no); // since 0.2.9.2-alpha
- use_cached_ipv4 = ! no;
+ cfg->entry_cfg.use_cached_ipv4_answers = ! no;
continue;
} else if (!strcasecmp(elt, "UseIPv6Cache")) {
warn_client_dns_cache(elt, no); // since 0.2.9.2-alpha
- use_cached_ipv6 = ! no;
+ cfg->entry_cfg.use_cached_ipv6_answers = ! no;
continue;
} else if (!strcasecmp(elt, "UseDNSCache")) {
warn_client_dns_cache(elt, no); // since 0.2.9.2-alpha
- use_cached_ipv4 = use_cached_ipv6 = ! no;
+ cfg->entry_cfg.use_cached_ipv4_answers = ! no;
+ cfg->entry_cfg.use_cached_ipv6_answers = ! no;
continue;
} else if (!strcasecmp(elt, "PreferIPv6Automap")) {
- prefer_ipv6_automap = ! no;
+ cfg->entry_cfg.prefer_ipv6_virtaddr = ! no;
continue;
} else if (!strcasecmp(elt, "PreferSOCKSNoAuth")) {
- prefer_no_auth = ! no;
+ cfg->entry_cfg.socks_prefer_no_auth = ! no;
continue;
} else if (!strcasecmp(elt, "KeepAliveIsolateSOCKSAuth")) {
- socks_iso_keep_alive = ! no;
+ cfg->entry_cfg.socks_iso_keep_alive = ! no;
continue;
} else if (!strcasecmp(elt, "ExtendedErrors")) {
- extended_errors = ! no;
+ cfg->entry_cfg.extended_socks5_codes = ! no;
continue;
}
@@ -6405,9 +6404,9 @@ port_parse_config(smartlist_t *out,
}
if (no) {
- isolation &= ~isoflag;
+ cfg->entry_cfg.isolation_flags &= ~isoflag;
} else {
- isolation |= isoflag;
+ cfg->entry_cfg.isolation_flags |= isoflag;
}
} SMARTLIST_FOREACH_END(elt);
}
@@ -6417,51 +6416,51 @@ port_parse_config(smartlist_t *out,
else
got_zero_port = 1;
- if (dns_request == 0 && listener_type == CONN_TYPE_AP_DNS_LISTENER) {
+ if (cfg->entry_cfg.dns_request == 0 &&
+ listener_type == CONN_TYPE_AP_DNS_LISTENER) {
log_warn(LD_CONFIG, "You have a %sPort entry with DNS disabled; that "
"won't work.", portname);
goto err;
}
-
- if (ipv4_traffic == 0 && ipv6_traffic == 0 && onion_traffic == 0
- && listener_type != CONN_TYPE_AP_DNS_LISTENER) {
+ if (cfg->entry_cfg.ipv4_traffic == 0 &&
+ cfg->entry_cfg.ipv6_traffic == 0 &&
+ cfg->entry_cfg.onion_traffic == 0 &&
+ listener_type != CONN_TYPE_AP_DNS_LISTENER) {
log_warn(LD_CONFIG, "You have a %sPort entry with all of IPv4 and "
"IPv6 and .onion disabled; that won't work.", portname);
goto err;
}
-
- if (dns_request == 1 && ipv4_traffic == 0 && ipv6_traffic == 0
- && listener_type != CONN_TYPE_AP_DNS_LISTENER) {
+ if (cfg->entry_cfg.dns_request == 1 &&
+ cfg->entry_cfg.ipv4_traffic == 0 &&
+ cfg->entry_cfg.ipv6_traffic == 0 &&
+ listener_type != CONN_TYPE_AP_DNS_LISTENER) {
log_warn(LD_CONFIG, "You have a %sPort entry with DNSRequest enabled, "
"but IPv4 and IPv6 disabled; DNS-based sites won't work.",
portname);
goto err;
}
-
if ( has_used_unix_socket_only_option && ! unix_socket_path) {
log_warn(LD_CONFIG, "You have a %sPort entry with GroupWritable, "
"WorldWritable, or RelaxDirModeCheck, but it is not a "
"unix socket.", portname);
goto err;
}
-
- if (!(isolation & ISO_SOCKSAUTH) && socks_iso_keep_alive) {
+ if (!(cfg->entry_cfg.isolation_flags & ISO_SOCKSAUTH) &&
+ cfg->entry_cfg.socks_iso_keep_alive) {
log_warn(LD_CONFIG, "You have a %sPort entry with both "
"NoIsolateSOCKSAuth and KeepAliveIsolateSOCKSAuth set.",
portname);
goto err;
}
-
- if (unix_socket_path && (isolation & ISO_CLIENTADDR)) {
+ if (unix_socket_path &&
+ (cfg->entry_cfg.isolation_flags & ISO_CLIENTADDR)) {
/* `IsolateClientAddr` is nonsensical in the context of AF_LOCAL.
* just silently remove the isolation flag.
*/
- isolation &= ~ISO_CLIENTADDR;
+ cfg->entry_cfg.isolation_flags &= ~ISO_CLIENTADDR;
}
-
if (out && port) {
size_t namelen = unix_socket_path ? strlen(unix_socket_path) : 0;
- port_cfg_t *cfg = port_cfg_new(namelen);
if (unix_socket_path) {
tor_addr_make_unspec(&cfg->addr);
memcpy(cfg->unix_addr, unix_socket_path, namelen + 1);
@@ -6472,33 +6471,13 @@ port_parse_config(smartlist_t *out,
cfg->port = port;
}
cfg->type = listener_type;
- cfg->is_world_writable = world_writable;
- cfg->is_group_writable = group_writable;
- cfg->relax_dirmode_check = relax_dirmode_check;
- cfg->entry_cfg.isolation_flags = isolation;
- cfg->entry_cfg.session_group = sessiongroup;
- cfg->server_cfg.no_advertise = no_advertise;
- cfg->server_cfg.no_listen = no_listen;
- cfg->server_cfg.all_addrs = all_addrs;
- cfg->server_cfg.bind_ipv4_only = bind_ipv4_only;
- cfg->server_cfg.bind_ipv6_only = bind_ipv6_only;
- cfg->entry_cfg.ipv4_traffic = ipv4_traffic;
- cfg->entry_cfg.ipv6_traffic = ipv6_traffic;
- cfg->entry_cfg.prefer_ipv6 = prefer_ipv6;
- cfg->entry_cfg.dns_request = dns_request;
- cfg->entry_cfg.onion_traffic = onion_traffic;
- cfg->entry_cfg.cache_ipv4_answers = cache_ipv4;
- cfg->entry_cfg.cache_ipv6_answers = cache_ipv6;
- cfg->entry_cfg.use_cached_ipv4_answers = use_cached_ipv4;
- cfg->entry_cfg.use_cached_ipv6_answers = use_cached_ipv6;
- cfg->entry_cfg.prefer_ipv6_virtaddr = prefer_ipv6_automap;
- cfg->entry_cfg.socks_prefer_no_auth = prefer_no_auth;
- if (! (isolation & ISO_SOCKSAUTH))
+ if (! (cfg->entry_cfg.isolation_flags & ISO_SOCKSAUTH))
cfg->entry_cfg.socks_prefer_no_auth = 1;
- cfg->entry_cfg.socks_iso_keep_alive = socks_iso_keep_alive;
- cfg->entry_cfg.extended_socks5_codes = extended_errors;
-
smartlist_add(out, cfg);
+ /* out owns cfg now, don't re-use or free it */
+ cfg = NULL;
+ } else {
+ tor_free(cfg);
}
SMARTLIST_FOREACH(elts, char *, cp, tor_free(cp));
smartlist_clear(elts);
@@ -6524,10 +6503,22 @@ port_parse_config(smartlist_t *out,
retval = 0;
err:
+ /* There are two ways we can error out:
+ * 1. part way through the loop: cfg needs to be freed;
+ * 2. ending the loop normally: cfg is always NULL.
+ * In this case, cfg has either been:
+ * - added to out, then set to NULL, or
+ * - freed and set to NULL (because out is NULL, or port is 0).
+ */
+ tor_free(cfg);
+
+ /* Free the other variables from the loop.
+ * elts is always non-NULL here, but it may or may not be empty. */
SMARTLIST_FOREACH(elts, char *, cp, tor_free(cp));
smartlist_free(elts);
tor_free(unix_socket_path);
tor_free(addrport);
+
return retval;
}
More information about the tor-commits
mailing list