Deduplicate peer certificate chain processing code.

Rather than reimplement this in each TLS client and server, deduplicate it
into a single function. Furthermore, rather than dealing with the API
hazard that is SSL_get_peer_cert_chain() in this code, simply produce two
chains - one that has the leaf and one that does not.
SSL_get_peer_cert_chain() can then return the appropriate one.

This also moves the peer cert chain from the SSL_SESSION to the
SSL_HANDSHAKE, which makes more sense since it is not available on
resumption.

ok tb@

10 files changed, 121 insertions, 109 deletions

diff --git a/src/lib/libssl/Makefile b/src/lib/libssl/Makefile
index d0d7bc4e02..1788cd75a3 100644
--- a/src/lib/libssl/Makefile
+++ b/src/lib/libssl/Makefile
@@ -1,4 +1,4 @@
-# $OpenBSD: Makefile,v 1.76 2022/07/24 14:28:16 jsing Exp $
+# $OpenBSD: Makefile,v 1.77 2022/08/17 07:39:19 jsing Exp $
 
 .include <bsd.own.mk>
 .ifndef NOMAN
@@ -85,7 +85,8 @@ SRCS= \
         tls13_server.c \
         tls_buffer.c \
         tls_content.c \
-        tls_key_share.c
+        tls_key_share.c \
+        tls_lib.c
 
 HDRS=   dtls1.h srtp.h ssl.h ssl2.h ssl23.h ssl3.h tls1.h
 
diff --git a/src/lib/libssl/s3_lib.c b/src/lib/libssl/s3_lib.c
index b6a2c26938..2726744357 100644
--- a/src/lib/libssl/s3_lib.c
+++ b/src/lib/libssl/s3_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: s3_lib.c,v 1.235 2022/07/02 16:31:04 tb Exp $ */
+/* $OpenBSD: s3_lib.c,v 1.236 2022/08/17 07:39:19 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1559,8 +1559,10 @@ ssl3_free(SSL *s)
         tls1_cleanup_key_block(s);
         ssl3_release_read_buffer(s);
         ssl3_release_write_buffer(s);
-        freezero(s->s3->hs.sigalgs, s->s3->hs.sigalgs_len);
 
+        freezero(s->s3->hs.sigalgs, s->s3->hs.sigalgs_len);
+        sk_X509_pop_free(s->s3->hs.peer_certs, X509_free);
+        sk_X509_pop_free(s->s3->hs.peer_certs_no_leaf, X509_free);
         tls_key_share_free(s->s3->hs.key_share);
 
         tls13_secrets_destroy(s->s3->hs.tls13.secrets);
@@ -1586,8 +1588,8 @@ ssl3_free(SSL *s)
 void
 ssl3_clear(SSL *s)
 {
-        unsigned char   *rp, *wp;
-        size_t           rlen, wlen;
+        unsigned char *rp, *wp;
+        size_t rlen, wlen;
 
         tls1_cleanup_key_block(s);
         sk_X509_NAME_pop_free(s->s3->hs.tls12.ca_names, X509_NAME_free);
@@ -1598,6 +1600,11 @@ ssl3_clear(SSL *s)
         s->s3->hs.sigalgs = NULL;
         s->s3->hs.sigalgs_len = 0;
 
+        sk_X509_pop_free(s->s3->hs.peer_certs, X509_free);
+        s->s3->hs.peer_certs = NULL;
+        sk_X509_pop_free(s->s3->hs.peer_certs_no_leaf, X509_free);
+        s->s3->hs.peer_certs_no_leaf = NULL;
+
         tls_key_share_free(s->s3->hs.key_share);
         s->s3->hs.key_share = NULL;
 
diff --git a/src/lib/libssl/ssl_clnt.c b/src/lib/libssl/ssl_clnt.c
index 1319684868..0e50285898 100644
--- a/src/lib/libssl/ssl_clnt.c
+++ b/src/lib/libssl/ssl_clnt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_clnt.c,v 1.152 2022/08/15 10:45:25 tb Exp $ */
+/* $OpenBSD: ssl_clnt.c,v 1.153 2022/08/17 07:39:19 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1090,8 +1090,6 @@ ssl3_get_server_certificate(SSL *s)
         STACK_OF(X509) *certs = NULL;
         X509 *cert = NULL;
         const uint8_t *p;
-        EVP_PKEY *pkey;
-        int cert_type;
         int al, ret;
 
         if ((ret = ssl3_get_message(s, SSL3_ST_CR_CERT_A,
@@ -1156,37 +1154,11 @@ ssl3_get_server_certificate(SSL *s)
                 SSLerror(s, SSL_R_CERTIFICATE_VERIFY_FAILED);
                 goto fatal_err;
         }
-        ERR_clear_error(); /* but we keep s->verify_result */
-
-        /*
-         * Inconsistency alert: cert_chain does include the peer's
-         * certificate, which we don't include in s3_srvr.c
-         */
-        cert = sk_X509_value(certs, 0);
-        X509_up_ref(cert);
-
-        if ((pkey = X509_get0_pubkey(cert)) == NULL ||
-            EVP_PKEY_missing_parameters(pkey)) {
-                al = SSL3_AL_FATAL;
-                SSLerror(s, SSL_R_UNABLE_TO_FIND_PUBLIC_KEY_PARAMETERS);
-                goto fatal_err;
-        }
-        if ((cert_type = ssl_cert_type(pkey)) < 0) {
-                al = SSL3_AL_FATAL;
-                SSLerror(s, SSL_R_UNKNOWN_CERTIFICATE_TYPE);
-                goto fatal_err;
-        }
-
-        X509_free(s->session->peer_cert);
-        X509_up_ref(cert);
-        s->session->peer_cert = cert;
-        s->session->peer_cert_type = cert_type;
-
         s->session->verify_result = s->verify_result;
