# 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
 #