Add an option to permute ClientHello extension order.

Although not permitted by the TLS specification, systems sometimes
ossify TLS extension order, or byte offsets of various fields. To
keep the ecosystem healthy, add an API to reorder ClientHello
extensions.

Since ECH, HelloRetryRequest, and HelloVerifyRequest are sensitive to
extension order, I've implemented this by per-connection permutation of
the indices in the kExtensions structure. This ensures that all
ClientHellos within a connection are consistently ordered. As follow-up
work, permuting the other messages would also be nice, though any server
messages would need to be incorporated in handshake hints.

Change-Id: I18ce39b4df5ee376c654943f07ec26a50e0923a9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48045
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/base.h b/include/openssl/base.h
index b486f16..88cfb8f 100644
--- a/include/openssl/base.h
+++ b/include/openssl/base.h
@@ -195,7 +195,7 @@
 // A consumer may use this symbol in the preprocessor to temporarily build
 // against multiple revisions of BoringSSL at the same time. It is not
 // recommended to do so for longer than is necessary.
-#define BORINGSSL_API_VERSION 15
+#define BORINGSSL_API_VERSION 16
 
 #if defined(BORINGSSL_SHARED_LIBRARY)
 
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 1ff9379..dc3a79d 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -4301,6 +4301,14 @@
 // GREASE. See RFC 8701.
 OPENSSL_EXPORT void SSL_CTX_set_grease_enabled(SSL_CTX *ctx, int enabled);
 
+// SSL_CTX_set_permute_extensions configures whether sockets on |ctx| should
+// permute extensions. For now, this is only implemented for the ClientHello.
+OPENSSL_EXPORT void SSL_CTX_set_permute_extensions(SSL_CTX *ctx, int enabled);
+
+// SSL_set_permute_extensions configures whether sockets on |ssl| should
+// permute extensions. For now, this is only implemented for the ClientHello.
+OPENSSL_EXPORT void SSL_set_permute_extensions(SSL *ssl, int enabled);
+
 // SSL_max_seal_overhead returns the maximum overhead, in bytes, of sealing a
 // record with |ssl|.
 OPENSSL_EXPORT size_t SSL_max_seal_overhead(const SSL *ssl);
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index ae8aabd..1fb43fb 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -535,6 +535,7 @@
   }
 
   if (!ssl_setup_key_shares(hs, /*override_group_id=*/0) ||
+      !ssl_setup_extension_permutation(hs) ||
       !ssl_encrypt_client_hello(hs, MakeConstSpan(ech_enc, ech_enc_len)) ||
       !ssl_add_client_hello(hs)) {
     return ssl_hs_error;
diff --git a/ssl/internal.h b/ssl/internal.h
index 3728f70..cb5bf75 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1855,6 +1855,11 @@
   // peer_key is the peer's ECDH key for a TLS 1.2 client.
   Array<uint8_t> peer_key;
 
+  // extension_permutation is the permutation to apply to ClientHello
+  // extensions. It maps indices into the |kExtensions| table into other
+  // indices.
+  Array<uint8_t> extension_permutation;
+
   // cert_compression_alg_id, for a server, contains the negotiated certificate
   // compression algorithm for this client. It is only valid if
   // |cert_compression_negotiated| is true.
@@ -2100,6 +2105,10 @@
 bssl::UniquePtr<SSL_SESSION> tls13_create_session_with_ticket(SSL *ssl,
                                                               CBS *body);
 
+// ssl_setup_extension_permutation computes a ClientHello extension permutation
+// for |hs|, if applicable. It returns true on success and false on error.
+bool ssl_setup_extension_permutation(SSL_HANDSHAKE *hs);
+
 // ssl_setup_key_shares computes client key shares and saves them in |hs|. It
 // returns true on success and false on failure. If |override_group_id| is zero,
 // it offers the default groups, including GREASE. If it is non-zero, it offers
@@ -3028,6 +3037,9 @@
   // QUIC drafts up to and including 32 used a different TLS extension
   // codepoint to convey QUIC's transport parameters.
   bool quic_use_legacy_codepoint : 1;
+
+  // permute_extensions is whether to permute extensions when sending messages.
+  bool permute_extensions : 1;
 };
 
 // From RFC 8446, used in determining PSK modes.
