# HG changeset patch

# User Daiki Ueno <dueno@redhat.com>

# Date 1559121620 -7200

#      Wed May 29 11:20:20 2019 +0200

# Node ID 29a48b604602a523defd6f9322a5adeca7e284a5

# Parent  43a7fb4f994a31222c308113b0fccdd5480d5b8e

Bug 1553443, send session ticket only after handshake is marked as finished

Reviewers: mt

Reviewed By: mt

Bug #: 1553443

Differential Revision: https://phabricator.services.mozilla.com/D32128

diff --git a/gtests/ssl_gtest/ssl_auth_unittest.cc b/gtests/ssl_gtest/ssl_auth_unittest.cc

--- a/gtests/ssl_gtest/ssl_auth_unittest.cc

+++ b/gtests/ssl_gtest/ssl_auth_unittest.cc

@@ -537,6 +537,40 @@ TEST_F(TlsConnectStreamTls13, PostHandsh

                       capture_cert_req->buffer().len()));

 }

+// Check if post-handshake auth still works when session tickets are enabled:

+// https://bugzilla.mozilla.org/show_bug.cgi?id=1553443

+TEST_F(TlsConnectStreamTls13, PostHandshakeAuthWithSessionTicketsEnabled) {

+  EnsureTlsSetup();

+  client_->SetupClientAuth();

+  EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),

+                                      SSL_ENABLE_POST_HANDSHAKE_AUTH, PR_TRUE));

+  EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),

+                                      SSL_ENABLE_SESSION_TICKETS, PR_TRUE));

+  EXPECT_EQ(SECSuccess, SSL_OptionSet(server_->ssl_fd(),

+                                      SSL_ENABLE_SESSION_TICKETS, PR_TRUE));

+  size_t called = 0;

+  server_->SetAuthCertificateCallback(

+      [&called](TlsAgent*, PRBool, PRBool) -> SECStatus {

+        called++;

+        return SECSuccess;

+      });

+  Connect();

+  EXPECT_EQ(0U, called);

+  // Send CertificateRequest.

+  EXPECT_EQ(SECSuccess, SSL_GetClientAuthDataHook(

+                            client_->ssl_fd(), GetClientAuthDataHook, nullptr));

+  EXPECT_EQ(SECSuccess, SSL_SendCertificateRequest(server_->ssl_fd()))

+      << "Unexpected error: " << PORT_ErrorToName(PORT_GetError());

+  server_->SendData(50);

+  client_->ReadBytes(50);

+  client_->SendData(50);

+  server_->ReadBytes(50);

+  EXPECT_EQ(1U, called);

+  ScopedCERTCertificate cert1(SSL_PeerCertificate(server_->ssl_fd()));

+  ScopedCERTCertificate cert2(SSL_LocalCertificate(client_->ssl_fd()));

+  EXPECT_TRUE(SECITEM_ItemsAreEqual(&cert1->derCert, &cert2->derCert));

+}

+

 // In TLS 1.3, the client sends its cert rejection on the

 // second flight, and since it has already received the

 // server's Finished, it transitions to complete and

diff --git a/lib/ssl/tls13con.c b/lib/ssl/tls13con.c

--- a/lib/ssl/tls13con.c

+++ b/lib/ssl/tls13con.c

@@ -4561,6 +4561,11 @@ tls13_ServerHandleFinished(sslSocket *ss

         return SECFailure;

     }

+    rv = tls13_FinishHandshake(ss);

+    if (rv != SECSuccess) {

+        return SECFailure;

+    }

+

     ssl_GetXmitBufLock(ss);

     if (ss->opt.enableSessionTickets) {

         rv = tls13_SendNewSessionTicket(ss, NULL, 0);

@@ -4573,8 +4578,7 @@ tls13_ServerHandleFinished(sslSocket *ss

         }

     }

     ssl_ReleaseXmitBufLock(ss);

-

-    return tls13_FinishHandshake(ss);

+    return SECSuccess;

 loser:

     ssl_ReleaseXmitBufLock(ss);

diff --git a/tests/ssl/sslauth.txt b/tests/ssl/sslauth.txt

--- a/tests/ssl/sslauth.txt

+++ b/tests/ssl/sslauth.txt

@@ -42,6 +42,7 @@

   noECC     0       -r_-r_-r_-r_-E  -V_tls1.3:tls1.3_-E_-n_TestUser_-w_nss TLS 1.3 Require client auth on post hs (client auth)

   noECC     0       -r_-r_-r_-E  -V_tls1.3:tls1.3_-E_-n_none_-w_nss TLS 1.3 Request don't require client auth on post hs (client does not provide auth)

   noECC     1       -r_-r_-r_-r_-E  -V_tls1.3:tls1.3_-E_-n_none_-w_nss TLS 1.3 Require client auth on post hs (client does not provide auth)

+  noECC     0       -r_-r_-r_-E_-u  -V_tls1.3:tls1.3_-E_-n_TestUser_-w_nss TLS 1.3 Request don't require client auth on post hs with session ticket (client auth)

 #

 # Use EC cert for client authentication

 #