[openssl-commits] [openssl] OpenSSL_0_9_8-stable update

Matt Caswell matt at openssl.org
Tue Jun 2 11:52:53 UTC 2015


The branch OpenSSL_0_9_8-stable has been updated
       via  467daf6b6ef0753ccfc5c024c2f63c948354d698 (commit)
      from  113d36a3fb4c157242fa995d0cdfe7e36107fba6 (commit)


- Log -----------------------------------------------------------------
commit 467daf6b6ef0753ccfc5c024c2f63c948354d698
Author: Matt Caswell <matt at openssl.org>
Date:   Mon May 18 16:27:48 2015 +0100

    Fix race condition in NewSessionTicket
    
    If a NewSessionTicket is received by a multi-threaded client when
    attempting to reuse a previous ticket then a race condition can occur
    potentially leading to a double free of the ticket data.
    
    CVE-2015-1791
    
    This also fixes RT#3808 where a session ID is changed for a session already
    in the client session cache. Since the session ID is the key to the cache
    this breaks the cache access.
    
    Parts of this patch were inspired by this Akamai change:
    https://github.com/akamai/openssl/commit/c0bf69a791239ceec64509f9f19fcafb2461b0d3
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (cherry picked from commit 27c76b9b8010b536687318739c6f631ce4194688)
    
    Conflicts:
    	ssl/ssl.h
    	ssl/ssl_err.c

-----------------------------------------------------------------------

Summary of changes:
 ssl/s3_clnt.c  | 32 ++++++++++++++++++++++++++
 ssl/ssl.h      |  1 +
 ssl/ssl_err.c  |  1 +
 ssl/ssl_locl.h |  1 +
 ssl/ssl_sess.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 107 insertions(+)

diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 118856f..8d035f7 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -1722,6 +1722,38 @@ int ssl3_get_new_session_ticket(SSL *s)
     }
 
     p = d = (unsigned char *)s->init_msg;
+
+    if (s->session->session_id_length > 0) {
+        int i = s->session_ctx->session_cache_mode;
+        SSL_SESSION *new_sess;
+        /*
+         * We reused an existing session, so we need to replace it with a new
+         * one
+         */
+        if (i & SSL_SESS_CACHE_CLIENT) {
+            /*
+             * Remove the old session from the cache
+             */
+            if (i & SSL_SESS_CACHE_NO_INTERNAL_STORE) {
+                if (s->session_ctx->remove_session_cb != NULL)
+                    s->session_ctx->remove_session_cb(s->session_ctx,
+                                                      s->session);
+            } else {
+                /* We carry on if this fails */
+                SSL_CTX_remove_session(s->session_ctx, s->session);
+            }
+        }
+
+        if ((new_sess = ssl_session_dup(s->session, 0)) == 0) {
+            al = SSL_AD_INTERNAL_ERROR;
+            SSLerr(SSL_F_SSL3_GET_NEW_SESSION_TICKET, ERR_R_MALLOC_FAILURE);
+            goto f_err;
+        }
+
+        SSL_SESSION_free(s->session);
+        s->session = new_sess;
+    }
+
     n2l(p, s->session->tlsext_tick_lifetime_hint);
     n2s(p, ticklen);
     /* ticket_lifetime_hint + ticket_length + ticket */
diff --git a/ssl/ssl.h b/ssl/ssl.h
index ee9944f..2dcc3b8 100644
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -1945,6 +1945,7 @@ void ERR_load_SSL_strings(void);
 # define SSL_F_SSL_READ                                   223
 # define SSL_F_SSL_RSA_PRIVATE_DECRYPT                    187
 # define SSL_F_SSL_RSA_PUBLIC_ENCRYPT                     188
+# define SSL_F_SSL_SESSION_DUP                            348
 # define SSL_F_SSL_SESSION_NEW                            189
 # define SSL_F_SSL_SESSION_PRINT_FP                       190
 # define SSL_F_SSL_SESS_CERT_NEW                          225
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index c874591..65c8f61 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -279,6 +279,7 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_SSL_READ), "SSL_read"},
     {ERR_FUNC(SSL_F_SSL_RSA_PRIVATE_DECRYPT), "SSL_RSA_PRIVATE_DECRYPT"},
     {ERR_FUNC(SSL_F_SSL_RSA_PUBLIC_ENCRYPT), "SSL_RSA_PUBLIC_ENCRYPT"},
