CVE-2020-28018: Use-after-free in tls-openssl.c

[exim.git] / src / src / tls-openssl.c

diff --git a/src/src/tls-openssl.c b/src/src/tls-openssl.c
index 99d3f87f4795803adee75965bd7ab42016f18589..499384b50e34c6e6159c79a9858abe8bcc4bb133 100644 (file)
--- a/src/src/tls-openssl.c
+++ b/src/src/tls-openssl.c
@@ -3,6 +3,7 @@
 *************************************************/
 
 /* Copyright (c) University of Cambridge 1995 - 2019 */
+/* Copyright (c) The Exim Maintainers 2020 */
 /* See the file NOTICE for conditions of use and distribution. */
 
 /* Portions Copyright (c) The OpenSSL Project 1999 */
@@ -371,10 +372,10 @@ typedef struct ocsp_resp {
 } ocsp_resplist;
 
 typedef struct tls_ext_ctx_cb {
-  tls_support * tlsp;
-  uschar *certificate;
-  uschar *privatekey;
-  BOOL is_server;
+  tls_support *        tlsp;
+  uschar *     certificate;
+  uschar *     privatekey;
+  BOOL         is_server;
 #ifndef DISABLE_OCSP
   STACK_OF(X509) *verify_stack;                /* chain for verifying the proof */
   union {
@@ -389,14 +390,14 @@ typedef struct tls_ext_ctx_cb {
     } client;
   } u_ocsp;
 #endif
-  uschar *dhparam;
+  uschar *     dhparam;
   /* these are cached from first expand */
-  uschar *server_cipher_list;
+  uschar *     server_cipher_list;
   /* only passed down to tls_error: */
-  host_item *host;
+  host_item *  host;
   const uschar * verify_cert_hostnames;
 #ifndef DISABLE_EVENT
-  uschar * event_action;
+  uschar *     event_action;
 #endif
 } tls_ext_ctx_cb;
 
@@ -2498,7 +2499,7 @@ This is inconsistent with the need to verify the OCSP proof of the server cert.
 #endif
        }
 
-      /* If a certificate file is empty, the next function fails with an
+      /* If a certificate file is empty, the load function fails with an
       unhelpful error message. If we skip it, we get the correct behaviour (no
       certificates are recognized, but the error message is still misleading (it
       says no certificate was supplied).  But this is better. */
@@ -2507,9 +2508,9 @@ This is inconsistent with the need to verify the OCSP proof of the server cert.
          && !SSL_CTX_load_verify_locations(sctx, CS file, CS dir))
        return tls_error(US"SSL_CTX_load_verify_locations", host, NULL, errstr);
 