+        ERR_clear_error();
 
-        sk_X509_pop_free(s->session->cert_chain, X509_free);
-        s->session->cert_chain = certs;
-        certs = NULL;
+        if (!tls_process_peer_certs(s, certs))
+                goto err;
 
         ret = 1;
 
diff --git a/src/lib/libssl/ssl_lib.c b/src/lib/libssl/ssl_lib.c
index e346e3cf7f..9af1934dd6 100644
--- a/src/lib/libssl/ssl_lib.c
+++ b/src/lib/libssl/ssl_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_lib.c,v 1.300 2022/07/24 15:05:16 jsing Exp $ */
+/* $OpenBSD: ssl_lib.c,v 1.301 2022/08/17 07:39:19 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -880,14 +880,17 @@ SSL_get_peer_certificate(const SSL *s)
 STACK_OF(X509) *
 SSL_get_peer_cert_chain(const SSL *s)
 {
-        if (s == NULL || s->session == NULL)
+        if (s == NULL)
                 return NULL;
 
         /*
-         * If we are a client, cert_chain includes the peer's own
-         * certificate; if we are a server, it does not.
+         * Achtung! Due to API inconsistency, a client includes the peer's leaf
+         * certificate in the peer certificate chain, while a server does not.
          */
-        return s->session->cert_chain;
+        if (!s->server)
+                return s->s3->hs.peer_certs;
+
+        return s->s3->hs.peer_certs_no_leaf;
 }
 
 STACK_OF(X509) *
