commit 365ca3ca0f5d9d391a2156436d1a7c620906e23a Author: teor <teor2345@gmail.com> Date: Tue Sep 13 17:20:46 2016 +1000 Refactor Single Onion code to improve consistency * Check consistency between the two single onion torrc options * Use the more relevant option each time we check for single onion mode * Clarify log messages * Clarify comments * Otherwise, no behaviour change --- src/or/circuitstats.c | 2 +- src/or/config.c | 72 ++++++++++++++++++++++++------------------------- src/or/control.c | 21 ++++++++------- src/or/or.h | 7 +++-- src/or/rendcommon.c | 8 +++--- src/or/rendservice.c | 56 +++++++++++++++++++++++++------------- src/test/test_options.c | 8 +++--- 7 files changed, 96 insertions(+), 78 deletions(-) diff --git a/src/or/circuitstats.c b/src/or/circuitstats.c index 296f852..3d64113 100644 --- a/src/or/circuitstats.c +++ b/src/or/circuitstats.c @@ -104,7 +104,7 @@ circuit_build_times_disabled(void) int config_disabled = !options->LearnCircuitBuildTimeout; int dirauth_disabled = options->AuthoritativeDir; int state_disabled = did_last_state_file_write_fail() ? 1 : 0; - /* LearnCircuitBuildTimeout and Tor2webMode/OnionServiceSingleHopMode are + /* LearnCircuitBuildTimeout and Tor2web/Single Onion Services are * incompatible in two ways: * * - LearnCircuitBuildTimeout results in a low CBT, which diff --git a/src/or/config.c b/src/or/config.c index 48f1ab9..6d168c4 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1730,17 +1730,17 @@ options_act(const or_options_t *old_options) * poisoning code checks for existing keys, and refuses to modify their * directories. */ - /* If we use the insecure OnionServiceSingleHopMode, make sure we poison any + /* If we use non-anonymous single onion services, make sure we poison any new hidden service directories, so that we never accidentally launch the non-anonymous hidden services thinking they are anonymous. */ - if (running_tor && rend_service_allow_non_anonymous_connection(options)) { + if (running_tor && rend_service_non_anonymous_mode_enabled(options)) { if (options->RendConfigLines && !num_rend_services()) { log_warn(LD_BUG,"Error: hidden services configured, but not parsed."); return -1; } if (rend_service_poison_new_single_onion_dirs(NULL) < 0) { - log_warn(LD_GENERAL,"Failed to mark new hidden services as Single " - "Onion."); + log_warn(LD_GENERAL,"Failed to mark new hidden services as non-anonymous" + "."); return -1; } } @@ -2818,63 +2818,61 @@ warn_about_relative_paths(or_options_t *options) } } -/* Validate options related to OnionServiceSingleHopMode. - * Modifies some options that are incompatible with OnionServiceSingleHopMode. +/* Validate options related to single onion services. + * Modifies some options that are incompatible with single onion services. * On failure returns -1, and sets *msg to an error string. * Returns 0 on success. */ STATIC int options_validate_single_onion(or_options_t *options, char **msg) { - /* You must set OnionServiceNonAnonymousMode to 1 to use - * OnionServiceSingleHopMode */ + /* The two single onion service options must have matching values. */ if (options->OnionServiceSingleHopMode && - !rend_service_non_anonymous_mode_enabled(options)) { + !options->OnionServiceNonAnonymousMode) { REJECT("OnionServiceSingleHopMode does not provide any server anonymity. " "It must be used with OnionServiceNonAnonymousMode set to 1."); } - - /* If you have OnionServiceNonAnonymousMode set, you must use - * OnionServiceSingleHopMode. */ - if (rend_service_non_anonymous_mode_enabled(options) && + if (options->OnionServiceNonAnonymousMode && !options->OnionServiceSingleHopMode) { REJECT("OnionServiceNonAnonymousMode does not provide any server " "anonymity. It must be used with OnionServiceSingleHopMode set to " "1."); } + /* Now that we've checked that the two options are consistent, we can safely + * call the rend_service_* functions that abstract these options. */ + /* If you run an anonymous client with an active Single Onion service, the * client loses anonymity. */ const int client_port_set = (options->SocksPort_set || options->TransPort_set || options->NATDPort_set || options->DNSPort_set); - if (options->OnionServiceSingleHopMode && client_port_set && + if (rend_service_non_anonymous_mode_enabled(options) && client_port_set && !options->Tor2webMode) { - REJECT("OnionServiceSingleHopMode is incompatible with using Tor as an " + REJECT("OnionServiceNonAnonymousMode is incompatible with using Tor as an " "anonymous client. Please set Socks/Trans/NATD/DNSPort to 0, or " - "OnionServiceSingleHopMode to 0, or use the non-anonymous " + "OnionServiceNonAnonymousMode to 0, or use the non-anonymous " "Tor2webMode."); } /* If you run a hidden service in non-anonymous mode, the hidden service * loses anonymity, even if SOCKSPort / Tor2web mode isn't used. */ - if (!options->OnionServiceSingleHopMode && options->RendConfigLines - && options->Tor2webMode) { + if (!rend_service_non_anonymous_mode_enabled(options) && + options->RendConfigLines && options->Tor2webMode) { REJECT("Non-anonymous (Tor2web) mode is incompatible with using Tor as a " "hidden service. Please remove all HiddenServiceDir lines, or use " "a version of tor compiled without --enable-tor2web-mode, or use " - "the non-anonymous OnionServiceSingleHopMode."); + " OnionServiceNonAnonymousMode."); } - if (options->OnionServiceSingleHopMode + if (rend_service_allow_non_anonymous_connection(options) && options->UseEntryGuards) { - /* Single Onion services do not (and should not) use entry guards - * in any meaningful way. Further, Single Onions causes the hidden - * service code to do things which break the path bias + /* Single Onion services only use entry guards when uploading descriptors, + * all other connections are one-hop. Further, Single Onions causes the + * hidden service code to do things which break the path bias * detector, and it's far easier to turn off entry guards (and * thus the path bias detector with it) than to figure out how to - * make a piece of code which cannot possibly help Single Onions, - * compatible with OnionServiceSingleHopMode. + * make path bias compatible with single onions. */ log_notice(LD_CONFIG, "OnionServiceSingleHopMode is enabled; disabling " @@ -2882,12 +2880,12 @@ options_validate_single_onion(or_options_t *options, char **msg) options->UseEntryGuards = 0; } - /* Check if existing hidden service keys were created with a different - * setting of OnionServiceNonAnonymousMode, and refuse to launch if they + /* Check if existing hidden service keys were created in a different + * single onion service mode, and refuse to launch if they * have. We'll poison new keys in options_act() just before we create them. */ if (rend_service_list_verify_single_onion_poison(NULL, options) < 0) { - log_warn(LD_GENERAL, "We are configured with OnionServiceSingleHopMode " + log_warn(LD_GENERAL, "We are configured with OnionServiceNonAnonymousMode " "%d, but one or more hidden service keys were created in %s " "mode. This is not allowed.", rend_service_non_anonymous_mode_enabled(options) ? 1 : 0, @@ -3427,7 +3425,7 @@ options_validate(or_options_t *old_options, or_options_t *options, if (!(options->UseEntryGuards) && (options->RendConfigLines != NULL) && - !rend_service_non_anonymous_mode_enabled(options)) { + !rend_service_allow_non_anonymous_connection(options)) { log_warn(LD_CONFIG, "UseEntryGuards is disabled, but you have configured one or more " "hidden services on this Tor instance. Your hidden services " @@ -3450,15 +3448,15 @@ options_validate(or_options_t *old_options, or_options_t *options, return -1; } - /* OnionServiceSingleHopMode: one hop between the onion service server and - * intro and rendezvous points */ - if (options->OnionServiceSingleHopMode) { + /* Single Onion Services: non-anonymous hidden services */ + if (rend_service_non_anonymous_mode_enabled(options)) { log_warn(LD_CONFIG, - "OnionServiceSingleHopMode is set. Every hidden service on this " - "tor instance is NON-ANONYMOUS. If OnionServiceSingleHopMode is " - "disabled, Tor will refuse to launch hidden services from the " - "same directories, to protect against config errors. This " - "setting is for experimental use only."); + "OnionServiceNonAnonymousNode is set. Every hidden service on " + "this tor instance is NON-ANONYMOUS. If " + "the OnionServiceNonAnonymousMode option is changed, Tor will " + "refuse to launch hidden services from the same directories, to " + "protect your anonymity against config errors. This setting is " + "for experimental use only."); } if (!options->LearnCircuitBuildTimeout && options->CircuitBuildTimeout && diff --git a/src/or/control.c b/src/or/control.c index 8f3909b..8962075 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -4249,7 +4249,7 @@ handle_control_add_onion(control_connection_t *conn, int max_streams = 0; int max_streams_close_circuit = 0; rend_auth_type_t auth_type = REND_NO_AUTH; - /* Default to anonymous if no flag is given */ + /* Default to adding an anonymous hidden service if no flag is given */ int non_anonymous = 0; for (size_t i = 1; i < arg_len; i++) { static const char *port_prefix = "Port="; @@ -4288,10 +4288,9 @@ handle_control_add_onion(control_connection_t *conn, * exceeded. * * 'BasicAuth' - Client authorization using the 'basic' method. * * 'NonAnonymous' - Add a non-anonymous Single Onion Service. If this - * flag is present, OnionServiceSingleHopMode and - * OnionServiceNonAnonymousMode must both be 1. If - * this flag is absent, both these options must be - * 0. + * flag is present, tor must be in non-anonymous + * hidden service mode. If this flag is absent, + * tor must be in anonymous hidden service mode. */ static const char *discard_flag = "DiscardPK"; static const char *detach_flag = "Detach"; @@ -4388,15 +4387,17 @@ handle_control_add_onion(control_connection_t *conn, smartlist_len(auth_clients) > 16)) { connection_printf_to_buf(conn, "512 Too many auth clients\r\n"); goto out; - } else if (non_anonymous != rend_service_allow_non_anonymous_connection( + } else if (non_anonymous != rend_service_non_anonymous_mode_enabled( get_options())) { - /* If we failed, and non-anonymous is set, Tor must be in anonymous mode. + /* If we failed, and the non-anonymous flag is set, Tor must be in + * anonymous hidden service mode. * The error message changes based on the current Tor config: - * 512 Tor is in anonymous onion mode - * 512 Tor is in non-anonymous onion mode + * 512 Tor is in anonymous hidden service mode + * 512 Tor is in non-anonymous hidden service mode * (I've deliberately written them out in full here to aid searchability.) */ - connection_printf_to_buf(conn, "512 Tor is in %sanonymous onion mode\r\n", + connection_printf_to_buf(conn, "512 Tor is in %sanonymous hidden service " + "mode\r\n", non_anonymous ? "" : "non-"); goto out; } diff --git a/src/or/or.h b/src/or/or.h index 07f94b7..849cd4c 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3706,8 +3706,7 @@ typedef struct { * rendezvous points. (Onion service descriptors are still posted using * 3-hop paths, to avoid onion service directories blocking the service.) * This option makes every hidden service instance hosted by - * this tor instance a Single Onion Service. One-hop circuits make Single - * Onion servers easily locatable, but clients remain location-anonymous. + * this tor instance a Single Onion Service. * OnionServiceSingleHopMode requires OnionServiceNonAnonymousMode to be set * to 1. * Use rend_service_allow_non_anonymous_connection() or @@ -3716,8 +3715,8 @@ typedef struct { int OnionServiceSingleHopMode; /* Makes hidden service clients and servers non-anonymous on this tor * instance. Allows the non-anonymous OnionServiceSingleHopMode. Enables - * direct connections in the hidden service protocol. - * Use rend_service_non_anonymous_mode() instead of using this option + * non-anonymous behaviour in the hidden service protocol. + * Use rend_service_non_anonymous_mode_enabled() instead of using this option * directly. */ int OnionServiceNonAnonymousMode; diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index 12dd0f4..083efd0 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -1097,11 +1097,11 @@ rend_non_anonymous_mode_enabled(const or_options_t *options) /* Make sure that tor only builds one-hop circuits when they would not * compromise user anonymity. * - * One-hop circuits are permitted in Tor2webMode or OnionServiceSingleHopMode. + * One-hop circuits are permitted in Tor2web or Single Onion modes. * - * Tor2webMode and OnionServiceSingleHopMode are also allowed to make - * multi-hop circuits. For example, single onion HSDir circuits are 3-hop to - * prevent denial of service. + * Tor2web or Single Onion modes are also allowed to make multi-hop circuits. + * For example, single onion HSDir circuits are 3-hop to prevent denial of + * service. */ void assert_circ_anonymity_ok(origin_circuit_t *circ, diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 04894dd..9400b59 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1025,10 +1025,10 @@ rend_service_private_key_exists(const rend_service_t *service) /** Check the single onion service poison state of all existing hidden service * directories: - * - If each service is poisoned, and we are in OnionServiceSingleHopMode, + * - If each service is poisoned, and we are in Single Onion Mode, + * return 0, + * - If each service is not poisoned, and we are not in Single Onion Mode, * return 0, - * - If each service is not poisoned, and we are not in - * OnionServiceSingleHopMode, return 0, * - Otherwise, the poison state is invalid, and a service that was created in * one mode is being used in the other, return -1. * Hidden service directories without keys are not checked for consistency. @@ -1054,7 +1054,7 @@ rend_service_list_verify_single_onion_poison(const smartlist_t *service_list, int consistent = 1; SMARTLIST_FOREACH_BEGIN(s_list, const rend_service_t *, s) { if (service_is_single_onion_poisoned(s) != - rend_service_allow_non_anonymous_connection(options) && + rend_service_non_anonymous_mode_enabled(options) && rend_service_private_key_exists(s)) { consistent = 0; } @@ -1063,25 +1063,25 @@ rend_service_list_verify_single_onion_poison(const smartlist_t *service_list, return consistent ? 0 : -1; } -/*** Helper for rend_service_poison_new_single_onion_dirs(). When in single - * onion mode, add a file to this hidden service directory that marks it as a - * single onion hidden service. Returns 0 when a directory is successfully - * poisoned, or if it is already poisoned. Returns -1 on a failure to read - * the directory or write the poison file, or if there is an existing private - * key file in the directory. (The service should have been poisoned when the - * key was created.) */ +/*** Helper for rend_service_poison_new_single_onion_dirs(). Add a file to + * this hidden service directory that marks it as a single onion service. + * Tor must be in single onion mode before calling this function. + * Returns 0 when a directory is successfully poisoned, or if it is already + * poisoned. Returns -1 on a failure to read the directory or write the poison + * file, or if there is an existing private key file in the directory. (The + * service should have been poisoned when the key was created.) */ static int poison_new_single_onion_hidden_service_dir(const rend_service_t *service) { /* We must only poison directories if we're in Single Onion mode */ - tor_assert(rend_service_allow_non_anonymous_connection(get_options())); + tor_assert(rend_service_non_anonymous_mode_enabled(get_options())); int fd; int retval = -1; char *poison_fname = NULL; if (!service->directory) { - log_info(LD_REND, "Ephemeral HS started in OnionServiceSingleHopMode."); + log_info(LD_REND, "Ephemeral HS started in non-anonymous mode."); return 0; } @@ -1126,7 +1126,7 @@ poison_new_single_onion_hidden_service_dir(const rend_service_t *service) return retval; } -/** We just got launched in OnionServiceSingleHopMode. That's a non-anoymous +/** We just got launched in Single Onion Mode. That's a non-anoymous * mode for hidden services; hence we should mark all new hidden service * directories appropriately so that they are never launched as * location-private hidden services again. (New directories don't have private @@ -1138,7 +1138,7 @@ int rend_service_poison_new_single_onion_dirs(const smartlist_t *service_list) { /* We must only poison directories if we're in Single Onion mode */ - tor_assert(rend_service_allow_non_anonymous_connection(get_options())); + tor_assert(rend_service_non_anonymous_mode_enabled(get_options())); const smartlist_t *s_list; /* If no special service list is provided, then just use the global one. */ @@ -4179,12 +4179,25 @@ rend_service_set_connection_addr_port(edge_connection_t *conn, return -2; } +/* Are OnionServiceSingleHopMode and OnionServiceNonAnonymousMode consistent? + */ +static int +rend_service_non_anonymous_mode_consistent(const or_options_t *options) +{ + /* !! is used to make these options boolean */ + return (!! options->OnionServiceSingleHopMode == + !! options->OnionServiceNonAnonymousMode); +} + /* Do the options allow onion services to make direct (non-anonymous) * connections to introduction or rendezvous points? + * Must only be called after options_validate_single_onion() has successfully + * checked onion service option consistency. * Returns true if tor is in OnionServiceSingleHopMode. */ int rend_service_allow_non_anonymous_connection(const or_options_t *options) { + tor_assert(rend_service_non_anonymous_mode_consistent(options)); return options->OnionServiceSingleHopMode ? 1 : 0; } @@ -4192,17 +4205,24 @@ rend_service_allow_non_anonymous_connection(const or_options_t *options) * service? * Single Onion Services prioritise availability over hiding their * startup time, as their IP address is publicly discoverable anyway. - * Returns true if tor is in OnionServiceSingleHopMode. */ + * Must only be called after options_validate_single_onion() has successfully + * checked onion service option consistency. + * Returns true if tor is in non-anonymous hidden service mode. */ int rend_service_reveal_startup_time(const or_options_t *options) { - return rend_service_allow_non_anonymous_connection(options); + tor_assert(rend_service_non_anonymous_mode_consistent(options)); + return rend_service_non_anonymous_mode_enabled(options); } /* Is non-anonymous mode enabled using the OnionServiceNonAnonymousMode - * config option? */ + * config option? + * Must only be called after options_validate_single_onion() has successfully + * checked onion service option consistency. + */ int rend_service_non_anonymous_mode_enabled(const or_options_t *options) { + tor_assert(rend_service_non_anonymous_mode_consistent(options)); return options->OnionServiceNonAnonymousMode ? 1 : 0; } diff --git a/src/test/test_options.c b/src/test/test_options.c index cd1821a..f01e137 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -2813,10 +2813,10 @@ test_options_validate__single_onion(void *ignored) ); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, "OnionServiceSingleHopMode is incompatible with using " - "Tor as an anonymous client. Please set Socks/Trans/NATD/DNSPort " - "to 0, or OnionServiceSingleHopMode to 0, or use the " - "non-anonymous Tor2webMode."); + tt_str_op(msg, OP_EQ, "OnionServiceNonAnonymousMode is incompatible with " + "using Tor as an anonymous client. Please set " + "Socks/Trans/NATD/DNSPort to 0, or OnionServiceNonAnonymousMode " + "to 0, or use the non-anonymous Tor2webMode."); tor_free(msg); free_options_test_data(tdata);