@@ -3615,6 +3627,9 @@
   // grease_enabled is whether GREASE (RFC 8701) is enabled.
   bool grease_enabled : 1;
 
+  // permute_extensions is whether to permute extensions when sending messages.
+  bool permute_extensions : 1;
+
   // allow_unknown_alpn_protos is whether the client allows unsolicited ALPN
   // protocols from the peer.
   bool allow_unknown_alpn_protos : 1;
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 31ab3bb..1211761 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -562,6 +562,7 @@
       signed_cert_timestamps_enabled(false),
       channel_id_enabled(false),
       grease_enabled(false),
+      permute_extensions(false),
       allow_unknown_alpn_protos(false),
       false_start_allowed_without_alpn(false),
       handoff(false),
@@ -684,6 +685,7 @@
   ssl->config->custom_verify_callback = ctx->custom_verify_callback;
   ssl->config->retain_only_sha256_of_client_certs =
       ctx->retain_only_sha256_of_client_certs;
+  ssl->config->permute_extensions = ctx->permute_extensions;
 
   if (!ssl->config->supported_group_list.CopyFrom(ctx->supported_group_list) ||
       !ssl->config->alpn_client_proto_list.CopyFrom(
@@ -730,7 +732,8 @@
       handoff(false),
       shed_handshake_config(false),
       jdk11_workaround(false),
-      quic_use_legacy_codepoint(false) {
+      quic_use_legacy_codepoint(false),
+      permute_extensions(false) {
   assert(ssl);
 }
 
@@ -2915,6 +2918,17 @@
   ctx->grease_enabled = !!enabled;
 }
 
+void SSL_CTX_set_permute_extensions(SSL_CTX *ctx, int enabled) {
+  ctx->permute_extensions = !!enabled;
+}
+
+void SSL_set_permute_extensions(SSL *ssl, int enabled) {
+  if (!ssl->config) {
+    return;
+  }
+  ssl->config->permute_extensions = !!enabled;
+}
+
 int32_t SSL_get_ticket_age_skew(const SSL *ssl) {
   return ssl->s3->ticket_age_skew;
 }
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index e2708b1..e2a41b8 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -26,6 +26,7 @@
 
 #include <openssl/aead.h>
 #include <openssl/base64.h>
+#include <openssl/bytestring.h>
 #include <openssl/bio.h>
 #include <openssl/cipher.h>
 #include <openssl/crypto.h>
@@ -7369,5 +7370,126 @@
   }
 }
 