diff --git a/src/lib/libssl/ssl_locl.h b/src/lib/libssl/ssl_locl.h
index 18daf791f0..1bfeeb9740 100644
--- a/src/lib/libssl/ssl_locl.h
+++ b/src/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.417 2022/07/24 14:28:16 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.418 2022/08/17 07:39:19 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -518,8 +518,6 @@ struct ssl_session_st {
          * not_resumable_session_cb to disable session caching and tickets. */
         int not_resumable;
 
-        STACK_OF(X509) *cert_chain; /* as received from peer */
-
         size_t tlsext_ecpointformatlist_length;
         uint8_t *tlsext_ecpointformatlist; /* peer's list */
         size_t tlsext_supportedgroups_length;
@@ -645,6 +643,10 @@ typedef struct ssl_handshake_st {
         uint8_t peer_finished[EVP_MAX_MD_SIZE];
         size_t peer_finished_len;
 
+        /* List of certificates received from our peer. */
+        STACK_OF(X509) *peer_certs;
+        STACK_OF(X509) *peer_certs_no_leaf;
+
         SSL_HANDSHAKE_TLS12 tls12;
         SSL_HANDSHAKE_TLS13 tls13;
 } SSL_HANDSHAKE;
@@ -1566,6 +1568,8 @@ int srtp_find_profile_by_num(unsigned int profile_num,
 
 #endif /* OPENSSL_NO_SRTP */
 
+int tls_process_peer_certs(SSL *s, STACK_OF(X509) *peer_certs);
+
 __END_HIDDEN_DECLS
 
 #endif
diff --git a/src/lib/libssl/ssl_sess.c b/src/lib/libssl/ssl_sess.c
index fcb259f6a2..7cf36f8984 100644
--- a/src/lib/libssl/ssl_sess.c
+++ b/src/lib/libssl/ssl_sess.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_sess.c,v 1.116 2022/06/07 17:49:22 tb Exp $ */
+/* $OpenBSD: ssl_sess.c,v 1.117 2022/08/17 07:39:19 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -230,6 +230,8 @@ SSL_SESSION_new(void)
         ss->next = NULL;
         ss->tlsext_hostname = NULL;
 
+        ss->peer_cert_type = -1;
+
         ss->tlsext_ecpointformatlist_length = 0;
         ss->tlsext_ecpointformatlist = NULL;
         ss->tlsext_supportedgroups_length = 0;
@@ -761,8 +763,6 @@ SSL_SESSION_free(SSL_SESSION *ss)
         explicit_bzero(ss->master_key, sizeof ss->master_key);
         explicit_bzero(ss->session_id, sizeof ss->session_id);
 
-        sk_X509_pop_free(ss->cert_chain, X509_free);
-
         X509_free(ss->peer_cert);
 
         sk_SSL_CIPHER_free(ss->ciphers);
diff --git a/src/lib/libssl/ssl_srvr.c b/src/lib/libssl/ssl_srvr.c
index d665a568d1..acdcb15398 100644
--- a/src/lib/libssl/ssl_srvr.c
+++ b/src/lib/libssl/ssl_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.148 2022/07/03 14:58:00 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.149 2022/08/17 07:39:19 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -2175,6 +2175,11 @@ ssl3_get_client_certificate(SSL *s)
                         al = SSL_AD_HANDSHAKE_FAILURE;
                         goto fatal_err;
                 }
+
+                /*
+                 * If we asked for a client certificate and the client has none,
+                 * it must respond with a certificate list of length zero.
+                 */
                 if (s->s3->hs.tls12.cert_request != 0) {
                         SSLerror(s, SSL_R_TLS_PEER_DID_NOT_RESPOND_WITH_CERTIFICATE_LIST);
                         al = SSL_AD_UNEXPECTED_MESSAGE;
@@ -2244,19 +2249,11 @@ ssl3_get_client_certificate(SSL *s)
                 SSLerror(s, SSL_R_NO_CERTIFICATE_RETURNED);
                 goto fatal_err;
         }
-
-        X509_free(s->session->peer_cert);
-        s->session->peer_cert = sk_X509_shift(certs);
-
-        /*
-         * Inconsistency alert: cert_chain does *not* include the
-         * peer's own certificate, while we do include it in s3_clnt.c
-         */
-        sk_X509_pop_free(s->session->cert_chain, X509_free);
-        s->session->cert_chain = certs;
-        certs = NULL;
-
         s->session->verify_result = s->verify_result;
+        ERR_clear_error();
+
+        if (!tls_process_peer_certs(s, certs))
+                goto err;
 
  done:
         ret = 1;
diff --git a/src/lib/libssl/tls13_client.c b/src/lib/libssl/tls13_client.c
index b1efafdfdd..87759632f9 100644
--- a/src/lib/libssl/tls13_client.c
+++ b/src/lib/libssl/tls13_client.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls13_client.c,v 1.97 2022/07/24 14:16:29 jsing Exp $ */
+/* $OpenBSD: tls13_client.c,v 1.98 2022/08/17 07:39:19 jsing Exp $ */
 /*
  * Copyright (c) 2018, 2019 Joel Sing <jsing@openbsd.org>
  *
@@ -553,9 +553,8 @@ tls13_server_certificate_recv(struct tls13_ctx *ctx, CBS *cbs)
         struct stack_st_X509 *certs = NULL;
         SSL *s = ctx->ssl;
         X509 *cert = NULL;
-        EVP_PKEY *pkey;
         const uint8_t *p;
-        int alert_desc, cert_type;
+        int alert_desc;
         int ret = 0;
 
         if ((certs = sk_X509_new_null()) == NULL)
@@ -610,28 +609,11 @@ tls13_server_certificate_recv(struct tls13_ctx *ctx, CBS *cbs)
                     "failed to verify peer certificate", NULL);
                 goto err;
         }
+        s->session->verify_result = s->verify_result;
         ERR_clear_error();
 
-        cert = sk_X509_value(certs, 0);
-        X509_up_ref(cert);
-
-        if ((pkey = X509_get0_pubkey(cert)) == NULL)
-                goto err;
-        if (EVP_PKEY_missing_parameters(pkey))
+        if (!tls_process_peer_certs(s, certs))
                 goto err;
-        if ((cert_type = ssl_cert_type(pkey)) < 0)
-                goto err;
-
-        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))
diff --git a/src/lib/libssl/tls13_server.c b/src/lib/libssl/tls13_server.c
index 5aee5f1a93..8f225433f0 100644
--- a/src/lib/libssl/tls13_server.c
+++ b/src/lib/libssl/tls13_server.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls13_server.c,v 1.100 2022/07/24 14:16:29 jsing Exp $ */
+/* $OpenBSD: tls13_server.c,v 1.101 2022/08/17 07:39:19 jsing Exp $ */
 /*
  * Copyright (c) 2019, 2020 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2020 Bob Beck <beck@openbsd.org>
@@ -860,9 +860,7 @@ tls13_client_certificate_recv(struct tls13_ctx *ctx, CBS *cbs)
         struct stack_st_X509 *certs = NULL;
         SSL *s = ctx->ssl;
         X509 *cert = NULL;
-        EVP_PKEY *pkey;
         const uint8_t *p;
-        int cert_type;
         int ret = 0;
 
         if (!CBS_get_u8_length_prefixed(cbs, &cert_request_context))
@@ -911,31 +909,11 @@ tls13_client_certificate_recv(struct tls13_ctx *ctx, CBS *cbs)
                     "failed to verify peer certificate", NULL);
                 goto err;
         }
+        s->session->verify_result = s->verify_result;
         ERR_clear_error();
 
-        /*
-         * Achtung! Due to API inconsistency, a client includes the peer's leaf
-         * certificate in the stored certificate chain, while a server does not.
-         */
-        cert = sk_X509_shift(certs);
-
-        if ((pkey = X509_get0_pubkey(cert)) == NULL)
+        if (!tls_process_peer_certs(s, certs))
                 goto err;
-        if (EVP_PKEY_missing_parameters(pkey))
-                goto err;
-        if ((cert_type = ssl_cert_type(pkey)) < 0)
-                goto err;
-
-        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;
diff --git a/src/lib/libssl/tls_lib.c b/src/lib/libssl/tls_lib.c
new file mode 100644
index 0000000000..48db071803
--- /dev/null
+++ b/src/lib/libssl/tls_lib.c
@@ -0,0 +1,68 @@
+/* $OpenBSD: tls_lib.c,v 1.1 2022/08/17 07:39:19 jsing Exp $ */
+/*
+ * Copyright (c) 2019, 2021 Joel Sing <jsing@openbsd.org>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include "ssl_locl.h"
+
+int
+tls_process_peer_certs(SSL *s, STACK_OF(X509) *peer_certs)
+{
+        STACK_OF(X509) *peer_certs_no_leaf;
+        X509 *peer_cert = NULL;
+        EVP_PKEY *pkey;
+        int cert_type;
+        int ret = 0;
+
+        if (sk_X509_num(peer_certs) < 1)
+                goto err;
+        peer_cert = sk_X509_value(peer_certs, 0);
+        X509_up_ref(peer_cert);
+
+        if ((pkey = X509_get0_pubkey(peer_cert)) == NULL) {
+                SSLerror(s, SSL_R_NO_PUBLICKEY);
+                goto err;
+        }
+        if (EVP_PKEY_missing_parameters(pkey)) {
+                SSLerror(s, SSL_R_UNABLE_TO_FIND_PUBLIC_KEY_PARAMETERS);
+                goto err;
+        }
+        if ((cert_type = ssl_cert_type(pkey)) < 0) {
+                SSLerror(s, SSL_R_UNKNOWN_CERTIFICATE_TYPE);
+                goto err;
+        }
+
+        s->session->peer_cert_type = cert_type;
+
+        X509_free(s->session->peer_cert);
+        s->session->peer_cert = peer_cert;
+        peer_cert = NULL;
+        
+        sk_X509_pop_free(s->s3->hs.peer_certs, X509_free);
+        if ((s->s3->hs.peer_certs = X509_chain_up_ref(peer_certs)) == NULL)
+                goto err;
+
+        if ((peer_certs_no_leaf = X509_chain_up_ref(peer_certs)) == NULL)
+                goto err;
+        X509_free(sk_X509_shift(peer_certs_no_leaf));
+        sk_X509_pop_free(s->s3->hs.peer_certs_no_leaf, X509_free);
+        s->s3->hs.peer_certs_no_leaf = peer_certs_no_leaf;
+
+        ret = 1;
+ err:
+        X509_free(peer_cert);
+
+        return ret;
+}