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@

diff --git a/lib/libssl/Makefile b/lib/libssl/Makefile
index d0d7bc4..1788cd7 100644 (file)
--- a/lib/libssl/Makefile
+++ b/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/lib/libssl/s3_lib.c b/lib/libssl/s3_lib.c
index b6a2c26..2726744 100644 (file)
--- a/lib/libssl/s3_lib.c
+++ b/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/lib/libssl/ssl_clnt.c b/lib/libssl/ssl_clnt.c
index 1319684..0e50285 100644 (file)
--- a/lib/libssl/ssl_clnt.c
+++ b/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/lib/libssl/ssl_lib.c b/lib/libssl/ssl_lib.c
index e346e3c..9af1934 100644 (file)
--- a/lib/libssl/ssl_lib.c
+++ b/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/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index 18daf79..1bfeeb9 100644 (file)
--- a/lib/libssl/ssl_locl.h
+++ b/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/lib/libssl/ssl_sess.c b/lib/libssl/ssl_sess.c
index fcb259f..7cf36f8 100644 (file)
--- a/lib/libssl/ssl_sess.c
+++ b/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/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c
index d665a56..acdcb15 100644 (file)
--- a/lib/libssl/ssl_srvr.c
+++ b/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/lib/libssl/tls13_client.c b/lib/libssl/tls13_client.c
index b1efafd..8775963 100644 (file)
--- a/lib/libssl/tls13_client.c
+++ b/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/lib/libssl/tls13_server.c b/lib/libssl/tls13_server.c
index 5aee5f1..8f22543 100644 (file)
--- a/lib/libssl/tls13_server.c
+++ b/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/lib/libssl/tls_lib.c b/lib/libssl/tls_lib.c
new file mode 100644 (file)
index 0000000..48db071
--- /dev/null
+++ b/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;
+}