+// GetExtensionOrder sets |*out| to the list of extensions a client attached to
+// |ctx| will send in the ClientHello. If |ech_keys| is non-null, the client
+// will offer ECH with the public component. If |decrypt_ech| is true, |*out|
+// will be set to the ClientHelloInner's extensions, rather than
+// ClientHelloOuter.
+static bool GetExtensionOrder(SSL_CTX *client_ctx, std::vector<uint16_t> *out,
+                              SSL_ECH_KEYS *ech_keys, bool decrypt_ech) {
+  struct AppData {
+    std::vector<uint16_t> *out;
+    bool decrypt_ech;
+    bool callback_done = false;
+  };
+  AppData app_data;
+  app_data.out = out;
+  app_data.decrypt_ech = decrypt_ech;
+
+  bssl::UniquePtr<SSL_CTX> server_ctx =
+      CreateContextWithTestCertificate(TLS_method());
+  if (!server_ctx ||  //
+      !SSL_CTX_set_app_data(server_ctx.get(), &app_data) ||
+      (decrypt_ech && !SSL_CTX_set1_ech_keys(server_ctx.get(), ech_keys))) {
+    return false;
+  }
+
+  // Configure the server to record the ClientHello extension order. We use a
+  // server rather than |GetClientHello| so it can decrypt ClientHelloInner.
+  SSL_CTX_set_select_certificate_cb(
+      server_ctx.get(),
+      [](const SSL_CLIENT_HELLO *client_hello) -> ssl_select_cert_result_t {
+        AppData *app_data_ptr = static_cast<AppData *>(
+            SSL_CTX_get_app_data(SSL_get_SSL_CTX(client_hello->ssl)));
+        EXPECT_EQ(app_data_ptr->decrypt_ech ? 1 : 0,
+                  SSL_ech_accepted(client_hello->ssl));
+
+        app_data_ptr->out->clear();
+        CBS extensions;
+        CBS_init(&extensions, client_hello->extensions,
+                 client_hello->extensions_len);
+        while (CBS_len(&extensions)) {
+          uint16_t type;
+          CBS body;
+          if (!CBS_get_u16(&extensions, &type) ||
+              !CBS_get_u16_length_prefixed(&extensions, &body)) {
+            return ssl_select_cert_error;
+          }
+          app_data_ptr->out->push_back(type);
+        }
+
+        // Don't bother completing the handshake.
+        app_data_ptr->callback_done = true;
+        return ssl_select_cert_error;
+      });
+
+  bssl::UniquePtr<SSL> client, server;
+  if (!CreateClientAndServer(&client, &server, client_ctx, server_ctx.get()) ||
+      (ech_keys != nullptr && !InstallECHConfigList(client.get(), ech_keys))) {
+    return false;
+  }
+
+  // Run the handshake far enough to process the ClientHello.
+  SSL_do_handshake(client.get());
+  SSL_do_handshake(server.get());
+  return app_data.callback_done;
+}
+
+// Test that, when extension permutation is enabled, the ClientHello extension
+// order changes, both with and without ECH, and in both ClientHelloInner and
+// ClientHelloOuter.
+TEST(SSLTest, PermuteExtensions) {
+  bssl::UniquePtr<SSL_ECH_KEYS> keys = MakeTestECHKeys();
+  ASSERT_TRUE(keys);
+  for (bool offer_ech : {false, true}) {
+    SCOPED_TRACE(offer_ech);
+    SSL_ECH_KEYS *maybe_keys = offer_ech ? keys.get() : nullptr;
+    for (bool decrypt_ech : {false, true}) {
+      SCOPED_TRACE(decrypt_ech);
+      if (!offer_ech && decrypt_ech) {
+        continue;
+      }
+
+      // When extension permutation is disabled, the order should be consistent.
+      bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method()));
+      ASSERT_TRUE(ctx);
+      std::vector<uint16_t> order1, order2;
+      ASSERT_TRUE(
+          GetExtensionOrder(ctx.get(), &order1, maybe_keys, decrypt_ech));
+      ASSERT_TRUE(
+          GetExtensionOrder(ctx.get(), &order2, maybe_keys, decrypt_ech));
+      EXPECT_EQ(order1, order2);
+
+      ctx.reset(SSL_CTX_new(TLS_method()));
+      ASSERT_TRUE(ctx);
+      SSL_CTX_set_permute_extensions(ctx.get(), 1);
+
+      // When extension permutation is enabled, each ClientHello should have a
+      // different order.
+      //
+      // This test is inherently flaky, so we run it multiple times. We send at
+      // least five extensions by default from TLS 1.3: supported_versions,
+      // key_share, supported_groups, psk_key_exchange_modes, and
+      // signature_algorithms. That means the probability of a false negative is
+      // at most 1/120. Repeating the test 14 times lowers false negative rate
+      // to under 2^-96.
+      ASSERT_TRUE(
+          GetExtensionOrder(ctx.get(), &order1, maybe_keys, decrypt_ech));
+      EXPECT_GE(order1.size(), 5u);
+      static const int kNumIterations = 14;
+      bool passed = false;
+      for (int i = 0; i < kNumIterations; i++) {
+        ASSERT_TRUE(
+            GetExtensionOrder(ctx.get(), &order2, maybe_keys, decrypt_ech));
+        if (order1 != order2) {
+          passed = true;
+          break;
+        }
+      }
+      EXPECT_TRUE(passed) << "Extensions were not permuted";
+    }
+  }
+}
+
 }  // namespace
 BSSL_NAMESPACE_END
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index 90ad5eb..69a2bdc 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -127,6 +127,7 @@
 #include <openssl/hpke.h>
 #include <openssl/mem.h>
 #include <openssl/nid.h>
