From: Dr. Stephen Henson <steve@openssl.org>
Date: Tue, 12 May 2015 16:17:37 +0000 (+0100)
Subject: CERT tidy
X-Git-Tag: OpenSSL_1_1_0-pre1~1124
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=76106e60a827ddaefe1fee28a749018241d8f517;p=openssl

CERT tidy

Move per-connection state out of the CERT structure: which should just be
for shared configuration data (e.g. certificates to use).

In particular move temporary premaster secret, raw ciphers, peer signature
algorithms and shared signature algorithms.

Reviewed-by: Rich Salz <rsalz@openssl.org>
---

diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index c0dec1ef18..3486b944d4 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -3003,13 +3003,13 @@ int ssl3_send_client_key_exchange(SSL *s)
 #endif
         /* If we haven't written everything save PMS */
     if (n <= 0) {
-        s->cert->pms = pms;
-        s->cert->pmslen = pmslen;
+        s->s3->tmp.pms = pms;
+        s->s3->tmp.pmslen = pmslen;
     } else {
         /* If we don't have a PMS restore */
         if (pms == NULL) {
-            pms = s->cert->pms;
-            pmslen = s->cert->pmslen;
+            pms = s->s3->tmp.pms;
+            pmslen = s->s3->tmp.pmslen;
         }
         if (pms == NULL) {
             ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
@@ -3022,7 +3022,7 @@ int ssl3_send_client_key_exchange(SSL *s)
                                                         session->master_key,
                                                         pms, pmslen);
         OPENSSL_clear_free(pms, pmslen);
-        s->cert->pms = NULL;
+        s->s3->tmp.pms = NULL;
         if (s->session->master_key_length < 0) {
             ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
             SSLerr(SSL_F_SSL3_SEND_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
@@ -3035,7 +3035,7 @@ int ssl3_send_client_key_exchange(SSL *s)
     SSLerr(SSL_F_SSL3_SEND_CLIENT_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE);
  err:
     OPENSSL_clear_free(pms, pmslen);
-    s->cert->pms = NULL;
+    s->s3->tmp.pms = NULL;
 #ifndef OPENSSL_NO_EC
     BN_CTX_free(bn_ctx);
     OPENSSL_free(encodedPoint);
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 1a67e4ed2a..72c47d6473 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -2902,6 +2902,9 @@ void ssl3_free(SSL *s)
 #endif
 
     sk_X509_NAME_pop_free(s->s3->tmp.ca_names, X509_NAME_free);
+    OPENSSL_free(s->s3->tmp.ciphers_raw);
+    OPENSSL_clear_free(s->s3->tmp.pms, s->s3->tmp.pmslen);
+    OPENSSL_free(s->s3->tmp.peer_sigalgs);
     BIO_free(s->s3->handshake_buffer);
     if (s->s3->handshake_dgst)
         ssl3_free_digest_list(s);
@@ -2922,6 +2925,12 @@ void ssl3_clear(SSL *s)
 
     ssl3_cleanup_key_block(s);
     sk_X509_NAME_pop_free(s->s3->tmp.ca_names, X509_NAME_free);
+    OPENSSL_free(s->s3->tmp.ciphers_raw);
+    s->s3->tmp.ciphers_raw = NULL;
+    OPENSSL_clear_free(s->s3->tmp.pms, s->s3->tmp.pmslen);
+    s->s3->tmp.pms = NULL;
+    OPENSSL_free(s->s3->tmp.peer_sigalgs);
+    s->s3->tmp.peer_sigalgs = NULL;
 
 #ifndef OPENSSL_NO_DH
     DH_free(s->s3->tmp.dh);
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index ce092a70a3..6bc80d579b 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -3572,13 +3572,13 @@ STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, unsigned char *p,
         sk_SSL_CIPHER_zero(sk);
     }
 
-    OPENSSL_free(s->cert->ciphers_raw);
-    s->cert->ciphers_raw = BUF_memdup(p, num);
-    if (s->cert->ciphers_raw == NULL) {
+    OPENSSL_free(s->s3->tmp.ciphers_raw);
+    s->s3->tmp.ciphers_raw = BUF_memdup(p, num);
+    if (s->s3->tmp.ciphers_raw == NULL) {
         SSLerr(SSL_F_SSL_BYTES_TO_CIPHER_LIST, ERR_R_MALLOC_FAILURE);
         goto err;
     }
-    s->cert->ciphers_rawlen = (size_t)num;
+    s->s3->tmp.ciphers_rawlen = (size_t)num;
 
     for (i = 0; i < num; i += n) {
         /* Check for TLS_EMPTY_RENEGOTIATION_INFO_SCSV */
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index d8b47e6209..fd7a9a60c1 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -311,11 +311,7 @@ CERT *ssl_cert_dup(CERT *cert)
      * will be set during handshake.
      */
     ssl_cert_set_default_md(ret);
-    /* Peer sigalgs set to NULL as we get these from handshake too */
-    ret->peer_sigalgs = NULL;
-    ret->peer_sigalgslen = 0;
-    /* Configured sigalgs however we copy across */
-
+    /* Configured sigalgs copied across */
     if (cert->conf_sigalgs) {
         ret->conf_sigalgs = OPENSSL_malloc(cert->conf_sigalgslen);
         if (!ret->conf_sigalgs)
@@ -361,8 +357,6 @@ CERT *ssl_cert_dup(CERT *cert)
         ret->chain_store = cert->chain_store;
     }
 
-    ret->ciphers_raw = NULL;
-
     ret->sec_cb = cert->sec_cb;
     ret->sec_level = cert->sec_level;
     ret->sec_ex = cert->sec_ex;
@@ -438,20 +432,16 @@ void ssl_cert_free(CERT *c)
 #endif
 
     ssl_cert_clear_certs(c);
-    OPENSSL_free(c->peer_sigalgs);
     OPENSSL_free(c->conf_sigalgs);
     OPENSSL_free(c->client_sigalgs);
     OPENSSL_free(c->shared_sigalgs);
     OPENSSL_free(c->ctypes);
     X509_STORE_free(c->verify_store);
     X509_STORE_free(c->chain_store);
-    OPENSSL_free(c->ciphers_raw);
 #ifndef OPENSSL_NO_TLSEXT
     custom_exts_free(&c->cli_ext);
     custom_exts_free(&c->srv_ext);
 #endif
-    OPENSSL_clear_free(c->pms, c->pmslen);
-    c->pms = NULL;
     OPENSSL_free(c);
 }
 
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index b9ae0258a0..e143bc991e 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1076,10 +1076,10 @@ long SSL_ctrl(SSL *s, int cmd, long larg, void *parg)
 
     case SSL_CTRL_GET_RAW_CIPHERLIST:
         if (parg) {
-            if (s->cert->ciphers_raw == NULL)
+            if (s->s3->tmp.ciphers_raw == NULL)
                 return 0;
-            *(unsigned char **)parg = s->cert->ciphers_raw;
-            return (int)s->cert->ciphers_rawlen;
+            *(unsigned char **)parg = s->s3->tmp.ciphers_raw;
+            return (int)s->s3->tmp.ciphers_rawlen;
         } else
             return ssl_put_cipher_by_char(s, NULL, NULL);
     case SSL_CTRL_GET_EXTMS_SUPPORT:
@@ -2826,15 +2826,6 @@ SSL_CTX *SSL_set_SSL_CTX(SSL *ssl, SSL_CTX *ctx)
     if (new_cert == NULL) {
         return NULL;
     }
-    /* Preserve any already negotiated parameters */
-    if (ssl->server) {
-        new_cert->peer_sigalgs = ssl->cert->peer_sigalgs;
-        new_cert->peer_sigalgslen = ssl->cert->peer_sigalgslen;
-        ssl->cert->peer_sigalgs = NULL;
-        new_cert->ciphers_raw = ssl->cert->ciphers_raw;
-        new_cert->ciphers_rawlen = ssl->cert->ciphers_rawlen;
-        ssl->cert->ciphers_raw = NULL;
-    }
     ssl_cert_free(ssl->cert);
     ssl->cert = new_cert;
 
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 91eb1196f7..28a120158d 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1277,6 +1277,20 @@ typedef struct ssl3_state_st {
         char *new_compression;
 #  endif
         int cert_request;
+        /* Raw values of the cipher list from a client */
+        unsigned char *ciphers_raw;
+        size_t ciphers_rawlen;
+        /* Temporary storage for premaster secret */
+        unsigned char *pms;
+        size_t pmslen;
+        /*
+         * signature algorithms peer reports: e.g. supported signature
+         * algorithms extension for server or as part of a certificate
+         * request for client.
+         */
+        unsigned char *peer_sigalgs;
+        /* Size of above array */
+        size_t peer_sigalgslen;
     } tmp;
 
     /* Connection binding to prevent renegotiation attacks */
@@ -1531,16 +1545,6 @@ typedef struct cert_st {
      */
     unsigned char *ctypes;
     size_t ctype_num;
-    /* Temporary storage for premaster secret */
-    unsigned char *pms;
-    size_t pmslen;
-    /*
-     * signature algorithms peer reports: e.g. supported signature algorithms
-     * extension for server or as part of a certificate request for client.
-     */
-    unsigned char *peer_sigalgs;
-    /* Size of above array */
-    size_t peer_sigalgslen;
     /*
      * suppported signature algorithms. When set on a client this is sent in
      * the client hello as the supported signature algorithms extension. For
@@ -1580,9 +1584,6 @@ typedef struct cert_st {
      */
     X509_STORE *chain_store;
     X509_STORE *verify_store;
-    /* Raw values of the cipher list from a client */
-    unsigned char *ciphers_raw;
-    size_t ciphers_rawlen;
     /* Custom extension methods for server and client */
     custom_ext_methods cli_ext;
     custom_ext_methods srv_ext;
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index af0be02f0c..0df324dbde 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1889,8 +1889,8 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
 # endif                         /* !OPENSSL_NO_EC */
 
     /* Clear any signature algorithms extension received */
-    OPENSSL_free(s->cert->peer_sigalgs);
-    s->cert->peer_sigalgs = NULL;
+    OPENSSL_free(s->s3->tmp.peer_sigalgs);
+    s->s3->tmp.peer_sigalgs = NULL;
 # ifdef TLSEXT_TYPE_encrypt_then_mac
     s->s3->flags &= ~TLS1_FLAGS_ENCRYPT_THEN_MAC;
 # endif
@@ -2107,7 +2107,7 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
             }
         } else if (type == TLSEXT_TYPE_signature_algorithms) {
             int dsize;
-            if (s->cert->peer_sigalgs || size < 2) {
+            if (s->s3->tmp.peer_sigalgs || size < 2) {
                 *al = SSL_AD_DECODE_ERROR;
                 return 0;
             }
@@ -2684,7 +2684,7 @@ int tls1_set_server_sigalgs(SSL *s)
     }
 
     /* If sigalgs received process it. */
-    if (s->cert->peer_sigalgs) {
+    if (s->s3->tmp.peer_sigalgs) {
         if (!tls1_process_sigalgs(s)) {
             SSLerr(SSL_F_TLS1_SET_SERVER_SIGALGS, ERR_R_MALLOC_FAILURE);
             al = SSL_AD_INTERNAL_ERROR;
@@ -3386,13 +3386,13 @@ static int tls1_set_shared_sigalgs(SSL *s)
     if (s->options & SSL_OP_CIPHER_SERVER_PREFERENCE || is_suiteb) {
         pref = conf;
         preflen = conflen;
-        allow = c->peer_sigalgs;
-        allowlen = c->peer_sigalgslen;
+        allow = s->s3->tmp.peer_sigalgs;
+        allowlen = s->s3->tmp.peer_sigalgslen;
     } else {
         allow = conf;
         allowlen = conflen;
-        pref = c->peer_sigalgs;
-        preflen = c->peer_sigalgslen;
+        pref = s->s3->tmp.peer_sigalgs;
+        preflen = s->s3->tmp.peer_sigalgslen;
     }
     nmatch = tls12_shared_sigalgs(s, NULL, pref, preflen, allow, allowlen);
     if (nmatch) {
@@ -3420,12 +3420,12 @@ int tls1_save_sigalgs(SSL *s, const unsigned char *data, int dsize)
     if (!c)
         return 0;
 
-    OPENSSL_free(c->peer_sigalgs);
-    c->peer_sigalgs = OPENSSL_malloc(dsize);
-    if (!c->peer_sigalgs)
+    OPENSSL_free(s->s3->tmp.peer_sigalgs);
+    s->s3->tmp.peer_sigalgs = OPENSSL_malloc(dsize);
+    if (s->s3->tmp.peer_sigalgs == NULL)
         return 0;
-    c->peer_sigalgslen = dsize;
-    memcpy(c->peer_sigalgs, data, dsize);
+    s->s3->tmp.peer_sigalgslen = dsize;
+    memcpy(s->s3->tmp.peer_sigalgs, data, dsize);
     return 1;
 }
 
@@ -3510,12 +3510,12 @@ int SSL_get_sigalgs(SSL *s, int idx,
                     int *psign, int *phash, int *psignhash,
                     unsigned char *rsig, unsigned char *rhash)
 {
-    const unsigned char *psig = s->cert->peer_sigalgs;
+    const unsigned char *psig = s->s3->tmp.peer_sigalgs;
     if (psig == NULL)
         return 0;
     if (idx >= 0) {
         idx <<= 1;
-        if (idx >= (int)s->cert->peer_sigalgslen)
+        if (idx >= (int)s->s3->tmp.peer_sigalgslen)
             return 0;
         psig += idx;
         if (rhash)
@@ -3524,7 +3524,7 @@ int SSL_get_sigalgs(SSL *s, int idx,
             *rsig = psig[1];
         tls1_lookup_sigalg(phash, psign, psignhash, psig);
     }
-    return s->cert->peer_sigalgslen / 2;
+    return s->s3->tmp.peer_sigalgslen / 2;
 }
 
 int SSL_get_shared_sigalgs(SSL *s, int idx,
@@ -3923,7 +3923,7 @@ int tls1_check_chain(SSL *s, X509 *x, EVP_PKEY *pk, STACK_OF(X509) *chain,
     if (TLS1_get_version(s) >= TLS1_2_VERSION && strict_mode) {
         int default_nid;
         unsigned char rsign = 0;
-        if (c->peer_sigalgs)
+        if (s->s3->tmp.peer_sigalgs)
             default_nid = 0;
         /* If no sigalgs extension use defaults from RFC5246 */
         else {