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 <levitte@openssl.org>
This commit is contained in:
Dr. Stephen Henson
2015-06-21 19:08:57 +01:00
parent 8d92c1f8a3
commit a273c6eeee
3 changed files with 20 additions and 92 deletions

View File

@@ -1367,15 +1367,7 @@ int ssl3_get_server_certificate(SSL *s)
SSL_R_WRONG_CERTIFICATE_TYPE); SSL_R_WRONG_CERTIFICATE_TYPE);
goto f_err; goto f_err;
} }
sc->peer_cert_type = i; s->session->peer_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]);
X509_free(s->session->peer); X509_free(s->session->peer);
CRYPTO_add(&x->references, 1, CRYPTO_LOCK_X509); 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 */ /* We must check if there is a certificate */
# ifndef OPENSSL_NO_RSA # ifndef OPENSSL_NO_RSA
if (alg_a & SSL_aRSA) if (alg_a & SSL_aRSA)
pkey = pkey = X509_get_pubkey(s->session->peer);
X509_get_pubkey(s->session->
sess_cert->peer_pkeys[SSL_PKEY_RSA_ENC].x509);
# else # else
if (0) ; if (0) ;
# endif # endif
# ifndef OPENSSL_NO_DSA # ifndef OPENSSL_NO_DSA
else if (alg_a & SSL_aDSS) else if (alg_a & SSL_aDSS)
pkey = pkey = X509_get_pubkey(s->session->peer);
X509_get_pubkey(s->session->
sess_cert->peer_pkeys[SSL_PKEY_DSA_SIGN].
x509);
# endif # endif
} else } else
#endif /* !OPENSSL_NO_SRP */ #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 */ /* this should be because we are using an export cipher */
if (alg_a & SSL_aRSA) if (alg_a & SSL_aRSA)
pkey = pkey = X509_get_pubkey(s->session->peer);
X509_get_pubkey(s->session->
sess_cert->peer_pkeys[SSL_PKEY_RSA_ENC].x509);
else { else {
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
goto err; goto err;
@@ -1791,18 +1776,13 @@ int ssl3_get_key_exchange(SSL *s)
} }
# ifndef OPENSSL_NO_RSA # ifndef OPENSSL_NO_RSA
if (alg_a & SSL_aRSA) if (alg_a & SSL_aRSA)
pkey = pkey = X509_get_pubkey(s->session->peer);
X509_get_pubkey(s->session->
sess_cert->peer_pkeys[SSL_PKEY_RSA_ENC].x509);
# else # else
if (0) ; if (0) ;
# endif # endif
# ifndef OPENSSL_NO_DSA # ifndef OPENSSL_NO_DSA
else if (alg_a & SSL_aDSS) else if (alg_a & SSL_aDSS)
pkey = pkey = X509_get_pubkey(s->session->peer);
X509_get_pubkey(s->session->
sess_cert->peer_pkeys[SSL_PKEY_DSA_SIGN].
x509);
# endif # endif
/* else anonymous DH, so no certificate or pkey. */ /* else anonymous DH, so no certificate or pkey. */
@@ -1905,15 +1885,11 @@ int ssl3_get_key_exchange(SSL *s)
if (0) ; if (0) ;
# ifndef OPENSSL_NO_RSA # ifndef OPENSSL_NO_RSA
else if (alg_a & SSL_aRSA) else if (alg_a & SSL_aRSA)
pkey = pkey = X509_get_pubkey(s->session->peer);
X509_get_pubkey(s->session->
sess_cert->peer_pkeys[SSL_PKEY_RSA_ENC].x509);
# endif # endif
# ifndef OPENSSL_NO_EC # ifndef OPENSSL_NO_EC
else if (alg_a & SSL_aECDSA) else if (alg_a & SSL_aECDSA)
pkey = pkey = X509_get_pubkey(s->session->peer);
X509_get_pubkey(s->session->
sess_cert->peer_pkeys[SSL_PKEY_ECC].x509);
# endif # endif
/* else anonymous ECDH, so no certificate or pkey. */ /* else anonymous ECDH, so no certificate or pkey. */
EC_KEY_set_public_key(ecdh, srvr_ecpoint); 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) if (s->s3->peer_rsa_tmp != NULL)
rsa = s->s3->peer_rsa_tmp; rsa = s->s3->peer_rsa_tmp;
else { else {
pkey = pkey = X509_get_pubkey(s->session->peer);
X509_get_pubkey(s->session->
sess_cert->peer_pkeys[SSL_PKEY_RSA_ENC].
x509);
if ((pkey == NULL) || (pkey->type != EVP_PKEY_RSA) if ((pkey == NULL) || (pkey->type != EVP_PKEY_RSA)
|| (pkey->pkey.rsa == NULL)) { || (pkey->pkey.rsa == NULL)) {
SSLerr(SSL_F_SSL3_SEND_CLIENT_KEY_EXCHANGE, 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; dh_srvr = s->s3->peer_dh_tmp;
else { else {
/* we get them from the cert */ /* we get them from the cert */
int idx = scert->peer_cert_type;
EVP_PKEY *spkey = NULL; EVP_PKEY *spkey = NULL;
dh_srvr = NULL; dh_srvr = NULL;
if (idx >= 0) spkey = X509_get_pubkey(s->session->peer);
spkey = X509_get_pubkey(scert->peer_pkeys[idx].x509);
if (spkey) { if (spkey) {
dh_srvr = EVP_PKEY_get1_DH(spkey); dh_srvr = EVP_PKEY_get1_DH(spkey);
EVP_PKEY_free(spkey); EVP_PKEY_free(spkey);
@@ -2628,9 +2599,7 @@ int ssl3_send_client_key_exchange(SSL *s)
tkey = s->s3->peer_ecdh_tmp; tkey = s->s3->peer_ecdh_tmp;
} else { } else {
/* Get the Server Public Key from Cert */ /* Get the Server Public Key from Cert */
srvr_pub_pkey = srvr_pub_pkey = X509_get_pubkey(s->session->peer);
X509_get_pubkey(s->session->
sess_cert->peer_pkeys[SSL_PKEY_ECC].x509);
if ((srvr_pub_pkey == NULL) if ((srvr_pub_pkey == NULL)
|| (srvr_pub_pkey->type != EVP_PKEY_EC) || (srvr_pub_pkey->type != EVP_PKEY_EC)
|| (srvr_pub_pkey->pkey.ec == NULL)) { || (srvr_pub_pkey->pkey.ec == NULL)) {
@@ -2758,7 +2727,6 @@ int ssl3_send_client_key_exchange(SSL *s)
X509 *peer_cert; X509 *peer_cert;
size_t msglen; size_t msglen;
unsigned int md_len; unsigned int md_len;
int keytype;
unsigned char shared_ukm[32], tmp[256]; unsigned char shared_ukm[32], tmp[256];
EVP_MD_CTX *ukm_hash; EVP_MD_CTX *ukm_hash;
EVP_PKEY *pub_key; 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 * Get server sertificate PKEY and create ctx from it
*/ */
peer_cert = peer_cert = s->session->peer;
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;
if (!peer_cert) { if (!peer_cert) {
SSLerr(SSL_F_SSL3_SEND_CLIENT_KEY_EXCHANGE, SSLerr(SSL_F_SSL3_SEND_CLIENT_KEY_EXCHANGE,
SSL_R_NO_GOST_CERTIFICATE_SENT_BY_PEER); 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; alg_k = s->s3->tmp.new_cipher->algorithm_mkey;
/* See if we can use client certificate for fixed DH */ /* See if we can use client certificate for fixed DH */
if (alg_k & (SSL_kDHr | SSL_kDHd)) { if (alg_k & (SSL_kDHr | SSL_kDHd)) {
SESS_CERT *scert = s->session->sess_cert; int i = s->session->peer_type;
int i = scert->peer_cert_type;
EVP_PKEY *clkey = NULL, *spkey = NULL; EVP_PKEY *clkey = NULL, *spkey = NULL;
clkey = s->cert->key->privatekey; clkey = s->cert->key->privatekey;
/* If client key not DH assume it can be used */ /* If client key not DH assume it can be used */
if (EVP_PKEY_id(clkey) != EVP_PKEY_DH) if (EVP_PKEY_id(clkey) != EVP_PKEY_DH)
return 1; return 1;
if (i >= 0) if (i >= 0)
spkey = X509_get_pubkey(scert->peer_pkeys[i].x509); spkey = X509_get_pubkey(s->session->peer);
if (spkey) { if (spkey) {
/* Compare server and client parameters */ /* Compare server and client parameters */
i = EVP_PKEY_cmp_parameters(clkey, spkey); i = EVP_PKEY_cmp_parameters(clkey, spkey);
@@ -3365,10 +3326,10 @@ int ssl3_check_cert_and_algorithm(SSL *s)
/* This is the passed certificate */ /* This is the passed certificate */
idx = sc->peer_cert_type; idx = s->session->peer_type;
#ifndef OPENSSL_NO_EC #ifndef OPENSSL_NO_EC
if (idx == SSL_PKEY_ECC) { 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 */ /* check failed */
SSLerr(SSL_F_SSL3_CHECK_CERT_AND_ALGORITHM, SSL_R_BAD_ECC_CERT); SSLerr(SSL_F_SSL3_CHECK_CERT_AND_ALGORITHM, SSL_R_BAD_ECC_CERT);
goto f_err; goto f_err;
@@ -3384,9 +3345,9 @@ int ssl3_check_cert_and_algorithm(SSL *s)
goto f_err; goto f_err;
} }
#endif #endif
pkey = X509_get_pubkey(sc->peer_pkeys[idx].x509); pkey = X509_get_pubkey(s->session->peer);
pkey_bits = EVP_PKEY_bits(pkey); 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); EVP_PKEY_free(pkey);
/* Check that we have a certificate if we require one */ /* Check that we have a certificate if we require one */

View File

@@ -530,7 +530,6 @@ SESS_CERT *ssl_sess_cert_new(void)
} }
memset(ret, 0, sizeof(*ret)); memset(ret, 0, sizeof(*ret));
ret->peer_key = &(ret->peer_pkeys[SSL_PKEY_RSA_ENC]);
ret->references = 1; ret->references = 1;
return ret; return ret;
@@ -558,27 +557,9 @@ void ssl_sess_cert_free(SESS_CERT *sc)
/* i == 0 */ /* i == 0 */
sk_X509_pop_free(sc->cert_chain, X509_free); 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); 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) int ssl_verify_cert_chain(SSL *s, STACK_OF(X509) *sk)
{ {
X509 *x; X509 *x;

View File

@@ -626,13 +626,9 @@ struct ssl_session_st {
int not_resumable; int not_resumable;
/* The cert is the certificate used to establish this connection */ /* The cert is the certificate used to establish this connection */
struct sess_cert_st /* SESS_CERT */ *sess_cert; struct sess_cert_st /* SESS_CERT */ *sess_cert;
/* /* This is the cert and type for the other end. */
* 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).
*/
X509 *peer; X509 *peer;
int peer_type;
/* /*
* when app_verify_callback accepts a session where the peer's * when app_verify_callback accepts a session where the peer's
* certificate is not ok, we must remember the error for session reuse: * 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 { typedef struct sess_cert_st {
STACK_OF(X509) *cert_chain; /* as received from peer */ 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 */ int references; /* actually always 1 at the moment */
} SESS_CERT; } SESS_CERT;
/* Structure containing decoded values of signature algorithms extension */ /* 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); void ssl_cert_free(CERT *c);
__owur SESS_CERT *ssl_sess_cert_new(void); __owur SESS_CERT *ssl_sess_cert_new(void);
void ssl_sess_cert_free(SESS_CERT *sc); 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_new_session(SSL *s, int session);
__owur int ssl_get_prev_session(SSL *s, unsigned char *session, int len, __owur int ssl_get_prev_session(SSL *s, unsigned char *session, int len,
const unsigned char *limit); const unsigned char *limit);