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

…nished Reviewers: mt Reviewed By: mt Bug #: 1553443 Differential Revision: https://phabricator.services.mozilla.com/D32128 --HG-- extra : rebase_source : 6f09980632b4212d67ea84bb5e93dbe5b9c08562
sailfishos-mirror · May 29, 2019 · b49aaa6 · b49aaa6
1 parent 0368477
commit b49aaa6
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 2 deletions.
diff --git a/gtests/ssl_gtest/ssl_auth_unittest.cc b/gtests/ssl_gtest/ssl_auth_unittest.cc
@@ -537,6 +537,40 @@ TEST_F(TlsConnectStreamTls13, PostHandshakeAuthDecline) {
                       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
@@ -4561,6 +4561,11 @@ tls13_ServerHandleFinished(sslSocket *ss, PRUint8 *b, PRUint32 length)
         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, PRUint8 *b, PRUint32 length)
         }
     }
     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
@@ -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
 #