From e28489467233bff4500a70f8a7b22e42ca3b3e68 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 30 May 2012 12:14:38 -0400 Subject: Add __attribute__(format)s for our varargs printf/scanf wrappers It turns out that if you set the third argument of __attribute__(format) to 0, GCC and Clang will check the format argument without expecting to find variadic arguments. This is the correct behavior for vsnprintf, vasprintf, and vscanf. I'm hoping this will fix bug 5969 (a clang warning) by telling clang that the format argument to tor_vasprintf is indeed a format string. --- changes/bug5969_022 | 7 +++++++ src/common/compat.h | 5 +++-- src/common/util.h | 6 +++++- 3 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 changes/bug5969_022 diff --git a/changes/bug5969_022 b/changes/bug5969_022 new file mode 100644 index 0000000000..57c8744267 --- /dev/null +++ b/changes/bug5969_022 @@ -0,0 +1,7 @@ + o Minor bugfixes + - Fix a build warning with Clang 3.1 related to our use of vasprint. + Fix for bug 5969. Bugfix on 0.2.2.11-alpha. + + o Compilation improvements: + - Tell GCC and Clang to check for any errors in format strings passed + to the tor_v*(print|scan)f functions. diff --git a/src/common/compat.h b/src/common/compat.h index d2f1fd1295..fc70caf50c 100644 --- a/src/common/compat.h +++ b/src/common/compat.h @@ -261,11 +261,12 @@ void tor_munmap_file(tor_mmap_t *handle) ATTR_NONNULL((1)); int tor_snprintf(char *str, size_t size, const char *format, ...) CHECK_PRINTF(3,4) ATTR_NONNULL((1,3)); int tor_vsnprintf(char *str, size_t size, const char *format, va_list args) - ATTR_NONNULL((1,3)); + CHECK_PRINTF(3,0) ATTR_NONNULL((1,3)); int tor_asprintf(char **strp, const char *fmt, ...) CHECK_PRINTF(2,3); -int tor_vasprintf(char **strp, const char *fmt, va_list args); +int tor_vasprintf(char **strp, const char *fmt, va_list args) + CHECK_PRINTF(2,0); const void *tor_memmem(const void *haystack, size_t hlen, const void *needle, size_t nlen) ATTR_PURE ATTR_NONNULL((1,3)); diff --git a/src/common/util.h b/src/common/util.h index b9db25ca73..d4771562ee 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -211,7 +211,11 @@ const char *escaped(const char *string); struct smartlist_t; void wrap_string(struct smartlist_t *out, const char *string, size_t width, const char *prefix0, const char *prefixRest); -int tor_vsscanf(const char *buf, const char *pattern, va_list ap); +int tor_vsscanf(const char *buf, const char *pattern, va_list ap) +#ifdef __GNUC__ + __attribute__((format(scanf, 2, 0))) +#endif + ; int tor_sscanf(const char *buf, const char *pattern, ...) #ifdef __GNUC__ __attribute__((format(scanf, 2, 3))) -- cgit v1.2.3-54-g00ecf From 3a9351b57e528b1d0bd2e72bcf78db7c91b2ff8f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 30 May 2012 19:57:02 -0400 Subject: Fix more clang format-nonliteral warnings (bug 5969) --- src/common/log.c | 7 +++++++ src/or/control.c | 16 +++++++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/common/log.c b/src/common/log.c index ac98f13539..f2999f4e66 100644 --- a/src/common/log.c +++ b/src/common/log.c @@ -137,6 +137,13 @@ static void close_log(logfile_t *victim); static char *domain_to_string(log_domain_mask_t domain, char *buf, size_t buflen); +static INLINE char *format_msg(char *buf, size_t buf_len, + log_domain_mask_t domain, int severity, const char *funcname, + const char *format, va_list ap, size_t *msg_len_out) + CHECK_PRINTF(6,0); +static void logv(int severity, log_domain_mask_t domain, const char *funcname, + const char *format, va_list ap) + CHECK_PRINTF(4,0); /** Name of the application: used to generate the message we write at the * start of each new log. */ diff --git a/src/or/control.c b/src/or/control.c index d6e693285c..8aa4240f12 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -126,6 +126,13 @@ typedef int event_format_t; static void connection_printf_to_buf(control_connection_t *conn, const char *format, ...) CHECK_PRINTF(2,3); +static void send_control_event_impl(uint16_t event, event_format_t which, + const char *format, va_list ap) + CHECK_PRINTF(3,0); +static int control_event_status(int type, int severity, const char *format, + va_list args) + CHECK_PRINTF(3,0); + static void send_control_done(control_connection_t *conn); static void send_control_event(uint16_t event, event_format_t which, const char *format, ...) @@ -3918,6 +3925,7 @@ control_event_my_descriptor_changed(void) static int control_event_status(int type, int severity, const char *format, va_list args) { + char *user_buf = NULL; char format_buf[160]; const char *status, *sev; @@ -3949,13 +3957,15 @@ control_event_status(int type, int severity, const char *format, va_list args) log_warn(LD_BUG, "Unrecognized status severity %d", severity); return -1; } - if (tor_snprintf(format_buf, sizeof(format_buf), "650 %s %s %s\r\n", - status, sev, format)<0) { + if (tor_snprintf(format_buf, sizeof(format_buf), "650 %s %s\r\n", + status, sev)<0) { log_warn(LD_BUG, "Format string too long."); return -1; } + tor_vasprintf(&user_buf, format, args); - send_control_event_impl(type, ALL_FORMATS, format_buf, args); + send_control_event(type, ALL_FORMATS, "%s %s", format_buf, user_buf); + tor_free(user_buf); return 0; } -- cgit v1.2.3-54-g00ecf From 834654f145cc1205e20cf5f07a37bef2e11252ce Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 15 May 2012 09:41:45 -0400 Subject: Make all begindir or one-hop circuits internal This solves bug 5283, where client traffic could get sent over the same circuit as an anonymized connection to a directory, even if that circuit used an exit node unsuitable for clients. By marking the directory connection as needs_internal, we ensure that the (non-internal!) client-traffic connection won't be sent over the same circuit. --- src/or/circuituse.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 0ad8b3b51b..df33f63bb9 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1229,7 +1229,13 @@ circuit_get_open_circ_or_launch(edge_connection_t *conn, need_uptime = !conn->want_onehop && !conn->use_begindir && smartlist_string_num_isin(options->LongLivedPorts, conn->socks_request->port); - need_internal = desired_circuit_purpose != CIRCUIT_PURPOSE_C_GENERAL; + + if (desired_circuit_purpose != CIRCUIT_PURPOSE_C_GENERAL) + need_internal = 1; + else if (conn->use_begindir || conn->want_onehop) + need_internal = 1; + else + need_internal = 0; circ = circuit_get_best(conn, 1, desired_circuit_purpose, need_uptime, need_internal); -- cgit v1.2.3-54-g00ecf From b7e863c07305941d0c12b46da503fca694148abf Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Tue, 15 May 2012 20:50:29 -0400 Subject: add changes file for bug 5283 I called it a bugfix on 0.2.0.10-alpha, since git commit e5885deab is where we introduced anonymized begin_dir connections. --- changes/bug5283 | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changes/bug5283 diff --git a/changes/bug5283 b/changes/bug5283 new file mode 100644 index 0000000000..f0325cf26c --- /dev/null +++ b/changes/bug5283 @@ -0,0 +1,6 @@ + o Major bugfixes: + - Fix an edge case where if we fetch or publish a hidden service + descriptor, we might build a 4-hop circuit and then use that circuit + for exiting afterwards -- even if the new last hop doesn't obey our + ExitNodes config option. Fixes bug 5283; bugfix on 0.2.0.10-alpha. + -- cgit v1.2.3-54-g00ecf From 841a8d551abd191b23ad2f78dfb07d9e4ff8ace2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 2 Jun 2012 20:05:32 -0400 Subject: Work around a bug in OpenSSL 1.0.1's TLS 1.1 and TLS 1.2 support It appears that when OpenSSL negotiates a 1.1 or 1.2 connection, and it decides to renegotiate, the client will send a record with version "1.0" rather than with the current TLS version. This would cause the connection to fail whenever both sides had OpenSSL 1.0.1, and the v2 Tor handshake was in use. As a workaround, disable TLS 1.1 and TLS 1.2. When a later version of OpenSSL is released, we can make this conditional on running a fixed version of OpenSSL. Alternatively, we could disable TLS 1.1 and TLS 1.2 only on the client side. But doing it this way for now means that we not only fix TLS with patched clients; we also fix TLS when the server has this patch and the client does not. That could be important to keep the network running well. Fixes bug 6033. --- changes/bug6033 | 6 ++++++ src/common/tortls.c | 15 +++++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 changes/bug6033 diff --git a/changes/bug6033 b/changes/bug6033 new file mode 100644 index 0000000000..56cffd68b7 --- /dev/null +++ b/changes/bug6033 @@ -0,0 +1,6 @@ + o Major bugfixes: + - Work around a bug in OpenSSL that broke renegotiation with + TLS 1.1 and TLS 1.2. Without this workaround, all attempts + to speak the v2 Tor network protocol when both sides were + using OpenSSL 1.0.1 would fail. Fix for bug 6033, which is + not a bug in Tor. diff --git a/src/common/tortls.c b/src/common/tortls.c index 4c9d2188d4..c6316120f9 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -790,6 +790,21 @@ tor_tls_context_new(crypto_pk_env_t *identity, unsigned int key_lifetime, goto error; SSL_CTX_set_options(result->ctx, SSL_OP_NO_SSLv2); + /* Disable TLS1.1 and TLS1.2 if they exist. We need to do this to + * workaround a bug present in all OpenSSL 1.0.1 versions (as of 1 + * June 2012), wherein renegotiating while using one of these TLS + * protocols will cause the client to send a TLS 1.0 ServerHello + * rather than a ServerHello written with the appropriate protocol + * version. Once some version of OpenSSL does TLS1.1 and TLS1.2 + * renegotiation properly, we can turn them back on when built with + * that version. */ +#ifdef SSL_OP_NO_TLSv1_2 + SSL_CTX_set_options(result->ctx, SSL_OP_NO_TLSv1_2); +#endif +#ifdef SSL_OP_NO_TLSv1_1 + SSL_CTX_set_options(result->ctx, SSL_OP_NO_TLSv1_1); +#endif + if ( #ifdef DISABLE_SSL3_HANDSHAKE 1 || -- cgit v1.2.3-54-g00ecf From af54a0182870babec62bf07d067ca82a67c423de Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 31 May 2012 11:19:35 -0400 Subject: Kill non-open OR connections with any data on their inbufs. This fixes a DoS issue where a client could send so much data in 5 minutes that they exhausted the server's RAM. Fix for bug 5934 and 6007. Bugfix on 0.2.0.20-rc, which enabled the v2 handshake. --- changes/bug6007 | 5 +++++ src/or/connection_or.c | 22 ++++++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 changes/bug6007 diff --git a/changes/bug6007 b/changes/bug6007 new file mode 100644 index 0000000000..4e815754aa --- /dev/null +++ b/changes/bug6007 @@ -0,0 +1,5 @@ + o Major bugfixes (security): + - When waiting for a client to renegotiate, don't allow it to add + any bytes to the input buffer. This fixes a DoS issue. Fix for + bugs 6007 and 5934; bugfix on 0.2.0.20-rc. + diff --git a/src/or/connection_or.c b/src/or/connection_or.c index dc8850ea3f..cb0082bdc2 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -209,7 +209,12 @@ connection_or_reached_eof(or_connection_t *conn) int connection_or_process_inbuf(or_connection_t *conn) { - int ret; + /** Don't let the inbuf of a nonopen OR connection grow beyond this many + * bytes: it's either a broken client, a non-Tor client, or a DOS + * attempt. */ +#define MAX_OR_INBUF_WHEN_NONOPEN 0 + + int ret = 0; tor_assert(conn); switch (conn->_base.state) { @@ -231,8 +236,21 @@ connection_or_process_inbuf(or_connection_t *conn) case OR_CONN_STATE_OR_HANDSHAKING: return connection_or_process_cells_from_inbuf(conn); default: - return 0; /* don't do anything */ + break; /* don't do anything */ } + + if (buf_datalen(conn->_base.inbuf) > MAX_OR_INBUF_WHEN_NONOPEN) { + log_fn(LOG_PROTOCOL_WARN, LD_NET, "Accumulated too much data (%d bytes) " + "on nonopen OR connection %s %s:%u in state %s; closing.", + (int)buf_datalen(conn->_base.inbuf), + connection_or_nonopen_was_started_here(conn) ? "to" : "from", + conn->_base.address, conn->_base.port, + conn_state_to_string(conn->_base.type, conn->_base.state)); + connection_mark_for_close(TO_CONN(conn)); + ret = -1; + } + + return ret; } /** When adding cells to an OR connection's outbuf, keep adding until the -- cgit v1.2.3-54-g00ecf