From a273c6eeee6690d7061d3e647a5e648bbd3a997a Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Sun, 21 Jun 2015 19:08:57 +0100 Subject: [PATCH] Remove certificates from sess_cert As numerous comments indicate the certificate and key array is not an appopriate structure to store the peers certificate: so remove it and just the s->session->peer instead. Reviewed-by: Richard Levitte --- ssl/s3_clnt.c | 75 ++++++++++++-------------------------------------- ssl/ssl_cert.c | 19 ------------- ssl/ssl_locl.h | 18 ++---------- 3 files changed, 20 insertions(+), 92 deletions(-) diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index 524842f6ec..9227148651 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -1367,15 +1367,7 @@ int ssl3_get_server_certificate(SSL *s) SSL_R_WRONG_CERTIFICATE_TYPE); goto f_err; } - sc->peer_cert_type = i; - CRYPTO_add(&x->references, 1, CRYPTO_LOCK_X509); - /* - * Why would the following ever happen? We just created sc a couple - * of lines ago. - */ - X509_free(sc->peer_pkeys[i].x509); - sc->peer_pkeys[i].x509 = x; - sc->peer_key = &(sc->peer_pkeys[i]); + s->session->peer_type = i; X509_free(s->session->peer); CRYPTO_add(&x->references, 1, CRYPTO_LOCK_X509); @@ -1627,18 +1619,13 @@ int ssl3_get_key_exchange(SSL *s) /* We must check if there is a certificate */ # ifndef OPENSSL_NO_RSA if (alg_a & SSL_aRSA) - pkey = - X509_get_pubkey(s->session-> - sess_cert->peer_pkeys[SSL_PKEY_RSA_ENC].x509); + pkey = X509_get_pubkey(s->session->peer); # else if (0) ; # endif # ifndef OPENSSL_NO_DSA else if (alg_a & SSL_aDSS) - pkey = - X509_get_pubkey(s->session-> - sess_cert->peer_pkeys[SSL_PKEY_DSA_SIGN]. - x509); + pkey = X509_get_pubkey(s->session->peer); # endif } else #endif /* !OPENSSL_NO_SRP */ @@ -1697,9 +1684,7 @@ int ssl3_get_key_exchange(SSL *s) /* this should be because we are using an export cipher */ if (alg_a & SSL_aRSA) - pkey = - X509_get_pubkey(s->session-> - sess_cert->peer_pkeys[SSL_PKEY_RSA_ENC].x509); + pkey = X509_get_pubkey(s->session->peer); else { SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); goto err; @@ -1791,18 +1776,13 @@ int ssl3_get_key_exchange(SSL *s) } # ifndef OPENSSL_NO_RSA if (alg_a & SSL_aRSA) - pkey = - X509_get_pubkey(s->session-> - sess_cert->peer_pkeys[SSL_PKEY_RSA_ENC].x509); + pkey = X509_get_pubkey(s->session->peer); # else if (0) ; # endif # ifndef OPENSSL_NO_DSA else if (alg_a & SSL_aDSS) - pkey = - X509_get_pubkey(s->session-> - sess_cert->peer_pkeys[SSL_PKEY_DSA_SIGN]. - x509); + pkey = X509_get_pubkey(s->session->peer); # endif /* else anonymous DH, so no certificate or pkey. */ @@ -1905,15 +1885,11 @@ int ssl3_get_key_exchange(SSL *s) if (0) ; # ifndef OPENSSL_NO_RSA else if (alg_a & SSL_aRSA) - pkey = - X509_get_pubkey(s->session-> - sess_cert->peer_pkeys[SSL_PKEY_RSA_ENC].x509); + pkey = X509_get_pubkey(s->session->peer); # endif # ifndef OPENSSL_NO_EC else if (alg_a & SSL_aECDSA) - pkey = - X509_get_pubkey(s->session-> - sess_cert->peer_pkeys[SSL_PKEY_ECC].x509); + pkey = X509_get_pubkey(s->session->peer); # endif /* else anonymous ECDH, so no certificate or pkey. */ EC_KEY_set_public_key(ecdh, srvr_ecpoint); @@ -2449,10 +2425,7 @@ int ssl3_send_client_key_exchange(SSL *s) if (s->s3->peer_rsa_tmp != NULL) rsa = s->s3->peer_rsa_tmp; else { - pkey = - X509_get_pubkey(s->session-> - sess_cert->peer_pkeys[SSL_PKEY_RSA_ENC]. - x509); + pkey = X509_get_pubkey(s->session->peer); if ((pkey == NULL) || (pkey->type != EVP_PKEY_RSA) || (pkey->pkey.rsa == NULL)) { SSLerr(SSL_F_SSL3_SEND_CLIENT_KEY_EXCHANGE, @@ -2508,11 +2481,9 @@ int ssl3_send_client_key_exchange(SSL *s) dh_srvr = s->s3->peer_dh_tmp; else { /* we get them from the cert */ - int idx = scert->peer_cert_type; EVP_PKEY *spkey = NULL; dh_srvr = NULL; - if (idx >= 0) - spkey = X509_get_pubkey(scert->peer_pkeys[idx].x509); + spkey = X509_get_pubkey(s->session->peer); if (spkey) { dh_srvr = EVP_PKEY_get1_DH(spkey); EVP_PKEY_free(spkey); @@ -2628,9 +2599,7 @@ int ssl3_send_client_key_exchange(SSL *s) tkey = s->s3->peer_ecdh_tmp; } else { /* Get the Server Public Key from Cert */ - srvr_pub_pkey = - X509_get_pubkey(s->session-> - sess_cert->peer_pkeys[SSL_PKEY_ECC].x509); + srvr_pub_pkey = X509_get_pubkey(s->session->peer); if ((srvr_pub_pkey == NULL) || (srvr_pub_pkey->type != EVP_PKEY_EC) || (srvr_pub_pkey->pkey.ec == NULL)) { @@ -2758,7 +2727,6 @@ int ssl3_send_client_key_exchange(SSL *s) X509 *peer_cert; size_t msglen; unsigned int md_len; - int keytype; unsigned char shared_ukm[32], tmp[256]; EVP_MD_CTX *ukm_hash; EVP_PKEY *pub_key; @@ -2771,13 +2739,7 @@ int ssl3_send_client_key_exchange(SSL *s) /* * Get server sertificate PKEY and create ctx from it */ - peer_cert = - s->session-> - sess_cert->peer_pkeys[(keytype = SSL_PKEY_GOST01)].x509; - if (!peer_cert) - peer_cert = - s->session-> - sess_cert->peer_pkeys[(keytype = SSL_PKEY_GOST94)].x509; + peer_cert = s->session->peer; if (!peer_cert) { SSLerr(SSL_F_SSL3_SEND_CLIENT_KEY_EXCHANGE, SSL_R_NO_GOST_CERTIFICATE_SENT_BY_PEER); @@ -3218,15 +3180,14 @@ static int ssl3_check_client_certificate(SSL *s) alg_k = s->s3->tmp.new_cipher->algorithm_mkey; /* See if we can use client certificate for fixed DH */ if (alg_k & (SSL_kDHr | SSL_kDHd)) { - SESS_CERT *scert = s->session->sess_cert; - int i = scert->peer_cert_type; + int i = s->session->peer_type; EVP_PKEY *clkey = NULL, *spkey = NULL; clkey = s->cert->key->privatekey; /* If client key not DH assume it can be used */ if (EVP_PKEY_id(clkey) != EVP_PKEY_DH) return 1; if (i >= 0) - spkey = X509_get_pubkey(scert->peer_pkeys[i].x509); + spkey = X509_get_pubkey(s->session->peer); if (spkey) { /* Compare server and client parameters */ i = EVP_PKEY_cmp_parameters(clkey, spkey); @@ -3365,10 +3326,10 @@ int ssl3_check_cert_and_algorithm(SSL *s) /* This is the passed certificate */ - idx = sc->peer_cert_type; + idx = s->session->peer_type; #ifndef OPENSSL_NO_EC if (idx == SSL_PKEY_ECC) { - if (ssl_check_srvr_ecc_cert_and_alg(sc->peer_pkeys[idx].x509, s) == 0) { + if (ssl_check_srvr_ecc_cert_and_alg(s->session->peer, s) == 0) { /* check failed */ SSLerr(SSL_F_SSL3_CHECK_CERT_AND_ALGORITHM, SSL_R_BAD_ECC_CERT); goto f_err; @@ -3384,9 +3345,9 @@ int ssl3_check_cert_and_algorithm(SSL *s) goto f_err; } #endif - pkey = X509_get_pubkey(sc->peer_pkeys[idx].x509); + pkey = X509_get_pubkey(s->session->peer); pkey_bits = EVP_PKEY_bits(pkey); - i = X509_certificate_type(sc->peer_pkeys[idx].x509, pkey); + i = X509_certificate_type(s->session->peer, pkey); EVP_PKEY_free(pkey); /* Check that we have a certificate if we require one */ diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index badcc2d147..6cb967772a 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -530,7 +530,6 @@ SESS_CERT *ssl_sess_cert_new(void) } memset(ret, 0, sizeof(*ret)); - ret->peer_key = &(ret->peer_pkeys[SSL_PKEY_RSA_ENC]); ret->references = 1; return ret; @@ -558,27 +557,9 @@ void ssl_sess_cert_free(SESS_CERT *sc) /* i == 0 */ sk_X509_pop_free(sc->cert_chain, X509_free); - for (i = 0; i < SSL_PKEY_NUM; i++) { - X509_free(sc->peer_pkeys[i].x509); -#if 0 - /* - * We don't have the peer's private key. This line is just - * here as a reminder that we're still using a not-quite-appropriate - * data structure. - */ - EVP_PKEY_free(sc->peer_pkeys[i].privatekey); -#endif - } - OPENSSL_free(sc); } -int ssl_set_peer_cert_type(SESS_CERT *sc, int type) -{ - sc->peer_cert_type = type; - return (1); -} - int ssl_verify_cert_chain(SSL *s, STACK_OF(X509) *sk) { X509 *x; diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index b3fabf7341..97c0732ca3 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -626,13 +626,9 @@ struct ssl_session_st { int not_resumable; /* The cert is the certificate used to establish this connection */ struct sess_cert_st /* SESS_CERT */ *sess_cert; - /* - * This is the cert for the other end. On clients, it will be the same as - * sess_cert->peer_key->x509 (the latter is not enough as sess_cert is - * not retained in the external representation of sessions, see - * ssl_asn1.c). - */ + /* This is the cert and type for the other end. */ X509 *peer; + int peer_type; /* * when app_verify_callback accepts a session where the peer's * certificate is not ok, we must remember the error for session reuse: @@ -1592,15 +1588,6 @@ typedef struct cert_st { typedef struct sess_cert_st { STACK_OF(X509) *cert_chain; /* as received from peer */ - /* The 'peer_...' members are used only by clients. */ - int peer_cert_type; - CERT_PKEY *peer_key; /* points to an element of peer_pkeys (never - * NULL!) */ - CERT_PKEY peer_pkeys[SSL_PKEY_NUM]; - /* - * Obviously we don't have the private keys of these, so maybe we - * shouldn't even use the CERT_PKEY type here. - */ int references; /* actually always 1 at the moment */ } SESS_CERT; /* Structure containing decoded values of signature algorithms extension */ @@ -1859,7 +1846,6 @@ void ssl_cert_clear_certs(CERT *c); void ssl_cert_free(CERT *c); __owur SESS_CERT *ssl_sess_cert_new(void); void ssl_sess_cert_free(SESS_CERT *sc); -__owur int ssl_set_peer_cert_type(SESS_CERT *c, int type); __owur int ssl_get_new_session(SSL *s, int session); __owur int ssl_get_prev_session(SSL *s, unsigned char *session, int len, const unsigned char *limit); -- 2.40.0