Opened 3 years ago

Closed 3 years ago

Last modified 14 months ago

#2204 closed defect (fixed)

Recent openssls (1.0.0b and 0.9.8p) break relay handshakes

Reported by: stars Owned by:
Priority: blocker Milestone:
Component: Tor Version:
Keywords: tor-relay Cc: weasel
Actual Points: Parent ID:
Points:

Description

Hello,

The relay don't publish anymore to the consensus with patched Openssl against "buffer overflow vulnerability".

I running Kubuntu Lucid 10.04.1 on x86 64
Tor origin/master : commit 9cbe64db45de6d6f5f6adffda3586ca1e8d60d01
Libevent : commit 3a67d0bf42c69423fca36d001023d563b0326399

The relay can be reach but don't warn about Cert.

<rransom> I typed https://80.218.145.226/ into my browser, and it says it's connected, but it's not showing the Big Scary Certificate Warning.

The log look good and have confirmed that Dir and Orport was open and working like it does. After couple days it still out of consensus.

Exit policy: reject */*

Best regards
SwissTorExit

Child Tickets

Change History (19)

comment:1 Changed 3 years ago by rransom

The last descriptor for SwissTorHelp listed in a consensus (https://metrics.torproject.org/descriptor.html?desc-id=38b98abfac93606158da34aca4bb94c36e86a761) was generated by Tor git-3d7772ece3128097.

comment:2 Changed 3 years ago by rransom

SwissTorExit originally reported this problem with Tor git-4f66bf4fe7ec4c52ba9.

comment:3 Changed 3 years ago by stars

At "info level", i get :

nov. 20 16:20:09.618 [Info] TLS error while handshaking with [scrubbed]: parse tlsext (in SSL routines:SSL3_GET_CLIENT_HELLO:SSL3_ST_SR_CLNT_HELLO_C)
nov. 20 16:20:09.620 [Info] connection_tls_continue_handshake(): tls error [misc error]. breaking connection.

This message repeat almost all 4 minutes

comment:4 Changed 3 years ago by arma

  • Milestone set to Tor: 0.2.1.x-final
  • Summary changed from relay are not published anymore in consensus to Recent openssls (1.0.0b and 0.9.8p) break relay handshakes

comment:5 Changed 3 years ago by Sebastian

Here a few quick notes: This gets triggered by this call

    result = tor_tls_handshake(conn->tls);

inside connection_tls_continue_handshake. In the client use case,

     result = tor_tls_renegotiate(conn->tls);

doesn't trigger the issue.

The error we get is a decode error like it could be triggered by

+						if(s->session->tlsext_hostname)
+							{
+							*al = SSL_AD_DECODE_ERROR;
+							return 0;
+							}

which is part of the openssl patch.

Typical Tor log line:

[info] connection_tls_continue_handshake(): tls error [unexpected close]. breaking connection.
[info] TLS error while handshaking with [scrubbed]: parse tlsext (in SSL routines:SSL3_GET_CLIENT_HELLO)

comment:6 Changed 3 years ago by Sebastian

So here's a patch that makes a private network work in that relays are listed in the consensus etc. I'm very unsure about it being correct since we use the tlsext_hostname field because browsers do it too. This does make me hopeful that we can solve this in Tor, however.

diff --git a/src/common/tortls.c b/src/common/tortls.c
index c4b2500..9d4ca63 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -1054,18 +1054,18 @@ tor_tls_new(int sock, int isServer)
 
 #ifdef SSL_set_tlsext_host_name
   /* Browsers use the TLS hostname extension, so we should too. */
