Remove peer_pkeys from SSL_SESSION.
authorjsing <jsing@openbsd.org>
Tue, 11 Jan 2022 19:03:15 +0000 (19:03 +0000)
committerjsing <jsing@openbsd.org>
Tue, 11 Jan 2022 19:03:15 +0000 (19:03 +0000)
peer_pkeys comes from some world where peers can send multiple certificates
- in fact, one of each known type. Since we do not live in such a world,
get rid of peer_pkeys and simply use peer_cert instead (in both TLSv1.2
and TLSv1.3, both clients and servers can only send a single leaf
(aka end-entity) certificate).

ok inoguchi@ tb@

lib/libssl/ssl_clnt.c
lib/libssl/ssl_locl.h
lib/libssl/ssl_sess.c
lib/libssl/ssl_srvr.c
lib/libssl/tls13_client.c
lib/libssl/tls13_server.c

index 8b5ccd4..61c1d71 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_clnt.c,v 1.136 2022/01/11 18:39:28 jsing Exp $ */
+/* $OpenBSD: ssl_clnt.c,v 1.137 2022/01/11 19:03:15 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1071,12 +1071,13 @@ ssl3_get_server_hello(SSL *s)
 int
 ssl3_get_server_certificate(SSL *s)
 {
-       int al, i, ret;
        CBS cbs, cert_list;
        X509 *x = NULL;
        const unsigned char *q;
        STACK_OF(X509) *sk = NULL;
-       EVP_PKEY *pkey = NULL;
+       EVP_PKEY *pkey;
+       int cert_type;
+       int al, ret;
 
        if ((ret = ssl3_get_message(s, SSL3_ST_CR_CERT_A,
            SSL3_ST_CR_CERT_B, -1, s->internal->max_cert_list)) <= 0)
@@ -1144,12 +1145,11 @@ ssl3_get_server_certificate(SSL *s)
                x = NULL;
        }
 
-       i = ssl_verify_cert_chain(s, sk);
-       if ((s->verify_mode != SSL_VERIFY_NONE) && (i <= 0)) {
+       if (ssl_verify_cert_chain(s, sk) <= 0 &&
+           s->verify_mode != SSL_VERIFY_NONE) {
                al = ssl_verify_alarm_type(s->verify_result);
                SSLerror(s, SSL_R_CERTIFICATE_VERIFY_FAILED);
                goto fatal_err;
-
        }
        ERR_clear_error(); /* but we keep s->verify_result */
 
@@ -1159,39 +1159,31 @@ ssl3_get_server_certificate(SSL *s)
         */
        x = sk_X509_value(sk, 0);
 
