mirror of
https://gitlab.torproject.org/tpo/core/tor.git
synced 2024-11-28 14:23:30 +01:00
Refactoring to make parse_bridge_line() unittestable.
- Make parse_bridge_line() return a struct. - Make bridge_add_from_config() accept a struct. - Make string_is_key_value() less hysterical.
This commit is contained in:
George Kadianakis
parent
b5dceab175
commit
266f8cddd8
@ -866,9 +866,10 @@ tor_digest_is_zero(const char *digest)
|
||||
}
|
||||
|
||||
/** Return true if <b>string</b> is a valid '<key>=[<value>]' string.
|
||||
* <value> is optional, to indicate the empty string. */
|
||||
* <value> is optional, to indicate the empty string. Log at logging
|
||||
* <b>severity</b> if something ugly happens. */
|
||||
int
|
||||
string_is_key_value(const char *string)
|
||||
string_is_key_value(int severity, const char *string)
|
||||
{
|
||||
/* position of equal sign in string */
|
||||
const char *equal_sign_pos = NULL;
|
||||
@ -876,19 +877,21 @@ string_is_key_value(const char *string)
|
||||
tor_assert(string);
|
||||
|
||||
if (strlen(string) < 2) { /* "x=" is shortest args string */
|
||||
log_warn(LD_GENERAL, "'%s' is too short to be a k=v value.", escaped(string));
|
||||
tor_log(severity, LD_GENERAL, "'%s' is too short to be a k=v value.",
|
||||
escaped(string));
|
||||
return 0;
|
||||
}
|
||||
|
||||
equal_sign_pos = strchr(string, '=');
|
||||
if (!equal_sign_pos) {
|
||||
log_warn(LD_GENERAL, "'%s' is not a k=v value.", escaped(string));
|
||||
tor_log(severity, LD_GENERAL, "'%s' is not a k=v value.", escaped(string));
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* validate that the '=' is not in the beginning of the string. */
|
||||
if (equal_sign_pos == string) {
|
||||
log_warn(LD_GENERAL, "'%s' is not a valid k=v value.", escaped(string));
|
||||
tor_log(severity, LD_GENERAL, "'%s' is not a valid k=v value.",
|
||||
escaped(string));
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
||||
@ -209,7 +209,7 @@ const char *find_whitespace_eos(const char *s, const char *eos);
|
||||
const char *find_str_at_start_of_line(const char *haystack,
|
||||
const char *needle);
|
||||
int string_is_C_identifier(const char *string);
|
||||
int string_is_key_value(const char *string);
|
||||
int string_is_key_value(int severity, const char *string);
|
||||
|
||||
int tor_mem_is_zero(const char *mem, size_t len);
|
||||
int tor_digest_is_zero(const char *digest);
|
||||
|
||||
133
src/or/config.c
133
src/or/config.c
@ -481,7 +481,6 @@ static int options_transition_affects_descriptor(
|
||||
const or_options_t *old_options, const or_options_t *new_options);
|
||||
static int check_nickname_list(const char *lst, const char *name, char **msg);
|
||||
|
||||
static int parse_bridge_line(const char *line, int validate_only);
|
||||
static int parse_client_transport_line(const char *line, int validate_only);
|
||||
|
||||
static int parse_server_transport_line(const char *line, int validate_only);
|
||||
@ -1293,11 +1292,13 @@ options_act(const or_options_t *old_options)
|
||||
if (options->Bridges) {
|
||||
mark_bridge_list();
|
||||
for (cl = options->Bridges; cl; cl = cl->next) {
|
||||
if (parse_bridge_line(cl->value, 0)<0) {
|
||||
bridge_line_t *bridge_line = parse_bridge_line(cl->value);
|
||||
if (!bridge_line) {
|
||||
log_warn(LD_BUG,
|
||||
"Previously validated Bridge line could not be added!");
|
||||
return -1;
|
||||
}
|
||||
bridge_add_from_config(bridge_line);
|
||||
}
|
||||
sweep_bridge_list();
|
||||
}
|
||||
@ -2966,8 +2967,10 @@ options_validate(or_options_t *old_options, or_options_t *options,
|
||||
REJECT("If you set UseBridges, you must set TunnelDirConns.");
|
||||
|
||||
for (cl = options->Bridges; cl; cl = cl->next) {
|
||||
if (parse_bridge_line(cl->value, 1)<0)
|
||||
REJECT("Bridge line did not parse. See logs for details.");
|
||||
bridge_line_t *bridge_line = parse_bridge_line(cl->value);
|
||||
if (!bridge_line)
|
||||
REJECT("Bridge line did not parse. See logs for details.");
|
||||
bridge_line_free(bridge_line);
|
||||
}
|
||||
|
||||
for (cl = options->ClientTransportPlugin; cl; cl = cl->next) {
|
||||
@ -4038,8 +4041,10 @@ validate_transport_socks_arguments(const smartlist_t *args)
|
||||
tor_assert(smartlist_len(args) > 0);
|
||||
|
||||
SMARTLIST_FOREACH_BEGIN(args, const char *, s) {
|
||||
if (!string_is_key_value(s)) /* arguments should be k=v items */
|
||||
if (!string_is_key_value(LOG_WARN, s)) { /* items should be k=v items */
|
||||
log_warn(LD_CONFIG, "'%s' is not a k=v item.", s);
|
||||
return -1;
|
||||
}
|
||||
} SMARTLIST_FOREACH_END(s);
|
||||
|
||||
socks_string = pt_stringify_socks_args(args);
|
||||
@ -4059,22 +4064,36 @@ validate_transport_socks_arguments(const smartlist_t *args)
|
||||
return 0;
|
||||
}
|
||||
|
||||
/** Deallocate a bridge_line_t structure. */
|
||||
/* private */ void
|
||||
bridge_line_free(bridge_line_t *bridge_line)
|
||||
{
|
||||
if (!bridge_line)
|
||||
return;
|
||||
|
||||
if (bridge_line->socks_args) {
|
||||
SMARTLIST_FOREACH(bridge_line->socks_args, char*, s, tor_free(s));
|
||||
smartlist_free(bridge_line->socks_args);
|
||||
}
|
||||
tor_free(bridge_line->transport_name);
|
||||
tor_free(bridge_line);
|
||||
}
|
||||
|
||||
/** Read the contents of a Bridge line from <b>line</b>. Return 0
|
||||
* if the line is well-formed, and -1 if it isn't. If
|
||||
* <b>validate_only</b> is 0, and the line is well-formed, then add
|
||||
* the bridge described in the line to our internal bridge list. */
|
||||
static int
|
||||
parse_bridge_line(const char *line, int validate_only)
|
||||
* the bridge described in the line to our internal bridge list.
|
||||
*
|
||||
* Bridge line format:
|
||||
* Bridge [transport] IP:PORT [id-fingerprint] [k=v] [k=v] ...
|
||||
*/
|
||||
/* private */ bridge_line_t *
|
||||
parse_bridge_line(const char *line)
|
||||
{
|
||||
smartlist_t *items = NULL;
|
||||
int r;
|
||||
char *addrport=NULL, *fingerprint=NULL;
|
||||
char *transport_name=NULL;
|
||||
char *field=NULL;
|
||||
tor_addr_t addr;
|
||||
uint16_t port = 0;
|
||||
char digest[DIGEST_LEN];
|
||||
smartlist_t *socks_args = NULL;
|
||||
bridge_line_t *bridge_line = tor_malloc_zero(sizeof(bridge_line_t));
|
||||
|
||||
items = smartlist_new();
|
||||
smartlist_split_string(items, line, NULL,
|
||||
@ -4084,47 +4103,49 @@ parse_bridge_line(const char *line, int validate_only)
|
||||
goto err;
|
||||
}
|
||||
|
||||
/* field is either a transport name or addrport */
|
||||
/* first field is either a transport name or addrport */
|
||||
field = smartlist_get(items, 0);
|
||||
smartlist_del_keeporder(items, 0);
|
||||
|
||||
if (!(strstr(field, ".") || strstr(field, ":"))) {
|
||||
/* new-style bridge line */
|
||||
transport_name = field;
|
||||
if (string_is_C_identifier(field)) {
|
||||
/* It's a transport name. */
|
||||
bridge_line->transport_name = field;
|
||||
if (smartlist_len(items) < 1) {
|
||||
log_warn(LD_CONFIG, "Too few items to Bridge line.");
|
||||
goto err;
|
||||
}
|
||||
addrport = smartlist_get(items, 0);
|
||||
addrport = smartlist_get(items, 0); /* Next field is addrport then. */
|
||||
smartlist_del_keeporder(items, 0);
|
||||
} else {
|
||||
addrport = field;
|
||||
}
|
||||
|
||||
if (tor_addr_port_lookup(addrport, &addr, &port)<0) {
|
||||
/* Parse addrport. */
|
||||
if (tor_addr_port_lookup(addrport,
|
||||
&bridge_line->addr, &bridge_line->port)<0) {
|
||||
log_warn(LD_CONFIG, "Error parsing Bridge address '%s'", addrport);
|
||||
goto err;
|
||||
}
|
||||
if (!port) {
|
||||
if (!bridge_line->port) {
|
||||
log_info(LD_CONFIG,
|
||||
"Bridge address '%s' has no port; using default port 443.",
|
||||
addrport);
|
||||
port = 443;
|
||||
bridge_line->port = 443;
|
||||
}
|
||||
|
||||
/* If transports are enabled, next field could be a fingerprint or a
|
||||
socks argument. If transports are disabled, next field should be
|
||||
socks argument. If transports are disabled, next field must be
|
||||
a fingerprint. */
|
||||
if (smartlist_len(items)) {
|
||||
if (transport_name) { /* transports enabled: */
|
||||
if (bridge_line->transport_name) { /* transports enabled: */
|
||||
field = smartlist_get(items, 0);
|
||||
smartlist_del_keeporder(items, 0);
|
||||
|
||||
/* If it's a key=value pair, then it's a SOCKS argument for the
|
||||
transport proxy... */
|
||||
if (string_is_key_value(field)) {
|
||||
socks_args = smartlist_new();
|
||||
smartlist_add(socks_args, field);
|
||||
if (string_is_key_value(LOG_DEBUG, field)) {
|
||||
bridge_line->socks_args = smartlist_new();
|
||||
smartlist_add(bridge_line->socks_args, field);
|
||||
} else { /* ...otherwise, it's the bridge fingerprint. */
|
||||
fingerprint = field;
|
||||
}
|
||||
@ -4134,78 +4155,50 @@ parse_bridge_line(const char *line, int validate_only)
|
||||
}
|
||||
}
|
||||
|
||||
/* Handle fingerprint, if it was provided. */
|
||||
if (fingerprint) {
|
||||
if (strlen(fingerprint) != HEX_DIGEST_LEN) {
|
||||
log_warn(LD_CONFIG, "Key digest for Bridge is wrong length.");
|
||||
goto err;
|
||||
}
|
||||
if (base16_decode(digest, DIGEST_LEN, fingerprint, HEX_DIGEST_LEN)<0) {
|
||||
if (base16_decode(bridge_line->digest, DIGEST_LEN,
|
||||
fingerprint, HEX_DIGEST_LEN)<0) {
|
||||
log_warn(LD_CONFIG, "Unable to decode Bridge key digest.");
|
||||
goto err;
|
||||
}
|
||||
}
|
||||
|
||||
/* If we are using transports, any remaining items in the smartlist
|
||||
must be k=v values. */
|
||||
if (transport_name && smartlist_len(items)) {
|
||||
if (!socks_args)
|
||||
socks_args = smartlist_new();
|
||||
should be k=v values. */
|
||||
if (bridge_line->transport_name && smartlist_len(items)) {
|
||||
if (!bridge_line->socks_args)
|
||||
bridge_line->socks_args = smartlist_new();
|
||||
|
||||
/* append remaining items of 'items' to 'socks_args' */
|
||||
smartlist_add_all(socks_args, items);
|
||||
smartlist_add_all(bridge_line->socks_args, items);
|
||||
smartlist_clear(items);
|
||||
|
||||
tor_assert(smartlist_len(socks_args) > 0);
|
||||
tor_assert(smartlist_len(bridge_line->socks_args) > 0);
|
||||
}
|
||||
|
||||
if (!validate_only) {
|
||||
log_debug(LD_DIR, "Bridge at %s (transport: %s) (%s)",
|
||||
fmt_addrport(&addr, port),
|
||||
transport_name ? transport_name : "no transport",
|
||||
fingerprint ? fingerprint : "no key listed");
|
||||
|
||||
if (socks_args) { /* print socks arguments */
|
||||
int i = 0;
|
||||
|
||||
tor_assert(smartlist_len(socks_args) > 0);
|
||||
|
||||
log_debug(LD_DIR, "Bridge uses %d SOCKS arguments:",
|
||||
smartlist_len(socks_args));
|
||||
SMARTLIST_FOREACH(socks_args, const char *, arg,
|
||||
log_debug(LD_CONFIG, "%d: %s", ++i, arg));
|
||||
}
|
||||
|
||||
bridge_add_from_config(&addr, port,
|
||||
fingerprint ? digest : NULL,
|
||||
transport_name, socks_args);
|
||||
} else {
|
||||
if (socks_args) {
|
||||
if (validate_transport_socks_arguments(socks_args) < 0)
|
||||
goto err;
|
||||
}
|
||||
if (bridge_line->socks_args) {
|
||||
if (validate_transport_socks_arguments(bridge_line->socks_args) < 0)
|
||||
goto err;
|
||||
}
|
||||
|
||||
r = 0;
|
||||
goto done;
|
||||
|
||||
err:
|
||||
r = -1;
|
||||
bridge_line_free(bridge_line);
|
||||
bridge_line = NULL;
|
||||
|
||||
done:
|
||||
SMARTLIST_FOREACH(items, char*, s, tor_free(s));
|
||||
smartlist_free(items);
|
||||
tor_free(addrport);
|
||||
tor_free(transport_name);
|
||||
tor_free(fingerprint);
|
||||
|
||||
/* We only have to free socks_args if we are validating, since
|
||||
otherwise bridge_add_from_config() steals its reference. */
|
||||
if (socks_args && validate_only) {
|
||||
SMARTLIST_FOREACH(socks_args, char*, s, tor_free(s));
|
||||
smartlist_free(socks_args);
|
||||
}
|
||||
|
||||
return r;
|
||||
return bridge_line;
|
||||
}
|
||||
|
||||
/** Read the contents of a ClientTransportPlugin line from
|
||||
|
||||
@ -96,5 +96,19 @@ int addressmap_register_auto(const char *from, const char *to,
|
||||
addressmap_entry_source_t addrmap_source,
|
||||
const char **msg);
|
||||
|
||||
/** Represents the information stored in a torrc Bridge line. */
|
||||
typedef struct bridge_line_t {
|
||||
tor_addr_t addr; /* The IP address of the bridge. */
|
||||
uint16_t port; /* The TCP port of the bridge. */
|
||||
char *transport_name; /* The name of the pluggable transport that
|
||||
should be used to connect to the bridge. */
|
||||
char digest[DIGEST_LEN]; /* The bridge's identity key digest. */
|
||||
smartlist_t *socks_args;; /* SOCKS arguments for the pluggable
|
||||
transport proxy. */
|
||||
} bridge_line_t;
|
||||
|
||||
void bridge_line_free(bridge_line_t *bridge_line);
|
||||
bridge_line_t *parse_bridge_line(const char *line);
|
||||
|
||||
#endif
|
||||
|
||||
|
||||
@ -1633,37 +1633,52 @@ bridge_resolve_conflicts(const tor_addr_t *addr, uint16_t port,
|
||||
} SMARTLIST_FOREACH_END(bridge);
|
||||
}
|
||||
|
||||
/** Remember a new bridge at <b>addr</b>:<b>port</b>. If <b>digest</b>
|
||||
* is set, it tells us the identity key too. If we already had the
|
||||
* bridge in our list, unmark it, and don't actually add anything new.
|
||||
* If <b>transport_name</b> is non-NULL - the bridge is associated with a
|
||||
* pluggable transport - we assign the transport to the bridge.
|
||||
* If <b>transport_name</b> is non-NULL - the bridge is associated
|
||||
* with a pluggable transport - we assign the transport to the bridge.
|
||||
* If <b>socks_args</b> is non-NULL, it's a smartlist carrying
|
||||
* key=value pairs to be passed to the pluggable transports
|
||||
* proxy. This function steals reference of the smartlist. */
|
||||
/** Register the bridge information in <b>bridge_line</b> to the
|
||||
* bridge subsystem. Steals reference of <b>bridge_line</b>. */
|
||||
void
|
||||
bridge_add_from_config(const tor_addr_t *addr, uint16_t port,
|
||||
const char *digest, const char *transport_name,
|
||||
smartlist_t *socks_args)
|
||||
bridge_add_from_config(bridge_line_t *bridge_line)
|
||||
{
|
||||
bridge_info_t *b;
|
||||
|
||||
bridge_resolve_conflicts(addr, port, digest, transport_name);
|
||||
{ /* Log the bridge we are about to register: */
|
||||
log_debug(LD_GENERAL, "Registering bridge at %s (transport: %s) (%s)",
|
||||
fmt_addrport(&bridge_line->addr, bridge_line->port),
|
||||
bridge_line->transport_name ?
|
||||
bridge_line->transport_name : "no transport",
|
||||
tor_digest_is_zero(bridge_line->digest) ?
|
||||
"no key listed" : hex_str(bridge_line->digest, DIGEST_LEN));
|
||||
|
||||
if (bridge_line->socks_args) { /* print socks arguments */
|
||||
int i = 0;
|
||||
|
||||
tor_assert(smartlist_len(bridge_line->socks_args) > 0);
|
||||
|
||||
log_debug(LD_GENERAL, "Bridge uses %d SOCKS arguments:",
|
||||
smartlist_len(bridge_line->socks_args));
|
||||
SMARTLIST_FOREACH(bridge_line->socks_args, const char *, arg,
|
||||
log_debug(LD_CONFIG, "%d: %s", ++i, arg));
|
||||
}
|
||||
}
|
||||
|
||||
bridge_resolve_conflicts(&bridge_line->addr,
|
||||
bridge_line->port,
|
||||
bridge_line->digest,
|
||||
bridge_line->transport_name);
|
||||
|
||||
b = tor_malloc_zero(sizeof(bridge_info_t));
|
||||
tor_addr_copy(&b->addr, addr);
|
||||
b->port = port;
|
||||
if (digest)
|
||||
memcpy(b->identity, digest, DIGEST_LEN);
|
||||
if (transport_name)
|
||||
b->transport_name = tor_strdup(transport_name);
|
||||
tor_addr_copy(&b->addr, &bridge_line->addr);
|
||||
b->port = bridge_line->port;
|
||||
if (bridge_line->digest)
|
||||
memcpy(b->identity, bridge_line->digest, DIGEST_LEN);
|
||||
if (bridge_line->transport_name)
|
||||
b->transport_name = bridge_line->transport_name;
|
||||
b->fetch_status.schedule = DL_SCHED_BRIDGE;
|
||||
b->socks_args = socks_args;
|
||||
b->socks_args = bridge_line->socks_args;
|
||||
if (!bridge_list)
|
||||
bridge_list = smartlist_new();
|
||||
|
||||
tor_free(bridge_line); /* Deallocate bridge_line now. */
|
||||
|
||||
smartlist_add(bridge_list, b);
|
||||
}
|
||||
|
||||
|
||||
@ -97,11 +97,8 @@ int routerinfo_is_a_configured_bridge(const routerinfo_t *ri);
|
||||
int node_is_a_configured_bridge(const node_t *node);
|
||||
void learned_router_identity(const tor_addr_t *addr, uint16_t port,
|
||||
const char *digest);
|
||||
struct smartlist_t;
|
||||
void bridge_add_from_config(const tor_addr_t *addr, uint16_t port,
|
||||
const char *digest,
|
||||
const char *transport_name,
|
||||
struct smartlist_t *socks_args);
|
||||
struct bridge_line_t;
|
||||
void bridge_add_from_config(struct bridge_line_t *bridge_line);
|
||||
void retry_bridge_descriptor_fetch_directly(const char *digest);
|
||||
void fetch_bridge_descriptors(const or_options_t *options, time_t now);
|
||||
void learned_bridge_descriptor(routerinfo_t *ri, int from_cache);
|
||||
|
||||
@ -838,17 +838,17 @@ static void
|
||||
test_util_string_is_key_value(void *ptr)
|
||||
{
|
||||
(void)ptr;
|
||||
test_assert(string_is_key_value("key=value"));
|
||||
test_assert(string_is_key_value("k=v"));
|
||||
test_assert(string_is_key_value("key="));
|
||||
test_assert(string_is_key_value("x="));
|
||||
test_assert(string_is_key_value("xx="));
|
||||
test_assert(!string_is_key_value("=value"));
|
||||
test_assert(!string_is_key_value("=x"));
|
||||
test_assert(!string_is_key_value("="));
|
||||
test_assert(string_is_key_value(LOG_WARN, "key=value"));
|
||||
test_assert(string_is_key_value(LOG_WARN, "k=v"));
|
||||
test_assert(string_is_key_value(LOG_WARN, "key="));
|
||||
test_assert(string_is_key_value(LOG_WARN, "x="));
|
||||
test_assert(string_is_key_value(LOG_WARN, "xx="));
|
||||
test_assert(!string_is_key_value(LOG_WARN, "=value"));
|
||||
test_assert(!string_is_key_value(LOG_WARN, "=x"));
|
||||
test_assert(!string_is_key_value(LOG_WARN, "="));
|
||||
|
||||
/* ??? */
|
||||
/* test_assert(!string_is_key_value("===")); */
|
||||
/* test_assert(!string_is_key_value(LOG_WARN, "===")); */
|
||||
done:
|
||||
;
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user