-      /* Load the list of CAs for which we will accept certs, for sending
-      to the client.  This is only for the one-file tls_verify_certificates
-      variant.
+      /* On the server load the list of CAs for which we will accept certs, for
+      sending to the client.  This is only for the one-file
+      tls_verify_certificates variant.
       If a list isn't loaded into the server, but some verify locations are set,
       the server end appears to make a wildcard request for client certs.
       Meanwhile, the client library as default behaviour *ignores* the list
@@ -2521,7 +2522,7 @@ This is inconsistent with the need to verify the OCSP proof of the server cert.
        {
        STACK_OF(X509_NAME) * names = SSL_load_client_CA_file(CS file);
 
-       SSL_CTX_set_client_CA_list(sctx, names);
+       if (!host) SSL_CTX_set_client_CA_list(sctx, names);
        DEBUG(D_tls) debug_printf("Added %d certificate authorities.\n",
                                    sk_X509_NAME_num(names));
        }
@@ -2764,8 +2765,12 @@ if (rc <= 0)
     case SSL_ERROR_SSL:
       {
       uschar * s = US"SSL_accept";
-      unsigned long e = ERR_peek_error();
-      if (ERR_GET_REASON(e) == SSL_R_WRONG_VERSION_NUMBER)
+      int r = ERR_GET_REASON(ERR_peek_error());
+      if (  r == SSL_R_WRONG_VERSION_NUMBER
+#ifdef SSL_R_VERSION_TOO_LOW
+         || r == SSL_R_VERSION_TOO_LOW
+#endif
+         || r == SSL_R_UNKNOWN_PROTOCOL || r == SSL_R_UNSUPPORTED_PROTOCOL)
        s = string_sprintf("%s (%s)", s, SSL_get_version(server_ssl));
       (void) tls_error(s, NULL, sigalrm_seen ? US"timed out" : NULL, errstr);
       return FAIL;
@@ -2910,9 +2915,9 @@ if (verify_check_given_host(CUSS &ob->tls_verify_cert_hostnames, host) == OK)
   {
   cbinfo->verify_cert_hostnames =
 #ifdef SUPPORT_I18N
-    string_domain_utf8_to_alabel(host->name, NULL);
+    string_domain_utf8_to_alabel(host->certname, NULL);
 #else
-    host->name;
+    host->certname;
 #endif
   DEBUG(D_tls) debug_printf("Cert hostname to check: \"%s\"\n",
                    cbinfo->verify_cert_hostnames);
@@ -2994,7 +2999,7 @@ if (tlsp->host_resumable)
 
   tlsp->resumption |= RESUME_CLIENT_REQUESTED;
   DEBUG(D_tls) debug_printf("checking for resumable session for %s\n", key);
-  if ((dbm_file = dbfn_open(US"tls", O_RDONLY, &dbblock, FALSE, FALSE)))
+  if ((dbm_file = dbfn_open(US"tls", O_RDWR, &dbblock, FALSE, FALSE)))
     {
     /* key for the db is the IP */
     if ((dt = dbfn_read_with_length(dbm_file, key, &len)))
@@ -3192,6 +3197,7 @@ tlsp->tlsa_usage = 0;
 #ifndef DISABLE_OCSP
   {
 # ifdef SUPPORT_DANE
+  /*XXX this should be moved to caller, to be common across gnutls/openssl */
   if (  conn_args->dane
      && ob->hosts_request_ocsp[0] == '*'
      && ob->hosts_request_ocsp[1] == '\0'
@@ -3453,6 +3459,7 @@ int inbytes;
 DEBUG(D_tls) debug_printf("Calling SSL_read(%p, %p, %u)\n", server_ssl,
   ssl_xfer_buffer, ssl_xfer_buffer_size);
 
+ERR_clear_error();
 if (smtp_receive_timeout > 0) ALARM(smtp_receive_timeout);
 inbytes = SSL_read(server_ssl, CS ssl_xfer_buffer,
                  MIN(ssl_xfer_buffer_size, lim));
@@ -3602,6 +3609,7 @@ int error;
 DEBUG(D_tls) debug_printf("Calling SSL_read(%p, %p, %u)\n", ssl,
   buff, (unsigned int)len);
 
+ERR_clear_error();
 inbytes = SSL_read(ssl, CS buff, len);
 error = SSL_get_error(ssl, inbytes);
 
@@ -3667,16 +3675,12 @@ if ((more || corked))
   {
   if (!len) buff = US &error;  /* dummy just so that string_catn is ok */
 
-#ifndef DISABLE_PIPE_CONNECT
   int save_pool = store_pool;
   store_pool = POOL_PERM;
-#endif
 
   corked = string_catn(corked, buff, len);
 
-#ifndef DISABLE_PIPE_CONNECT
   store_pool = save_pool;
-#endif
 
   if (more)
     {
@@ -3691,6 +3695,7 @@ if ((more || corked))
 for (int left = len; left > 0;)
   {
   DEBUG(D_tls) debug_printf("SSL_write(%p, %p, %d)\n", ssl, buff, left);
+  ERR_clear_error();
   outbytes = SSL_write(ssl, CS buff, left);
   error = SSL_get_error(ssl, outbytes);
   DEBUG(D_tls) debug_printf("outbytes=%d error=%d\n", outbytes, error);