-       pkey = X509_get_pubkey(x);
-
-       if (pkey == NULL || EVP_PKEY_missing_parameters(pkey)) {
+       if ((pkey = X509_get0_pubkey(x)) == NULL ||
+           EVP_PKEY_missing_parameters(pkey)) {
                x = NULL;
                al = SSL3_AL_FATAL;
                SSLerror(s, SSL_R_UNABLE_TO_FIND_PUBLIC_KEY_PARAMETERS);
                goto fatal_err;
        }
-
-       i = ssl_cert_type(x, pkey);
-       if (i < 0) {
+       if ((cert_type = ssl_cert_type(x, pkey)) < 0) {
                x = NULL;
                al = SSL3_AL_FATAL;
                SSLerror(s, SSL_R_UNKNOWN_CERTIFICATE_TYPE);
                goto fatal_err;
        }
-       s->session->peer_cert_type = i;
-
-       sk_X509_pop_free(s->session->cert_chain, X509_free);
-       s->session->cert_chain = sk;
-       sk = NULL;
-
-       X509_up_ref(x);
-       X509_free(s->session->peer_pkeys[i].x509);
-       s->session->peer_pkeys[i].x509 = x;
-       s->session->peer_key = &s->session->peer_pkeys[i];
 
        X509_up_ref(x);
        X509_free(s->session->peer_cert);
        s->session->peer_cert = x;
+       s->session->peer_cert_type = cert_type;
 
        s->session->verify_result = s->verify_result;
 
+       sk_X509_pop_free(s->session->cert_chain, X509_free);
+       s->session->cert_chain = sk;
+       sk = NULL;
+
        x = NULL;
        ret = 1;
 
@@ -1204,7 +1196,6 @@ ssl3_get_server_certificate(SSL *s)
                ssl3_send_alert(s, SSL3_AL_FATAL, al);
        }
  err:
-       EVP_PKEY_free(pkey);
        X509_free(x);
        sk_X509_pop_free(sk, X509_free);
 
@@ -1377,12 +1368,12 @@ ssl3_get_server_key_exchange(SSL *s)
                EVP_PKEY_CTX *pctx;
                EVP_PKEY *pkey = NULL;
 
-               if ((alg_a & SSL_aRSA) != 0) {
-                       pkey = X509_get0_pubkey(
-                           s->session->peer_pkeys[SSL_PKEY_RSA].x509);
-               } else if ((alg_a & SSL_aECDSA) != 0) {
-                       pkey = X509_get0_pubkey(
-                           s->session->peer_pkeys[SSL_PKEY_ECC].x509);
+               if ((alg_a & SSL_aRSA) != 0 &&
+                   s->session->peer_cert_type == SSL_PKEY_RSA) {
+                       pkey = X509_get0_pubkey(s->session->peer_cert);
+               } else if ((alg_a & SSL_aECDSA) != 0 &&
+                   s->session->peer_cert_type == SSL_PKEY_ECC) {
+                       pkey = X509_get0_pubkey(s->session->peer_cert);
                }
                if (pkey == NULL) {
                        al = SSL_AD_ILLEGAL_PARAMETER;
@@ -1800,7 +1791,7 @@ ssl3_send_client_kex_rsa(SSL *s, CBB *cbb)
        unsigned char pms[SSL_MAX_MASTER_KEY_LENGTH];
        unsigned char *enc_pms = NULL;
        uint16_t max_legacy_version;
-       EVP_PKEY *pkey = NULL;
+       EVP_PKEY *pkey;
        RSA *rsa;
        int ret = 0;
        int enc_len;
@@ -1810,7 +1801,7 @@ ssl3_send_client_kex_rsa(SSL *s, CBB *cbb)
         * RSA-Encrypted Premaster Secret Message - RFC 5246 section 7.4.7.1.
         */
 
-       pkey = X509_get_pubkey(s->session->peer_pkeys[SSL_PKEY_RSA].x509);
+       pkey = X509_get0_pubkey(s->session->peer_cert);
        if (pkey == NULL || (rsa = EVP_PKEY_get0_RSA(pkey)) == NULL) {
                SSLerror(s, ERR_R_INTERNAL_ERROR);
                goto err;
@@ -1855,7 +1846,6 @@ ssl3_send_client_kex_rsa(SSL *s, CBB *cbb)
 
  err:
        explicit_bzero(pms, sizeof(pms));
-       EVP_PKEY_free(pkey);
        free(enc_pms);
 
        return ret;
@@ -1938,8 +1928,7 @@ ssl3_send_client_kex_gost(SSL *s, CBB *cbb)
        unsigned char premaster_secret[32], shared_ukm[32], tmp[256];
        EVP_PKEY_CTX *pkey_ctx = NULL;
        EVP_MD_CTX *ukm_hash = NULL;
-       EVP_PKEY *pub_key;
-       X509 *peer_cert;
+       EVP_PKEY *pkey;
        size_t msglen;
        unsigned int md_len;
        CBB gostblob;
@@ -1947,12 +1936,12 @@ ssl3_send_client_kex_gost(SSL *s, CBB *cbb)
        int ret = 0;
 
        /* Get server sertificate PKEY and create ctx from it */
-       peer_cert = s->session->peer_pkeys[SSL_PKEY_GOST01].x509;
-       if ((pub_key = X509_get0_pubkey(peer_cert)) == NULL) {
+       pkey = X509_get0_pubkey(s->session->peer_cert);
+       if (pkey == NULL || s->session->peer_cert_type != SSL_PKEY_GOST01) {
                SSLerror(s, SSL_R_NO_GOST_CERTIFICATE_SENT_BY_PEER);
                goto err;
        }
-       if ((pkey_ctx = EVP_PKEY_CTX_new(pub_key, NULL)) == NULL) {
+       if ((pkey_ctx = EVP_PKEY_CTX_new(pkey, NULL)) == NULL) {
                SSLerror(s, ERR_R_MALLOC_FAILURE);
                goto err;
        }
@@ -2449,9 +2438,8 @@ int
 ssl3_check_cert_and_algorithm(SSL *s)
 {
        long alg_k, alg_a;
-       EVP_PKEY *pkey = NULL;
        int nid = NID_undef;
-       int i, idx;
+       int i;
 
        alg_k = S3I(s)->hs.cipher->algorithm_mkey;
        alg_a = S3I(s)->hs.cipher->algorithm_auth;
@@ -2465,20 +2453,15 @@ ssl3_check_cert_and_algorithm(SSL *s)
 
        /* This is the passed certificate. */
 
-       idx = s->session->peer_cert_type;
-       if (idx == SSL_PKEY_ECC) {
-               if (!ssl_check_srvr_ecc_cert_and_alg(s,
-                   s->session->peer_pkeys[idx].x509)) {
-                       /* check failed */
+       if (s->session->peer_cert_type == SSL_PKEY_ECC) {
+               if (!ssl_check_srvr_ecc_cert_and_alg(s, s->session->peer_cert)) {
                        SSLerror(s, SSL_R_BAD_ECC_CERT);
                        goto fatal_err;
-               } else {
-                       return (1);
                }
+               return (1);
        }
-       pkey = X509_get_pubkey(s->session->peer_pkeys[idx].x509);
-       i = X509_certificate_type(s->session->peer_pkeys[idx].x509, pkey);
-       EVP_PKEY_free(pkey);
+
+       i = X509_certificate_type(s->session->peer_cert, NULL);
 
        /* Check that we have a certificate if we require one. */
        if ((alg_a & SSL_aRSA) && !has_bits(i, EVP_PK_RSA|EVP_PKT_SIGN)) {
index 36823d6..546854b 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.382 2022/01/11 18:39:28 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.383 2022/01/11 19:03:15 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -475,8 +475,9 @@ struct ssl_session_st {
        unsigned int sid_ctx_length;
        unsigned char sid_ctx[SSL_MAX_SID_CTX_LENGTH];
 
-       /* This is the cert for the other end. */
+       /* Peer provided leaf (end-entity) certificate. */
        X509 *peer_cert;
+       int peer_cert_type;
 
        /* when app_verify_callback accepts a session where the peer's certificate
         * is not ok, we must remember the error for session reuse: */
@@ -513,14 +514,6 @@ struct ssl_session_st {
 
        STACK_OF(X509) *cert_chain; /* as received from peer */
 
-       /* The 'peer_...' members are used only by clients. */
-       int peer_cert_type;
-
-       /* Obviously we don't have the private keys of these,
-        * so maybe we shouldn't even use the SSL_CERT_PKEY type here. */
-       SSL_CERT_PKEY *peer_key; /* points to an element of peer_pkeys (never NULL!) */
-       SSL_CERT_PKEY peer_pkeys[SSL_PKEY_NUM];
-
        size_t tlsext_ecpointformatlist_length;
        uint8_t *tlsext_ecpointformatlist; /* peer's list */
        size_t tlsext_supportedgroups_length;
index a49076b..44c2e84 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_sess.c,v 1.108 2022/01/11 18:39:28 jsing Exp $ */
+/* $OpenBSD: ssl_sess.c,v 1.109 2022/01/11 19:03:15 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -230,8 +230,6 @@ SSL_SESSION_new(void)
        ss->next = NULL;
        ss->tlsext_hostname = NULL;
 
-       ss->peer_key = &ss->peer_pkeys[SSL_PKEY_RSA];
-
        ss->tlsext_ecpointformatlist_length = 0;
        ss->tlsext_ecpointformatlist = NULL;
        ss->tlsext_supportedgroups_length = 0;
@@ -763,8 +761,6 @@ SSL_SESSION_free(SSL_SESSION *ss)
        explicit_bzero(ss->session_id, sizeof ss->session_id);
 
        sk_X509_pop_free(ss->cert_chain, X509_free);
-       for (i = 0; i < SSL_PKEY_NUM; i++)
-               X509_free(ss->peer_pkeys[i].x509);
 
        X509_free(ss->peer_cert);
 
index 786362e..3054532 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.139 2022/01/11 18:39:28 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.140 2022/01/11 19:03:15 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1905,7 +1905,7 @@ ssl3_get_cert_verify(SSL *s)
        CBS cbs, signature;
        const struct ssl_sigalg *sigalg = NULL;
        uint16_t sigalg_value = SIGALG_NONE;
-       EVP_PKEY *pkey = NULL;
+       EVP_PKEY *pkey;
        X509 *peer_cert = NULL;
        EVP_MD_CTX *mctx = NULL;
        int al, verify;
@@ -1928,11 +1928,9 @@ ssl3_get_cert_verify(SSL *s)
 
        CBS_init(&cbs, s->internal->init_msg, s->internal->init_num);
 
-       if (s->session->peer_cert != NULL) {
-               peer_cert = s->session->peer_cert;
-               pkey = X509_get_pubkey(peer_cert);
-               type = X509_certificate_type(peer_cert, pkey);
-       }
+       peer_cert = s->session->peer_cert;
+       pkey = X509_get0_pubkey(peer_cert);
+       type = X509_certificate_type(peer_cert, pkey);
 
        if (S3I(s)->hs.tls12.message_type != SSL3_MT_CERTIFICATE_VERIFY) {
                S3I(s)->hs.tls12.reuse_message = 1;
@@ -2131,7 +2129,7 @@ ssl3_get_cert_verify(SSL *s)
        tls1_transcript_free(s);
  err:
        EVP_MD_CTX_free(mctx);
-       EVP_PKEY_free(pkey);
+
        return (ret);
 }
 
index 3e168a0..4b52f6c 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls13_client.c,v 1.92 2022/01/11 18:39:28 jsing Exp $ */
+/* $OpenBSD: tls13_client.c,v 1.93 2022/01/11 19:03:15 jsing Exp $ */
 /*
  * Copyright (c) 2018, 2019 Joel Sing <jsing@openbsd.org>
  *
@@ -561,7 +561,7 @@ tls13_server_certificate_recv(struct tls13_ctx *ctx, CBS *cbs)
        X509 *cert = NULL;
        EVP_PKEY *pkey;
        const uint8_t *p;
-       int cert_idx, alert_desc;
+       int alert_desc, cert_type;
        int ret = 0;
 
        if ((certs = sk_X509_new_null()) == NULL)
@@ -625,24 +625,20 @@ tls13_server_certificate_recv(struct tls13_ctx *ctx, CBS *cbs)
                goto err;
        if (EVP_PKEY_missing_parameters(pkey))
                goto err;
-       if ((cert_idx = ssl_cert_type(cert, pkey)) < 0)
+       if ((cert_type = ssl_cert_type(cert, pkey)) < 0)
                goto err;
 
-       sk_X509_pop_free(s->session->cert_chain, X509_free);
-       s->session->cert_chain = certs;
-       certs = NULL;
-
-       X509_up_ref(cert);
-       X509_free(s->session->peer_pkeys[cert_idx].x509);
-       s->session->peer_pkeys[cert_idx].x509 = cert;
-       s->session->peer_key = &s->session->peer_pkeys[cert_idx];
-
        X509_up_ref(cert);
        X509_free(s->session->peer_cert);
        s->session->peer_cert = cert;
+       s->session->peer_cert_type = cert_type;
 
        s->session->verify_result = s->verify_result;
 
+       sk_X509_pop_free(s->session->cert_chain, X509_free);
+       s->session->cert_chain = certs;
+       certs = NULL;
+
        if (ctx->ocsp_status_recv_cb != NULL &&
            !ctx->ocsp_status_recv_cb(ctx))
                goto err;
index 3330023..10e4910 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls13_server.c,v 1.94 2022/01/11 18:39:28 jsing Exp $ */
+/* $OpenBSD: tls13_server.c,v 1.95 2022/01/11 19:03:15 jsing Exp $ */
 /*
  * Copyright (c) 2019, 2020 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2020 Bob Beck <beck@openbsd.org>
@@ -857,7 +857,7 @@ tls13_client_certificate_recv(struct tls13_ctx *ctx, CBS *cbs)
        X509 *cert = NULL;
        EVP_PKEY *pkey;
        const uint8_t *p;
-       int cert_idx;
+       int cert_type;
        int ret = 0;
 
        if (!CBS_get_u8_length_prefixed(cbs, &cert_request_context))
@@ -918,24 +918,20 @@ tls13_client_certificate_recv(struct tls13_ctx *ctx, CBS *cbs)
                goto err;
        if (EVP_PKEY_missing_parameters(pkey))
                goto err;
-       if ((cert_idx = ssl_cert_type(cert, pkey)) < 0)
+       if ((cert_type = ssl_cert_type(cert, pkey)) < 0)
                goto err;
 
-       sk_X509_pop_free(s->session->cert_chain, X509_free);
-       s->session->cert_chain = certs;
-       certs = NULL;
-
-       X509_up_ref(cert);
-       X509_free(s->session->peer_pkeys[cert_idx].x509);
-       s->session->peer_pkeys[cert_idx].x509 = cert;
-       s->session->peer_key = &s->session->peer_pkeys[cert_idx];
-
        X509_up_ref(cert);
        X509_free(s->session->peer_cert);
        s->session->peer_cert = cert;
+       s->session->peer_cert_type = cert_type;
 
        s->session->verify_result = s->verify_result;
 
+       sk_X509_pop_free(s->session->cert_chain, X509_free);
+       s->session->cert_chain = certs;
+       certs = NULL;
+
        ctx->handshake_stage.hs_type |= WITH_CCV;
        ret = 1;