February 2018 - tor-commits - lists.torproject.org

[tor/master] Merge remote-tracking branch 'valentecaio/t-24714'
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 3d7bf98d13ffc090e5ba1b918f53668018690dce Merge: 3c8a48159 7884ce76e Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 15 20:19:53 2018 -0500 Merge remote-tracking branch 'valentecaio/t-24714' changes/ticket24714 | 6 ++++++ src/or/circuituse.c | 2 +- src/or/connection.c | 14 +++++++------- src/or/connection_edge.c | 6 +++--- src/or/directory.c | 2 +- src/or/hs_client.c | 4 ++-- src/or/main.c | 18 ++++++++++-------- src/or/or.h | 8 ++++---- src/or/relay.c | 2 +- src/or/rendclient.c | 4 ++-- 10 files changed, 37 insertions(+), 29 deletions(-)

1 0

[tor/master] Add a new tor_addr_from_getsockname()
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 2a7bfec364a30a92c3babe658ece2db1a8a40d03 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jan 26 11:48:45 2018 -0500 Add a new tor_addr_from_getsockname() We use this pattern all over, and this should simplify matters a bit. Part of 18105. --- src/common/compat.c | 18 ++++++++++++++++++ src/common/compat.h | 2 ++ 2 files changed, 20 insertions(+) diff --git a/src/common/compat.c b/src/common/compat.c index 6a61c6c91..c457e6383 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -1392,6 +1392,24 @@ tor_getsockname,(tor_socket_t sock, struct sockaddr *address, return getsockname(sock, address, address_len); } +/** + * Find the local address associated with the socket sock, and + * place it in *addr_out. Return 0 on success, -1 on failure. + * + * (As tor_getsockname, but instead places the result in a tor_addr_t.) */ +int +tor_addr_from_getsockname(tor_addr_t *addr_out, tor_socket_t sock) +{ + struct sockaddr_storage ss; + socklen_t ss_len = sizeof(ss); + memset(ss, 0, sizeof(ss)); + + if (tor_getsockname(sock, (struct sockaddr *) &ss, &ss_len) < 0) + return -1; + + return tor_addr_from_sockaddr(addr_out, (struct sockaddr *)&ss, NULL); +} + /** Turn socket into a nonblocking socket. Return 0 on success, -1 * on failure. */ diff --git a/src/common/compat.h b/src/common/compat.h index 93301feda..f7932c914 100644 --- a/src/common/compat.h +++ b/src/common/compat.h @@ -510,6 +510,8 @@ int get_n_open_sockets(void); MOCK_DECL(int, tor_getsockname,(tor_socket_t socket, struct sockaddr *address, socklen_t *address_len)); +struct tor_addr_t; +int tor_addr_from_getsockname(struct tor_addr_t *addr_out, tor_socket_t sock); #define tor_socket_send(s, buf, len, flags) send(s, buf, len, flags) #define tor_socket_recv(s, buf, len, flags) recv(s, buf, len, flags)

1 0

