The branch OpenSSL_1_1_1-stable has been updated via de8848aeafd4210bcbbc6742b8947b37cb7ed8cb (commit) via a2388b50afc5136a1b65d0bf794f0398c31a1acb (commit) from 5cf0f0e70887fbe9d94a95e25e379a64e1676010 (commit)
- Log ----------------------------------------------------------------- commit de8848aeafd4210bcbbc6742b8947b37cb7ed8cb Author: Matt Caswell <m...@openssl.org> Date: Tue Oct 16 12:42:59 2018 +0100 Add a client_cert_cb test Reviewed-by: Ben Kaduk <ka...@mit.edu> (Merged from https://github.com/openssl/openssl/pull/7413) (cherry picked from commit 6e46c065b9b97212d63ef1f321b08fb7fa6b320d) commit a2388b50afc5136a1b65d0bf794f0398c31a1acb Author: Matt Caswell <m...@openssl.org> Date: Thu Oct 11 17:01:06 2018 +0100 Don't call the client_cert_cb immediately in TLSv1.3 In TLSv1.2 and below a CertificateRequest is sent after the Certificate from the server. This means that by the time the client_cert_cb is called on receipt of the CertificateRequest a call to SSL_get_peer_certificate() will return the server certificate as expected. In TLSv1.3 a CertificateRequest is sent before a Certificate message so calling SSL_get_peer_certificate() returns NULL. To workaround this we delay calling the client_cert_cb until after we have processed the CertificateVerify message, when we are doing TLSv1.3. Fixes #7384 Reviewed-by: Ben Kaduk <ka...@mit.edu> (Merged from https://github.com/openssl/openssl/pull/7413) (cherry picked from commit e45620140fce22c3251440063bc17440289d730c) ----------------------------------------------------------------------- Summary of changes: ssl/statem/statem_clnt.c | 12 +++++++ ssl/statem/statem_lib.c | 13 ++++++- test/sslapitest.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 8c658da..0a11b88 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1095,6 +1095,7 @@ WORK_STATE ossl_statem_client_post_process_message(SSL *s, WORK_STATE wst) ERR_R_INTERNAL_ERROR); return WORK_ERROR; + case TLS_ST_CR_CERT_VRFY: case TLS_ST_CR_CERT_REQ: return tls_prepare_client_certificate(s, wst); } @@ -2563,6 +2564,17 @@ MSG_PROCESS_RETURN tls_process_certificate_request(SSL *s, PACKET *pkt) /* we should setup a certificate to return.... */ s->s3->tmp.cert_req = 1; + /* + * In TLSv1.3 we don't prepare the client certificate yet. We wait until + * after the CertificateVerify message has been received. This is because + * in TLSv1.3 the CertificateRequest arrives before the Certificate message + * but in TLSv1.2 it is the other way around. We want to make sure that + * SSL_get_peer_certificate() returns something sensible in + * client_cert_cb. + */ + if (SSL_IS_TLS13(s) && s->post_handshake_auth != SSL_PHA_REQUESTED) + return MSG_PROCESS_CONTINUE_READING; + return MSG_PROCESS_CONTINUE_PROCESSING; } diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index e6e61f7..75cf321 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -495,7 +495,18 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt) } } - ret = MSG_PROCESS_CONTINUE_READING; + /* + * In TLSv1.3 on the client side we make sure we prepare the client + * certificate after the CertVerify instead of when we get the + * CertificateRequest. This is because in TLSv1.3 the CertificateRequest + * comes *before* the Certificate message. In TLSv1.2 it comes after. We + * want to make sure that SSL_get_peer_certificate() will return the actual + * server certificate from the client_cert_cb callback. + */ + if (!s->server && SSL_IS_TLS13(s) && s->s3->tmp.cert_req == 1) + ret = MSG_PROCESS_CONTINUE_PROCESSING; + else + ret = MSG_PROCESS_CONTINUE_READING; err: BIO_free(s->s3->handshake_buffer); s->s3->handshake_buffer = NULL; diff --git a/test/sslapitest.c b/test/sslapitest.c index d87e9f6..0b8f98f 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -5593,6 +5593,99 @@ static int test_cert_cb(int tst) return testresult; } +static int client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey) +{ + X509 *xcert, *peer; + EVP_PKEY *privpkey; + BIO *in = NULL; + + /* Check that SSL_get_peer_certificate() returns something sensible */ + peer = SSL_get_peer_certificate(ssl); + if (!TEST_ptr(peer)) + return 0; + X509_free(peer); + + in = BIO_new_file(cert, "r"); + if (!TEST_ptr(in)) + return 0; + + xcert = PEM_read_bio_X509(in, NULL, NULL, NULL); + BIO_free(in); + if (!TEST_ptr(xcert)) + return 0; + + in = BIO_new_file(privkey, "r"); + if (!TEST_ptr(in)) { + X509_free(xcert); + return 0; + } + + privpkey = PEM_read_bio_PrivateKey(in, NULL, NULL, NULL); + BIO_free(in); + if (!TEST_ptr(privpkey)) { + X509_free(xcert); + return 0; + } + + *x509 = xcert; + *pkey = privpkey; + + return 1; +} + +static int verify_cb(int preverify_ok, X509_STORE_CTX *x509_ctx) +{ + return 1; +} + +static int test_client_cert_cb(int tst) +{ + SSL_CTX *cctx = NULL, *sctx = NULL; + SSL *clientssl = NULL, *serverssl = NULL; + int testresult = 0; + +#ifdef OPENSSL_NO_TLS1_2 + if (tst == 0) + return 1; +#endif +#ifdef OPENSSL_NO_TLS1_3 + if (tst == 1) + return 1; +#endif + + if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(), + TLS_client_method(), + TLS1_VERSION, + tst == 0 ? TLS1_2_VERSION + : TLS1_3_VERSION, + &sctx, &cctx, cert, privkey))) + goto end; + + /* + * Test that setting a client_cert_cb results in a client certificate being + * sent. + */ + SSL_CTX_set_client_cert_cb(cctx, client_cert_cb); + SSL_CTX_set_verify(sctx, + SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, + verify_cb); + if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, + NULL, NULL)) + || !TEST_true(create_ssl_connection(serverssl, clientssl, + SSL_ERROR_NONE))) + goto end; + + testresult = 1; + + end: + SSL_free(serverssl); + SSL_free(clientssl); + SSL_CTX_free(sctx); + SSL_CTX_free(cctx); + + return testresult; +} + int setup_tests(void) { if (!TEST_ptr(cert = test_get_argument(0)) @@ -5696,6 +5789,7 @@ int setup_tests(void) ADD_ALL_TESTS(test_ticket_callbacks, 12); ADD_ALL_TESTS(test_shutdown, 7); ADD_ALL_TESTS(test_cert_cb, 3); + ADD_ALL_TESTS(test_client_cert_cb, 2); return 1; } _____ openssl-commits mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-commits