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

added component::core tor/tor priority::immediate reporter::stars resolution::fixed severity::normal status::closed tor-relay type::defect labels

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

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

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

Trac:
Username: stars

Trac:
Summary: relay are not published anymore in consensus to Recent openssls (1.0.0b and 0.9.8p) break relay handshakes
Milestone: N/A to Tor: 0.2.1.x-final

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)

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;

(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)

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.)

Trac:
Priority: critical to blocker

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).

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?

Seems to work fine.

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".

Trac:
Status: new to needs_review

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

Trac:
Cc: N/A to weasel

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

Trac:
Username: stars

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

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

Trac:
Keywords: N/A deleted, tor-relay added

Trac:
Component: Tor Relay to Tor

Milestone Tor: 0.2.1.x-final deleted

Trac:
Milestone: Tor: 0.2.1.x-final to N/A

Set all tickets without a severity to "Normal"

Trac:
Severity: N/A to Normal

closed

mentioned in issue #2205 (moved)

mentioned in issue #2282 (moved)

mentioned in issue #2299 (moved)

moved to tpo/core/tor#2204 (closed)

mentioned in issue tpo/core/tor#2282 (closed)

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

Child items ...

Activity