From 76106e60a827ddaefe1fee28a749018241d8f517 Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Tue, 12 May 2015 17:17:37 +0100 Subject: [PATCH] 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 --- ssl/s3_clnt.c | 12 ++++++------ ssl/s3_lib.c | 9 +++++++++ ssl/s3_srvr.c | 8 ++++---- ssl/ssl_cert.c | 12 +----------- ssl/ssl_lib.c | 15 +++------------ ssl/ssl_locl.h | 27 ++++++++++++++------------- ssl/t1_lib.c | 34 +++++++++++++++++----------------- 7 files changed, 54 insertions(+), 63 deletions(-) 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 { -- 2.25.1