diff options
36 files changed, 773 insertions, 240 deletions
@@ -1854,7 +1854,7 @@ Changes in version 0.2.8.2-alpha - 2016-03-28 testing versions of the static libraries. Fixes bug 18490; bugfix on 0.2.7.1-alpha. - Avoid spurious failures from configure files related to calling - exit(0) in TOR_SEARCH_LIBRARY. Fixes bug 18625; bugfix on + exit(0) in TOR_SEARCH_LIBRARY. Fixes bug 18626; bugfix on 0.2.0.1-alpha. Patch from "cypherpunks". - Silence spurious clang-scan warnings in the ed25519_donna code by explicitly initializing some objects. Fixes bug 18384; bugfix on diff --git a/ReleaseNotes b/ReleaseNotes index 96c2235351..23fbfcfb2e 100644 --- a/ReleaseNotes +++ b/ReleaseNotes @@ -1192,7 +1192,7 @@ Changes in version 0.2.8.6 - 2016-08-02 o Minor bugfixes (build): - Avoid spurious failures from configure files related to calling - exit(0) in TOR_SEARCH_LIBRARY. Fixes bug 18625; bugfix on + exit(0) in TOR_SEARCH_LIBRARY. Fixes bug 18626; bugfix on 0.2.0.1-alpha. Patch from "cypherpunks". - Do not link the unit tests against both the testing and non- testing versions of the static libraries. Fixes bug 18490; bugfix diff --git a/changes/bug19025 b/changes/bug19025 new file mode 100644 index 0000000000..0f365f52ba --- /dev/null +++ b/changes/bug19025 @@ -0,0 +1,4 @@ + o Major bugfixes (DNS): + - Fix a bug that prevented exit nodes from caching DNS records for more + than 60 seconds. + Fixes bug 19025; bugfix on 0.2.4.7-alpha. diff --git a/changes/bug19953 b/changes/bug19953 new file mode 100644 index 0000000000..919018dbb7 --- /dev/null +++ b/changes/bug19953 @@ -0,0 +1,6 @@ + o Minor bugfixes (relay) + - Honor DataDirectoryGroupReadable when tor is a relay. Previously, the + initialization of the keys would reset the DataDirectory to 0700 instead + of 0750 if DataDirectoryGroupReadable was set to 1. Fixes #19953. Patch + by "redfish".; bugfix on tor-0.0.2pre16. + diff --git a/changes/bug20307 b/changes/bug20307 new file mode 100644 index 0000000000..9112c9c78d --- /dev/null +++ b/changes/bug20307 @@ -0,0 +1,7 @@ + o Minor bugfixes (circuit, hidden service) + - When closing a circuit, the reason for doing so was assigned from an int + value to a uint16_t which is quite a problem for negative values that are + our internal reasons (ex: END_CIRC_REASON_IP_NOW_REDUNDANT). On the HS + side, this was causing introduction points to be flagged as unusable + because the reason wasn't the right one due to the bad conversion. + Partially fixes bug 21056 and fixes bug 20307; Bugfix on 0.2.8.1-alpha. diff --git a/changes/bug20502 b/changes/bug20502 new file mode 100644 index 0000000000..d956daf0b9 --- /dev/null +++ b/changes/bug20502 @@ -0,0 +1,5 @@ + o Minor features + - Require UseEntryGuards when UseBridges is set in order to make sure + bridges aren't bypassed. Resolves ticket 20502. + - Add UseEntryGuards to TEST_OPTIONS_DEFAULT_VALUES in order to not break + regresson tests. diff --git a/changes/bug20996 b/changes/bug20996 new file mode 100644 index 0000000000..b2be805b0a --- /dev/null +++ b/changes/bug20996 @@ -0,0 +1,9 @@ + o Minor bugfixes (IPv6): + - Make IP6-using clients try harder to find an IPv6 directory server. + Fixes bug 20999; bugfix on 77a9de0 from 17840 in 0.2.8.2-alpha. + - When IPv6 addresses have not been downloaded, use hard-coded address + info for authorities, fallbacks, and configured bridges. + (When IPv6-only clients receive a microdesc consensus, it has no IPv6 + addresses, so they can't use it until microdescs are downloaded.) + This allows IPv6-only clients to use microdescriptors. + Fixes bug 20996; bugfix on b167e82 from 19608 in 0.2.8.5-alpha. diff --git a/changes/bug21033 b/changes/bug21033 new file mode 100644 index 0000000000..c4b3deab43 --- /dev/null +++ b/changes/bug21033 @@ -0,0 +1,5 @@ + o Minor bugfixes (hidden service): + - Restrict the number of introduction point from 0 to the maximum allowed + instead of the default value to the maximum. That was stopping operators + from using less than 3 introduction points. Closes #21033; bugfix on + tor-0.2.7.2-alpha. diff --git a/changes/bug21054 b/changes/bug21054 new file mode 100644 index 0000000000..bc499490ee --- /dev/null +++ b/changes/bug21054 @@ -0,0 +1,5 @@ + o Minor bugfixes (hidden service): + - Fix the config reload pruning of old vs new services so it actually + works when both ephemeral and non ephemeral services were configured + which lead to a BUG() stacktrace. Close #21054; Bugfix on + tor-0.3.0.1-alpha. diff --git a/changes/bug21058 b/changes/bug21058 new file mode 100644 index 0000000000..194b529b4b --- /dev/null +++ b/changes/bug21058 @@ -0,0 +1,3 @@ + o Documentation (man page): + - Clarify a number of options in tor.1 and add some min/max values for + HiddenService options. Closes #21058. diff --git a/changes/bug21118 b/changes/bug21118 new file mode 100644 index 0000000000..88e860c683 --- /dev/null +++ b/changes/bug21118 @@ -0,0 +1,6 @@ + o Major bugfixes (client, guard, crash): + - In circuit_get_global_origin_list(), return the actual list of + origin circuits. The previous version of this code returned + the list of all the circuits, and could have caused strange bugs, + including possible crashes. Fixes bug 21118; bugfix on 0.3.0.1-alpha. + diff --git a/changes/bug21142 b/changes/bug21142 new file mode 100644 index 0000000000..e232f43fe7 --- /dev/null +++ b/changes/bug21142 @@ -0,0 +1,4 @@ + o Minor bugfixes (client, guards): + - Fix bug where Tor would think that there are circuits waiting for better + guards even though those circuits have been freed. Fixes bug #21142; + bugfix on 0.3.0.1-alpha. diff --git a/changes/ticket19769 b/changes/ticket19769 new file mode 100644 index 0000000000..9fc05c3e9e --- /dev/null +++ b/changes/ticket19769 @@ -0,0 +1,7 @@ + o Major features (security): + - Change the algorithm used to decide DNS TTLs on client and server side, + to better resist DNS-based correlation attacks like the DefecTor attack + of Greschbach, Pulls, Roberts, Winter, and Feamster). Now + relays only return one of two possible DNS TTL values, and clients + are willing to believe DNS TTL values up to 3 hours long. + Closes ticket 19769. diff --git a/changes/ticket20921 b/changes/ticket20921 new file mode 100644 index 0000000000..7bc0074fdb --- /dev/null +++ b/changes/ticket20921 @@ -0,0 +1,4 @@ + o Code simplification and refactoring: + - Refactor code to manipulate global_origin_circuit_list into separate + functions. Closes ticket 20921. + diff --git a/doc/HACKING/HelpfulTools.md b/doc/HACKING/HelpfulTools.md index a7f36e6c7e..67481ace43 100644 --- a/doc/HACKING/HelpfulTools.md +++ b/doc/HACKING/HelpfulTools.md @@ -142,6 +142,12 @@ run `make test-network`. We also have scripts to run integration tests using Stem. To try them, set `STEM_SOURCE_DIR` to your Stem source directory, and run `test-stem`. +Profiling Tor +------------- + +Ongoing notes about Tor profiling can be found at +https://pad.riseup.net/p/profiling-tor + Profiling Tor with oprofile --------------------------- @@ -168,6 +174,55 @@ Here are some basic instructions * `opreport -l that_dir/*` - Profit +Profiling Tor with perf +----------------------- + +This works with a running Tor, and requires root. + +1. Decide how long you want to profile for. Start with (say) 30 seconds. If that + works, try again with longer times. + +2. Find the PID of your running tor process. + +3. Run `perf record --call-graph dwarf -p <PID> sleep <SECONDS>` + + (You may need to do this as root.) + + You might need to add `-e cpu-clock` as an option to the perf record line + above, if you are on an older CPU without access to hardware profiling + events, or in a VM, or something. + +4. Now you have a perf.data file. Have a look at it with `perf report + --no-children --sort symbol,dso` or `perf report --no-children --sort + symbol,dso --stdio --header`. How does it look? + +5a. Once you have a nice big perf.data file, you can compress it, encrypt it, + and send it to your favorite Tor developers. + +5b. Or maybe you'd rather not send a nice big perf.data file. Who knows what's + in that!? It's kinda scary. To generate a less scary file, you can use `perf + report -g > <FILENAME>.out`. Then you can compress that and put it somewhere + public. + +Profiling Tor with gperftools aka Google-performance-tools +---------------------------------------------------------- + +This should work on nearly any unixy system. It doesn't seem to be compatible +with RunAsDaemon though. + +Beforehand, install google-perftools. + +1. You need to rebuild Tor, hack the linking steps to add `-lprofiler` to the + libs. You can do this by adding `LIBS=-lprofiler` when you call `./configure`. + +Now you can run Tor with profiling enabled, and use the pprof utility to look at +performance! See the gperftools manual for more info, but basically: + +2. Run `env CPUPROFILE=/tmp/profile src/or/tor -f <path/torrc>`. The profile file + is not written to until Tor finishes execuction. + +3. Run `pprof src/or/tor /tm/profile` to start the REPL. + Generating and analyzing a callgraph ------------------------------------ diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 32f89086aa..46b10773bd 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -846,7 +846,7 @@ The following options are useful only for clients (that is, if [[ExcludeExitNodes]] **ExcludeExitNodes** __node__,__node__,__...__:: A list of identity fingerprints, country codes, and address patterns of nodes to never use when picking an exit node---that is, a - node that delivers traffic for you outside the Tor network. Note that any + node that delivers traffic for you *outside* the Tor network. Note that any node listed in ExcludeNodes is automatically considered to be part of this list too. See the **ExcludeNodes** option for more information on how to specify @@ -863,7 +863,7 @@ The following options are useful only for clients (that is, if [[ExitNodes]] **ExitNodes** __node__,__node__,__...__:: A list of identity fingerprints, country codes, and address patterns of nodes to use as exit node---that is, a - node that delivers traffic for you outside the Tor network. See + node that delivers traffic for you *outside* the Tor network. See the **ExcludeNodes** option for more information on how to specify nodes. + + Note that if you list too few nodes here, or if you exclude too many exit @@ -871,7 +871,7 @@ The following options are useful only for clients (that is, if if none of the exits you list allows traffic on port 80 or 443, you won't be able to browse the web. + + - Note also that not every circuit is used to deliver traffic outside of + Note also that not every circuit is used to deliver traffic *outside* of the Tor network. It is normal to see non-exit circuits (such as those used to connect to hidden services, those that do directory fetches, those used for relay reachability self-tests, and so on) that end @@ -897,16 +897,16 @@ The following options are useful only for clients (that is, if the **ExcludeNodes** option for more information on how to specify nodes. [[StrictNodes]] **StrictNodes** **0**|**1**:: - If StrictNodes is set to 1, Tor will treat the ExcludeNodes option as a - requirement to follow for all the circuits you generate, even if doing so - will break functionality for you. If StrictNodes is set to 0, Tor will + If StrictNodes is set to 1, Tor will treat solely the ExcludeNodes option + as a requirement to follow for all the circuits you generate, even if + doing so will break functionality for you (StrictNodes applies to neither + ExcludeExitNodes nor to ExitNodes). If StrictNodes is set to 0, Tor will still try to avoid nodes in the ExcludeNodes list, but it will err on the - side of avoiding unexpected errors. Specifically, StrictNodes 0 tells - Tor that it is okay to use an excluded node when it is *necessary* to - perform relay reachability self-tests, connect to - a hidden service, provide a hidden service to a client, fulfill a .exit - request, upload directory information, or download directory information. - (Default: 0) + side of avoiding unexpected errors. Specifically, StrictNodes 0 tells Tor + that it is okay to use an excluded node when it is *necessary* to perform + relay reachability self-tests, connect to a hidden service, provide a + hidden service to a client, fulfill a .exit request, upload directory + information, or download directory information. (Default: 0) [[FascistFirewall]] **FascistFirewall** **0**|**1**:: If 1, Tor will only create outgoing connections to ORs running on ports @@ -2412,8 +2412,8 @@ The following options are used to configure a hidden service. [[HiddenServiceMaxStreams]] **HiddenServiceMaxStreams** __N__:: The maximum number of simultaneous streams (connections) per rendezvous - circuit. (Setting this to 0 will allow an unlimited number of simultanous - streams.) (Default: 0) + circuit. The maximum value allowed is 65535. (Setting this to 0 will allow + an unlimited number of simultanous streams.) (Default: 0) [[HiddenServiceMaxStreamsCloseCircuit]] **HiddenServiceMaxStreamsCloseCircuit** **0**|**1**:: If set to 1, then exceeding **HiddenServiceMaxStreams** will cause the @@ -2422,8 +2422,9 @@ The following options are used to configure a hidden service. [[RendPostPeriod]] **RendPostPeriod** __N__ **seconds**|**minutes**|**hours**|**days**|**weeks**:: Every time the specified period elapses, Tor uploads any rendezvous - service descriptors to the directory servers. This information is also - uploaded whenever it changes. (Default: 1 hour) + service descriptors to the directory servers. This information is also + uploaded whenever it changes. Minimum value allowed is 10 minutes and + maximum is 3.5 days. (Default: 1 hour) [[HiddenServiceDirGroupReadable]] **HiddenServiceDirGroupReadable** **0**|**1**:: If this option is set to 1, allow the filesystem group to read the diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 5943e516ff..54a7db9dbf 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -509,6 +509,39 @@ circuit_count_pending_on_channel(channel_t *chan) return cnt; } +/** Remove <b>origin_circ</b> from the global list of origin circuits. + * Called when we are freeing a circuit. + */ +static void +circuit_remove_from_origin_circuit_list(origin_circuit_t *origin_circ) +{ + int origin_idx = origin_circ->global_origin_circuit_list_idx; + if (origin_idx < 0) + return; + origin_circuit_t *c2; + tor_assert(origin_idx <= smartlist_len(global_origin_circuit_list)); + c2 = smartlist_get(global_origin_circuit_list, origin_idx); + tor_assert(origin_circ == c2); + smartlist_del(global_origin_circuit_list, origin_idx); + if (origin_idx < smartlist_len(global_origin_circuit_list)) { + origin_circuit_t *replacement = + smartlist_get(global_origin_circuit_list, origin_idx); + replacement->global_origin_circuit_list_idx = origin_idx; + } + origin_circ->global_origin_circuit_list_idx = -1; +} + +/** Add <b>origin_circ</b> to the global list of origin circuits. Called + * when creating the circuit. */ +static void +circuit_add_to_origin_circuit_list(origin_circuit_t *origin_circ) +{ + tor_assert(origin_circ->global_origin_circuit_list_idx == -1); + smartlist_t *lst = circuit_get_global_origin_circuit_list(); + smartlist_add(lst, origin_circ); + origin_circ->global_origin_circuit_list_idx = smartlist_len(lst) - 1; +} + /** Detach from the global circuit list, and deallocate, all * circuits that have been marked for close. */ @@ -533,15 +566,7 @@ circuit_close_all_marked(void) /* Remove it from the origin circuit list, if appropriate. */ if (CIRCUIT_IS_ORIGIN(circ)) { - origin_circuit_t *origin_circ = TO_ORIGIN_CIRCUIT(circ); - int origin_idx = origin_circ->global_origin_circuit_list_idx; - smartlist_del(global_origin_circuit_list, origin_idx); - if (origin_idx < smartlist_len(global_origin_circuit_list)) { - origin_circuit_t *replacement = - smartlist_get(global_origin_circuit_list, origin_idx); - replacement->global_origin_circuit_list_idx = origin_idx; - } - origin_circ->global_origin_circuit_list_idx = -1; + circuit_remove_from_origin_circuit_list(TO_ORIGIN_CIRCUIT(circ)); } circuit_about_to_free(circ); @@ -566,7 +591,7 @@ circuit_get_global_origin_circuit_list(void) { if (NULL == global_origin_circuit_list) global_origin_circuit_list = smartlist_new(); - return global_circuitlist; + return global_origin_circuit_list; } /** Function to make circ-\>state human-readable */ @@ -811,11 +836,8 @@ origin_circuit_new(void) init_circuit_base(TO_CIRCUIT(circ)); /* Add to origin-list. */ - if (!global_origin_circuit_list) - global_origin_circuit_list = smartlist_new(); - smartlist_add(global_origin_circuit_list, circ); - circ->global_origin_circuit_list_idx = - smartlist_len(global_origin_circuit_list) - 1; + circ->global_origin_circuit_list_idx = -1; + circuit_add_to_origin_circuit_list(circ); circuit_build_times_update_last_circ(get_circuit_build_times_mutable()); @@ -875,16 +897,7 @@ circuit_free(circuit_t *circ) memlen = sizeof(origin_circuit_t); tor_assert(circ->magic == ORIGIN_CIRCUIT_MAGIC); - if (ocirc->global_origin_circuit_list_idx != -1) { - int idx = ocirc->global_origin_circuit_list_idx; - origin_circuit_t *c2 = smartlist_get(global_origin_circuit_list, idx); - tor_assert(c2 == ocirc); - smartlist_del(global_origin_circuit_list, idx); - if (idx < smartlist_len(global_origin_circuit_list)) { - c2 = smartlist_get(global_origin_circuit_list, idx); - c2->global_origin_circuit_list_idx = idx; - } - } + circuit_remove_from_origin_circuit_list(ocirc); if (ocirc->build_state) { extend_info_free(ocirc->build_state->chosen_exit); @@ -1440,6 +1453,41 @@ circuit_get_ready_rend_circ_by_rend_data(const rend_data_t *rend_data) return NULL; } +/** Return the first service introduction circuit originating from the global + * circuit list after <b>start</b> or at the start of the list if <b>start</b> + * is NULL. Return NULL if no circuit is found. + * + * A service introduction point circuit has a purpose of either + * CIRCUIT_PURPOSE_S_ESTABLISH_INTRO or CIRCUIT_PURPOSE_S_INTRO. This does not + * return a circuit marked for close and its state must be open. */ +origin_circuit_t * +circuit_get_next_service_intro_circ(origin_circuit_t *start) +{ + int idx = 0; + smartlist_t *lst = circuit_get_global_list(); + + if (start) { + idx = TO_CIRCUIT(start)->global_circuitlist_idx + 1; + } + + for ( ; idx < smartlist_len(lst); ++idx) { + circuit_t *circ = smartlist_get(lst, idx); + + /* Ignore a marked for close circuit or purpose not matching a service + * intro point or if the state is not open. */ + if (circ->marked_for_close || circ->state != CIRCUIT_STATE_OPEN || + (circ->purpose != CIRCUIT_PURPOSE_S_ESTABLISH_INTRO && + circ->purpose != CIRCUIT_PURPOSE_S_INTRO)) { + continue; + } + /* The purposes we are looking for are only for origin circuits so the + * following is valid. */ + return TO_ORIGIN_CIRCUIT(circ); + } + /* Not found. */ + return NULL; +} + /** Return the first circuit originating here in global_circuitlist after * <b>start</b> whose purpose is <b>purpose</b>, and where <b>digest</b> (if * set) matches the private key digest of the rend data associated with the @@ -1810,6 +1858,9 @@ circuit_about_to_free(circuit_t *circ) if (circuits_pending_chans) smartlist_remove(circuits_pending_chans, circ); } + if (circuits_pending_other_guards) { + smartlist_remove(circuits_pending_other_guards, circ); + } if (CIRCUIT_IS_ORIGIN(circ)) { control_event_circuit_status(TO_ORIGIN_CIRCUIT(circ), (circ->state == CIRCUIT_STATE_OPEN || diff --git a/src/or/circuitlist.h b/src/or/circuitlist.h index d83801a7a8..6abee37dc4 100644 --- a/src/or/circuitlist.h +++ b/src/or/circuitlist.h @@ -47,6 +47,7 @@ origin_circuit_t *circuit_get_ready_rend_circ_by_rend_data( const rend_data_t *rend_data); origin_circuit_t *circuit_get_next_by_pk_and_purpose(origin_circuit_t *start, const uint8_t *digest, uint8_t purpose); +origin_circuit_t *circuit_get_next_service_intro_circ(origin_circuit_t *start); origin_circuit_t *circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info, int flags); void circuit_mark_all_unused_circs(void); diff --git a/src/or/config.c b/src/or/config.c index d4177878e7..c4e9ca2403 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -3361,23 +3361,6 @@ options_validate(or_options_t *old_options, or_options_t *options, "of the Internet, so they must not set Reachable*Addresses " "or FascistFirewall or FirewallPorts or ClientUseIPv4 0."); - /* We check if Reachable*Addresses blocks all addresses in - * parse_reachable_addresses(). */ - -#define WARN_PLEASE_USE_IPV6_LOG_MSG \ - "ClientPreferIPv6%sPort 1 is ignored unless tor is using IPv6. " \ - "Please set ClientUseIPv6 1, ClientUseIPv4 0, or configure bridges." - - if (!fascist_firewall_use_ipv6(options) - && options->ClientPreferIPv6ORPort == 1) - log_warn(LD_CONFIG, WARN_PLEASE_USE_IPV6_LOG_MSG, "OR"); - - if (!fascist_firewall_use_ipv6(options) - && options->ClientPreferIPv6DirPort == 1) - log_warn(LD_CONFIG, WARN_PLEASE_USE_IPV6_LOG_MSG, "Dir"); - -#undef WARN_PLEASE_USE_IPV6_LOG_MSG - if (options->UseBridges && server_mode(options)) REJECT("Servers must be able to freely connect to the rest " @@ -3389,6 +3372,11 @@ options_validate(or_options_t *old_options, or_options_t *options, if (options->UseBridges && options->EntryNodes) REJECT("You cannot set both UseBridges and EntryNodes."); + /* If we have UseBridges as 1 and UseEntryGuards as 0, we end up bypassing + * the use of bridges */ + if (options->UseBridges && !options->UseEntryGuards) + REJECT("Setting UseBridges requires also setting UseEntryGuards."); + options->MaxMemInQueues = compute_real_max_mem_in_queues(options->MaxMemInQueues_raw, server_mode(options)); diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 415f9e480b..82d5d3c5a2 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -329,6 +329,33 @@ relay_send_end_cell_from_edge(streamid_t stream_id, circuit_t *circ, payload, 1, cpath_layer); } +/* If the connection <b>conn</b> is attempting to connect to an external + * destination that is an hidden service and the reason is a connection + * refused or timeout, log it so the operator can take appropriate actions. + * The log statement is a rate limited warning. */ +static void +warn_if_hs_unreachable(const edge_connection_t *conn, uint8_t reason) +{ + tor_assert(conn); + + if (conn->base_.type == CONN_TYPE_EXIT && + connection_edge_is_rendezvous_stream(conn) && + (reason == END_STREAM_REASON_CONNECTREFUSED || + reason == END_STREAM_REASON_TIMEOUT)) { +#define WARN_FAILED_HS_CONNECTION 300 + static ratelim_t warn_limit = RATELIM_INIT(WARN_FAILED_HS_CONNECTION); + char *m; + if ((m = rate_limit_log(&warn_limit, approx_time()))) { + log_warn(LD_EDGE, "Onion service connection to %s failed (%s)", + (conn->base_.socket_family == AF_UNIX) ? + safe_str(conn->base_.address) : + safe_str(fmt_addrport(&conn->base_.addr, conn->base_.port)), + stream_end_reason_to_string(reason)); + tor_free(m); + } + } +} + /** Send a relay end cell from stream <b>conn</b> down conn's circuit, and * remember that we've done so. If this is not a client connection, set the * relay end cell's reason for closing as <b>reason</b>. @@ -386,6 +413,9 @@ connection_edge_end(edge_connection_t *conn, uint8_t reason) conn->base_.s); connection_edge_send_command(conn, RELAY_COMMAND_END, payload, payload_len); + /* We'll log warn if the connection was an hidden service and couldn't be + * made because the service wasn't available. */ + warn_if_hs_unreachable(conn, control_reason); } else { log_debug(LD_EDGE,"No circ to send end on conn " "(fd "TOR_SOCKET_T_FORMAT").", @@ -3521,7 +3551,7 @@ connection_exit_connect_dir(edge_connection_t *exitconn) * it is a general stream. */ int -connection_edge_is_rendezvous_stream(edge_connection_t *conn) +connection_edge_is_rendezvous_stream(const edge_connection_t *conn) { tor_assert(conn); if (conn->rend_data) diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h index 5dfc8af901..61b5752aed 100644 --- a/src/or/connection_edge.h +++ b/src/or/connection_edge.h @@ -60,7 +60,7 @@ void connection_ap_handshake_socks_resolved_addr(entry_connection_t *conn, int connection_exit_begin_conn(cell_t *cell, circuit_t *circ); int connection_exit_begin_resolve(cell_t *cell, or_circuit_t *circ); void connection_exit_connect(edge_connection_t *conn); -int connection_edge_is_rendezvous_stream(edge_connection_t *conn); +int connection_edge_is_rendezvous_stream(const edge_connection_t *conn); int connection_ap_can_use_exit(const entry_connection_t *conn, const node_t *exit); void connection_ap_expire_beginning(void); diff --git a/src/or/dns.c b/src/or/dns.c index 388104f8da..7651501ec3 100644 --- a/src/or/dns.c +++ b/src/or/dns.c @@ -243,29 +243,19 @@ has_dns_init_failed(void) } /** Helper: Given a TTL from a DNS response, determine what TTL to give the - * OP that asked us to resolve it. */ + * OP that asked us to resolve it, and how long to cache that record + * ourselves. */ uint32_t dns_clip_ttl(uint32_t ttl) { - if (ttl < MIN_DNS_TTL) - return MIN_DNS_TTL; - else if (ttl > MAX_DNS_TTL) - return MAX_DNS_TTL; - else - return ttl; -} - -/** Helper: Given a TTL from a DNS response, determine how long to hold it in - * our cache. */ -STATIC uint32_t -dns_get_expiry_ttl(uint32_t ttl) -{ - if (ttl < MIN_DNS_TTL) - return MIN_DNS_TTL; - else if (ttl > MAX_DNS_ENTRY_AGE) - return MAX_DNS_ENTRY_AGE; + /* This logic is a defense against "DefectTor" DNS-based traffic + * confirmation attacks, as in https://nymity.ch/tor-dns/tor-dns.pdf . + * We only give two values: a "low" value and a "high" value. + */ + if (ttl < MIN_DNS_TTL_AT_EXIT) + return MIN_DNS_TTL_AT_EXIT; else - return ttl; + return MAX_DNS_TTL_AT_EXIT; } /** Helper: free storage held by an entry in the DNS cache. */ @@ -336,7 +326,7 @@ cached_resolve_add_answer(cached_resolve_t *resolve, resolve->result_ipv4.err_ipv4 = dns_result; resolve->res_status_ipv4 = RES_STATUS_DONE_ERR; } - + resolve->ttl_ipv4 = ttl; } else if (query_type == DNS_IPv6_AAAA) { if (resolve->res_status_ipv6 != RES_STATUS_INFLIGHT) return; @@ -351,6 +341,7 @@ cached_resolve_add_answer(cached_resolve_t *resolve, resolve->result_ipv6.err_ipv6 = dns_result; resolve->res_status_ipv6 = RES_STATUS_DONE_ERR; } + resolve->ttl_ipv6 = ttl; } } @@ -1317,7 +1308,7 @@ make_pending_resolve_cached(cached_resolve_t *resolve) resolve->ttl_hostname < ttl) ttl = resolve->ttl_hostname; - set_expiry(new_resolve, time(NULL) + dns_get_expiry_ttl(ttl)); + set_expiry(new_resolve, time(NULL) + dns_clip_ttl(ttl)); } assert_cache_ok(); diff --git a/src/or/dns.h b/src/or/dns.h index b14f7dd29c..951a2a3467 100644 --- a/src/or/dns.h +++ b/src/or/dns.h @@ -12,6 +12,18 @@ #ifndef TOR_DNS_H #define TOR_DNS_H +/** Lowest value for DNS ttl that a server will give. */ +#define MIN_DNS_TTL_AT_EXIT (5*60) +/** Highest value for DNS ttl that a server will give. */ +#define MAX_DNS_TTL_AT_EXIT (60*60) + +/** How long do we keep DNS cache entries before purging them (regardless of + * their TTL)? */ +#define MAX_DNS_ENTRY_AGE (3*60*60) +/** How long do we cache/tell clients to cache DNS records when no TTL is + * known? */ +#define DEFAULT_DNS_TTL (30*60) + int dns_init(void); int has_dns_init_failed(void); void dns_free_all(void); @@ -31,8 +43,6 @@ void dump_dns_mem_usage(int severity); #ifdef DNS_PRIVATE #include "dns_structs.h" -STATIC uint32_t dns_get_expiry_ttl(uint32_t ttl); - MOCK_DECL(STATIC int,dns_resolve_impl,(edge_connection_t *exitconn, int is_resolve,or_circuit_t *oncirc, char **hostname_out, int *made_connection_pending_out, cached_resolve_t **resolve_out)); diff --git a/src/or/nodelist.c b/src/or/nodelist.c index 2bcedbfb0c..804af297ba 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -1126,6 +1126,9 @@ node_get_prim_orport(const node_t *node, tor_addr_port_t *ap_out) node_assert_ok(node); tor_assert(ap_out); + /* Check ri first, because rewrite_node_address_for_bridge() updates + * node->ri with the configured bridge address. */ + RETURN_IPV4_AP(node->ri, or_port, ap_out); RETURN_IPV4_AP(node->rs, or_port, ap_out); /* Microdescriptors only have an IPv6 address */ @@ -1156,9 +1159,11 @@ node_get_pref_ipv6_orport(const node_t *node, tor_addr_port_t *ap_out) node_assert_ok(node); tor_assert(ap_out); - /* Prefer routerstatus over microdesc for consistency with the - * fascist_firewall_* functions. Also check if the address or port are valid, - * and try another alternative if they are not. */ + /* Check ri first, because rewrite_node_address_for_bridge() updates + * node->ri with the configured bridge address. + * Prefer rs over md for consistency with the fascist_firewall_* functions. + * Check if the address or port are valid, and try another alternative + * if they are not. */ if (node->ri && tor_addr_port_is_valid(&node->ri->ipv6_addr, node->ri->ipv6_orport, 0)) { @@ -1218,6 +1223,9 @@ node_get_prim_dirport(const node_t *node, tor_addr_port_t *ap_out) node_assert_ok(node); tor_assert(ap_out); + /* Check ri first, because rewrite_node_address_for_bridge() updates + * node->ri with the configured bridge address. */ + RETURN_IPV4_AP(node->ri, dir_port, ap_out); RETURN_IPV4_AP(node->rs, dir_port, ap_out); /* Microdescriptors only have an IPv6 address */ @@ -1250,8 +1258,11 @@ node_get_pref_ipv6_dirport(const node_t *node, tor_addr_port_t *ap_out) node_assert_ok(node); tor_assert(ap_out); - /* Check if the address or port are valid, and try another alternative if - * they are not. Note that microdescriptors have no dir_port. */ + /* Check ri first, because rewrite_node_address_for_bridge() updates + * node->ri with the configured bridge address. + * Prefer rs over md for consistency with the fascist_firewall_* functions. + * Check if the address or port are valid, and try another alternative + * if they are not. */ /* Assume IPv4 and IPv6 dirports are the same */ if (node->ri && tor_addr_port_is_valid(&node->ri->ipv6_addr, diff --git a/src/or/or.h b/src/or/or.h index 600bd9dbe6..6292c499df 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -147,18 +147,6 @@ /** Maximum size of a single extrainfo document, as above. */ #define MAX_EXTRAINFO_UPLOAD_SIZE 50000 -/** How long do we keep DNS cache entries before purging them (regardless of - * their TTL)? */ -#define MAX_DNS_ENTRY_AGE (30*60) -/** How long do we cache/tell clients to cache DNS records when no TTL is - * known? */ -#define DEFAULT_DNS_TTL (30*60) -/** How long can a TTL be before we stop believing it? */ -#define MAX_DNS_TTL (3*60*60) -/** How small can a TTL be before we stop believing it? Provides rudimentary - * pinning. */ -#define MIN_DNS_TTL 60 - /** How often do we rotate onion keys? */ #define MIN_ONION_KEY_LIFETIME (7*24*60*60) /** How often do we rotate TLS contexts? */ @@ -3039,11 +3027,11 @@ typedef struct circuit_t { /** For what reason (See END_CIRC_REASON...) is this circuit being closed? * This field is set in circuit_mark_for_close and used later in * circuit_about_to_free. */ - uint16_t marked_for_close_reason; + int marked_for_close_reason; /** As marked_for_close_reason, but reflects the underlying reason for * closing this circuit. */ - uint16_t marked_for_close_orig_reason; + int marked_for_close_orig_reason; /** Unique ID for measuring tunneled network status requests. */ uint64_t dirreq_id; diff --git a/src/or/policies.c b/src/or/policies.c index f4c0cddbcc..84600f7ef8 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -20,6 +20,7 @@ #include "or.h" #include "config.h" #include "dirserv.h" +#include "microdesc.h" #include "networkstatus.h" #include "nodelist.h" #include "policies.h" @@ -297,8 +298,8 @@ parse_reachable_addresses(void) } else if (fascist_firewall_use_ipv6(options) && (policy_is_reject_star(reachable_or_addr_policy, AF_INET6, 0) || policy_is_reject_star(reachable_dir_addr_policy, AF_INET6, 0))) { - log_warn(LD_CONFIG, "You have configured tor to use IPv6 " - "(ClientUseIPv6 1 or UseBridges 1), but " + log_warn(LD_CONFIG, "You have configured tor to use or prefer IPv6 " + "(or UseBridges 1), but " "ReachableAddresses, ReachableORAddresses, or " "ReachableDirAddresses reject all IPv6 addresses. " "Tor will not connect using IPv6."); @@ -316,10 +317,8 @@ firewall_is_fascist_impl(void) const or_options_t *options = get_options(); /* Assume every non-bridge relay has an IPv4 address. * Clients which use bridges may only know the IPv6 address of their - * bridge. */ - return (options->ClientUseIPv4 == 0 - || (!fascist_firewall_use_ipv6(options) - && options->UseBridges == 1)); + * bridge, but they will connect regardless of the ClientUseIPv6 setting. */ + return options->ClientUseIPv4 == 0; } /** Return true iff the firewall options, including ClientUseIPv4 0 and @@ -426,6 +425,9 @@ fascist_firewall_allows_address(const tor_addr_t *addr, } /** Is this client configured to use IPv6? + * Returns true if the client might use IPv6 for some of its connections + * (including dual-stack and IPv6-only clients), and false if it will never + * use IPv6 for any connections. * Use node_ipv6_or/dir_preferred() when checking a specific node and OR/Dir * port: it supports bridge client per-node IPv6 preferences. */ @@ -433,9 +435,11 @@ int fascist_firewall_use_ipv6(const or_options_t *options) { /* Clients use IPv6 if it's set, or they use bridges, or they don't use - * IPv4 */ - return (options->ClientUseIPv6 == 1 || options->UseBridges == 1 - || options->ClientUseIPv4 == 0); + * IPv4, or they prefer it. + * ClientPreferIPv6DirPort is deprecated, but check it anyway. */ + return (options->ClientUseIPv6 == 1 || options->ClientUseIPv4 == 0 || + options->ClientPreferIPv6ORPort == 1 || + options->ClientPreferIPv6DirPort == 1 || options->UseBridges == 1); } /** Do we prefer to connect to IPv6, ignoring ClientPreferIPv6ORPort and @@ -888,6 +892,33 @@ fascist_firewall_choose_address_ipv4h(uint32_t ipv4h_addr, pref_ipv6, ap); } +/* The microdescriptor consensus has no IPv6 addresses in rs: they are in + * the microdescriptors. This means we can't rely on the node's IPv6 address + * until its microdescriptor is available (when using microdescs). + * But for bridges, rewrite_node_address_for_bridge() updates node->ri with + * the configured address, so we can trust bridge addresses. + * (Bridges could gain an IPv6 address if their microdescriptor arrives, but + * this will never be their preferred address: that is in the config.) + * Returns true if the node needs a microdescriptor for its IPv6 address, and + * false if the addresses in the node are already up-to-date. + */ +static int +node_awaiting_ipv6(const or_options_t* options, const node_t *node) +{ + tor_assert(node); + + /* There's no point waiting for an IPv6 address if we'd never use it */ + if (!fascist_firewall_use_ipv6(options)) { + return 0; + } + + /* We are waiting if we_use_microdescriptors_for_circuits() and we have no + * md. Bridges have a ri based on their config. They would never use the + * address from their md, so there's no need to wait for it. */ + return (!node->md && we_use_microdescriptors_for_circuits(options) && + !node->ri); +} + /** Like fascist_firewall_choose_address_base(), but takes <b>rs</b>. * Consults the corresponding node, then falls back to rs if node is NULL. * This should only happen when there's no valid consensus, and rs doesn't @@ -904,15 +935,15 @@ fascist_firewall_choose_address_rs(const routerstatus_t *rs, tor_assert(ap); + const or_options_t *options = get_options(); const node_t *node = node_get_by_id(rs->identity_digest); - if (node) { + if (node && !node_awaiting_ipv6(options, node)) { return fascist_firewall_choose_address_node(node, fw_connection, pref_only, ap); } else { /* There's no node-specific IPv6 preference, so use the generic IPv6 * preference instead. */ - const or_options_t *options = get_options(); int pref_ipv6 = (fw_connection == FIREWALL_OR_CONNECTION ? fascist_firewall_prefer_ipv6_orport(options) : fascist_firewall_prefer_ipv6_dirport(options)); @@ -946,6 +977,18 @@ fascist_firewall_choose_address_node(const node_t *node, node_assert_ok(node); + /* Calling fascist_firewall_choose_address_node() when the node is missing + * IPv6 information breaks IPv6-only clients. + * If the node is a hard-coded fallback directory or authority, call + * fascist_firewall_choose_address_rs() on the fake (hard-coded) routerstatus + * for the node. + * If it is not hard-coded, check that the node has a microdescriptor, full + * descriptor (routerinfo), or is one of our configured bridges before + * calling this function. */ + if (BUG(node_awaiting_ipv6(get_options(), node))) { + return 0; + } + const int pref_ipv6_node = (fw_connection == FIREWALL_OR_CONNECTION ? node_ipv6_or_preferred(node) : node_ipv6_dir_preferred(node)); diff --git a/src/or/rendservice.c b/src/or/rendservice.c index ce205313f6..00f251588c 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -542,6 +542,95 @@ rend_service_check_dir_and_add(smartlist_t *service_list, return rend_add_service(s_list, service); } +/* If this is a reload and there were hidden services configured before, + * keep the introduction points that are still needed and close the + * other ones. */ +STATIC void +prune_services_on_reload(smartlist_t *old_service_list, + smartlist_t *new_service_list) +{ + origin_circuit_t *ocirc = NULL; + smartlist_t *surviving_services = NULL; + + tor_assert(old_service_list); + tor_assert(new_service_list); + + /* This contains all _existing_ services that survives the relaod that is + * that haven't been removed from the configuration. The difference between + * this list and the new service list is that the new list can possibly + * contain newly configured service that have no introduction points opened + * yet nor key material loaded or generated. */ + surviving_services = smartlist_new(); + + /* Preserve the existing ephemeral services. + * + * This is the ephemeral service equivalent of the "Copy introduction + * points to new services" block, except there's no copy required since + * the service structure isn't regenerated. + * + * After this is done, all ephemeral services will be: + * * Removed from old_service_list, so the equivalent non-ephemeral code + * will not attempt to preserve them. + * * Added to the new_service_list (that previously only had the + * services listed in the configuration). + * * Added to surviving_services, which is the list of services that + * will NOT have their intro point closed. + */ + SMARTLIST_FOREACH_BEGIN(old_service_list, rend_service_t *, old) { + if (rend_service_is_ephemeral(old)) { + SMARTLIST_DEL_CURRENT(old_service_list, old); + smartlist_add(surviving_services, old); + smartlist_add(new_service_list, old); + } + } SMARTLIST_FOREACH_END(old); + + /* Copy introduction points to new services. This is O(n^2), but it's only + * called on reconfigure, so it's ok performance wise. */ + SMARTLIST_FOREACH_BEGIN(new_service_list, rend_service_t *, new) { + SMARTLIST_FOREACH_BEGIN(old_service_list, rend_service_t *, old) { + /* Skip ephemeral services as we only want to copy introduction points + * from current services to newly configured one that already exists. + * The same directory means it's the same service. */ + if (rend_service_is_ephemeral(new) || rend_service_is_ephemeral(old) || + strcmp(old->directory, new->directory)) { + continue; + } + smartlist_add_all(new->intro_nodes, old->intro_nodes); + smartlist_clear(old->intro_nodes); + smartlist_add_all(new->expiring_nodes, old->expiring_nodes); + smartlist_clear(old->expiring_nodes); + /* This regular service will survive the closing IPs step after. */ + smartlist_add(surviving_services, old); + break; + } SMARTLIST_FOREACH_END(old); + } SMARTLIST_FOREACH_END(new); + + /* For every service introduction circuit we can find, see if we have a + * matching surviving configured service. If not, close the circuit. */ + while ((ocirc = circuit_get_next_service_intro_circ(ocirc))) { + int keep_it = 0; + tor_assert(ocirc->rend_data); + SMARTLIST_FOREACH_BEGIN(surviving_services, const rend_service_t *, s) { + if (rend_circuit_pk_digest_eq(ocirc, (uint8_t *) s->pk_digest)) { + /* Keep this circuit as we have a matching configured service. */ + keep_it = 1; + break; + } + } SMARTLIST_FOREACH_END(s); + if (keep_it) { + continue; + } + log_info(LD_REND, "Closing intro point %s for service %s.", + safe_str_client(extend_info_describe( + ocirc->build_state->chosen_exit)), + safe_str_client(rend_data_get_address(ocirc->rend_data))); + /* Reason is FINISHED because service has been removed and thus the + * circuit is considered old/uneeded. */ + circuit_mark_for_close(TO_CIRCUIT(ocirc), END_CIRC_REASON_FINISHED); + } + smartlist_free(surviving_services); +} + /** Set up rend_service_list, based on the values of HiddenServiceDir and * HiddenServicePort in <b>options</b>. Return 0 on success and -1 on * failure. (If <b>validate_only</b> is set, parse, warn and return as @@ -602,7 +691,6 @@ rend_config_services(const or_options_t *options, int validate_only) log_warn(LD_CONFIG, "HiddenServiceAllowUnknownPorts should be 0 or 1, not %s", line->value); - smartlist_free(temp_service_list); goto free_and_return; } log_info(LD_CONFIG, @@ -653,14 +741,12 @@ rend_config_services(const or_options_t *options, int validate_only) } else if (!strcasecmp(line->key, "HiddenServiceNumIntroductionPoints")) { service->n_intro_points_wanted = (unsigned int) tor_parse_long(line->value, 10, - NUM_INTRO_POINTS_DEFAULT, - NUM_INTRO_POINTS_MAX, &ok, NULL); + 0, NUM_INTRO_POINTS_MAX, &ok, NULL); if (!ok) { log_warn(LD_CONFIG, "HiddenServiceNumIntroductionPoints " "should be between %d and %d, not %s", - NUM_INTRO_POINTS_DEFAULT, NUM_INTRO_POINTS_MAX, - line->value); + 0, NUM_INTRO_POINTS_MAX, line->value); goto free_and_return; } log_info(LD_CONFIG, "HiddenServiceNumIntroductionPoints=%d for %s", @@ -795,84 +881,11 @@ rend_config_services(const or_options_t *options, int validate_only) * keep the introduction points that are still needed and close the * other ones. */ if (old_service_list && !validate_only) { - smartlist_t *surviving_services = smartlist_new(); - - /* Preserve the existing ephemeral services. - * - * This is the ephemeral service equivalent of the "Copy introduction - * points to new services" block, except there's no copy required since - * the service structure isn't regenerated. - * - * After this is done, all ephemeral services will be: - * * Removed from old_service_list, so the equivalent non-ephemeral code - * will not attempt to preserve them. - * * Added to the new rend_service_list (that previously only had the - * services listed in the configuration). - * * Added to surviving_services, which is the list of services that - * will NOT have their intro point closed. - */ - SMARTLIST_FOREACH(old_service_list, rend_service_t *, old, { - if (rend_service_is_ephemeral(old)) { - SMARTLIST_DEL_CURRENT(old_service_list, old); - smartlist_add(surviving_services, old); - smartlist_add(rend_service_list, old); - } - }); - - /* Copy introduction points to new services. */ - /* XXXX This is O(n^2), but it's only called on reconfigure, so it's - * probably ok? */ - SMARTLIST_FOREACH_BEGIN(rend_service_list, rend_service_t *, new) { - SMARTLIST_FOREACH_BEGIN(old_service_list, rend_service_t *, old) { - if (BUG(rend_service_is_ephemeral(new)) || - BUG(rend_service_is_ephemeral(old))) { - continue; - } - if (BUG(!new->directory) || BUG(!old->directory) || - strcmp(old->directory, new->directory)) { - continue; - } - smartlist_add_all(new->intro_nodes, old->intro_nodes); - smartlist_clear(old->intro_nodes); - smartlist_add_all(new->expiring_nodes, old->expiring_nodes); - smartlist_clear(old->expiring_nodes); - smartlist_add(surviving_services, old); - break; - } SMARTLIST_FOREACH_END(old); - } SMARTLIST_FOREACH_END(new); - - /* Close introduction circuits of services we don't serve anymore. */ - /* XXXX it would be nicer if we had a nicer abstraction to use here, - * so we could just iterate over the list of services to close, but - * once again, this isn't critical-path code. */ - SMARTLIST_FOREACH_BEGIN(circuit_get_global_list(), circuit_t *, circ) { - if (!circ->marked_for_close && - circ->state == CIRCUIT_STATE_OPEN && - (circ->purpose == CIRCUIT_PURPOSE_S_ESTABLISH_INTRO || - circ->purpose == CIRCUIT_PURPOSE_S_INTRO)) { - origin_circuit_t *oc = TO_ORIGIN_CIRCUIT(circ); - int keep_it = 0; - tor_assert(oc->rend_data); - SMARTLIST_FOREACH(surviving_services, rend_service_t *, ptr, { - if (rend_circuit_pk_digest_eq(oc, (uint8_t *) ptr->pk_digest)) { - keep_it = 1; - break; - } - }); - if (keep_it) - continue; - log_info(LD_REND, "Closing intro point %s for service %s.", - safe_str_client(extend_info_describe( - oc->build_state->chosen_exit)), - rend_data_get_address(oc->rend_data)); - circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED); - /* XXXX Is there another reason we should use here? */ - } - } - SMARTLIST_FOREACH_END(circ); - smartlist_free(surviving_services); - SMARTLIST_FOREACH(old_service_list, rend_service_t *, ptr, - rend_service_free(ptr)); + prune_services_on_reload(old_service_list, rend_service_list); + /* Every remaining service in the old list have been removed from the + * configuration so clean them up safely. */ + SMARTLIST_FOREACH(old_service_list, rend_service_t *, s, + rend_service_free(s)); smartlist_free(old_service_list); } diff --git a/src/or/rendservice.h b/src/or/rendservice.h index 4e6b9a2536..3bfac0bece 100644 --- a/src/or/rendservice.h +++ b/src/or/rendservice.h @@ -132,6 +132,9 @@ STATIC int rend_service_poison_new_single_onion_dir( STATIC ssize_t encode_establish_intro_cell_legacy(char *cell_body_out, crypto_pk_t *intro_key, char *rend_circ_nonce); +STATIC void prune_services_on_reload(smartlist_t *old_service_list, + smartlist_t *new_service_list); + #endif int num_rend_services(void); diff --git a/src/or/router.c b/src/or/router.c index 2d8208aa04..e4fa72a283 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -849,7 +849,12 @@ init_keys(void) if (init_keys_common() < 0) return -1; /* Make sure DataDirectory exists, and is private. */ - if (check_private_dir(options->DataDirectory, CPD_CREATE, options->User)) { + cpd_check_t cpd_opts = CPD_CREATE; + if (options->DataDirectoryGroupReadable) + cpd_opts |= CPD_GROUP_READ; + if (check_private_dir(options->DataDirectory, cpd_opts, options->User)) { + log_err(LD_OR, "Can't create/check datadirectory %s", + options->DataDirectory); return -1; } /* Check the key directory. */ diff --git a/src/or/routerkeys.c b/src/or/routerkeys.c index 51802b15e5..e20787123e 100644 --- a/src/or/routerkeys.c +++ b/src/or/routerkeys.c @@ -785,8 +785,11 @@ load_ed_keys(const or_options_t *options, time_t now) if (options->command == CMD_KEYGEN) flags |= INIT_ED_KEY_TRY_ENCRYPTED; - /* Check the key directory */ - if (check_private_dir(options->DataDirectory, CPD_CREATE, options->User)) { + /* Check/Create the key directory */ + cpd_check_t cpd_opts = CPD_CREATE; + if (options->DataDirectoryGroupReadable) + cpd_opts |= CPD_GROUP_READ; + if (check_private_dir(options->DataDirectory, cpd_opts, options->User)) { log_err(LD_OR, "Can't create/check datadirectory %s", options->DataDirectory); goto err; diff --git a/src/or/routerlist.c b/src/or/routerlist.c index afa618167c..72474b1152 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -949,7 +949,7 @@ authority_certs_fetch_resource_impl(const char *resource, /* If we've just downloaded a consensus from a bridge, re-use that * bridge */ - if (options->UseBridges && node && !get_via_tor) { + if (options->UseBridges && node && node->ri && !get_via_tor) { /* clients always make OR connections to bridges */ tor_addr_port_t or_ap; /* we are willing to use a non-preferred address if we need to */ diff --git a/src/test/test_cell_formats.c b/src/test/test_cell_formats.c index bbcff38d6d..22c34b6d6c 100644 --- a/src/test/test_cell_formats.c +++ b/src/test/test_cell_formats.c @@ -346,9 +346,9 @@ test_cfmt_connected_cells(void *arg) memset(&cell, 0, sizeof(cell)); tor_addr_parse(&addr, "30.40.50.60"); rh.length = connected_cell_format_payload(cell.payload+RELAY_HEADER_SIZE, - &addr, 128); + &addr, 1024); tt_int_op(rh.length, OP_EQ, 8); - test_memeq_hex(cell.payload+RELAY_HEADER_SIZE, "1e28323c" "00000080"); + test_memeq_hex(cell.payload+RELAY_HEADER_SIZE, "1e28323c" "00000e10"); /* Try parsing it. */ tor_addr_make_unspec(&addr); @@ -356,7 +356,7 @@ test_cfmt_connected_cells(void *arg) tt_int_op(r, OP_EQ, 0); tt_int_op(tor_addr_family(&addr), OP_EQ, AF_INET); tt_str_op(fmt_addr(&addr), OP_EQ, "30.40.50.60"); - tt_int_op(ttl, OP_EQ, 128); + tt_int_op(ttl, OP_EQ, 3600); /* not 1024, since we clipped to 3600 */ /* Try an IPv6 address */ memset(&rh, 0, sizeof(rh)); diff --git a/src/test/test_dns.c b/src/test/test_dns.c index 8346c0a33f..6a8e92cb47 100644 --- a/src/test/test_dns.c +++ b/src/test/test_dns.c @@ -16,30 +16,11 @@ NS(test_main)(void *arg) { (void)arg; - uint32_t ttl_mid = MIN_DNS_TTL / 2 + MAX_DNS_TTL / 2; + uint32_t ttl_mid = MIN_DNS_TTL_AT_EXIT / 2 + MAX_DNS_TTL_AT_EXIT / 2; - tt_int_op(dns_clip_ttl(MIN_DNS_TTL - 1),==,MIN_DNS_TTL); - tt_int_op(dns_clip_ttl(ttl_mid),==,ttl_mid); - tt_int_op(dns_clip_ttl(MAX_DNS_TTL + 1),==,MAX_DNS_TTL); - - done: - return; -} - -#undef NS_SUBMODULE - -#define NS_SUBMODULE expiry_ttl - -static void -NS(test_main)(void *arg) -{ - (void)arg; - - uint32_t ttl_mid = MIN_DNS_TTL / 2 + MAX_DNS_ENTRY_AGE / 2; - - tt_int_op(dns_get_expiry_ttl(MIN_DNS_TTL - 1),==,MIN_DNS_TTL); - tt_int_op(dns_get_expiry_ttl(ttl_mid),==,ttl_mid); - tt_int_op(dns_get_expiry_ttl(MAX_DNS_ENTRY_AGE + 1),==,MAX_DNS_ENTRY_AGE); + tt_int_op(dns_clip_ttl(MIN_DNS_TTL_AT_EXIT - 1),==,MIN_DNS_TTL_AT_EXIT); + tt_int_op(dns_clip_ttl(ttl_mid),==,MAX_DNS_TTL_AT_EXIT); + tt_int_op(dns_clip_ttl(MAX_DNS_TTL_AT_EXIT + 1),==,MAX_DNS_TTL_AT_EXIT); done: return; @@ -749,7 +730,6 @@ NS(test_main)(void *arg) struct testcase_t dns_tests[] = { TEST_CASE(clip_ttl), - TEST_CASE(expiry_ttl), TEST_CASE(resolve), TEST_CASE_ASPECT(resolve_impl, addr_is_ip_no_need_to_resolve), TEST_CASE_ASPECT(resolve_impl, non_exit), diff --git a/src/test/test_hs.c b/src/test/test_hs.c index ac9788ceea..fbaabe91d8 100644 --- a/src/test/test_hs.c +++ b/src/test/test_hs.c @@ -787,6 +787,126 @@ test_single_onion_poisoning(void *arg) tor_free(mock_options->DataDirectory); } +static rend_service_t * +helper_create_rend_service(const char *path) +{ + rend_service_t *s = tor_malloc_zero(sizeof(rend_service_t)); + s->ports = smartlist_new(); + s->intro_nodes = smartlist_new(); + s->expiring_nodes = smartlist_new(); + if (path) { + s->directory = tor_strdup(path); + } + return s; +} + +static void +test_prune_services_on_reload(void *arg) +{ + smartlist_t *new = smartlist_new(), *old = smartlist_new(); + /* Non ephemeral service. */ + rend_service_t *s1 = helper_create_rend_service("SomePath"); + /* Create a non ephemeral service with the _same_ path as so we can test the + * transfer of introduction point between the same services on reload. */ + rend_service_t *s2 = helper_create_rend_service(s1->directory); + /* Ephemeral service (directory is NULL). */ + rend_service_t *e1 = helper_create_rend_service(NULL); + rend_service_t *e2 = helper_create_rend_service(NULL); + + (void) arg; + + { + /* Add both services to the old list. */ + smartlist_add(old, s1); + smartlist_add(old, e1); + /* Only put the non ephemeral in the new list. */ + smartlist_add(new, s1); + prune_services_on_reload(old, new); + /* We expect that the ephemeral one is in the new list but removed from + * the old one. */ + tt_int_op(smartlist_len(old), OP_EQ, 1); + tt_assert(smartlist_get(old, 0) == s1); + tt_int_op(smartlist_len(new), OP_EQ, 2); + tt_assert(smartlist_get(new, 0) == s1); + tt_assert(smartlist_get(new, 1) == e1); + /* Cleanup for next test. */ + smartlist_clear(new); + smartlist_clear(old); + } + + { + /* This test will make sure that only the ephemeral service is kept if the + * new list is empty. The old list should contain only the non ephemeral + * one. */ + smartlist_add(old, s1); + smartlist_add(old, e1); + prune_services_on_reload(old, new); + tt_int_op(smartlist_len(old), OP_EQ, 1); + tt_assert(smartlist_get(old, 0) == s1); + tt_int_op(smartlist_len(new), OP_EQ, 1); + tt_assert(smartlist_get(new, 0) == e1); + /* Cleanup for next test. */ + smartlist_clear(new); + smartlist_clear(old); + } + + { + /* This test makes sure that the new list stays the same even from the old + * list being completely different. */ + smartlist_add(new, s1); + smartlist_add(new, e1); + prune_services_on_reload(old, new); + tt_int_op(smartlist_len(old), OP_EQ, 0); + tt_int_op(smartlist_len(new), OP_EQ, 2); + tt_assert(smartlist_get(new, 0) == s1); + tt_assert(smartlist_get(new, 1) == e1); + /* Cleanup for next test. */ + smartlist_clear(new); + } + + { + rend_intro_point_t ip1; + /* This IP should be found in the s2 service after pruning. */ + smartlist_add(s1->intro_nodes, &ip1); + /* Setup our list. */ + smartlist_add(old, s1); + smartlist_add(new, s2); + prune_services_on_reload(old, new); + tt_int_op(smartlist_len(old), OP_EQ, 1); + /* Intro nodes have been moved to the s2 in theory so it must be empty. */ + tt_int_op(smartlist_len(s1->intro_nodes), OP_EQ, 0); + tt_int_op(smartlist_len(new), OP_EQ, 1); + rend_service_t *elem = smartlist_get(new, 0); + tt_assert(elem); + tt_assert(elem == s2); + tt_int_op(smartlist_len(elem->intro_nodes), OP_EQ, 1); + tt_assert(smartlist_get(elem->intro_nodes, 0) == &ip1); + smartlist_clear(s1->intro_nodes); + smartlist_clear(s2->intro_nodes); + /* Cleanup for next test. */ + smartlist_clear(new); + smartlist_clear(old); + } + + { + /* Test two ephemeral services. */ + smartlist_add(old, e1); + smartlist_add(old, e2); + prune_services_on_reload(old, new); + /* Check if they've all been transfered. */ + tt_int_op(smartlist_len(old), OP_EQ, 0); + tt_int_op(smartlist_len(new), OP_EQ, 2); + } + + done: + rend_service_free(s1); + rend_service_free(s2); + rend_service_free(e1); + rend_service_free(e2); + smartlist_free(new); + smartlist_free(old); +} + struct testcase_t hs_tests[] = { { "hs_rend_data", test_hs_rend_data, TT_FORK, NULL, NULL }, @@ -807,6 +927,9 @@ struct testcase_t hs_tests[] = { TT_FORK, &passthrough_setup, (void*)(CREATE_HS_DIR2) }, { "single_onion_poisoning_create_dir_both", test_single_onion_poisoning, TT_FORK, &passthrough_setup, (void*)(CREATE_HS_DIR1 | CREATE_HS_DIR2) }, + { "prune_services_on_reload", test_prune_services_on_reload, TT_FORK, + NULL, NULL }, + END_OF_TESTCASES }; diff --git a/src/test/test_hs_intropoint.c b/src/test/test_hs_intropoint.c index 608988ba9a..dbd884cf3b 100644 --- a/src/test/test_hs_intropoint.c +++ b/src/test/test_hs_intropoint.c @@ -12,12 +12,14 @@ #define CIRCUITLIST_PRIVATE #include "test.h" +#include "log_test_helpers.h" #include "crypto.h" #include "or.h" #include "ht.h" #include "hs/cell_establish_intro.h" +#include "hs_common.h" #include "hs_service.h" #include "hs_circuitmap.h" #include "hs_intropoint.h" @@ -143,6 +145,163 @@ test_establish_intro_wrong_keytype2(void *arg) circuit_free(TO_CIRCUIT(intro_circ)); } +/* Send a legit ESTABLISH_INTRO cell but with a wrong MAC. Should fail. */ +static void +test_establish_intro_wrong_mac(void *arg) +{ + int retval; + hs_cell_establish_intro_t *establish_intro_cell = NULL; + or_circuit_t *intro_circ = or_circuit_new(0,NULL);; + uint8_t cell_body[RELAY_PAYLOAD_SIZE]; + ssize_t cell_len = 0; + uint8_t circuit_key_material[DIGEST_LEN] = {0}; + + (void)arg; + + /* Get the auth key of the intro point */ + crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material)); + helper_prepare_circ_for_intro(intro_circ, circuit_key_material); + + /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we + attempt to parse it. */ + establish_intro_cell = generate_establish_intro_cell(circuit_key_material, + sizeof(circuit_key_material)); + tt_assert(establish_intro_cell); + /* Mangle one byte of the MAC. */ + uint8_t *handshake_ptr = + hs_cell_establish_intro_getarray_handshake_mac(establish_intro_cell); + handshake_ptr[TRUNNEL_SHA3_256_LEN - 1]++; + /* We need to resign the payload with that change. */ + { + ed25519_signature_t sig; + ed25519_keypair_t key_struct; + /* New keypair for the signature since we don't have access to the private + * key material generated earlier when creating the cell. */ + retval = ed25519_keypair_generate(&key_struct, 0); + tt_int_op(retval, OP_EQ, 0); + uint8_t *auth_key_ptr = + hs_cell_establish_intro_getarray_auth_key(establish_intro_cell); + memcpy(auth_key_ptr, key_struct.pubkey.pubkey, ED25519_PUBKEY_LEN); + /* Encode payload so we can sign it. */ + cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body), + establish_intro_cell); + tt_int_op(cell_len, >, 0); + + retval = ed25519_sign_prefixed(&sig, cell_body, + cell_len - + (ED25519_SIG_LEN + + sizeof(establish_intro_cell->sig_len)), + ESTABLISH_INTRO_SIG_PREFIX, &key_struct); + tt_int_op(retval, OP_EQ, 0); + /* And write the signature to the cell */ + uint8_t *sig_ptr = + hs_cell_establish_intro_getarray_sig(establish_intro_cell); + memcpy(sig_ptr, sig.sig, establish_intro_cell->sig_len); + /* Re-encode with the new signature. */ + cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body), + establish_intro_cell); + } + + /* Receive the cell. Should fail because our MAC is wrong. */ + setup_full_capture_of_logs(LOG_INFO); + retval = hs_intro_received_establish_intro(intro_circ, cell_body, cell_len); + expect_log_msg_containing("ESTABLISH_INTRO handshake_auth not as expected"); + teardown_capture_of_logs(); + tt_int_op(retval, ==, -1); + + done: + hs_cell_establish_intro_free(establish_intro_cell); + circuit_free(TO_CIRCUIT(intro_circ)); +} + +/* Send a legit ESTABLISH_INTRO cell but with a wrong auth key length. Should + * fail. */ +static void +test_establish_intro_wrong_auth_key_len(void *arg) +{ + int retval; + hs_cell_establish_intro_t *establish_intro_cell = NULL; + or_circuit_t *intro_circ = or_circuit_new(0,NULL);; + uint8_t cell_body[RELAY_PAYLOAD_SIZE]; + ssize_t cell_len = 0; + size_t bad_auth_key_len = ED25519_PUBKEY_LEN - 1; + uint8_t circuit_key_material[DIGEST_LEN] = {0}; + + (void)arg; + + /* Get the auth key of the intro point */ + crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material)); + helper_prepare_circ_for_intro(intro_circ, circuit_key_material); + + /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we + attempt to parse it. */ + establish_intro_cell = generate_establish_intro_cell(circuit_key_material, + sizeof(circuit_key_material)); + tt_assert(establish_intro_cell); + /* Mangle the auth key length. */ + hs_cell_establish_intro_set_auth_key_len(establish_intro_cell, + bad_auth_key_len); + hs_cell_establish_intro_setlen_auth_key(establish_intro_cell, + bad_auth_key_len); + cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body), + establish_intro_cell); + tt_int_op(cell_len, >, 0); + + /* Receive the cell. Should fail. */ + setup_full_capture_of_logs(LOG_INFO); + retval = hs_intro_received_establish_intro(intro_circ, cell_body, cell_len); + expect_log_msg_containing("ESTABLISH_INTRO auth key length is invalid"); + teardown_capture_of_logs(); + tt_int_op(retval, ==, -1); + + done: + hs_cell_establish_intro_free(establish_intro_cell); + circuit_free(TO_CIRCUIT(intro_circ)); +} + +/* Send a legit ESTABLISH_INTRO cell but with a wrong sig length. Should + * fail. */ +static void +test_establish_intro_wrong_sig_len(void *arg) +{ + int retval; + hs_cell_establish_intro_t *establish_intro_cell = NULL; + or_circuit_t *intro_circ = or_circuit_new(0,NULL);; + uint8_t cell_body[RELAY_PAYLOAD_SIZE]; + ssize_t cell_len = 0; + size_t bad_sig_len = ED25519_SIG_LEN - 1; + uint8_t circuit_key_material[DIGEST_LEN] = {0}; + + (void)arg; + + /* Get the auth key of the intro point */ + crypto_rand((char *) circuit_key_material, sizeof(circuit_key_material)); + helper_prepare_circ_for_intro(intro_circ, circuit_key_material); + + /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we + attempt to parse it. */ + establish_intro_cell = generate_establish_intro_cell(circuit_key_material, + sizeof(circuit_key_material)); + tt_assert(establish_intro_cell); + /* Mangle the signature length. */ + hs_cell_establish_intro_set_sig_len(establish_intro_cell, bad_sig_len); + hs_cell_establish_intro_setlen_sig(establish_intro_cell, bad_sig_len); + cell_len = get_establish_intro_payload(cell_body, sizeof(cell_body), + establish_intro_cell); + tt_int_op(cell_len, >, 0); + + /* Receive the cell. Should fail. */ + setup_full_capture_of_logs(LOG_INFO); + retval = hs_intro_received_establish_intro(intro_circ, cell_body, cell_len); + expect_log_msg_containing("ESTABLISH_INTRO sig len is invalid"); + teardown_capture_of_logs(); + tt_int_op(retval, ==, -1); + + done: + hs_cell_establish_intro_free(establish_intro_cell); + circuit_free(TO_CIRCUIT(intro_circ)); +} + /* Send a legit ESTABLISH_INTRO cell but slightly change the signature. Should * fail. */ static void @@ -356,6 +515,15 @@ struct testcase_t hs_intropoint_tests[] = { { "receive_establish_intro_wrong_sig", test_establish_intro_wrong_sig, TT_FORK, NULL, NULL }, + { "receive_establish_intro_wrong_sig_len", + test_establish_intro_wrong_sig_len, TT_FORK, NULL, NULL }, + + { "receive_establish_intro_wrong_auth_key_len", + test_establish_intro_wrong_auth_key_len, TT_FORK, NULL, NULL }, + + { "receive_establish_intro_wrong_mac", + test_establish_intro_wrong_mac, TT_FORK, NULL, NULL }, + END_OF_TESTCASES }; diff --git a/src/test/test_options.c b/src/test/test_options.c index 29ec1e9824..41c015bdd4 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -333,7 +333,8 @@ fixed_get_uname(void) "VirtualAddrNetworkIPv4 127.192.0.0/10\n" \ "VirtualAddrNetworkIPv6 [FE80::]/10\n" \ "SchedulerHighWaterMark__ 42\n" \ - "SchedulerLowWaterMark__ 10\n" + "SchedulerLowWaterMark__ 10\n" \ + "UseEntryGuards 1\n" typedef struct { or_options_t *old_opt; @@ -1797,14 +1798,6 @@ test_options_validate__reachable_addresses(void *ignored) /* Test IPv4-only clients setting IPv6 preferences */ -#define WARN_PLEASE_USE_IPV6_OR_LOG_MSG \ - "ClientPreferIPv6ORPort 1 is ignored unless tor is using IPv6. " \ - "Please set ClientUseIPv6 1, ClientUseIPv4 0, or configure bridges.\n" - -#define WARN_PLEASE_USE_IPV6_DIR_LOG_MSG \ - "ClientPreferIPv6DirPort 1 is ignored unless tor is using IPv6. " \ - "Please set ClientUseIPv6 1, ClientUseIPv4 0, or configure bridges.\n" - free_options_test_data(tdata); tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES "ClientUseIPv4 1\n" @@ -1814,7 +1807,6 @@ test_options_validate__reachable_addresses(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - expect_log_msg(WARN_PLEASE_USE_IPV6_OR_LOG_MSG); tor_free(msg); free_options_test_data(tdata); @@ -1826,7 +1818,6 @@ test_options_validate__reachable_addresses(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - expect_log_msg(WARN_PLEASE_USE_IPV6_DIR_LOG_MSG); tor_free(msg); /* Now test an IPv4/IPv6 client setting IPv6 preferences */ @@ -1948,6 +1939,19 @@ test_options_validate__use_bridges(void *ignored) tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES "UseBridges 1\n" "Bridge 10.0.0.1\n" + "UseEntryGuards 0\n" + ); + + 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, + "Setting UseBridges requires also setting UseEntryGuards."); + tor_free(msg); + + free_options_test_data(tdata); + tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES + "UseBridges 1\n" + "Bridge 10.0.0.1\n" "Bridge !!!\n" ); |