+#include <openssl/rand.h>
 
 #include "../crypto/internal.h"
 #include "internal.h"
@@ -3271,6 +3272,30 @@
                   sizeof(((SSL_HANDSHAKE *)NULL)->extensions.received) * 8,
               "too many extensions for received bitset");
 
+bool ssl_setup_extension_permutation(SSL_HANDSHAKE *hs) {
+  if (!hs->config->permute_extensions) {
+    return true;
+  }
+
+  static_assert(kNumExtensions <= UINT8_MAX,
+                "extensions_permutation type is too small");
+  uint32_t seeds[kNumExtensions - 1];
+  Array<uint8_t> permutation;
+  if (!RAND_bytes(reinterpret_cast<uint8_t *>(seeds), sizeof(seeds)) ||
+      !permutation.Init(kNumExtensions)) {
+    return false;
+  }
+  for (size_t i = 0; i < kNumExtensions; i++) {
+    permutation[i] = i;
+  }
+  for (size_t i = kNumExtensions - 1; i > 0; i--) {
+    // Set element |i| to a randomly-selected element 0 <= j <= i.
+    std::swap(permutation[i], permutation[seeds[i - 1] % (i + 1)]);
+  }
+  hs->extension_permutation = std::move(permutation);
+  return true;
+}
+
 static const struct tls_extension *tls_extension_find(uint32_t *out_index,
                                                       uint16_t value) {
   unsigned i;
@@ -3328,7 +3353,10 @@
     }
   }
 
