Clean up TLSv1.2 certificate request handshake data.

Currently cert_req is used by clients and cert_request is used by servers.
Replace this by a single cert_request used by either client or server.
Remove the certificate types as they are currently unused. This also fixes
a bug whereby if the number of certificate types exceeds SSL3_CT_NUMBER
the number of bytes read in is insufficient, which will break decoding.

ok inoguchi@ tb@

diff --git a/lib/libssl/s3_lib.c b/lib/libssl/s3_lib.c
index 6563de5..9dd6343 100644 (file)
--- a/lib/libssl/s3_lib.c
+++ b/lib/libssl/s3_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: s3_lib.c,v 1.207 2021/04/19 16:47:25 jsing Exp $ */
+/* $OpenBSD: s3_lib.c,v 1.208 2021/04/21 19:27:56 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1570,7 +1570,7 @@ ssl3_free(SSL *s)
        freezero(S3I(s)->hs.tls13.cookie, S3I(s)->hs.tls13.cookie_len);
        tls13_clienthello_hash_clear(&S3I(s)->hs.tls13);
 
-       sk_X509_NAME_pop_free(S3I(s)->tmp.ca_names, X509_NAME_free);
+       sk_X509_NAME_pop_free(S3I(s)->hs.tls12.ca_names, X509_NAME_free);
 
        tls1_transcript_free(s);
        tls1_transcript_hash_free(s);
@@ -1591,7 +1591,7 @@ ssl3_clear(SSL *s)
        size_t           rlen, wlen;
 
        tls1_cleanup_key_block(s);
-       sk_X509_NAME_pop_free(S3I(s)->tmp.ca_names, X509_NAME_free);
+       sk_X509_NAME_pop_free(S3I(s)->hs.tls12.ca_names, X509_NAME_free);
 
        DH_free(S3I(s)->tmp.dh);
        S3I(s)->tmp.dh = NULL;

diff --git a/lib/libssl/ssl_cert.c b/lib/libssl/ssl_cert.c
index 03ef856..d122c80 100644 (file)
--- a/lib/libssl/ssl_cert.c
+++ b/lib/libssl/ssl_cert.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_cert.c,v 1.81 2021/03/27 17:56:28 tb Exp $ */
+/* $OpenBSD: ssl_cert.c,v 1.82 2021/04/21 19:27:56 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -508,7 +508,7 @@ SSL_get_client_CA_list(const SSL *s)
        if (!s->server) {
                /* We are in the client. */
                if ((s->version >> 8) == SSL3_VERSION_MAJOR)
