[openssl-commits] [openssl] OpenSSL_1_1_1-stable update

Matt Caswell matt at openssl.org
Tue Oct 30 12:24:34 UTC 2018


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 <matt at openssl.org>
Date:   Tue Oct 16 12:42:59 2018 +0100

    Add a client_cert_cb test
    
    Reviewed-by: Ben Kaduk <kaduk at mit.edu>
    (Merged from https://github.com/openssl/openssl/pull/7413)
    
    (cherry picked from commit 6e46c065b9b97212d63ef1f321b08fb7fa6b320d)

commit a2388b50afc5136a1b65d0bf794f0398c31a1acb
Author: Matt Caswell <matt at 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 <kaduk at 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;
 }
 


More information about the openssl-commits mailing list