Commit bf413829 authored by Nick Mathewson's avatar Nick Mathewson 🤹
Browse files

Detect openssl bug 7712 and work around it. 

In theory it would be better to detect this bug in advance, but this
approach is much simpler, and therefore safer to backport.

This closes tor issue 28973.
parent 93019b47

changes/ticket28973

0 → 100644
+6 −0
Original line number Diff line number Diff line 
  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.

src/common/tortls.c

+45 −1
Original line number Diff line number Diff line
@@ -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;

}

src/or/connection_or.c

+10 −7
Original line number Diff line number Diff line
@@ -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,

    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;

}