-                       return (S3I(s)->tmp.ca_names);
+                       return (S3I(s)->hs.tls12.ca_names);
                else
                        return (NULL);
        } else {

diff --git a/lib/libssl/ssl_clnt.c b/lib/libssl/ssl_clnt.c
index 6b43b56..7f69b8b 100644 (file)
--- a/lib/libssl/ssl_clnt.c
+++ b/lib/libssl/ssl_clnt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_clnt.c,v 1.91 2021/04/19 16:51:56 jsing Exp $ */
+/* $OpenBSD: ssl_clnt.c,v 1.92 2021/04/21 19:27:56 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -391,7 +391,7 @@ ssl3_connect(SSL *s)
                                goto end;
                        if (SSL_is_dtls(s))
                                dtls1_stop_timer(s);
-                       if (S3I(s)->tmp.cert_req)
+                       if (S3I(s)->hs.tls12.cert_request)
                                S3I(s)->hs.state = SSL3_ST_CW_CERT_A;
                        else
                                S3I(s)->hs.state = SSL3_ST_CW_KEY_EXCH_A;
@@ -435,7 +435,7 @@ ssl3_connect(SSL *s)
                         * message when client's ECDH public key is sent
                         * inside the client certificate.
                         */
-                       if (S3I(s)->tmp.cert_req == 1) {
+                       if (S3I(s)->hs.tls12.cert_request == 1) {
                                S3I(s)->hs.state = SSL3_ST_CW_CERT_VRFY_A;
                        } else {
                                S3I(s)->hs.state = SSL3_ST_CW_CHANGE_A;
@@ -1650,8 +1650,7 @@ ssl3_get_certificate_request(SSL *s)
 {
        int                      ok, ret = 0;
        long                     n;
-       uint8_t                  ctype_num;
-       CBS                      cert_request, ctypes, rdn_list;
+       CBS                      cert_request, cert_types, rdn_list;
        X509_NAME               *xn = NULL;
        const unsigned char     *q;
        STACK_OF(X509_NAME)     *ca_sk = NULL;
@@ -1661,7 +1660,7 @@ ssl3_get_certificate_request(SSL *s)
        if (!ok)
                return ((int)n);
 
-       S3I(s)->tmp.cert_req = 0;
+       S3I(s)->hs.tls12.cert_request = 0;
 
        if (S3I(s)->hs.tls12.message_type == SSL3_MT_SERVER_DONE) {
                S3I(s)->hs.tls12.reuse_message = 1;
@@ -1695,19 +1694,9 @@ ssl3_get_certificate_request(SSL *s)
                goto err;
        }
 
-       /* get the certificate types */
-       if (!CBS_get_u8(&cert_request, &ctype_num))
+       if (!CBS_get_u8_length_prefixed(&cert_request, &cert_types))
                goto decode_err;
 
-       if (ctype_num > SSL3_CT_NUMBER)
-               ctype_num = SSL3_CT_NUMBER;
-       if (!CBS_get_bytes(&cert_request, &ctypes, ctype_num) ||
-           !CBS_write_bytes(&ctypes, (uint8_t *)S3I(s)->tmp.ctype,
-           sizeof(S3I(s)->tmp.ctype), NULL)) {
-               SSLerror(s, SSL_R_DATA_LENGTH_TOO_LONG);
-               goto err;
-       }
-
        if (SSL_USE_SIGALGS(s)) {
                CBS sigalgs;
 
@@ -1778,10 +1767,9 @@ ssl3_get_certificate_request(SSL *s)
        }
 
        /* we should setup a certificate to return.... */
-       S3I(s)->tmp.cert_req = 1;
-       S3I(s)->tmp.ctype_num = ctype_num;
-       sk_X509_NAME_pop_free(S3I(s)->tmp.ca_names, X509_NAME_free);
-       S3I(s)->tmp.ca_names = ca_sk;
+       S3I(s)->hs.tls12.cert_request = 1;
+       sk_X509_NAME_pop_free(S3I(s)->hs.tls12.ca_names, X509_NAME_free);
+       S3I(s)->hs.tls12.ca_names = ca_sk;
        ca_sk = NULL;
 
        ret = 1;
@@ -2228,7 +2216,7 @@ ssl3_send_client_kex_gost(SSL *s, SESS_CERT *sess_cert, CBB *cbb)
        /*
         * If we have client certificate, use its secret as peer key.
         */
-       if (S3I(s)->tmp.cert_req && s->cert->key->privatekey) {
+       if (S3I(s)->hs.tls12.cert_request && s->cert->key->privatekey) {
                if (EVP_PKEY_derive_set_peer(pkey_ctx,
                    s->cert->key->privatekey) <=0) {
                        /*
@@ -2681,7 +2669,7 @@ ssl3_send_client_certificate(SSL *s)
                X509_free(x509);
                EVP_PKEY_free(pkey);
                if (i == 0) {
-                       S3I(s)->tmp.cert_req = 2;
+                       S3I(s)->hs.tls12.cert_request = 2;
 
                        /* There is no client certificate to verify. */
                        tls1_transcript_free(s);
@@ -2696,7 +2684,7 @@ ssl3_send_client_certificate(SSL *s)
                    SSL3_MT_CERTIFICATE))
                        goto err;
                if (!ssl3_output_cert_chain(s, &client_cert,
-                   (S3I(s)->tmp.cert_req == 2) ? NULL : s->cert->key))
+                   (S3I(s)->hs.tls12.cert_request == 2) ? NULL : s->cert->key))
                        goto err;
                if (!ssl3_handshake_msg_finish(s, &cbb))
                        goto err;

diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index 86d1b6e..2739730 100644 (file)
--- a/lib/libssl/ssl_locl.h
+++ b/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.336 2021/04/19 17:26:39 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.337 2021/04/21 19:27:56 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -427,6 +427,10 @@ typedef struct ssl_handshake_tls12_st {
        /* Reuse current handshake message. */
        int reuse_message;
 
+       /* Client certificate requests. */
+       int cert_request;
+       STACK_OF(X509_NAME) *ca_names;
+
        /* Size of the MAC secret. */
        int mac_secret_size;
 
@@ -946,14 +950,6 @@ typedef struct ssl3_state_internal_st {
                int ecdh_nid;
 
                uint8_t *x25519;
-
-               /* used for certificate requests */
-               int cert_req;
-               int ctype_num;
-               char ctype[SSL3_CT_NUMBER];
-               STACK_OF(X509_NAME) *ca_names;
-
-               int cert_request;
        } tmp;
 
        /* Connection binding to prevent renegotiation attacks */

diff --git a/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c
index 8241a59..c85a251 100644 (file)
--- a/lib/libssl/ssl_srvr.c
+++ b/lib/libssl/ssl_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.102 2021/04/19 16:51:56 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.103 2021/04/21 19:27:56 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -464,13 +464,13 @@ ssl3_accept(SSL *s)
                             SSL_VERIFY_FAIL_IF_NO_PEER_CERT))) {
                                /* No cert request. */
                                skip = 1;
-                               S3I(s)->tmp.cert_request = 0;
+                               S3I(s)->hs.tls12.cert_request = 0;
                                S3I(s)->hs.state = SSL3_ST_SW_SRVR_DONE_A;
 
                                if (!SSL_is_dtls(s))
                                        tls1_transcript_free(s);
                        } else {
-                               S3I(s)->tmp.cert_request = 1;
+                               S3I(s)->hs.tls12.cert_request = 1;
                                if (SSL_is_dtls(s))
                                        dtls1_start_timer(s);
                                ret = ssl3_send_certificate_request(s);
@@ -522,7 +522,7 @@ ssl3_accept(SSL *s)
 
                case SSL3_ST_SR_CERT_A:
                case SSL3_ST_SR_CERT_B:
-                       if (S3I(s)->tmp.cert_request) {
+                       if (S3I(s)->hs.tls12.cert_request) {
                                ret = ssl3_get_client_certificate(s);
                                if (ret <= 0)
                                        goto end;
@@ -2379,7 +2379,7 @@ ssl3_get_client_certificate(SSL *s)
                 * If tls asked for a client cert,
                 * the client must return a 0 list.
                 */
-               if (S3I(s)->tmp.cert_request) {
+               if (S3I(s)->hs.tls12.cert_request) {
                        SSLerror(s, SSL_R_TLS_PEER_DID_NOT_RESPOND_WITH_CERTIFICATE_LIST
                            );
                        al = SSL_AD_UNEXPECTED_MESSAGE;