+    {ERR_FUNC(SSL_F_SSL_SESSION_DUP), "ssl_session_dup"},
     {ERR_FUNC(SSL_F_SSL_SESSION_NEW), "SSL_SESSION_new"},
     {ERR_FUNC(SSL_F_SSL_SESSION_PRINT_FP), "SSL_SESSION_print_fp"},
     {ERR_FUNC(SSL_F_SSL_SESS_CERT_NEW), "SSL_SESS_CERT_NEW"},
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 9fa209d..038554f 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -760,6 +760,7 @@ int ssl_set_peer_cert_type(SESS_CERT *c, int type);
 int ssl_get_new_session(SSL *s, int session);
 int ssl_get_prev_session(SSL *s, unsigned char *session, int len,
                          const unsigned char *limit);
+SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket);
 int ssl_cipher_id_cmp(const SSL_CIPHER *a, const SSL_CIPHER *b);
 int ssl_cipher_ptr_id_cmp(const SSL_CIPHER *const *ap,
                           const SSL_CIPHER *const *bp);
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index fc31296..9baa090 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -135,6 +135,78 @@ SSL_SESSION *SSL_SESSION_new(void)
     return (ss);
 }
 
+/*
+ * Create a new SSL_SESSION and duplicate the contents of |src| into it. If
+ * ticket == 0 then no ticket information is duplicated, otherwise it is.
+ */
+SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket)
+{
+    SSL_SESSION *dest;
+
+    dest = OPENSSL_malloc(sizeof(*src));
+    if (dest == NULL) {
+        goto err;
+    }
+    memcpy(dest, src, sizeof(*dest));
+
+#ifndef OPENSSL_NO_KRB5
+    dest->krb5_client_princ_len = dest->krb5_client_princ_len;
+    if (src->krb5_client_princ_len > 0)
+        memcpy(dest->krb5_client_princ, src->krb5_client_princ,
+               src->krb5_client_princ_len);
+#endif
+
+    if (src->sess_cert != NULL)
+        CRYPTO_add(&src->sess_cert->references, 1, CRYPTO_LOCK_SSL_SESS_CERT);
+
+    if (src->peer != NULL)
+        CRYPTO_add(&src->peer->references, 1, CRYPTO_LOCK_X509);
+
+    dest->references = 1;
+
+    if(src->ciphers != NULL) {
+        dest->ciphers = sk_SSL_CIPHER_dup(src->ciphers);
+        if (dest->ciphers == NULL)
+            goto err;
+    } else {
+        dest->ciphers = NULL;
+    }
+
+    if (!CRYPTO_dup_ex_data(CRYPTO_EX_INDEX_SSL_SESSION,
+                                            &dest->ex_data, &src->ex_data)) {
+        goto err;
+    }
+
+    /* We deliberately don't copy the prev and next pointers */
+    dest->prev = NULL;
+    dest->next = NULL;
+
+#ifndef OPENSSL_NO_TLSEXT
+    if (src->tlsext_hostname) {
+        dest->tlsext_hostname = BUF_strdup(src->tlsext_hostname);
+        if (dest->tlsext_hostname == NULL) {
+            goto err;
+        }
+    } else {
+        dest->tlsext_hostname = NULL;
+    }
+#endif
+
+    if (ticket != 0) {
+        dest->tlsext_tick_lifetime_hint = src->tlsext_tick_lifetime_hint;
+        dest->tlsext_ticklen = src->tlsext_ticklen;
+        if((dest->tlsext_tick = OPENSSL_malloc(src->tlsext_ticklen)) == NULL) {
+            goto err;
+        }
+    }
+
+    return dest;
+err:
+    SSLerr(SSL_F_SSL_SESSION_DUP, ERR_R_MALLOC_FAILURE);
+    SSL_SESSION_free(dest);
+    return NULL;
+}
+
 const unsigned char *SSL_SESSION_get_id(const SSL_SESSION *s,
                                         unsigned int *len)
 {


More information about the openssl-commits mailing list