From c735b60e4ce2d2806d8cc285d7b9b2795f9732b4 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Sat, 13 Sep 2014 16:25:48 +0300 Subject: [PATCH 1/5] New API for policies_parse_exit_policy(). --- src/or/policies.c | 63 ++++++++++++++++++++++++++++++++++++++++------- src/or/policies.h | 14 +++++++++++ src/or/router.c | 6 ++--- 3 files changed, 70 insertions(+), 13 deletions(-) diff --git a/src/or/policies.c b/src/or/policies.c index 535271ba3f..9739fd5d7c 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -62,6 +62,14 @@ static const char *private_nets[] = { NULL }; + +static int policies_parse_exit_policy_internal(config_line_t *cfg, + smartlist_t **dest, + int ipv6_exit, + int rejectprivate, + uint32_t local_address, + int add_default_policy); + /** Replace all "private" entries in *policy with their expanded * equivalents. */ void @@ -423,11 +431,9 @@ validate_addr_policies(const or_options_t *options, char **msg) smartlist_t *addr_policy=NULL; *msg = NULL; - if (policies_parse_exit_policy(options->ExitPolicy, &addr_policy, - options->IPv6Exit, - options->ExitPolicyRejectPrivate, 0, - !options->BridgeRelay)) + if (policies_parse_exit_policy_from_options(options,0,&addr_policy)) { REJECT("Error in ExitPolicy entry."); + } /* The rest of these calls *append* to addr_policy. So don't actually * use the results for anything other than checking if they parse! */ @@ -948,11 +954,12 @@ exit_policy_remove_redundancies(smartlist_t *dest) * the functions used to parse the exit policy from a router descriptor, * see router_add_exit_policy. */ -int -policies_parse_exit_policy(config_line_t *cfg, smartlist_t **dest, - int ipv6_exit, - int rejectprivate, uint32_t local_address, - int add_default_policy) +static int +policies_parse_exit_policy_internal(config_line_t *cfg, smartlist_t **dest, + int ipv6_exit, + int rejectprivate, + uint32_t local_address, + int add_default_policy) { if (!ipv6_exit) { append_exit_policy_string(dest, "reject *6:*"); @@ -978,6 +985,44 @@ policies_parse_exit_policy(config_line_t *cfg, smartlist_t **dest, return 0; } +int +policies_parse_exit_policy(config_line_t *cfg, smartlist_t **dest, + exit_policy_parser_cfg_t options, + uint32_t local_address) +{ + int ipv6_enabled = (options & EXIT_POLICY_IPV6_ENABLED) ? 1 : 0; + int reject_private = (options & EXIT_POLICY_REJECT_PRIVATE) ? 1 : 0; + int add_default = (options & EXIT_POLICY_ADD_DEFAULT) ? 1 : 0; + + return policies_parse_exit_policy_internal(cfg,dest,ipv6_enabled, + reject_private, + local_address, + add_default); +} + +int +policies_parse_exit_policy_from_options(const or_options_t *or_options, + uint32_t local_address, + smartlist_t **result) +{ + exit_policy_parser_cfg_t parser_cfg = 0; + + if (or_options->IPv6Exit) { + parser_cfg |= EXIT_POLICY_IPV6_ENABLED; + } + + if (or_options->ExitPolicyRejectPrivate) { + parser_cfg |= EXIT_POLICY_REJECT_PRIVATE; + } + + if (!or_options->BridgeRelay) { + parser_cfg |= EXIT_POLICY_ADD_DEFAULT; + } + + return policies_parse_exit_policy(or_options->ExitPolicy,result, + parser_cfg,local_address); +} + /** Add "reject *:*" to the end of the policy in *dest, allocating * *dest as needed. */ void diff --git a/src/or/policies.h b/src/or/policies.h index da1feaf02d..0b47b761ec 100644 --- a/src/or/policies.h +++ b/src/or/policies.h @@ -18,6 +18,12 @@ */ #define POLICY_BUF_LEN 72 +#define EXIT_POLICY_IPV6_ENABLED (1 << 0) +#define EXIT_POLICY_REJECT_PRIVATE (1 << 1) +#define EXIT_POLICY_ADD_DEFAULT (1 << 2) + +typedef int exit_policy_parser_cfg_t; + int firewall_is_fascist_or(void); int fascist_firewall_allows_address_or(const tor_addr_t *addr, uint16_t port); int fascist_firewall_allows_or(const routerinfo_t *ri); @@ -42,10 +48,18 @@ MOCK_DECL(addr_policy_result_t, compare_tor_addr_to_addr_policy, addr_policy_result_t compare_tor_addr_to_node_policy(const tor_addr_t *addr, uint16_t port, const node_t *node); +/* int policies_parse_exit_policy(config_line_t *cfg, smartlist_t **dest, int ipv6exit, int rejectprivate, uint32_t local_address, int add_default_policy); +*/ +int policies_parse_exit_policy_from_options(const or_options_t *or_options, + uint32_t local_address, + smartlist_t **result); +int policies_parse_exit_policy(config_line_t *cfg, smartlist_t **dest, + exit_policy_parser_cfg_t options, + uint32_t local_address); void policies_exit_policy_append_reject_star(smartlist_t **dest); void addr_policy_append_reject_addr(smartlist_t **dest, const tor_addr_t *addr); diff --git a/src/or/router.c b/src/or/router.c index 7bcc02ef39..4af8d262f9 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1855,10 +1855,8 @@ router_rebuild_descriptor(int force) /* DNS is screwed up; don't claim to be an exit. */ policies_exit_policy_append_reject_star(&ri->exit_policy); } else { - policies_parse_exit_policy(options->ExitPolicy, &ri->exit_policy, - options->IPv6Exit, - options->ExitPolicyRejectPrivate, - ri->addr, !options->BridgeRelay); + policies_parse_exit_policy_from_options(options,ri->addr, + &ri->exit_policy); } ri->policy_is_reject_star = policy_is_reject_star(ri->exit_policy, AF_INET) && From 0eaf82947d862852e59b17a1763e49d57de03873 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Sat, 13 Sep 2014 19:32:35 +0300 Subject: [PATCH 2/5] Using the new API in unit-test. --- src/test/test_policy.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/test/test_policy.c b/src/test/test_policy.c index 4cdcd034bb..5f044d05b7 100644 --- a/src/test/test_policy.c +++ b/src/test/test_policy.c @@ -47,7 +47,9 @@ test_policy_summary_helper(const char *policy_str, line.value = (char *)policy_str; line.next = NULL; - r = policies_parse_exit_policy(&line, &policy, 1, 0, 0, 1); + r = policies_parse_exit_policy(&line, &policy, + EXIT_POLICY_IPV6_ENABLED | + EXIT_POLICY_ADD_DEFAULT ,0); test_eq(r, 0); summary = policy_summarize(policy, AF_INET); @@ -106,7 +108,11 @@ test_policies_general(void *arg) test_assert(ADDR_POLICY_REJECTED == compare_tor_addr_to_addr_policy(&tar, 2, policy)); - test_assert(0 == policies_parse_exit_policy(NULL, &policy2, 1, 1, 0, 1)); + test_assert(0 == policies_parse_exit_policy(NULL, &policy2, + EXIT_POLICY_IPV6_ENABLED | + EXIT_POLICY_REJECT_PRIVATE | + EXIT_POLICY_ADD_DEFAULT, 0)); + test_assert(policy2); policy3 = smartlist_new(); @@ -193,7 +199,10 @@ test_policies_general(void *arg) line.key = (char*)"foo"; line.value = (char*)"accept *:80,reject private:*,reject *:*"; line.next = NULL; - test_assert(0 == policies_parse_exit_policy(&line, &policy, 1, 0, 0, 1)); + + test_assert(0 == policies_parse_exit_policy(&line,&policy, + EXIT_POLICY_IPV6_ENABLED | + EXIT_POLICY_ADD_DEFAULT,0)); test_assert(policy); //test_streq(policy->string, "accept *:80"); //test_streq(policy->next->string, "reject *:*"); From 2e951f8dda4046d940b9268a8f02fe221098ab58 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Sun, 14 Sep 2014 17:07:05 +0300 Subject: [PATCH 3/5] Whitespace fixes --- src/or/policies.c | 9 ++++----- src/test/test_policy.c | 8 ++++---- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/or/policies.c b/src/or/policies.c index 9739fd5d7c..ffeda1fa4c 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -62,11 +62,10 @@ static const char *private_nets[] = { NULL }; - -static int policies_parse_exit_policy_internal(config_line_t *cfg, +static int policies_parse_exit_policy_internal(config_line_t *cfg, smartlist_t **dest, int ipv6_exit, - int rejectprivate, + int rejectprivate, uint32_t local_address, int add_default_policy); @@ -957,7 +956,7 @@ exit_policy_remove_redundancies(smartlist_t *dest) static int policies_parse_exit_policy_internal(config_line_t *cfg, smartlist_t **dest, int ipv6_exit, - int rejectprivate, + int rejectprivate, uint32_t local_address, int add_default_policy) { @@ -985,7 +984,7 @@ policies_parse_exit_policy_internal(config_line_t *cfg, smartlist_t **dest, return 0; } -int +int policies_parse_exit_policy(config_line_t *cfg, smartlist_t **dest, exit_policy_parser_cfg_t options, uint32_t local_address) diff --git a/src/test/test_policy.c b/src/test/test_policy.c index 5f044d05b7..f1196c255e 100644 --- a/src/test/test_policy.c +++ b/src/test/test_policy.c @@ -47,8 +47,8 @@ test_policy_summary_helper(const char *policy_str, line.value = (char *)policy_str; line.next = NULL; - r = policies_parse_exit_policy(&line, &policy, - EXIT_POLICY_IPV6_ENABLED | + r = policies_parse_exit_policy(&line, &policy, + EXIT_POLICY_IPV6_ENABLED | EXIT_POLICY_ADD_DEFAULT ,0); test_eq(r, 0); summary = policy_summarize(policy, AF_INET); @@ -199,10 +199,10 @@ test_policies_general(void *arg) line.key = (char*)"foo"; line.value = (char*)"accept *:80,reject private:*,reject *:*"; line.next = NULL; - + test_assert(0 == policies_parse_exit_policy(&line,&policy, EXIT_POLICY_IPV6_ENABLED | - EXIT_POLICY_ADD_DEFAULT,0)); + EXIT_POLICY_ADD_DEFAULT,0)); test_assert(policy); //test_streq(policy->string, "accept *:80"); //test_streq(policy->next->string, "reject *:*"); From 45fc0612d3c496c33c33bdfc9d6c0c754a785d9a Mon Sep 17 00:00:00 2001 From: rl1987 Date: Sun, 14 Sep 2014 17:35:13 +0300 Subject: [PATCH 4/5] Adding changes file for 8197. --- changes/bug8197 | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changes/bug8197 diff --git a/changes/bug8197 b/changes/bug8197 new file mode 100644 index 0000000000..b8e467dc38 --- /dev/null +++ b/changes/bug8197 @@ -0,0 +1,6 @@ + o Minor refactoring: + - Reworking API of policies_parse_exit_policy() function to use a + bitmask to represent parsing options instead of a confusing mess + of booleans. Resolves ticket 8197. + - Introducing helper function to parse ExitPolicy in or_options_t + structure. From 80622c0664741e7947ace3972d106d94340aaf7c Mon Sep 17 00:00:00 2001 From: rl1987 Date: Sun, 21 Sep 2014 14:35:48 +0300 Subject: [PATCH 5/5] Writing comments for newly added functions. --- src/or/policies.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/or/policies.c b/src/or/policies.c index ffeda1fa4c..7090eda2c4 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -984,6 +984,18 @@ policies_parse_exit_policy_internal(config_line_t *cfg, smartlist_t **dest, return 0; } +/** Parse exit policy in cfg into dest smartlist. + * + * Add entry that rejects all IPv6 destinations unless + * EXIT_POLICY_IPV6_ENABLED bit is set in options bitmask. + * + * If EXIT_POLICY_REJECT_PRIVATE bit is set in options, + * do add entry that rejects all destinations in private subnetwork + * Tor is running in. + * + * Respectively, if EXIT_POLICY_ADD_DEFAULT bit is set, add + * default exit policy entries to result smartlist. + */ int policies_parse_exit_policy(config_line_t *cfg, smartlist_t **dest, exit_policy_parser_cfg_t options, @@ -999,6 +1011,18 @@ policies_parse_exit_policy(config_line_t *cfg, smartlist_t **dest, add_default); } +/** Parse ExitPolicy member of or_options into result + * smartlist. + * If or_options->IPv6Exit is false, add an entry that + * rejects all IPv6 destinations. + * + * If or_options->ExitPolicyRejectPrivate is true, add entry that + * rejects all destinations in the private subnetwork of machine Tor + * instance is running in. + * + * If or_options->BridgeRelay is false, add entries of default + * Tor exit policy into result smartlist. + */ int policies_parse_exit_policy_from_options(const or_options_t *or_options, uint32_t local_address,