-  for (size_t i = 0; i < kNumExtensions; i++) {
+  for (size_t unpermuted = 0; unpermuted < kNumExtensions; unpermuted++) {
+    size_t i = hs->extension_permutation.empty()
+                   ? unpermuted
+                   : hs->extension_permutation[unpermuted];
     const size_t len_before = CBB_len(&extensions);
     const size_t len_compressed_before = CBB_len(compressed.get());
     if (!kExtensions[i].add_clienthello(hs, &extensions, compressed.get(),
@@ -3462,7 +3490,10 @@
   }
 
   bool last_was_empty = false;
-  for (size_t i = 0; i < kNumExtensions; i++) {
+  for (size_t unpermuted = 0; unpermuted < kNumExtensions; unpermuted++) {
+    size_t i = hs->extension_permutation.empty()
+                   ? unpermuted
+                   : hs->extension_permutation[unpermuted];
     size_t bytes_written;
     if (omit_ech_len != 0 &&
         kExtensions[i].value == TLSEXT_TYPE_encrypted_client_hello) {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index d4dcd52..fded062 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -8208,6 +8208,78 @@
 					base64.StdEncoding.EncodeToString(testSCTList),
 				},
 			})
+
+			// Extension permutation should interact correctly with other extensions,
+			// HelloVerifyRequest, HelloRetryRequest, and ECH. SSLTest.PermuteExtensions
+			// in ssl_test.cc tests that the extensions are actually permuted. This
+			// tests the handshake still works.
+			//
+			// This test also tests that all our extensions interact with each other.
+			for _, ech := range []bool{false, true} {
+				if ech && ver.version < VersionTLS13 {
+					continue
+				}
+
+				test := testCase{
+					protocol:           protocol,
+					name:               "AllExtensions-Client-Permute",
+					skipQUICALPNConfig: true,
+					config: Config{
+						MinVersion:          ver.version,
+						MaxVersion:          ver.version,
+						NextProtos:          []string{"proto"},
+						ApplicationSettings: map[string][]byte{"proto": []byte("runner1")},
+						Bugs: ProtocolBugs{
+							SendServerNameAck: true,
+							ExpectServerName:  "example.com",
+							ExpectGREASE:      true,
+						},
+					},
+					resumeSession: true,
+					flags: []string{
+						"-permute-extensions",
+						"-enable-grease",
+						"-enable-ocsp-stapling",
+						"-enable-signed-cert-timestamps",
+						"-advertise-alpn", "\x05proto",
+						"-expect-alpn", "proto",
+						"-host-name", "example.com",
+					},
+				}
+
+				if ech {
+					test.name += "-ECH"
+					echConfig := generateServerECHConfig(&ECHConfig{ConfigID: 42})
+					test.config.ServerECHConfigs = []ServerECHConfig{echConfig}
+					test.flags = append(test.flags,
+						"-ech-config-list", base64.StdEncoding.EncodeToString(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+						"-expect-ech-accept",
+					)
+					test.expectations.echAccepted = true
+				}
+
+				if ver.version >= VersionTLS13 {
+					// Trigger a HelloRetryRequest to test both ClientHellos. Note
+					// our DTLS tests always enable HelloVerifyRequest.
+					test.name += "-HelloRetryRequest"
+
+					// ALPS is only available on TLS 1.3.
+					test.config.ApplicationSettings = map[string][]byte{"proto": []byte("runner")}
+					test.flags = append(test.flags,
+						"-application-settings", "proto,shim",
+						"-expect-peer-application-settings", "runner")
+					test.expectations.peerApplicationSettings = []byte("shim")
+				}
+
+				if protocol == dtls {
+					test.config.SRTPProtectionProfiles = []uint16{SRTP_AES128_CM_HMAC_SHA1_80}
+					test.flags = append(test.flags, "-srtp-profiles", "SRTP_AES128_CM_SHA1_80")
+					test.expectations.srtpProtectionProfile = SRTP_AES128_CM_HMAC_SHA1_80
+				}
+
+				test.name += "-" + suffix
+				testCases = append(testCases, test)
+			}
 		}
 	}
 
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index c62c765..6288da4 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -118,6 +118,7 @@
     {"-send-alert", &TestConfig::send_alert},
     {"-peek-then-read", &TestConfig::peek_then_read},
     {"-enable-grease", &TestConfig::enable_grease},
+    {"-permute-extensions", &TestConfig::permute_extensions},
     {"-use-exporter-between-reads", &TestConfig::use_exporter_between_reads},
     {"-retain-only-sha256-client-cert",
      &TestConfig::retain_only_sha256_client_cert},
@@ -1428,6 +1429,10 @@
     SSL_CTX_set_grease_enabled(ssl_ctx.get(), 1);
   }
 
+  if (permute_extensions) {
+    SSL_CTX_set_permute_extensions(ssl_ctx.get(), 1);
+  }
+
   if (!expect_server_name.empty()) {
     SSL_CTX_set_tlsext_servername_callback(ssl_ctx.get(), ServerNameCallback);
   }
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 9ef0ced..377a757 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -142,6 +142,7 @@
   bool send_alert = false;
   bool peek_then_read = false;
   bool enable_grease = false;
+  bool permute_extensions = false;
   int max_cert_list = 0;
   std::string ticket_key;
   bool use_exporter_between_reads = false;
diff --git a/tool/client.cc b/tool/client.cc
index 6f738e3..4301c22 100644
--- a/tool/client.cc
+++ b/tool/client.cc
@@ -123,6 +123,11 @@
         "-grease", kBooleanArgument, "Enable GREASE",
     },
     {
+        "-permute-extensions",
+        kBooleanArgument,
+        "Permute extensions in handshake messages",
+    },
+    {
         "-test-resumption", kBooleanArgument,
         "Connect to the server twice. The first connection is closed once a "
         "session is established. The second connection offers it.",
@@ -531,6 +536,10 @@
     SSL_CTX_set_grease_enabled(ctx.get(), 1);
   }
 
+  if (args_map.count("-permute-extensions") != 0) {
+    SSL_CTX_set_permute_extensions(ctx.get(), 1);
+  }
+
   if (args_map.count("-root-certs") != 0) {
     if (!SSL_CTX_load_verify_locations(
             ctx.get(), args_map["-root-certs"].c_str(), nullptr)) {