[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[or-cvs] [tor/maint-0.2.1] Make Tor work with OpenSSL 0.9.8l



Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date: Thu, 5 Nov 2009 18:13:08 -0500
Subject: Make Tor work with OpenSSL 0.9.8l
Commit: ce0a89e2624471272ffc4950c5069d9b81a7f0b9

To fix a major security problem related to incorrect use of
SSL/TLS renegotiation, OpenSSL has turned off renegotiation by
default.  We are not affected by this security problem, however,
since we do renegotiation right.  (Specifically, we never treat a
renegotiated credential as authenticating previous communication.)
Nevertheless, OpenSSL's new behavior requires us to explicitly
turn renegotiation back on in order to get our protocol working
again.

Amusingly, this is not so simple as "set the flag when you create
the SSL object" , since calling connect or accept seems to clear
the flags.

For belt-and-suspenders purposes, we clear the flag once the Tor
handshake is done.  There's no way to exploit a second handshake
either, but we might as well not allow it.
---
 ChangeLog              |    6 ++++++
 src/common/tortls.c    |   34 ++++++++++++++++++++++++++++++++++
 src/common/tortls.h    |    1 +
 src/or/connection_or.c |    2 ++
 4 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7023ea3..4799e74 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,10 @@
 Changes in Version 0.2.1.21 - 20??-??-??
+  o Major bugfixes:
+    - Work around a security feature in OpenSSL 0.9.8l that prevents our
+      handshake from working unless we explicitly tell OpenSSL that we are
+      using SSL renegotiation safely.  We are, of course, but OpenSSL
+      0.9.8l won't work unless we say we are.
+
   o Minor bugfixes:
     - Do not refuse to learn about authority certs and v2 networkstatus
       documents that are older than the latest consensus.  This bug might
diff --git a/src/common/tortls.c b/src/common/tortls.c
index c6b11e9..bcc6780 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -154,6 +154,7 @@ static X509* tor_tls_create_certificate(crypto_pk_env_t *rsa,
                                         const char *cname,
                                         const char *cname_sign,
                                         unsigned int lifetime);
+static void tor_tls_unblock_renegotiation(tor_tls_t *tls);
 
 /** Global tls context. We keep it here because nobody else needs to
  * touch it. */
@@ -904,6 +905,36 @@ tor_tls_set_renegotiate_callback(tor_tls_t *tls,
 #endif
 }
 
+/** If this version of openssl requires it, turn on renegotiation on
+ * <b>tls</b>.  (Our protocol never requires this for security, but it's nice
+ * to use belt-and-suspenders here.)
+ */
+static void
+tor_tls_unblock_renegotiation(tor_tls_t *tls)
+{
+#ifdef SSL3_FLAGS_ALLOW_UNSAFE_LEGACY_RENEGOTIATION
+  /* Yes, we know what we are doing here.  No, we do not treat a renegotiation
+   * as authenticating any earlier-received data. */
+  tls->ssl->s3->flags |= SSL3_FLAGS_ALLOW_UNSAFE_LEGACY_RENEGOTIATION;
+#else
+  (void)tls;
+#endif
+}
+
+/** If this version of openssl supports it, turn off renegotiation on
+ * <b>tls</b>.  (Our protocol never requires this for security, but it's nice
+ * to use belt-and-suspenders here.)
+ */
+void
+tor_tls_block_renegotiation(tor_tls_t *tls)
+{
+#ifdef SSL3_FLAGS_ALLOW_UNSAFE_LEGACY_RENEGOTIATION
+  tls->ssl->s3->flags &= ~SSL3_FLAGS_ALLOW_UNSAFE_LEGACY_RENEGOTIATION;
+#else
+  (void)tls;
+#endif
+}
+
 /** Return whether this tls initiated the connect (client) or
  * received it (server). */
 int
@@ -1026,6 +1057,9 @@ tor_tls_handshake(tor_tls_t *tls)
   } else {
     r = SSL_connect(tls->ssl);
   }
+  /* We need to call this here and not earlier, since OpenSSL has a penchant
+   * for clearing its flags when you say accept or connect. */
+  tor_tls_unblock_renegotiation(tls);
   r = tor_tls_get_error(tls,r,0, "handshaking", LOG_INFO);
   if (ERR_peek_error() != 0) {
     tls_log_errors(tls, tls->isServer ? LOG_INFO : LOG_WARN,
diff --git a/src/common/tortls.h b/src/common/tortls.h
index d006909..871fec3 100644
--- a/src/common/tortls.h
+++ b/src/common/tortls.h
@@ -65,6 +65,7 @@ int tor_tls_read(tor_tls_t *tls, char *cp, size_t len);
 int tor_tls_write(tor_tls_t *tls, const char *cp, size_t n);
 int tor_tls_handshake(tor_tls_t *tls);
 int tor_tls_renegotiate(tor_tls_t *tls);
+void tor_tls_block_renegotiation(tor_tls_t *tls);
 int tor_tls_shutdown(tor_tls_t *tls);
 int tor_tls_get_pending_bytes(tor_tls_t *tls);
 size_t tor_tls_get_forced_write_size(tor_tls_t *tls);
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index b4e8092..2a52b3f 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -844,6 +844,7 @@ connection_or_tls_renegotiated_cb(tor_tls_t *tls, void *_conn)
 
   /* Don't invoke this again. */
   tor_tls_set_renegotiate_callback(tls, NULL, NULL);
+  tor_tls_block_renegotiation(tls);
 
   if (connection_tls_finish_handshake(conn) < 0) {
     /* XXXX_TLS double-check that it's ok to do this from inside read. */
@@ -1087,6 +1088,7 @@ connection_tls_finish_handshake(or_connection_t *conn)
       connection_or_init_conn_from_address(conn, &conn->_base.addr,
                                            conn->_base.port, digest_rcvd, 0);
     }
+    tor_tls_block_renegotiation(conn->tls);
     return connection_or_set_state_open(conn);
   } else {
     conn->_base.state = OR_CONN_STATE_OR_HANDSHAKING;
-- 
1.5.6.5