-  {
+/*  {
     char *fake_hostname = crypto_random_hostname(4,25, "www.",".com");
     SSL_set_tlsext_host_name(result->ssl, fake_hostname);
     tor_free(fake_hostname);
-  }
+  }*/
 #endif
 
   if (!SSL_set_cipher_list(result->ssl,
                      isServer ? SERVER_CIPHER_LIST : CLIENT_CIPHER_LIST)) {
     tls_log_errors(NULL, LOG_WARN, LD_NET, "setting ciphers");
 #ifdef SSL_set_tlsext_host_name
-    SSL_set_tlsext_host_name(result->ssl, NULL);
+//    SSL_set_tlsext_host_name(result->ssl, NULL);
 #endif
     SSL_free(result->ssl);
     tor_free(result);
@@ -1078,7 +1078,7 @@ tor_tls_new(int sock, int isServer)
   if (! bio) {
     tls_log_errors(NULL, LOG_WARN, LD_NET, "opening BIO");
 #ifdef SSL_set_tlsext_host_name
-    SSL_set_tlsext_host_name(result->ssl, NULL);
+//    SSL_set_tlsext_host_name(result->ssl, NULL);
 #endif
     SSL_free(result->ssl);
     tor_free(result);
@@ -1204,7 +1204,7 @@ tor_tls_free(tor_tls_t *tls)
     log_warn(LD_BUG, "Freeing a TLS that was not in the ssl->tls map.");
   }
 #ifdef SSL_set_tlsext_host_name
-  SSL_set_tlsext_host_name(tls->ssl, NULL);
+//  SSL_set_tlsext_host_name(tls->ssl, NULL);
 #endif
   SSL_free(tls->ssl);
   tls->ssl = NULL;

comment:7 Changed 3 years ago by Sebastian

(Don't try this on master, use maint-0.2.2. Master has an unrelated bug that prevents the private network to handle client requests)

comment:8 Changed 3 years ago by nickm

  • Priority changed from critical to blocker

That fix won't work right, I think: I think only changes the client side behavior, not the server-side behavior, and the server-side behavior will remain broken.

(I'd love to be proven wrong about that, btw: one way to do that is try this on a private network where the client is unpatched and the servers have the patch. If you do, I think you'll see the client fail to connect to the servers. But if the client can successfully connect, then we may have the start of an approach.)

comment:9 Changed 3 years ago by Sebastian

I think I can prove you wrong here, I ran a network with all relays and authorities patched and client unpatched, and it worked (the network got set up, the client could make circuits, those handled requests, etc).

comment:10 Changed 3 years ago by nickm

That's good news; the fix that would have been needed if I had been right would have been ugly. Yours is cleaner. I've tried to clean it even harder in branch no_server_tlsext_hostname_022 in my public; how does that work?

comment:11 Changed 3 years ago by Sebastian

Seems to work fine.

comment:12 follow-up: Changed 3 years ago by nickm

  • Status changed from new to needs_review

Okay; next thing I'd want to test is to make sure that clients and servers with (0.9.8p+my patch) work fine with clients and servers with (older openssl, unpatched) and (older openssl, patched).

I have cleaned up my branch and made a new version of it to apply against 0.2.1. It is now called "fix2204".

comment:13 in reply to: ↑ 12 Changed 3 years ago by weasel

Replying to nickm:

Okay; next thing I'd want to test is to make sure that clients and servers
with (0.9.8p+my patch) work fine with clients and servers with (older
openssl, unpatched) and (older openssl, patched).

client and server are on debian stable.
server is a bridge relay.

 / patched ssl on server (0.9.8g-15+lenny9 vs. 0.9.8g-15+lenny8)
 | / patched tor 0.2.1.26 on server
 | | / patched ssl on client
 | | | / patched tor 0.2.1.26 on client

 0 0 0 0        ok
 0 0 0 1        ok
 0 0 1 0        ok
 0 0 1 1        ok
 0 1 0 0        ok
 0 1 0 1        ok
 0 1 1 0        ok
 0 1 1 1        ok

 1 0 0 0        bad
 1 0 0 1        bad
 1 0 1 0        bad
 1 0 1 1        bad
 1 1 0 0        ok
 1 1 0 1        ok
 1 1 1 0        ok
 1 1 1 1        ok

comment:14 Changed 3 years ago by weasel

  • Cc weasel added

comment:15 Changed 3 years ago by stars

I have tested on real environment the fix :

The fix on: nickm git://git.torproject.org/nickm/tor + nickm/fix2204 don't work.

The fix on: origin/maint-0.2.2 + nickm/fix2204 work well and have published my relay in consensus.

Thanks for the great work

comment:16 Changed 3 years ago by nickm

  • Resolution set to fixed
  • Status changed from needs_review to closed

Weasel's table is persuasive. Merging fix2204 to 0.2.1 and later; closing. Thanks to everyone for helping this get done so quickly.

comment:17 Changed 19 months ago by nickm

  • Keywords tor-relay added

comment:18 Changed 19 months ago by nickm

  • Component changed from Tor Relay to Tor

comment:19 Changed 14 months ago by nickm

  • Milestone Tor: 0.2.1.x-final deleted

Milestone Tor: 0.2.1.x-final deleted

Note: See TracTickets for help on using tickets.