Bug 1789436 - CH extension permutation. r=djackson

Depends on D161806

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

6 files changed, 113 insertions, 0 deletions

diff --git a/gtests/nss_bogo_shim/nss_bogo_shim.cc b/gtests/nss_bogo_shim/nss_bogo_shim.cc
index 890ec7149..52a3e9a94 100644
--- a/gtests/nss_bogo_shim/nss_bogo_shim.cc
+++ b/gtests/nss_bogo_shim/nss_bogo_shim.cc
@@ -372,6 +372,12 @@ class TestAgent {
         if (rv != SECSuccess) return false;
       }
 
+      if (cfg_.get<bool>("permute-extensions")) {
+        rv = SSL_OptionSet(ssl_fd_.get(), SSL_ENABLE_CH_EXTENSION_PERMUTATION,
+                           PR_TRUE);
+        if (rv != SECSuccess) return false;
+      }
+
     } else {
       // GREASE - BoGo expects servers to enable GREASE by default
       rv = SSL_OptionSet(ssl_fd_.get(), SSL_ENABLE_GREASE, PR_TRUE);
@@ -907,6 +913,7 @@ std::unique_ptr<const Config> ReadConfig(int argc, char** argv) {
   cfg->AddEntry<bool>("enable-ech-grease", false);
   cfg->AddEntry<bool>("enable-early-data", false);
   cfg->AddEntry<bool>("enable-grease", false);
+  cfg->AddEntry<bool>("permute-extensions", false);
   cfg->AddEntry<bool>("on-resume-expect-reject-early-data", false);
   cfg->AddEntry<bool>("on-resume-expect-accept-early-data", false);
   cfg->AddEntry<bool>("expect-ticket-supports-early-data", false);
diff --git a/gtests/ssl_gtest/ssl_extension_unittest.cc b/gtests/ssl_gtest/ssl_extension_unittest.cc
index 08eb3cb02..59a6ea679 100644
--- a/gtests/ssl_gtest/ssl_extension_unittest.cc
+++ b/gtests/ssl_gtest/ssl_extension_unittest.cc
@@ -1357,6 +1357,61 @@ TEST_F(TlsConnectDatagram13, Dtls13RejectLegacyCookie) {
   client_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
 }
 
+TEST_P(TlsConnectGeneric, ClientHelloExtensionPermutation) {
+  EnsureTlsSetup();
+  PR_ASSERT(SSL_OptionSet(client_->ssl_fd(),
+                          SSL_ENABLE_CH_EXTENSION_PERMUTATION,
+                          PR_TRUE) == SECSuccess);
+  Connect();
+}
+
+/* This test checks that the ClientHello extension order is actually permuted
+ * if ss->opt.chXtnPermutation is set. It is asserted that at least one out of
+ * 10 extension orders differs from the others.
+ *
+ * This is a probabilistic test: The default TLS 1.3 ClientHello contains 8
+ * extensions, leading to a 1/8! probability for any extension order and the
+ * same probability for two drawn extension orders to coincide.
+ * Since all sequences are compared against each other this leads to a false
+ * positive rate of (1/8!)^(n^2-n).
+ * To achieve a spurious failure rate << 1/2^64, we compare n=10 drawn orders.
+ *
+ * This test assures that randomisation is happening but does not check quality
+ * of the used Fisher-Yates shuffle. */
+TEST_F(TlsConnectStreamTls13,
+       ClientHelloExtensionPermutationProbabilisticTest) {
+  std::vector<std::vector<uint16_t>> orders;
+
+  /* Capture the extension order of 10 ClientHello messages. */
+  for (size_t i = 0; i < 10; i++) {
+    client_->StartConnect();
+    /* Enable ClientHello extension permutation. */
+    PR_ASSERT(SSL_OptionSet(client_->ssl_fd(),
+                            SSL_ENABLE_CH_EXTENSION_PERMUTATION,
+                            PR_TRUE) == SECSuccess);
+    /* Capture extension order filter. */
+    auto filter = MakeTlsFilter<TlsExtensionOrderCapture>(
+        client_, kTlsHandshakeClientHello);
+    /* Send ClientHello. */
+    client_->Handshake();
+    /* Remember extension order. */
+    orders.push_back(filter->order);
+    /* Reset client / server state. */
+    Reset();
+  }
+
+  /* Check for extension order inequality. */
+  size_t inequal = 0;
+  for (auto& outerOrders : orders) {
+    for (auto& innerOrders : orders) {
+      if (outerOrders != innerOrders) {
+        inequal++;
+      }
+    }
+  }
+  PR_ASSERT(inequal >= 1);
+}
+
 INSTANTIATE_TEST_SUITE_P(
     ExtensionStream, TlsExtensionTestGeneric,
     ::testing::Combine(TlsConnectTestBase::kTlsVariantsStream,
diff --git a/gtests/ssl_gtest/tls_ech_unittest.cc b/gtests/ssl_gtest/tls_ech_unittest.cc
index ab61beec5..1e70a6ee5 100644
--- a/gtests/ssl_gtest/tls_ech_unittest.cc
+++ b/gtests/ssl_gtest/tls_ech_unittest.cc
@@ -2884,6 +2884,28 @@ TEST_F(TlsConnectStreamTls13Ech, EchPublicNameNotLdh) {
   ValidatePublicNames(kNotLdh, SECFailure);
 }
 
+TEST_F(TlsConnectStreamTls13, EchClientHelloExtensionPermutation) {
+  EnsureTlsSetup();
+  PR_ASSERT(SSL_OptionSet(client_->ssl_fd(),
+                          SSL_ENABLE_CH_EXTENSION_PERMUTATION,
+                          PR_TRUE) == SECSuccess);
+  SetupEch(client_, server_);
+
+  client_->ExpectEch();
+  server_->ExpectEch();
+  Connect();
+}
+
+TEST_F(TlsConnectStreamTls13, EchGreaseClientHelloExtensionPermutation) {
+  EnsureTlsSetup();
+  PR_ASSERT(SSL_OptionSet(client_->ssl_fd(),
+                          SSL_ENABLE_CH_EXTENSION_PERMUTATION,
+                          PR_TRUE) == SECSuccess);
+  PR_ASSERT(SSL_EnableTls13GreaseEch(client_->ssl_fd(), PR_FALSE) ==
+            SECSuccess);
+  Connect();
+}
+
 INSTANTIATE_TEST_SUITE_P(EchAgentTest, TlsAgentEchTest,
                          ::testing::Combine(TlsConnectTestBase::kTlsVariantsAll,
                                             TlsConnectTestBase::kTlsV13));
diff --git a/gtests/ssl_gtest/tls_filter.cc b/gtests/ssl_gtest/tls_filter.cc
index 00fececee..2851619b0 100644
--- a/gtests/ssl_gtest/tls_filter.cc
+++ b/gtests/ssl_gtest/tls_filter.cc
@@ -1015,6 +1015,12 @@ PacketFilter::Action TlsExtensionFilter::FilterExtensions(
   return KEEP;
 }
 
+PacketFilter::Action TlsExtensionOrderCapture::FilterExtension(
+    uint16_t extension_type, const DataBuffer& input, DataBuffer* output) {
+  order.push_back(extension_type);
+  return KEEP;
+}
+
 PacketFilter::Action TlsExtensionCapture::FilterExtension(
     uint16_t extension_type, const DataBuffer& input, DataBuffer* output) {
   if (extension_type == extension_ && (last_ || !captured_)) {
diff --git a/gtests/ssl_gtest/tls_filter.h b/gtests/ssl_gtest/tls_filter.h
index d7136c0fd..fc88d0b18 100644
--- a/gtests/ssl_gtest/tls_filter.h
+++ b/gtests/ssl_gtest/tls_filter.h
@@ -488,6 +488,19 @@ class TlsExtensionFilter : public TlsHandshakeFilter {
                                         DataBuffer* output);
 };
 
+class TlsExtensionOrderCapture : public TlsExtensionFilter {
+ public:
+  TlsExtensionOrderCapture(const std::shared_ptr<TlsAgent>& a, uint8_t message)
+      : TlsExtensionFilter(a, {message}){};
+
+  std::vector<uint16_t> order;
+
+ protected:
+  PacketFilter::Action FilterExtension(uint16_t extension_type,
+                                       const DataBuffer& input,
+                                       DataBuffer* output) override;
+};
+
 class TlsExtensionCapture : public TlsExtensionFilter {
  public:
   TlsExtensionCapture(const std::shared_ptr<TlsAgent>& a, uint16_t ext,
diff --git a/gtests/ssl_gtest/tls_grease_unittest.cc b/gtests/ssl_gtest/tls_grease_unittest.cc
index a3d5bbf45..8580ee1fc 100644
--- a/gtests/ssl_gtest/tls_grease_unittest.cc
+++ b/gtests/ssl_gtest/tls_grease_unittest.cc
@@ -473,6 +473,16 @@ TEST_P(GreaseTestStreamTls13, GreasedClientCertificateExtensions) {
   client_->CheckErrorCode(SSL_ERROR_DECRYPT_ERROR_ALERT);
 }
 
+TEST_F(TlsConnectStreamTls13, GreaseClientHelloExtensionPermutation) {
+  EnsureTlsSetup();
+  PR_ASSERT(SSL_OptionSet(client_->ssl_fd(),
+                          SSL_ENABLE_CH_EXTENSION_PERMUTATION,
+                          PR_TRUE) == SECSuccess);
+  PR_ASSERT(SSL_OptionSet(client_->ssl_fd(), SSL_ENABLE_GREASE, PR_TRUE) ==
+            SECSuccess);
+  Connect();
+}
+
 const uint8_t kTlsGreaseExtensionMessages[] = {kTlsHandshakeEncryptedExtensions,
                                                kTlsHandshakeCertificate};