Remove certificates from sess_cert
authorDr. Stephen Henson <steve@openssl.org>
Sun, 21 Jun 2015 18:08:57 +0000 (19:08 +0100)
committerDr. Stephen Henson <steve@openssl.org>
Mon, 22 Jun 2015 12:52:24 +0000 (13:52 +0100)
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>
ssl/s3_clnt.c
ssl/ssl_cert.c
ssl/ssl_locl.h

index 524842f..9227148 100644 (file)
@@ -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 */
index badcc2d..6cb9677 100644 (file)
@@ -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;
index b3fabf7..97c0732 100644 (file)
@@ -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);