[tor/master] Use tor_addr_from_getsockname() in several places
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 6ed384b827dce21ea3a44b58792cc28b2ed48056 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jan 26 11:59:01 2018 -0500 Use tor_addr_from_getsockname() in several places I'm leaving the getsockname code in transproxy alone, since it is comparatively isolated, rather platform-specific, and hard to test. Implements 18105. --- changes/18105 | 4 ++++ src/common/address.c | 18 ++++++++---------- src/common/compat.c | 2 +- src/or/connection.c | 10 ++-------- 4 files changed, 15 insertions(+), 19 deletions(-) diff --git a/changes/18105 b/changes/18105 new file mode 100644 index 000000000..87e6e61d2 --- /dev/null +++ b/changes/18105 @@ -0,0 +1,4 @@ + o Code simplification and refactoring: + - Introduce a function to call getsockname() and return + tor_addr_t, to save a little complexity throughout the codebase. + Closes ticket 18105. diff --git a/src/common/address.c b/src/common/address.c index d96ec514b..7d0fa5614 100644 --- a/src/common/address.c +++ b/src/common/address.c @@ -1672,7 +1672,7 @@ get_interface_address6_via_udp_socket_hack,(int severity, sa_family_t family, tor_addr_t *addr)) { - struct sockaddr_storage my_addr, target_addr; + struct sockaddr_storage target_addr; int sock=-1, r=-1; socklen_t addr_len; @@ -1715,21 +1715,19 @@ get_interface_address6_via_udp_socket_hack,(int severity, goto err; } - if (tor_getsockname(sock,(struct sockaddr*)&my_addr, &addr_len)) { + if (tor_addr_from_getsockname(addr, sock) < 0) { int e = tor_socket_errno(sock); log_fn(severity, LD_NET, "getsockname() to determine interface failed: %s", tor_socket_strerror(e)); goto err; } - if (tor_addr_from_sockaddr(addr, (struct sockaddr*)&my_addr, NULL) == 0) { - if (tor_addr_is_loopback(addr) || tor_addr_is_multicast(addr)) { - log_fn(severity, LD_NET, "Address that we determined via UDP socket" - " magic is unsuitable for public comms."); - } else { - r=0; - } - } + if (tor_addr_is_loopback(addr) || tor_addr_is_multicast(addr)) { + log_fn(severity, LD_NET, "Address that we determined via UDP socket" + " magic is unsuitable for public comms."); + } else { + r=0; + } err: if (sock >= 0) diff --git a/src/common/compat.c b/src/common/compat.c index c457e6383..204fbd62e 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -1402,7 +1402,7 @@ tor_addr_from_getsockname(tor_addr_t *addr_out, tor_socket_t sock) { struct sockaddr_storage ss; socklen_t ss_len = sizeof(ss); - memset(ss, 0, sizeof(ss)); + memset(&ss, 0, sizeof(ss)); if (tor_getsockname(sock, (struct sockaddr *) &ss, &ss_len) < 0) return -1; diff --git a/src/or/connection.c b/src/or/connection.c index a020bef77..e3b430782 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -1253,15 +1253,12 @@ connection_listener_new(const struct sockaddr *listensockaddr, gotPort = usePort; } else { tor_addr_t addr2; - struct sockaddr_storage ss; - socklen_t ss_len=sizeof(ss); - if (getsockname(s, (struct sockaddr*)&ss, &ss_len)<0) { + if (tor_addr_from_getsockname(&addr2, s)<0) { log_warn(LD_NET, "getsockname() couldn't learn address for %s: %s", conn_type_to_string(type), tor_socket_strerror(tor_socket_errno(s))); gotPort = 0; } - tor_addr_from_sockaddr(&addr2, (struct sockaddr*)&ss, &gotPort); } #ifdef HAVE_SYS_UN_H /* @@ -4514,8 +4511,6 @@ alloc_http_authenticator(const char *authenticator) static void client_check_address_changed(tor_socket_t sock) { - struct sockaddr_storage out_sockaddr; - socklen_t out_addr_len = (socklen_t) sizeof(out_sockaddr); tor_addr_t out_addr, iface_addr; tor_addr_t **last_interface_ip_ptr; sa_family_t family; @@ -4523,13 +4518,12 @@ client_check_address_changed(tor_socket_t sock) if (!outgoing_addrs) outgoing_addrs = smartlist_new(); - if (getsockname(sock, (struct sockaddr*)&out_sockaddr, &out_addr_len)<0) { + if (tor_addr_from_getsockname(&out_addr, sock) < 0) { int e = tor_socket_errno(sock); log_warn(LD_NET, "getsockname() to check for address change failed: %s", tor_socket_strerror(e)); return; } - tor_addr_from_sockaddr(&out_addr, (struct sockaddr*)&out_sockaddr, NULL); family = tor_addr_family(&out_addr); if (family == AF_INET)

1 0

[tor/master] Merge branch 'bug18105'
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 3c8a481599cf9c84530db1f304731b5f2ec9caef Merge: 3e2b48f8b 6ed384b82 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 15 20:17:31 2018 -0500 Merge branch 'bug18105' changes/18105 | 4 ++++ src/common/address.c | 18 ++++++++---------- src/common/compat.c | 18 ++++++++++++++++++ src/common/compat.h | 2 ++ src/or/connection.c | 10 ++-------- 5 files changed, 34 insertions(+), 18 deletions(-)

