Bug 1427675 - Add TlsAgent argument to TlsRecordFilter, r=ekr

This is a fairly disruptive change, but mostly just mechanical.  There are a few extra changes:

- I have renamed the TlsInspector* filters for consistency.  This was purely mechanical.

- I renamed the SetPacketFilter function to just SetFilter.  Also mechanical.

- TlsRecordFilter maintains a weak pointer reference to the TlsAgent now rather than using a bare pointer.  This meant that I had to change TlsAgentTestBase to use shared_ptr rather than unique_ptr to support of use of filters with those tests.

- I removed the helper function that enables decryption.  Enabling decryption is now more explicit.

- I ran a newer clang-format version and it fixed a few extra things, like the comments on the end of namespace {} blocks, some of which were wrong.

- I discovered a bug in some of the drop tests: in the 0-RTT tests, the filters were being installed on the client and server right at the start, which meant that they were capturing the first handshake and not the second one.  This was clearly against intent, but the tests were mostly right still, it was only the expected ACKs that were wrong.  We were expecting just one record to be ACKed by a server (Finished), but the record with EndOfEarlyData should have been acknowledged as well.

- In TlsSkipTest and Tls13SkipTest, I had to override SetUp() so that client_ and server_ are initialized prior to constructing filters.  In doing so, I noticed that we weren't being consistent about overriding SetUp properly, so I fixed the small number of instances of that by adding an override label to each and marking the base method virtual.

- The stateless HRR test for TLS 1.3 compat mode was replacing the server, but expecting to retain the same filters.  That wasn't a problem in that case, but I didn't want to have any places where the filter was set on a different agent from the one that was passed to it.

1 files changed, 54 insertions, 40 deletions

