From d9bd0de0628ff3ac9d84409315ad6df38981f019 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 31 Jul 2012 10:16:03 -0400 Subject: [PATCH 01/14] Fix some manpage typos This is based on a pair of patches from A. Costa. I couldn't apply those directly, since they changed the generated *roff files, not the asciidoc source. Fixes Tor bug 6500 and Debian bug 683359. --- changes/bug6500 | 2 ++ doc/tor.1.txt | 4 ++-- doc/torify.1.txt | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) create mode 100644 changes/bug6500 diff --git a/changes/bug6500 b/changes/bug6500 new file mode 100644 index 0000000000..cac2054a3a --- /dev/null +++ b/changes/bug6500 @@ -0,0 +1,2 @@ + o Minor bugfixes: + - Fix some typos in the manpages. Patch from A. Costa. Fixes bug 6500. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 78c34874c5..e7ba8485c0 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -169,7 +169,7 @@ GENERAL OPTIONS duty of that proxy to properly forward the traffic to the bridge. + + In its second form, when set along with a corresponding Bridge line, the Tor - client launches the pluggable transport proxy exacutable in + client launches the pluggable transport proxy executable in __path-to-binary__ using __options__ as its command-line options, and forwards its traffic to it. It's the duty of that proxy to properly forward the traffic to the bridge. @@ -1709,7 +1709,7 @@ DIRECTORY AUTHORITY SERVER OPTIONS **V3BandwidthsFile** __FILENAME__:: V3 authoritative directories only. Configures the location of the - bandiwdth-authority generated file storing information on relays' measured + bandwidth-authority generated file storing information on relays' measured bandwidth capacities. (Default: unset) **V3AuthUseLegacyKey** **0**|**1**:: diff --git a/doc/torify.1.txt b/doc/torify.1.txt index ca2c385c94..4a4be1250a 100644 --- a/doc/torify.1.txt +++ b/doc/torify.1.txt @@ -21,7 +21,7 @@ DESCRIPTION wrapper available on a system. It calls torsocks or tsocks with a tor specific configuration file. + -torsocks is an improved wrapper that explictly rejects UDP, safely resolves DNS +torsocks is an improved wrapper that explicitly rejects UDP, safely resolves DNS lookups and properly socksifies your TCP connections. + tsocks itself is a wrapper between the tsocks library and the application that From 2bd45213c991df62bf184758c5e8222685bda1ba Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 30 Jul 2012 11:58:55 -0400 Subject: [PATCH 02/14] Warn when accounting is used in a way likely to link hidden services Fix for 6490. --- changes/bug6490 | 4 ++++ src/or/config.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 changes/bug6490 diff --git a/changes/bug6490 b/changes/bug6490 new file mode 100644 index 0000000000..c92daad8f4 --- /dev/null +++ b/changes/bug6490 @@ -0,0 +1,4 @@ + o Minor features: + - Warn when Tor is configured to use accounting in a way that will + link a hidden service to some other hidden service or public + address. Fix for bug 6490. diff --git a/src/or/config.c b/src/or/config.c index 918efd4cf8..a369297518 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -594,6 +594,7 @@ static void option_reset(const config_format_t *fmt, or_options_t *options, const config_var_t *var, int use_defaults); static void config_free(const config_format_t *fmt, void *options); static int config_lines_eq(config_line_t *a, config_line_t *b); +static int config_count_key(const config_line_t *a, const char *key); static int option_is_same(const config_format_t *fmt, const or_options_t *o1, const or_options_t *o2, const char *name); @@ -3066,6 +3067,20 @@ config_lines_eq(config_line_t *a, config_line_t *b) return 1; } +/** Return the number of lines in a whose key is key. */ +static int +config_count_key(const config_line_t *a, const char *key) +{ + int n = 0; + while (a) { + if (!strcasecmp(a->key, key)) { + ++n; + } + a = a->next; + } + return n; +} + /** Return true iff the option name has the same value in o1 * and o2. Must not be called for LINELIST_S or OBSOLETE options. */ @@ -3848,6 +3863,20 @@ options_validate(or_options_t *old_options, or_options_t *options, if (accounting_parse_options(options, 1)<0) REJECT("Failed to parse accounting options. See logs for details."); + if (options->AccountingMax) { + if (options->RendConfigLines && server_mode(options)) { + log_warn(LD_CONFIG, "Using accounting with a hidden service and an " + "ORPort is risky: your hidden service(s) and your public " + "address will all turn off at the same time, which may alert " + "observers that they are being run by the same party."); + } else if (config_count_key(options->RendConfigLines, + "HiddenServiceDir") > 1) { + log_warn(LD_CONFIG, "Using accounting with multiple hidden services is " + "risky: they will all turn off at the same time, which may " + "alert observers that they are being run by the same party."); + } + } + if (options->HTTPProxy) { /* parse it now */ if (tor_addr_port_lookup(options->HTTPProxy, &options->HTTPProxyAddr, &options->HTTPProxyPort) < 0) From cf20fac37f78cc1212eaf0dc5260eabf46bcea92 Mon Sep 17 00:00:00 2001 From: Ondrej Mikle Date: Fri, 8 Jun 2012 15:41:53 +0200 Subject: [PATCH 03/14] Dependencies for RPM building with mock should work now --- doc/tor-rpm-creation.txt | 19 ++++++++++++++++++- tor.spec.in | 8 ++++---- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/doc/tor-rpm-creation.txt b/doc/tor-rpm-creation.txt index 3fc6bde79c..eb22c2225a 100644 --- a/doc/tor-rpm-creation.txt +++ b/doc/tor-rpm-creation.txt @@ -3,7 +3,7 @@ The process used to create the official rpms is as follows: Download latest stable libevent from -http://www.monkey.org/~provos/libevent/ +http://libevent.org/ The first step of compiling libevent is to configure it as follows: ./configure --enable-static --disable-shared @@ -24,3 +24,20 @@ make dist-rpm You should have at least two, maybe three, rpms. There should be the binary i386.rpm, a src.rpm, and on redhat/centos machines, a debuginfo.rpm. + + +## Instructions for building RPMs for multiple architectures or distributions +## using 'mock' + +Make sure you have mock installed and configured, see following HOWTOs for setup: +https://fedoraproject.org/wiki/How_to_create_an_RPM_package +https://fedoraproject.org/wiki/Using_Mock_to_test_package_builds + +Take the source RPM generated by previous step, and execute mock for every +target architecture (the names come from files in /etc/mock, strip the .cfg +extension in the -r parameter): + +mock --rebuild -r fedora-17-x86_64 tor-X.Y.Z.src.rpm + +(Note: don't build under OpenVZ - it breaks unshare() syscall, which in turn +breaks mock. It could save you several hours.) diff --git a/tor.spec.in b/tor.spec.in index 25557b73e2..127bf361d7 100644 --- a/tor.spec.in +++ b/tor.spec.in @@ -109,11 +109,11 @@ Vendor: The Tor Project (https://torproject.org) Packager: Erinn Clark %if %{is_suse} -Requires: openssl >= 0.9.7 -BuildRequires: openssl-devel >= 0.9.7, rpm >= 4.0, zlib-devel +Requires: openssl >= 0.9.7, libevent >= 1.4.13 +BuildRequires: openssl-devel >= 0.9.7, rpm >= 4.0, zlib-devel, libevent-devel >= 1.4.13, asciidoc %else -Requires: openssl >= 0.9.7 -BuildRequires: openssl-devel >= 0.9.7 +Requires: openssl >= 0.9.7, libevent >= 1.4.13 +BuildRequires: openssl-devel >= 0.9.7, libevent-devel >= 1.4.13, asciidoc %endif %if %{is_fc} BuildRequires: rpm-build >= 4.0 From 72260cfa680ac1fd77d8859d471f766853ddc063 Mon Sep 17 00:00:00 2001 From: Ondrej Mikle Date: Fri, 15 Jun 2012 23:59:35 +0200 Subject: [PATCH 04/14] Cleanup of RPM build process - no more .git in tarball, use automake's dist-gzip to create the tarball. Signed-off-by: Ondrej Mikle --- Makefile.am | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/Makefile.am b/Makefile.am index a80763d929..e25e8ff2a2 100644 --- a/Makefile.am +++ b/Makefile.am @@ -24,23 +24,18 @@ EXTRA_DIST = \ #install-data-local: # $(INSTALL) -m 755 -d $(LOCALSTATEDIR)/lib/tor -# Assume a tarball is in .. for now. -dist-rpm: - RPM_BUILD_DIR="/tmp/tor-rpm-build-$$$$"; \ - rm -rf $$RPM_BUILD_DIR; \ - mkdir $$RPM_BUILD_DIR || exit 1; \ - for subdir in BUILD RPMS SOURCES SPECS SRPMS; do \ - mkdir $$RPM_BUILD_DIR/$$subdir; \ - done; \ - mkdir $$RPM_BUILD_DIR/SOURCES/tor-$(VERSION); \ - cp -R ./ $$RPM_BUILD_DIR/SOURCES/tor-$(VERSION)/; \ - pushd $$RPM_BUILD_DIR/SOURCES/; \ - tar zcf tor-$(VERSION).tar.gz ./; \ - popd; \ - LIBS=-lrt rpmbuild -ba --define "_topdir $$RPM_BUILD_DIR" tor.spec; \ - mv $$RPM_BUILD_DIR/SRPMS/* .; \ - mv $$RPM_BUILD_DIR/RPMS/* .; \ - rm -rf $$RPM_BUILD_DIR +# Use automake's dist-gzip target to build the tarball +dist-rpm: dist-gzip + TIMESTAMP=$$(date +"%Y-%m-%d_%H.%M.%S"); \ + RPM_BUILD_DIR=$$(mktemp -d "/tmp/tor-rpm-build-$$TIMESTAMP-XXXX"); \ + mkdir -p "$$RPM_BUILD_DIR"/{BUILD,RPMS,SOURCES/"tor-$(VERSION)",SPECS,SRPMS}; \ + cp -fa "$(distdir).tar.gz" "$$RPM_BUILD_DIR"/SOURCES/; \ + LIBS=-lrt rpmbuild -ba --define "_topdir $$RPM_BUILD_DIR" tor.spec; \ + cp -fa "$$RPM_BUILD_DIR"/SRPMS/* .; \ + cp -fa "$$RPM_BUILD_DIR"/RPMS/* .; \ + rm -rf "$$RPM_BUILD_DIR"; \ + echo "RPM build finished"; \ + #end of dist-rpm dist: check From 0e778ac6044f991bdd144a36258df341aa969554 Mon Sep 17 00:00:00 2001 From: Ondrej Mikle Date: Sat, 16 Jun 2012 18:38:14 +0200 Subject: [PATCH 05/14] Workaround for building EL5 RPMs by specifying rpmbuild-md5. Updated old note about using static libevent when building RPMs. --- Makefile.am | 6 +++++- doc/tor-rpm-creation.txt | 35 +++++++++++++++++++++++------------ 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/Makefile.am b/Makefile.am index e25e8ff2a2..29bba715bd 100644 --- a/Makefile.am +++ b/Makefile.am @@ -24,13 +24,17 @@ EXTRA_DIST = \ #install-data-local: # $(INSTALL) -m 755 -d $(LOCALSTATEDIR)/lib/tor +# Allows to override rpmbuild with rpmbuild-md5 from fedora-packager so that +# building for EL5 won't fail on https://bugzilla.redhat.com/show_bug.cgi?id=490613 +RPMBUILD ?= rpmbuild + # Use automake's dist-gzip target to build the tarball dist-rpm: dist-gzip TIMESTAMP=$$(date +"%Y-%m-%d_%H.%M.%S"); \ RPM_BUILD_DIR=$$(mktemp -d "/tmp/tor-rpm-build-$$TIMESTAMP-XXXX"); \ mkdir -p "$$RPM_BUILD_DIR"/{BUILD,RPMS,SOURCES/"tor-$(VERSION)",SPECS,SRPMS}; \ cp -fa "$(distdir).tar.gz" "$$RPM_BUILD_DIR"/SOURCES/; \ - LIBS=-lrt rpmbuild -ba --define "_topdir $$RPM_BUILD_DIR" tor.spec; \ + LIBS=-lrt $(RPMBUILD) -ba --define "_topdir $$RPM_BUILD_DIR" tor.spec; \ cp -fa "$$RPM_BUILD_DIR"/SRPMS/* .; \ cp -fa "$$RPM_BUILD_DIR"/RPMS/* .; \ rm -rf "$$RPM_BUILD_DIR"; \ diff --git a/doc/tor-rpm-creation.txt b/doc/tor-rpm-creation.txt index eb22c2225a..a88f2e8290 100644 --- a/doc/tor-rpm-creation.txt +++ b/doc/tor-rpm-creation.txt @@ -2,18 +2,9 @@ ## The process used to create the official rpms is as follows: -Download latest stable libevent from -http://libevent.org/ - -The first step of compiling libevent is to configure it as follows: -./configure --enable-static --disable-shared - -Complete the "make" and "make install". You will need to be root, -or sudo -s, to complete the "make install". - -Check for a successful universal binary of libevent.a in, by default, -/usr/local/lib by using the following command: - "file /usr/local/lib/libevent.a" +You'll need to install libevent headers, usually located in package named +libevent-devel. Alternatively, you could download latest libevent from +http://libevent.org/ but that shouldn't be necessary. Download and Extract the latest tor source code from https://www.torproject.org/download @@ -25,6 +16,9 @@ make dist-rpm You should have at least two, maybe three, rpms. There should be the binary i386.rpm, a src.rpm, and on redhat/centos machines, a debuginfo.rpm. +This step suffices unless you want to create RPMs for distros other than the +one you used for building. + ## Instructions for building RPMs for multiple architectures or distributions ## using 'mock' @@ -39,5 +33,22 @@ extension in the -r parameter): mock --rebuild -r fedora-17-x86_64 tor-X.Y.Z.src.rpm +Building for EL5 from newer distro (e.g. EL6 or Fedora 17) will fail due to bug +(https://bugzilla.redhat.com/show_bug.cgi?id=490613). +Here's a workaround: + +Before even building the source RPM, install fedora-packager and instruct +the build system to use rpmbuild-md5 like this: + +yum install fedora-packager +export RPMBUILD=rpmbuild-md5 + +Then proceed as usual to create the source RPM and binary RPMs: + +LIBS=-lrt ./configure +make dist-rpm +mock --rebuild -r epel-5-x86_64 tor-X.Y.Z.src.rpm + + (Note: don't build under OpenVZ - it breaks unshare() syscall, which in turn breaks mock. It could save you several hours.) From a7fc797edffc3ae6c2813ae354be35a1cd3c175e Mon Sep 17 00:00:00 2001 From: Ondrej Mikle Date: Sat, 16 Jun 2012 22:41:59 +0200 Subject: [PATCH 06/14] Fixed creating _tor user in RPM-based installs. --- tor.spec.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tor.spec.in b/tor.spec.in index 127bf361d7..5605586323 100644 --- a/tor.spec.in +++ b/tor.spec.in @@ -213,7 +213,7 @@ if [ ! -n "`/usr/bin/id -u %{toruser} 2>/dev/null`" ]; then # One would also like to default the UID, but doing that properly would # also require thought. if [ -x %{_sbindir}/nologin ]; then - %{_sbindir}/useradd -r -g %{torgroup} -d% {_localstatedir}/lib/%{name} -s %{_sbindir}/nologin %{toruser} 2> /dev/null + %{_sbindir}/useradd -r -g %{torgroup} -d %{_localstatedir}/lib/%{name} -s %{_sbindir}/nologin %{toruser} 2> /dev/null else %{_sbindir}/useradd -r -g %{torgroup} -d %{_localstatedir}/lib/%{name} -s /bin/false %{toruser} 2> /dev/null fi From eeb81b5bb1135c1b85fff68da823c10fc4a99965 Mon Sep 17 00:00:00 2001 From: Ondrej Mikle Date: Thu, 21 Jun 2012 18:26:05 +0200 Subject: [PATCH 07/14] Fixes/beautification of RPM spec. Tiny improvements in RPM build docs. --- doc/tor-rpm-creation.txt | 6 ++++-- tor.spec.in | 21 +++++++++++++-------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/doc/tor-rpm-creation.txt b/doc/tor-rpm-creation.txt index a88f2e8290..a03891e2b9 100644 --- a/doc/tor-rpm-creation.txt +++ b/doc/tor-rpm-creation.txt @@ -14,14 +14,16 @@ LIBS=-lrt ./configure make dist-rpm You should have at least two, maybe three, rpms. There should be the binary -i386.rpm, a src.rpm, and on redhat/centos machines, a debuginfo.rpm. +(i686|x86_64).rpm, a src.rpm, and on redhat/centos machines, a debuginfo.rpm. +The debuginfo rpms are created if package redhat-rpm-config is installed (case +of redhat distros). This step suffices unless you want to create RPMs for distros other than the one you used for building. ## Instructions for building RPMs for multiple architectures or distributions -## using 'mock' +## using 'mock' on Fedora or RHEL (and clones) Make sure you have mock installed and configured, see following HOWTOs for setup: https://fedoraproject.org/wiki/How_to_create_an_RPM_package diff --git a/tor.spec.in b/tor.spec.in index 5605586323..b452c9bb35 100644 --- a/tor.spec.in +++ b/tor.spec.in @@ -6,7 +6,7 @@ # This should be incremented whenever the spec file changes, but # can drop back to zero at a new Tor version -%define specver 0 +%define specver 1 ## Things users may want to change # @@ -108,17 +108,14 @@ License: 3-clause BSD Vendor: The Tor Project (https://torproject.org) Packager: Erinn Clark -%if %{is_suse} -Requires: openssl >= 0.9.7, libevent >= 1.4.13 -BuildRequires: openssl-devel >= 0.9.7, rpm >= 4.0, zlib-devel, libevent-devel >= 1.4.13, asciidoc -%else Requires: openssl >= 0.9.7, libevent >= 1.4.13 BuildRequires: openssl-devel >= 0.9.7, libevent-devel >= 1.4.13, asciidoc -%endif -%if %{is_fc} -BuildRequires: rpm-build >= 4.0 + +# Fedora 16 and RHEL 5 have following conflicting packages according to rpm search +%if %{is_rh} Conflicts: tor-core, tor-lsb, tor-upstart %endif + Requires(pre): /usr/bin/id, /bin/date, /bin/sh Requires(pre): %{_sbindir}/useradd, %{_sbindir}/groupadd @@ -287,6 +284,14 @@ exit 0 %attr(0750,%{toruser},%{torgroup}) %dir %{_localstatedir}/log/%{name} %changelog +* Thu Jun 21 2012 Ondrej Mikle +- fixed to work with both rpmbuild and mock on RHEL/Fedora +- removed unnecessary files from rpm such as .git repo +- fixed build dependencies and package conflicts +- fixed creating _tor user on Fedora 17 (ancient typo) +- added/updated build instructions for RPM creation +- confirmed to build and run on EL5, EL6, Fedora 16/17, OpenSuse 12.1 + * Fri Aug 20 2010 Erinn Clark - add conflicts for Fedora packages - add logic for SuSE since it requires special doc handling From ae75fb13e175ec24cb3bff44150fd2f932aa5385 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 31 Jul 2012 11:49:20 -0400 Subject: [PATCH 08/14] changelog for bug6043 --- changes/bug6043 | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changes/bug6043 diff --git a/changes/bug6043 b/changes/bug6043 new file mode 100644 index 0000000000..b88bafb788 --- /dev/null +++ b/changes/bug6043 @@ -0,0 +1,6 @@ + o Packaging (RPM): + - Our default RPM spec files have been updated to work with mock + and rpmbuild on RHEL/Fedora. They have an updated set of + dependencies and conflicts, a fix for an ancient typo when creating + the "_tor" user, and better instructions. Thanks to Ondrej + Mikle for the patch series; fix for bug 6043. From 62637fa22405278758febb1743da9af562524d4c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 31 Jul 2012 12:58:19 -0400 Subject: [PATCH 09/14] Avoid hard (impossible?)-to-trigger double-free in dns_resolve() Fixes 6480; fix on 0.2.0.1-alpha; based on pseudonymous patch. --- changes/bug6480 | 5 +++++ src/or/dns.c | 30 ++++++++++++++++++------------ 2 files changed, 23 insertions(+), 12 deletions(-) create mode 100644 changes/bug6480 diff --git a/changes/bug6480 b/changes/bug6480 new file mode 100644 index 0000000000..83ae00b251 --- /dev/null +++ b/changes/bug6480 @@ -0,0 +1,5 @@ + o Major bugfixes: + - Avoid read-from-freed-RAM bug and related double-free bug that + could occur when a DNS request fails while launching it. Fixes + bug 6480; bugfix on 0.2.0.1-alpha. + diff --git a/src/or/dns.c b/src/or/dns.c index da11668c6d..3e88fad68c 100644 --- a/src/or/dns.c +++ b/src/or/dns.c @@ -168,7 +168,8 @@ static void add_wildcarded_test_address(const char *address); static int configure_nameservers(int force); static int answer_is_wildcarded(const char *ip); static int dns_resolve_impl(edge_connection_t *exitconn, int is_resolve, - or_circuit_t *oncirc, char **resolved_to_hostname); + or_circuit_t *oncirc, char **resolved_to_hostname, + int *made_connection_pending_out); #ifdef DEBUG_DNS_CACHE static void _assert_cache_ok(void); #define assert_cache_ok() _assert_cache_ok() @@ -596,10 +597,12 @@ dns_resolve(edge_connection_t *exitconn) { or_circuit_t *oncirc = TO_OR_CIRCUIT(exitconn->on_circuit); int is_resolve, r; + int made_connection_pending = 0; char *hostname = NULL; is_resolve = exitconn->_base.purpose == EXIT_PURPOSE_RESOLVE; - r = dns_resolve_impl(exitconn, is_resolve, oncirc, &hostname); + r = dns_resolve_impl(exitconn, is_resolve, oncirc, &hostname, + &made_connection_pending); switch (r) { case 1: @@ -639,16 +642,11 @@ dns_resolve(edge_connection_t *exitconn) dns_cancel_pending_resolve(exitconn->_base.address); - if (!exitconn->_base.marked_for_close) { + if (!made_connection_pending && !exitconn->_base.marked_for_close) { + /* If we made the connection pending, then we freed it already in + * dns_cancel_pending_resolve(). If we marked it for close, it'll + * get freed from the main loop. Otherwise, can free it now. */ connection_free(TO_CONN(exitconn)); - // XXX ... and we just leak exitconn otherwise? -RD - // If it's marked for close, it's on closeable_connection_lst in - // main.c. If it's on the closeable list, it will get freed from - // main.c. -NM - // " If that's true, there are other bugs around, where we - // don't check if it's marked, and will end up double-freeing." - // On the other hand, I don't know of any actual bugs here, so this - // shouldn't be holding up the rc. -RD } break; default: @@ -667,10 +665,15 @@ dns_resolve(edge_connection_t *exitconn) * Return -2 on a transient error. If it's a reverse resolve and it's * successful, sets *hostname_out to a newly allocated string * holding the cached reverse DNS value. + * + * Set *made_connection_pending_out to true if we have placed + * exitconn on the list of pending connections for some resolve; set it + * to false otherwise. */ static int dns_resolve_impl(edge_connection_t *exitconn, int is_resolve, - or_circuit_t *oncirc, char **hostname_out) + or_circuit_t *oncirc, char **hostname_out, + int *made_connection_pending_out) { cached_resolve_t *resolve; cached_resolve_t search; @@ -684,6 +687,7 @@ dns_resolve_impl(edge_connection_t *exitconn, int is_resolve, tor_assert(!SOCKET_OK(exitconn->_base.s)); assert_cache_ok(); tor_assert(oncirc); + *made_connection_pending_out = 0; /* first check if exitconn->_base.address is an IP. If so, we already * know the answer. */ @@ -757,6 +761,7 @@ dns_resolve_impl(edge_connection_t *exitconn, int is_resolve, pending_connection->conn = exitconn; pending_connection->next = resolve->pending_connections; resolve->pending_connections = pending_connection; + *made_connection_pending_out = 1; log_debug(LD_EXIT,"Connection (fd %d) waiting for pending DNS " "resolve of %s", exitconn->_base.s, escaped_safe_str(exitconn->_base.address)); @@ -797,6 +802,7 @@ dns_resolve_impl(edge_connection_t *exitconn, int is_resolve, pending_connection = tor_malloc_zero(sizeof(pending_connection_t)); pending_connection->conn = exitconn; resolve->pending_connections = pending_connection; + *made_connection_pending_out = 1; /* Add this resolve to the cache and priority queue. */ HT_INSERT(cache_map, &cache_root, resolve); From 57e35ad3d91724882c345ac709666a551a977f0f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 3 Aug 2012 10:53:00 -0400 Subject: [PATCH 10/14] Avoid possible segfault when handling networkstatus vote with bad flavor Fix for 6530; fix on 0.2.2.6-alpha. --- changes/bug6530 | 4 ++++ src/or/routerparse.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 changes/bug6530 diff --git a/changes/bug6530 b/changes/bug6530 new file mode 100644 index 0000000000..a6b7caa1a0 --- /dev/null +++ b/changes/bug6530 @@ -0,0 +1,4 @@ + o Major bugfixes: + - Avoid a read of uninitializd RAM when reading a vote or consensus + document with an unrecognized flavor name. Fixes bug 6530; bugfix on + 0.2.2.6-alpha. diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 8c4f582c07..2ff546bb1d 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -2821,7 +2821,7 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, int flavor = networkstatus_parse_flavor_name(tok->args[1]); if (flavor < 0) { log_warn(LD_DIR, "Can't parse document with unknown flavor %s", - escaped(tok->args[2])); + escaped(tok->args[1])); goto err; } ns->flavor = flav = flavor; From 55f635745afacefffdaafc72cc176ca7ab817546 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 3 Aug 2012 11:16:13 -0400 Subject: [PATCH 11/14] Clarify security impact of bug 6530 --- changes/bug6530 | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/changes/bug6530 b/changes/bug6530 index a6b7caa1a0..825bbb752a 100644 --- a/changes/bug6530 +++ b/changes/bug6530 @@ -1,4 +1,5 @@ - o Major bugfixes: + o Major security fixes: - Avoid a read of uninitializd RAM when reading a vote or consensus - document with an unrecognized flavor name. Fixes bug 6530; bugfix on - 0.2.2.6-alpha. + document with an unrecognized flavor name. This could lead to a + remote crash bug. Fixes bug 6530; bugfix on 0.2.2.6-alpha. + From 82c5e385cbddec4fd80618d6e96111ad73d5a22e Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Thu, 14 Jun 2012 15:41:11 +0000 Subject: [PATCH 12/14] Remove bogus comment claiming that an assertion is triggerable by consensus --- src/or/routerlist.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/or/routerlist.c b/src/or/routerlist.c index de1a66ce16..37de70f0b5 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -1983,9 +1983,10 @@ smartlist_choose_node_by_bandwidth(smartlist_t *sl, if (is_guard) bitarray_set(guard_bits, i); if (is_known) { - bandwidths[i] = (int32_t) this_bw; // safe since MAX_BELIEVABLE= 0); if (is_guard) total_guard_bw += this_bw; From 308f6dad20675c42b29862f4269ad1fbfb00dc9a Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Thu, 14 Jun 2012 17:15:54 +0000 Subject: [PATCH 13/14] Mitigate a side-channel leak of which relays Tor chooses for a circuit Tor's and OpenSSL's current design guarantee that there are other leaks, but this one is likely to be more easily exploitable, and is easy to fix. --- changes/pathsel-BUGGY-a | 12 ++++++++++++ src/or/routerlist.c | 22 ++++++++++++++++++---- 2 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 changes/pathsel-BUGGY-a diff --git a/changes/pathsel-BUGGY-a b/changes/pathsel-BUGGY-a new file mode 100644 index 0000000000..cad2af5c0d --- /dev/null +++ b/changes/pathsel-BUGGY-a @@ -0,0 +1,12 @@ + o Security fixes: + + - Try to leak less information about what relays a client is + choosing to a side-channel attacker. Previously, a Tor client + would stop iterating through the list of available relays as + soon as it had chosen one, thus leaking information about which + relays it picked for a circuit to a timing attack. (Tor is + likely to still leak information about which relays it has + chosen for a circuit to other processes on the same computer, + through e.g. which cache lines it loads while building the + circuit.) + diff --git a/src/or/routerlist.c b/src/or/routerlist.c index d21b40c57d..30c20bf6e6 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -1674,6 +1674,8 @@ smartlist_choose_by_bandwidth_weights(smartlist_t *sl, double *bandwidths; double tmp = 0; unsigned int i; + unsigned int i_chosen; + unsigned int i_has_been_chosen; int have_unknown = 0; /* true iff sl contains element not in consensus. */ /* Can't choose exit and guard at same time */ @@ -1835,12 +1837,17 @@ smartlist_choose_by_bandwidth_weights(smartlist_t *sl, * from 1 below. See bug 1203 for details. */ /* Last, count through sl until we get to the element we picked */ + i_chosen = (unsigned)smartlist_len(sl); + i_has_been_chosen = 0; tmp = 0.0; for (i=0; i < (unsigned)smartlist_len(sl); i++) { tmp += bandwidths[i]; - if (tmp >= rand_bw) - break; + if (tmp >= rand_bw && !i_has_been_chosen) { + i_chosen = i; + i_has_been_chosen = 1; + } } + i = i_chosen; if (i == (unsigned)smartlist_len(sl)) { /* This was once possible due to round-off error, but shouldn't be able @@ -1877,6 +1884,8 @@ smartlist_choose_by_bandwidth(smartlist_t *sl, bandwidth_weight_rule_t rule, int statuses) { unsigned int i; + unsigned int i_chosen; + unsigned int i_has_been_chosen; routerinfo_t *router; routerstatus_t *status=NULL; int32_t *bandwidths; @@ -2092,6 +2101,8 @@ smartlist_choose_by_bandwidth(smartlist_t *sl, bandwidth_weight_rule_t rule, /* Last, count through sl until we get to the element we picked */ tmp = 0; + i_chosen = (unsigned)smartlist_len(sl); + i_has_been_chosen = 0; for (i=0; i < (unsigned)smartlist_len(sl); i++) { is_exit = bitarray_is_set(exit_bits, i); is_guard = bitarray_is_set(guard_bits, i); @@ -2106,9 +2117,12 @@ smartlist_choose_by_bandwidth(smartlist_t *sl, bandwidth_weight_rule_t rule, else tmp += bandwidths[i]; - if (tmp >= rand_bw) - break; + if (tmp >= rand_bw && !i_has_been_chosen) { + i_chosen = i; + i_has_been_chosen = 1; + } } + i = i_chosen; if (i == (unsigned)smartlist_len(sl)) { /* This was once possible due to round-off error, but shouldn't be able * to occur any longer. */ From d48cebc5e498b0ae673635f40fc57cdddab45d5b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 3 Aug 2012 11:54:11 -0400 Subject: [PATCH 14/14] Try to clarify impact of bug 6537 I don't personally agree that this is likely to be easy to exploit, and some initial experimention I've done suggests that cache-miss times are just plain too fast to get useful info out of when they're mixed up with the rest of Tor's timing noise. Nevertheless, I'm leaving Robert's initial changelog entry in the git history so that he can be the voice of reason if I'm wrong. :) --- changes/pathsel-BUGGY-a | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/changes/pathsel-BUGGY-a b/changes/pathsel-BUGGY-a index cad2af5c0d..2e642c7953 100644 --- a/changes/pathsel-BUGGY-a +++ b/changes/pathsel-BUGGY-a @@ -3,10 +3,12 @@ - Try to leak less information about what relays a client is choosing to a side-channel attacker. Previously, a Tor client would stop iterating through the list of available relays as - soon as it had chosen one, thus leaking information about which - relays it picked for a circuit to a timing attack. (Tor is - likely to still leak information about which relays it has - chosen for a circuit to other processes on the same computer, - through e.g. which cache lines it loads while building the - circuit.) - + soon as it had chosen one, thus finishing a little earlier + when it picked a router earlier in the list. If an attacker + can recover this timing information (nontrivial but not + proven to be impossible), they could learn some coarse- + grained information about which relays a client was picking + (middle nodes in particular are likelier to be affected than + exits). The timing attack might be mitigated by other factors + (see bug #6537 for some discussion), but it's best not to + take chances. Fixes bug 6537; bugfix on 0.0.8rc1.