Bug 1614870 - Free sid->peerID before reallocating in ssl_DecodeResum…

…ptionToken. r=mt

This patch adds a missing `PORT_Free()` when reallocating `sid->PeerID`, and adds a test for a non-empty PeerID.

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

--HG--
extra : moz-landing-system : lando

gtests/ssl_gtest/ssl_resumption_unittest.cc

@@ -838,7 +838,7 @@ TEST_F(TlsConnectTest, TestTls13ResumptionDuplicateNST) {
 
   // Clear the session ticket keys to invalidate the old ticket.
   SSLInt_ClearSelfEncryptKey();
-  SSL_SendSessionTicket(server_->ssl_fd(), NULL, 0);
+  EXPECT_EQ(SECSuccess, SSL_SendSessionTicket(server_->ssl_fd(), NULL, 0));
 
   SendReceive();  // Need to read so that we absorb the session tickets.
   CheckKeys();
@@ -1005,7 +1005,8 @@ TEST_F(TlsConnectStreamTls13, ExternalResumptionUseSecondTicket) {
     state->invoked++;
     return SECSuccess;
   };
-  SSL_SetResumptionTokenCallback(client_->ssl_fd(), cb, &ticket_state);
+  EXPECT_EQ(SECSuccess, SSL_SetResumptionTokenCallback(client_->ssl_fd(), cb,
+                                                       &ticket_state));
 
   Connect();
   EXPECT_EQ(SECSuccess, SSL_SendSessionTicket(server_->ssl_fd(), nullptr, 0));
@@ -1446,4 +1447,34 @@ TEST_F(TlsConnectStreamTls13, ExternalTokenAfterHrr) {
   SendReceive();
 }
 
+TEST_F(TlsConnectStreamTls13, ExternalTokenWithPeerId) {
+  ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
+  EXPECT_EQ(SECSuccess, SSL_SetSockPeerID(client_->ssl_fd(), "testPeerId"));
+  std::vector<uint8_t> ticket_state;
+  auto cb = [](PRFileDesc* fd, const PRUint8* ticket, unsigned int ticket_len,
+               void* arg) -> SECStatus {
+    EXPECT_NE(0U, ticket_len);
+    EXPECT_NE(nullptr, ticket);
+    auto ticket_state_ = reinterpret_cast<std::vector<uint8_t>*>(arg);
+    ticket_state_->assign(ticket, ticket + ticket_len);
+    return SECSuccess;
+  };
+  EXPECT_EQ(SECSuccess, SSL_SetResumptionTokenCallback(client_->ssl_fd(), cb,
+                                                       &ticket_state));
+
+  Connect();
+  SendReceive();
+  EXPECT_NE(0U, ticket_state.size());
+
+  Reset();
+  ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
+  EXPECT_EQ(SECSuccess, SSL_SetSockPeerID(client_->ssl_fd(), "testPeerId"));
+  client_->SetResumptionToken(ticket_state);
+  ASSERT_TRUE(client_->MaybeSetResumptionToken());
+  ExpectResumption(RESUME_TICKET);
+  Connect();
+  SendReceive();
+}
+
 }  // namespace nss_test

lib/ssl/sslnonce.c

@@ -537,6 +537,9 @@ ssl_DecodeResumptionToken(sslSessionID *sid, const PRUint8 *encodedToken,
     }
     if (readerBuffer.len) {
         PORT_Assert(readerBuffer.buf);
+        if (sid->peerID) {
+            PORT_Free((void *)sid->peerID);
+        }
         sid->peerID = PORT_Strdup((const char *)readerBuffer.buf);
     }