Fix decodeAndGetParam cursor bug

Summary:
Fixes a bug in decodeAndGetParam().
This function parses the OuterClientHello extension from the client hello.
Currently this function utilizes a cursor that is moved pass the outer client
hello buffer range when multiple ECHConfigs are loaded in the ECHDecrypter.
This results in an out_of_range exception being thrown.

This diff fixes this by calling getExtension once and saving the result.
Note that the RFC specifies that the client should select **one** suitable
config to use ECH (https://www.ietf.org/archive/id/draft-ietf-tls-esni-20.html#section-6.1). It is the __client's__ job to select which ech config to use
from the server's set of ECHConfigs. There should not be a case where
multiple configs are utilized by the client.

Reviewed By: mingtaoy

Differential Revision: D60994613

fbshipit-source-id: 08625fd221738e29230202c670d204e17bda35a2

fizz/protocol/ech/Decrypter.cpp

@@ -30,10 +30,10 @@ folly::Optional<DecrypterLookupResult> decodeAndGetParam(
     const Extension& encodedECHExtension,
     const std::vector<DecrypterParams>& decrypterParams) {
   folly::io::Cursor cursor(encodedECHExtension.extension_data.get());
+  auto echExtension = getExtension<ech::OuterECHClientHello>(cursor);
   for (const auto& param : decrypterParams) {
     switch (param.echConfig.version) {
       case ECHVersion::Draft15: {
-        auto echExtension = getExtension<ech::OuterECHClientHello>(cursor);
         auto& echConfig = param.echConfig;
         auto currentConfigDraft = decode<ECHConfigContentDraft>(
             echConfig.ech_config_content->clone());

fizz/protocol/ech/test/DecrypterTest.cpp

@@ -20,11 +20,18 @@ using namespace fizz::test;
 namespace fizz {
 namespace ech {
 namespace test {
-ClientHello getChloOuterWithExt(std::unique_ptr<KeyExchange> kex) {
+ClientHello getChloOuterWithExt(
+    std::unique_ptr<KeyExchange> kex,
+    std::optional<ECHConfig> config = std::nullopt) {
+  if (!config) {
+    config = getECHConfig();
+  }
+
   // Setup ECH extension
-  auto echConfigContent = getECHConfigContent();
+  folly::io::Cursor c(config->ech_config_content.get());
+  auto echConfigContent = decode<ECHConfigContentDraft>(c);
   auto supportedECHConfig = SupportedECHConfig{
-      getECHConfig(),
+      std::move(config.value()),
       echConfigContent.key_config.config_id,
       echConfigContent.maximum_name_length,
       HpkeSymmetricCipherSuite{
@@ -222,6 +229,30 @@ TEST(DecrypterTest, TestDecodeHRRWithContextFailure) {
       FizzException);
 }
 
+TEST(DecrypterTest, TestDecodeMultipleDecrypterParam) {
+  auto targetEchConfigContent = getECHConfigContent();
+  targetEchConfigContent.key_config.config_id = 2;
+  ECHConfig targetECHConfig;
+  targetECHConfig.ech_config_content =
+      encode(std::move(targetEchConfigContent));
+  targetECHConfig.version = ECHVersion::Draft15;
+
+  auto kex = openssl::makeOpenSSLECKeyExchange<fizz::P256>();
+  kex->setPrivateKey(getPrivateKey(kP256Key));
+  ECHConfigManager decrypter;
+  // Load multiple decrypter params
+  decrypter.addDecryptionConfig(DecrypterParams{getECHConfig(), kex->clone()});
+  decrypter.addDecryptionConfig(DecrypterParams{targetECHConfig, kex->clone()});
+
+  Buf enc;
+  auto chlo = getChloOuterWithExt(kex->clone(), targetECHConfig);
+
+  // Decrypting chlo should result in the second decrypter param being used
+  auto result = decrypter.decryptClientHello(chlo);
+  ASSERT_TRUE(result.has_value());
+  EXPECT_TRUE(result->configId == 2);
+}
+
 } // namespace test
 } // namespace ech
 } // namespace fizz