From: jsing Date: Wed, 21 Apr 2021 19:27:56 +0000 (+0000) Subject: Clean up TLSv1.2 certificate request handshake data. X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=1f38de1dc356717682c253b7d6280ceb1ad92e5b;p=openbsd 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 6563de5be27..9dd6343b84e 100644 --- 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 03ef8565ac5..d122c80f2cd 100644 --- 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 6b43b565b98..7f69b8ba980 100644 --- 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 86d1b6e10b2..27397308efa 100644 --- 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 8241a59ac07..c85a25158f8 100644 --- 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;