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>

include/openssl/base.h

@@ -195,7 +195,7 @@ extern "C" {
 // 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)
 

include/openssl/ssl.h

@@ -4301,6 +4301,14 @@ OPENSSL_EXPORT void SSL_CTX_set_retain_only_sha256_of_client_certs(SSL_CTX *ctx,
 // 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);

ssl/handshake_client.cc

@@ -535,6 +535,7 @@ static enum ssl_hs_wait_t do_start_connect(SSL_HANDSHAKE *hs) {
   }
 
   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;

ssl/internal.h

@@ -1855,6 +1855,11 @@ struct SSL_HANDSHAKE {
   // 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 @@ bool tls13_process_new_session_ticket(SSL *ssl, const SSLMessage &msg);
 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 @@ struct SSL_CONFIG {
   // 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 @@ struct ssl_ctx_st {
   // 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;

ssl/ssl_lib.cc

@@ -562,6 +562,7 @@ ssl_ctx_st::ssl_ctx_st(const SSL_METHOD *ssl_method)
       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 *SSL_new(SSL_CTX *ctx) {
   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 @@ SSL_CONFIG::SSL_CONFIG(SSL *ssl_arg)
       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 @@ void SSL_CTX_set_grease_enabled(SSL_CTX *ctx, int enabled) {
   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;
 }

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 @@ TEST(SSLTest, CanReleasePrivateKey) {
   }
 }
 
+// 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

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 @@ static_assert(kNumExtensions <=
                   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 @@ static bool ssl_add_clienthello_tlsext_inner(SSL_HANDSHAKE *hs, CBB *out,
     }
   }
 
-  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 ssl_add_clienthello_tlsext(SSL_HANDSHAKE *hs, CBB *out, CBB *out_encoded,
   }
 
   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) {

ssl/test/runner/runner.go

@@ -8208,6 +8208,78 @@ func addExtensionTests() {
 					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)
+			}
 		}
 	}
 

ssl/test/test_config.cc

@@ -118,6 +118,7 @@ const Flag<bool> kBoolFlags[] = {
     {"-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 @@ bssl::UniquePtr<SSL_CTX> TestConfig::SetupCtx(SSL_CTX *old_ctx) const {
     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);
   }

ssl/test/test_config.h

@@ -142,6 +142,7 @@ struct TestConfig {
   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;

tool/client.cc

@@ -122,6 +122,11 @@ static const struct argument kArguments[] = {
     {
         "-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 "
@@ -531,6 +536,10 @@ bool Client(const std::vector<std::string> &args) {
     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)) {