Browse Source

Merge branch 'ticket28973_033_squashed' into maint-0.3.3

Nick Mathewson 5 years ago
parent
commit
1ea3127188
3 changed files with 61 additions and 8 deletions
  1. 6 0
      changes/ticket28973
  2. 45 1
      src/common/tortls.c
  3. 10 7
      src/or/connection_or.c

+ 6 - 0
changes/ticket28973

@@ -0,0 +1,6 @@
+  o Minor features (OpenSSL bug workaround):
+    - Work around a bug in OpenSSL 1.1.1a, which prevented the TLS 1.3
+      key export function from handling long labels.  When this bug
+      is detected, Tor will disable TLS 1.3.  We recommend upgrading to
+      a version of OpenSSL without this bug when it becomes available.
+      Closes ticket 28973.

+ 45 - 1
src/common/tortls.c

@@ -89,6 +89,9 @@ ENABLE_GCC_WARNING(redundant-decls)
 #define SSL3_FLAGS_ALLOW_UNSAFE_LEGACY_RENEGOTIATION 0x0010
 #endif
 
+/** Set to true iff openssl bug 7712 has been detected. */
+static int openssl_bug_7712_is_present = 0;
+
 /** Return values for tor_tls_classify_client_ciphers.
  *
  * @{
@@ -1701,6 +1704,13 @@ tor_tls_new(int sock, int isServer)
   }
 #endif /* defined(SSL_set_tlsext_host_name) */
 
+#ifdef SSL_CTRL_SET_MAX_PROTO_VERSION
+  if (openssl_bug_7712_is_present) {
+    /* We can't actually use TLS 1.3 until this bug is fixed. */
+    SSL_set_max_proto_version(result->ssl, TLS1_2_VERSION);
+  }
+#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");
@@ -2586,7 +2596,8 @@ tor_tls_get_tlssecrets,(tor_tls_t *tls, uint8_t *secrets_out))
  * provided <b>context</b> (<b>context_len</b> bytes long) and
  * <b>label</b> (a NUL-terminated string), compute a 32-byte secret in
  * <b>secrets_out</b> that only the parties to this TLS session can
- * compute.  Return 0 on success and -1 on failure.
+ * compute.  Return 0 on success; -1 on failure; and -2 on failure
+ * caused by OpenSSL bug 7712.
  */
 MOCK_IMPL(int,
 tor_tls_export_key_material,(tor_tls_t *tls, uint8_t *secrets_out,
@@ -2601,6 +2612,39 @@ tor_tls_export_key_material,(tor_tls_t *tls, uint8_t *secrets_out,
                                      secrets_out, DIGEST256_LEN,
                                      label, strlen(label),
                                      context, context_len, 1);
+
+  if (r != 1) {
+    int severity = openssl_bug_7712_is_present ? LOG_WARN : LOG_DEBUG;
+    tls_log_errors(tls, severity, LD_NET, "exporting keying material");
+  }
+
+#ifdef TLS1_3_VERSION
+  if (r != 1 &&
+      strlen(label) > 12 &&
+      SSL_version(tls->ssl) >= TLS1_3_VERSION) {
+
+    if (! openssl_bug_7712_is_present) {
+      /* We might have run into OpenSSL issue 7712, which caused OpenSSL
+       * 1.1.1a to not handle long labels.  Let's test to see if we have.
+       */
+      r = SSL_export_keying_material(tls->ssl, secrets_out, DIGEST256_LEN,
+                                     "short", 5, context, context_len, 1);
+      if (r == 1) {
+        /* A short label succeeds, but a long label fails. This was openssl
+         * issue 7712. */
+        openssl_bug_7712_is_present = 1;
+        log_warn(LD_GENERAL, "Detected OpenSSL bug 7712: disabling TLS 1.3 on "
+                 "future connections. A fix is expected to appear in OpenSSL "
+                 "1.1.1b.");
+      }
+    }
+    if (openssl_bug_7712_is_present)
+      return -2;
+    else
+      return -1;
+  }
+#endif
+
   return (r == 1) ? 0 : -1;
 }
 

+ 10 - 7
src/or/connection_or.c

@@ -2843,9 +2843,15 @@ connection_or_compute_authenticate_cell_body(or_connection_t *conn,
     char label[128];
     tor_snprintf(label, sizeof(label),
                  "EXPORTER FOR TOR TLS CLIENT BINDING %s", authtype_str);
-    tor_tls_export_key_material(conn->tls, auth->tlssecrets,
-                                auth->cid, sizeof(auth->cid),
-                                label);
+    int r = tor_tls_export_key_material(conn->tls, auth->tlssecrets,
+                                        auth->cid, sizeof(auth->cid),
+                                        label);
+    if (r < 0) {
+      if (r != -2)
+        log_warn(LD_BUG, "TLS key export failed for unknown reason.");
+      // If r == -2, this was openssl bug 7712.
+      goto err;
+    }
   }
 
   /* 8 octets were reserved for the current time, but we're trying to get out
@@ -2973,14 +2979,11 @@ connection_or_send_authenticate_cell,(or_connection_t *conn, int authtype))
                                                  get_current_auth_keypair(),
                                                  0 /* not server */);
   if (! cell) {
-    /* LCOV_EXCL_START */
-    log_warn(LD_BUG, "Unable to compute authenticate cell!");
+    log_fn(LOG_PROTOCOL_WARN, LD_NET, "Unable to compute authenticate cell!");
     return -1;
-    /* LCOV_EXCL_STOP */
   }
   connection_or_write_var_cell_to_buf(cell, conn);
   var_cell_free(cell);
 
   return 0;
 }
-