diff --git a/gtests/ssl_gtest/ssl_skip_unittest.cc b/gtests/ssl_gtest/ssl_skip_unittest.cc
index 335bfecfa..e4a9e5aed 100644
--- a/gtests/ssl_gtest/ssl_skip_unittest.cc
+++ b/gtests/ssl_gtest/ssl_skip_unittest.cc
@@ -22,8 +22,11 @@ namespace nss_test {
 class TlsHandshakeSkipFilter : public TlsRecordFilter {
  public:
   // A TLS record filter that skips handshake messages of the identified type.
-  TlsHandshakeSkipFilter(uint8_t handshake_type)
-      : handshake_type_(handshake_type), skipped_(false) {}
+  TlsHandshakeSkipFilter(const std::shared_ptr<TlsAgent>& agent,
+                         uint8_t handshake_type)
+      : TlsRecordFilter(agent),
+        handshake_type_(handshake_type),
+        skipped_(false) {}
 
  protected:
   // Takes a record; if it is a handshake record, it removes the first handshake
@@ -92,9 +95,14 @@ class TlsSkipTest : public TlsConnectTestBase,
   TlsSkipTest()
       : TlsConnectTestBase(std::get<0>(GetParam()), std::get<1>(GetParam())) {}
 
+  void SetUp() override {
+    TlsConnectTestBase::SetUp();
+    EnsureTlsSetup();
+  }
+
   void ServerSkipTest(std::shared_ptr<PacketFilter> filter,
                       uint8_t alert = kTlsAlertUnexpectedMessage) {
-    server_->SetPacketFilter(filter);
+    server_->SetFilter(filter);
     ConnectExpectAlert(client_, alert);
   }
 };
@@ -105,9 +113,14 @@ class Tls13SkipTest : public TlsConnectTestBase,
   Tls13SkipTest()
       : TlsConnectTestBase(GetParam(), SSL_LIBRARY_VERSION_TLS_1_3) {}
 
-  void ServerSkipTest(std::shared_ptr<TlsRecordFilter> filter, int32_t error) {
+  void SetUp() override {
+    TlsConnectTestBase::SetUp();
     EnsureTlsSetup();
-    server_->SetTlsRecordFilter(filter);
+  }
+
+  void ServerSkipTest(std::shared_ptr<TlsRecordFilter> filter, int32_t error) {
+    filter->EnableDecryption();
+    server_->SetFilter(filter);
     ExpectAlert(client_, kTlsAlertUnexpectedMessage);
     ConnectExpectFail();
     client_->CheckErrorCode(error);
@@ -115,8 +128,8 @@ class Tls13SkipTest : public TlsConnectTestBase,
   }
 
   void ClientSkipTest(std::shared_ptr<TlsRecordFilter> filter, int32_t error) {
-    EnsureTlsSetup();
-    client_->SetTlsRecordFilter(filter);
+    filter->EnableDecryption();
+    client_->SetFilter(filter);
     server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
     ConnectExpectFailOneSide(TlsAgent::SERVER);
 
@@ -129,48 +142,49 @@ class Tls13SkipTest : public TlsConnectTestBase,
 
 TEST_P(TlsSkipTest, SkipCertificateRsa) {
   EnableOnlyStaticRsaCiphers();
-  ServerSkipTest(
-      std::make_shared<TlsHandshakeSkipFilter>(kTlsHandshakeCertificate));
+  ServerSkipTest(std::make_shared<TlsHandshakeSkipFilter>(
+      server_, kTlsHandshakeCertificate));
   client_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_HELLO_DONE);
 }
 
 TEST_P(TlsSkipTest, SkipCertificateDhe) {
-  ServerSkipTest(
-      std::make_shared<TlsHandshakeSkipFilter>(kTlsHandshakeCertificate));
+  ServerSkipTest(std::make_shared<TlsHandshakeSkipFilter>(
+      server_, kTlsHandshakeCertificate));
   client_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_SERVER_KEY_EXCH);
 }
 
 TEST_P(TlsSkipTest, SkipCertificateEcdhe) {
-  ServerSkipTest(
-      std::make_shared<TlsHandshakeSkipFilter>(kTlsHandshakeCertificate));
+  ServerSkipTest(std::make_shared<TlsHandshakeSkipFilter>(
+      server_, kTlsHandshakeCertificate));
   client_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_SERVER_KEY_EXCH);
 }
 
 TEST_P(TlsSkipTest, SkipCertificateEcdsa) {
   Reset(TlsAgent::kServerEcdsa256);
-  ServerSkipTest(
-      std::make_shared<TlsHandshakeSkipFilter>(kTlsHandshakeCertificate));
+  ServerSkipTest(std::make_shared<TlsHandshakeSkipFilter>(
+      server_, kTlsHandshakeCertificate));
   client_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_SERVER_KEY_EXCH);
 }
 
 TEST_P(TlsSkipTest, SkipServerKeyExchange) {
-  ServerSkipTest(
-      std::make_shared<TlsHandshakeSkipFilter>(kTlsHandshakeServerKeyExchange));
+  ServerSkipTest(std::make_shared<TlsHandshakeSkipFilter>(
+      server_, kTlsHandshakeServerKeyExchange));
   client_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_HELLO_DONE);
 }
 
 TEST_P(TlsSkipTest, SkipServerKeyExchangeEcdsa) {
   Reset(TlsAgent::kServerEcdsa256);
-  ServerSkipTest(
-      std::make_shared<TlsHandshakeSkipFilter>(kTlsHandshakeServerKeyExchange));
+  ServerSkipTest(std::make_shared<TlsHandshakeSkipFilter>(
+      server_, kTlsHandshakeServerKeyExchange));
   client_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_HELLO_DONE);
 }
 
 TEST_P(TlsSkipTest, SkipCertAndKeyExch) {
-  auto chain = std::make_shared<ChainedPacketFilter>(ChainedPacketFilterInit{
-      std::make_shared<TlsHandshakeSkipFilter>(kTlsHandshakeCertificate),
-      std::make_shared<TlsHandshakeSkipFilter>(
-          kTlsHandshakeServerKeyExchange)});
+  auto chain = std::make_shared<ChainedPacketFilter>(
+      ChainedPacketFilterInit{std::make_shared<TlsHandshakeSkipFilter>(
+                                  server_, kTlsHandshakeCertificate),
+                              std::make_shared<TlsHandshakeSkipFilter>(
+                                  server_, kTlsHandshakeServerKeyExchange)});
   ServerSkipTest(chain);
   client_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_HELLO_DONE);
 }