1 0

[tor/master] Merge branch 'bug24484_squashed'
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 3e2b48f8b4eee456c4aa81f640185382642fa75c Merge: cfff582e4 4dc228e35 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 15 20:13:53 2018 -0500 Merge branch 'bug24484_squashed' changes/bug24484 | 4 ++++ configure.ac | 15 +++++++++++++++ src/common/util.h | 18 +++++++----------- 3 files changed, 26 insertions(+), 11 deletions(-)

1 0

[tor/master] Remove workaround code for systems where free(NULL) is busted.
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 4dc228e35bbedb6f451a621db4a756ead9560a76 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jan 25 17:04:12 2018 -0500 Remove workaround code for systems where free(NULL) is busted. Add an autoconf test to make sure we won't regret it. Closes ticket 24484. --- changes/bug24484 | 4 ++++ configure.ac | 15 +++++++++++++++ src/common/util.h | 18 +++++++----------- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/changes/bug24484 b/changes/bug24484 new file mode 100644 index 000000000..35a204492 --- /dev/null +++ b/changes/bug24484 @@ -0,0 +1,4 @@ + o Code simplification and refactoring: + - Since Tor requires C99, remove our old workaround code for libc + implementations where free(NULL) doesn't work. Closes ticket 24484. + diff --git a/configure.ac b/configure.ac index 3cb187b0e..3795faddc 100644 --- a/configure.ac +++ b/configure.ac @@ -615,6 +615,21 @@ fi AM_CONDITIONAL(BUILD_READPASSPHRASE_C, test "x$ac_cv_func_readpassphrase" = "xno" && test "$bwin32" = "false") +AC_MSG_CHECKING([whether free(NULL) works]) +AC_RUN_IFELSE([AC_LANG_PROGRAM([ + #include <stdlib.h> +], [ +char *p = NULL; +free(p); +])], +[free_null_ok=true; AC_MSG_RESULT(yes)], +[free_null_ok=false; AC_MSG_RESULT(no)], +[free_null_ok=cross; AC_MSG_RESULT(cross)]) + +if test "$free_null_ok" = "false"; then + AC_MSG_ERROR([Your libc implementation doesn't allow free(NULL), as required by C99.]) +fi + dnl ------------------------------------------------------ dnl Where do you live, libevent? And how do we call you? diff --git a/src/common/util.h b/src/common/util.h index 2ee0ea28c..653c154ab 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -73,9 +73,9 @@ extern int dmalloc_free(const char *file, const int line, void *pnt, } \ STMT_END #else /* !(defined(USE_DMALLOC)) */ -/** Release memory allocated by tor_malloc, tor_realloc, tor_strdup, etc. - * Unlike the free() function, tor_free() will still work on NULL pointers, - * and it sets the pointer value to NULL after freeing it. +/** Release memory allocated by tor_malloc, tor_realloc, tor_strdup, + * etc. Unlike the free() function, the tor_free() macro sets the + * pointer value to NULL after freeing it. * * This is a macro. If you need a function pointer to release memory from * tor_malloc(), use tor_free_(). @@ -88,17 +88,13 @@ extern int dmalloc_free(const char *file, const int line, void *pnt, #ifdef __GNUC__ #define tor_free(p) STMT_BEGIN \ typeof(&(p)) tor_free__tmpvar = &(p); \ - if (PREDICT_LIKELY((*tor_free__tmpvar)!=NULL)) { \ - raw_free(*tor_free__tmpvar); \ - *tor_free__tmpvar=NULL; \ - } \ + raw_free(*tor_free__tmpvar); \ + *tor_free__tmpvar=NULL; \ STMT_END #else #define tor_free(p) STMT_BEGIN \ - if (PREDICT_LIKELY((p)!=NULL)) { \ - raw_free(p); \ - (p)=NULL; \ - } \ + raw_free(p); \ + (p)=NULL; \ STMT_END #endif #endif /* defined(USE_DMALLOC) */

1 0

[tor/master] Bump version in master to 0.3.4.0-alpha-dev
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit cfff582e4d68b4026d19c4cee4c2d893aaab116f Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 15 20:11:25 2018 -0500 Bump version in master to 0.3.4.0-alpha-dev --- configure.ac | 2 +- contrib/win32build/tor-mingw.nsi.in | 2 +- src/win32/orconfig.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 1e27dfa56..cea02b7da 100644 --- a/configure.ac +++ b/configure.ac @@ -4,7 +4,7 @@ dnl Copyright (c) 2007-2017, The Tor Project, Inc. dnl See LICENSE for licensing information AC_PREREQ([2.63]) -AC_INIT([tor],[0.3.3.2-alpha-dev]) +AC_INIT([tor],[0.3.4.0-alpha-dev]) AC_CONFIG_SRCDIR([src/or/main.c]) AC_CONFIG_MACRO_DIR([m4]) diff --git a/contrib/win32build/tor-mingw.nsi.in b/contrib/win32build/tor-mingw.nsi.in index 77e75c450..dfcb0644d 100644 --- a/contrib/win32build/tor-mingw.nsi.in +++ b/contrib/win32build/tor-mingw.nsi.in @@ -8,7 +8,7 @@ !include "LogicLib.nsh" !include "FileFunc.nsh" !insertmacro GetParameters -!define VERSION "0.3.3.2-alpha-dev" +!define VERSION "0.3.4.0-alpha-dev" !define INSTALLER "tor-${VERSION}-win32.exe" !define WEBSITE "https://www.torproject.org/" !define LICENSE "LICENSE" diff --git a/src/win32/orconfig.h b/src/win32/orconfig.h index 218cc36ec..238e0f696 100644 --- a/src/win32/orconfig.h +++ b/src/win32/orconfig.h @@ -218,7 +218,7 @@ #define USING_TWOS_COMPLEMENT /* Version number of package */ -#define VERSION "0.3.3.2-alpha-dev" +#define VERSION "0.3.4.0-alpha-dev"

1 0

[tor/master] Rust protover compat: forbid more than MAX_VERSIONS_TO_EXPAND in a range
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit f69510ba4b196ed40fce64f24b5b7799b68d182b Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 15 08:37:19 2018 -0500 Rust protover compat: forbid more than MAX_VERSIONS_TO_EXPAND in a range Also correct MAX_VERSIONS_TO_EXPAND to match the C. NOTE that this patch leads to incorrect behavior: the C code allows huge ranges; it just doesn't allow votes on them (currently). For full compatibility, we'll need to make the rust code store ranges as ranges natively, possibly using something like the range_map crate. Still, this patch is smaller than a "proper" fix. Fixes TROVE-2018-003. --- src/rust/protover/protover.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 25f776aed..cc9be67b6 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -23,7 +23,7 @@ const FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS: &'static str = "0.2.9.3-alpha"; /// before concluding that someone is trying to DoS us /// /// C_RUST_COUPLED: src/or/protover.c `MAX_PROTOCOLS_TO_EXPAND` -const MAX_PROTOCOLS_TO_EXPAND: u32 = 500; +const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16); /// Currently supported protocols and their versions, as a byte-slice. /// @@ -209,7 +209,7 @@ impl Versions { )?); } - if versions.len() > MAX_PROTOCOLS_TO_EXPAND as usize { + if versions.len() > MAX_PROTOCOLS_TO_EXPAND { return Err("Too many versions to expand"); } } @@ -448,7 +448,13 @@ fn expand_version_range(range: &str) -> Result<Range<u32>, &'static str> { ))?; // We can use inclusive range syntax when it becomes stable. - Ok(lower..higher + 1) + let result = lower..higher + 1; + + if result.len() > MAX_PROTOCOLS_TO_EXPAND { + Err("Too many protocols in expanded range") + } else { + Ok(result) + } } /// Checks to see if there is a continuous range of integers, starting at the @@ -862,6 +868,9 @@ mod test { Err("cannot parse protocol range upper bound"), expand_version_range("1-a") ); + assert_eq!(Ok(1000..66536), expand_version_range("1000-66535")); + assert_eq!(Err("Too many protocols in expanded range"), + expand_version_range("1000-66536")); } #[test]

1 0

[tor/master] Forbid u32::MAX as a protover range element in rust
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit b58a2febe32f96204a94fb547e14a84c8fd32651 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 15 10:27:27 2018 -0500 Forbid u32::MAX as a protover range element in rust Part of the 25249 fix to make rust match the C. --- src/rust/protover/protover.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index cc9be67b6..64f350c6b 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -9,6 +9,7 @@ use std::fmt; use std::collections::{HashMap, HashSet}; use std::ops::Range; use std::string::String; +use std::u32; use tor_util::strings::NUL_BYTE; @@ -204,9 +205,13 @@ impl Versions { versions.insert(p); } } else { - versions.insert(u32::from_str(piece).or( + let v = u32::from_str(piece).or( Err("invalid protocol entry"), - )?); + )?; + if v == u32::MAX { + return Err("invalid protocol entry"); + } + versions.insert(v); } if versions.len() > MAX_PROTOCOLS_TO_EXPAND { @@ -447,6 +452,10 @@ fn expand_version_range(range: &str) -> Result<Range<u32>, &'static str> { "cannot parse protocol range upper bound", ))?; + if lower == u32::MAX || higher == u32::MAX { + return Err("protocol range value out of range"); + } + // We can use inclusive range syntax when it becomes stable. let result = lower..higher + 1;

1 0

[tor/master] rust protover: match the C implementation on empty-str cases
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 5af03c1ef3c4718b79abb1638f5a8c275129530a Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 15 10:47:09 2018 -0500 rust protover: match the C implementation on empty-str cases Empty versions lists are permitted; empty keywords are not. --- src/rust/protover/protover.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 64f350c6b..e5dc69b9a 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -193,10 +193,6 @@ impl Versions { fn from_version_string( version_string: &str, ) -> Result<Self, &'static str> { - if version_string.is_empty() { - return Err("version string is empty"); - } - let mut versions = HashSet::<Version>::new(); for piece in version_string.split(",") { @@ -204,6 +200,8 @@ impl Versions { for p in expand_version_range(piece)? { versions.insert(p); } + } else if piece == "" { + continue; } else { let v = u32::from_str(piece).or( Err("invalid protocol entry"), @@ -456,6 +454,10 @@ fn expand_version_range(range: &str) -> Result<Range<u32>, &'static str> { return Err("protocol range value out of range"); } + if lower > higher { + return Err("protocol range is badly formed"); + } + // We can use inclusive range syntax when it becomes stable. let result = lower..higher + 1; @@ -583,6 +585,7 @@ fn parse_protocols_from_string_with_no_validation<'a>( let mut parts = subproto.splitn(2, "="); let name = match parts.next() { + Some("") => return Err("invalid protover entry"), Some(n) => n, None => return Err("invalid protover entry"), }; @@ -650,7 +653,6 @@ pub fn compute_vote( Ok(result) => result, Err(_) => continue, }; - for (protocol, versions) in this_vote { let supported_vers: &mut HashMap<Version, usize> = all_count.entry(protocol).or_insert(HashMap::new()); @@ -794,7 +796,6 @@ mod test { use super::Versions; - assert_eq!(Err("version string is empty"), Versions::from_version_string("")); assert_eq!(Err("invalid protocol entry"), Versions::from_version_string("a,b")); assert_eq!(Err("invalid protocol entry"), Versions::from_version_string("1,!")); @@ -838,7 +839,7 @@ mod test { use super::contains_only_supported_protocols; assert_eq!(false, contains_only_supported_protocols("")); - assert_eq!(false, contains_only_supported_protocols("Cons=")); + assert_eq!(true, contains_only_supported_protocols("Cons=")); assert_eq!(true, contains_only_supported_protocols("Cons=1")); assert_eq!(false, contains_only_supported_protocols("Cons=0")); assert_eq!(false, contains_only_supported_protocols("Cons=0-1"));

1 0