@@ -178,48 +192,48 @@ TEST_P(TlsSkipTest, SkipCertAndKeyExch) {
 TEST_P(TlsSkipTest, SkipCertAndKeyExchEcdsa) {
   Reset(TlsAgent::kServerEcdsa256);
   auto chain = std::make_shared<ChainedPacketFilter>();
-  chain->Add(
-      std::make_shared<TlsHandshakeSkipFilter>(kTlsHandshakeCertificate));
-  chain->Add(
-      std::make_shared<TlsHandshakeSkipFilter>(kTlsHandshakeServerKeyExchange));
+  chain->Add(std::make_shared<TlsHandshakeSkipFilter>(
+      server_, kTlsHandshakeCertificate));
+  chain->Add(std::make_shared<TlsHandshakeSkipFilter>(
+      server_, kTlsHandshakeServerKeyExchange));
   ServerSkipTest(chain);
   client_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_HELLO_DONE);
 }
 
 TEST_P(Tls13SkipTest, SkipEncryptedExtensions) {
   ServerSkipTest(std::make_shared<TlsHandshakeSkipFilter>(
-                     kTlsHandshakeEncryptedExtensions),
+                     server_, kTlsHandshakeEncryptedExtensions),
                  SSL_ERROR_RX_UNEXPECTED_CERTIFICATE);
 }
 
 TEST_P(Tls13SkipTest, SkipServerCertificate) {
-  ServerSkipTest(
-      std::make_shared<TlsHandshakeSkipFilter>(kTlsHandshakeCertificate),
-      SSL_ERROR_RX_UNEXPECTED_CERT_VERIFY);
+  ServerSkipTest(std::make_shared<TlsHandshakeSkipFilter>(
+                     server_, kTlsHandshakeCertificate),
+                 SSL_ERROR_RX_UNEXPECTED_CERT_VERIFY);
 }
 
 TEST_P(Tls13SkipTest, SkipServerCertificateVerify) {
-  ServerSkipTest(
-      std::make_shared<TlsHandshakeSkipFilter>(kTlsHandshakeCertificateVerify),
-      SSL_ERROR_RX_UNEXPECTED_FINISHED);
+  ServerSkipTest(std::make_shared<TlsHandshakeSkipFilter>(
+                     server_, kTlsHandshakeCertificateVerify),
+                 SSL_ERROR_RX_UNEXPECTED_FINISHED);
 }
 
 TEST_P(Tls13SkipTest, SkipClientCertificate) {
   client_->SetupClientAuth();
   server_->RequestClientAuth(true);
   client_->ExpectReceiveAlert(kTlsAlertUnexpectedMessage);
-  ClientSkipTest(
-      std::make_shared<TlsHandshakeSkipFilter>(kTlsHandshakeCertificate),
-      SSL_ERROR_RX_UNEXPECTED_CERT_VERIFY);
+  ClientSkipTest(std::make_shared<TlsHandshakeSkipFilter>(
+                     client_, kTlsHandshakeCertificate),
+                 SSL_ERROR_RX_UNEXPECTED_CERT_VERIFY);
 }
 
 TEST_P(Tls13SkipTest, SkipClientCertificateVerify) {
   client_->SetupClientAuth();
   server_->RequestClientAuth(true);
   client_->ExpectReceiveAlert(kTlsAlertUnexpectedMessage);
-  ClientSkipTest(
-      std::make_shared<TlsHandshakeSkipFilter>(kTlsHandshakeCertificateVerify),
-      SSL_ERROR_RX_UNEXPECTED_FINISHED);
+  ClientSkipTest(std::make_shared<TlsHandshakeSkipFilter>(
+                     client_, kTlsHandshakeCertificateVerify),
+                 SSL_ERROR_RX_UNEXPECTED_FINISHED);
 }
 
 INSTANTIATE_TEST_CASE_P(