In TLS_Text_Policy be more careful to filter out unsupported groups

This had logic for X25519/X448 but did not consider the possibility of the PQC suites, which both can depend on a PQC algorithm or a PQC plus a curve in combination.
randombit · Aug 13, 2024 · ee8f6c0 · ee8f6c0
1 parent 2316613
commit ee8f6c0
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 17 deletions.
diff --git a/src/lib/tls/tls_algos.cpp b/src/lib/tls/tls_algos.cpp
@@ -134,6 +134,82 @@ Auth_Method auth_method_from_string(std::string_view str) {
    throw Invalid_Argument(fmt("Unknown TLS signature method '{}'", str));
 }
 
+bool Group_Params::is_available() const {
+#if !defined(BOTAN_HAS_X25519)
+   if(is_x25519()) {
+      return false;
+   }
+   if(is_pqc_hybrid() && pqc_hybrid_ecc() == Group_Params_Code::X25519) {
+      return false;
+   }
+#endif
+
+#if !defined(BOTAN_HAS_X448)
+   if(is_x448()) {
+      return false;
+   }
+   if(is_pqc_hybrid() && pqc_hybrid_ecc() == Group_Params_Code::X448) {
+      return false;
+   }
+#endif
+
+#if !defined(BOTAN_HAS_DIFFIE_HELLMAN)
+   if(is_in_ffdhe_range()) {
+      return false;
+   }
+#endif
+
+#if !defined(BOTAN_HAS_KYBER_ROUND3)
+   if(is_pure_kyber() || is_pqc_hybrid_kyber()) {
+      return false;
+   }
+#endif
+
+#if !defined(BOTAN_HAS_FRODOKEM)
+   if(is_pure_frodokem() || is_pqc_hybrid_frodokem()) {
+      return false;
+   }
+#endif
+
+   return true;
+}
+
+std::optional<Group_Params_Code> Group_Params::pqc_hybrid_ecc() const {
+   switch(m_code) {
+      case Group_Params_Code::HYBRID_X25519_KYBER_512_R3_CLOUDFLARE:
+      case Group_Params_Code::HYBRID_X25519_KYBER_512_R3_OQS:
+      case Group_Params_Code::HYBRID_X25519_KYBER_768_R3_OQS:
+
+      case Group_Params_Code::HYBRID_X25519_eFRODOKEM_640_SHAKE_OQS:
+      case Group_Params_Code::HYBRID_X25519_eFRODOKEM_640_AES_OQS:
+         return Group_Params_Code::X25519;
+
+      case Group_Params_Code::HYBRID_X448_KYBER_768_R3_OQS:
+      case Group_Params_Code::HYBRID_X448_eFRODOKEM_976_SHAKE_OQS:
+      case Group_Params_Code::HYBRID_X448_eFRODOKEM_976_AES_OQS:
+         return Group_Params_Code::X448;
+
+      case Group_Params_Code::HYBRID_SECP256R1_KYBER_512_R3_OQS:
+      case Group_Params_Code::HYBRID_SECP256R1_KYBER_768_R3_OQS:
+      case Group_Params_Code::HYBRID_SECP256R1_eFRODOKEM_640_SHAKE_OQS:
+      case Group_Params_Code::HYBRID_SECP256R1_eFRODOKEM_640_AES_OQS:
+         return Group_Params_Code::SECP256R1;
+
+      case Group_Params_Code::HYBRID_SECP384R1_KYBER_768_R3_OQS:
+      case Group_Params_Code::HYBRID_SECP384R1_eFRODOKEM_976_SHAKE_OQS:
+      case Group_Params_Code::HYBRID_SECP384R1_eFRODOKEM_976_AES_OQS:
+         return Group_Params_Code::SECP384R1;
+
+      case Group_Params_Code::HYBRID_SECP521R1_KYBER_1024_R3_OQS:
+      case Group_Params_Code::HYBRID_SECP521R1_eFRODOKEM_1344_SHAKE_OQS:
+      case Group_Params_Code::HYBRID_SECP521R1_eFRODOKEM_1344_AES_OQS:
+         return Group_Params_Code::SECP521R1;
+
+      default:
+         return {};
+   }
+}
+
 std::optional<Group_Params> Group_Params::from_string(std::string_view group_name) {
    if(group_name == "secp256r1") {
       return Group_Params::SECP256R1;

diff --git a/src/lib/tls/tls_algos.h b/src/lib/tls/tls_algos.h
@@ -173,6 +173,11 @@ class BOTAN_PUBLIC_API(3, 2) Group_Params final {
 
       constexpr uint16_t wire_code() const { return static_cast<uint16_t>(m_code); }
 
+      /**
+      * Returns false if this group/KEX is not available in the build configuration
+      */
+      bool is_available() const;
+
       constexpr bool is_x25519() const { return m_code == Group_Params_Code::X25519; }
 
       constexpr bool is_x448() const { return m_code == Group_Params_Code::X448; }
@@ -212,27 +217,35 @@ class BOTAN_PUBLIC_API(3, 2) Group_Params final {
 
       constexpr bool is_post_quantum() const { return is_pure_kyber() || is_pure_frodokem() || is_pqc_hybrid(); }
 
-      constexpr bool is_pqc_hybrid() const {
+      constexpr bool is_pqc_hybrid_kyber() const {
          return m_code == Group_Params_Code::HYBRID_X25519_KYBER_512_R3_CLOUDFLARE ||
                 m_code == Group_Params_Code::HYBRID_X25519_KYBER_512_R3_OQS ||
                 m_code == Group_Params_Code::HYBRID_X25519_KYBER_768_R3_OQS ||
                 m_code == Group_Params_Code::HYBRID_X448_KYBER_768_R3_OQS ||
-                m_code == Group_Params_Code::HYBRID_X25519_eFRODOKEM_640_SHAKE_OQS ||
+                m_code == Group_Params_Code::HYBRID_SECP256R1_KYBER_512_R3_OQS ||
+                m_code == Group_Params_Code::HYBRID_SECP256R1_KYBER_768_R3_OQS ||
+                m_code == Group_Params_Code::HYBRID_SECP384R1_KYBER_768_R3_OQS ||
+                m_code == Group_Params_Code::HYBRID_SECP521R1_KYBER_1024_R3_OQS;
+      }
+
+      constexpr bool is_pqc_hybrid_frodokem() const {
+         return m_code == Group_Params_Code::HYBRID_X25519_eFRODOKEM_640_SHAKE_OQS ||
                 m_code == Group_Params_Code::HYBRID_X25519_eFRODOKEM_640_AES_OQS ||
                 m_code == Group_Params_Code::HYBRID_X448_eFRODOKEM_976_SHAKE_OQS ||
                 m_code == Group_Params_Code::HYBRID_X448_eFRODOKEM_976_AES_OQS ||
-                m_code == Group_Params_Code::HYBRID_SECP256R1_KYBER_512_R3_OQS ||
-                m_code == Group_Params_Code::HYBRID_SECP256R1_KYBER_768_R3_OQS ||
                 m_code == Group_Params_Code::HYBRID_SECP256R1_eFRODOKEM_640_SHAKE_OQS ||
                 m_code == Group_Params_Code::HYBRID_SECP256R1_eFRODOKEM_640_AES_OQS ||
-                m_code == Group_Params_Code::HYBRID_SECP384R1_KYBER_768_R3_OQS ||
                 m_code == Group_Params_Code::HYBRID_SECP384R1_eFRODOKEM_976_SHAKE_OQS ||
                 m_code == Group_Params_Code::HYBRID_SECP384R1_eFRODOKEM_976_AES_OQS ||
-                m_code == Group_Params_Code::HYBRID_SECP521R1_KYBER_1024_R3_OQS ||
                 m_code == Group_Params_Code::HYBRID_SECP521R1_eFRODOKEM_1344_SHAKE_OQS ||
                 m_code == Group_Params_Code::HYBRID_SECP521R1_eFRODOKEM_1344_AES_OQS;
       }
 
+      // If this is a pqc hybrid group, returns the ECC ID
+      std::optional<Group_Params_Code> pqc_hybrid_ecc() const;
+
+      constexpr bool is_pqc_hybrid() const { return is_pqc_hybrid_kyber() || is_pqc_hybrid_frodokem(); }
+
       constexpr bool is_kem() const { return is_pure_kyber() || is_pure_frodokem() || is_pqc_hybrid(); }
 
       // Returns std::nullopt if the param has no known name

diff --git a/src/lib/tls/tls_policy.cpp b/src/lib/tls/tls_policy.cpp
@@ -169,7 +169,9 @@ std::vector<Group_Params> Policy::key_exchange_groups() const {
 
       Group_Params::SECP256R1,
 
+#if defined(BOTAN_HAS_X25519) && defined(BOTAN_HAS_KYBER_ROUND3) && defined(BOTAN_HAS_TLS_13_PQC)
       Group_Params::HYBRID_X25519_KYBER_512_R3_CLOUDFLARE,
+#endif
 
 #if defined(BOTAN_HAS_X448)
       Group_Params::X448,

diff --git a/src/lib/tls/tls_text_policy.cpp b/src/lib/tls/tls_text_policy.cpp
@@ -233,14 +233,9 @@ std::vector<Group_Params> Text_Policy::read_group_list(std::string_view group_st
    for(const auto& group_name : split_on(group_str, ' ')) {
       Group_Params group_id = Group_Params::from_string(group_name).value_or(Group_Params::NONE);
 
-#if !defined(BOTAN_HAS_X25519)
-      if(group_id == Group_Params::X25519)
+      if(!group_id.is_available()) {
          continue;
-#endif
-#if !defined(BOTAN_HAS_X448)
-      if(group_id == Group_Params::X448)
-         continue;
-#endif
+      }
 
       if(group_id == Group_Params::NONE) {
          try {

diff --git a/src/tests/data/tls-policy/datagram.txt b/src/tests/data/tls-policy/datagram.txt
@@ -8,7 +8,6 @@ signature_hashes = SHA-512 SHA-384 SHA-256
 signature_methods = ECDSA RSA
 key_exchange_methods = ECDH DH
 key_exchange_groups = x25519 secp256r1 x25519/Kyber-512-r3/cloudflare x448 secp384r1 secp521r1 brainpool256r1 brainpool384r1 brainpool512r1 ffdhe/ietf/2048 ffdhe/ietf/3072
-key_exchange_groups_to_offer = x25519
 allow_insecure_renegotiation = false
 include_time_in_hello_random = true
 allow_server_initiated_renegotiation = false

diff --git a/src/tests/data/tls-policy/default_tls13.txt b/src/tests/data/tls-policy/default_tls13.txt
@@ -8,7 +8,6 @@ signature_hashes = SHA-512 SHA-384 SHA-256
 signature_methods = ECDSA RSA
 key_exchange_methods = ECDH DH
 key_exchange_groups = x25519 secp256r1 x25519/Kyber-512-r3/cloudflare x448 secp384r1 secp521r1 brainpool256r1 brainpool384r1 brainpool512r1 ffdhe/ietf/2048 ffdhe/ietf/3072
-key_exchange_groups_to_offer = x25519
 allow_insecure_renegotiation = false
 include_time_in_hello_random = true
 allow_server_initiated_renegotiation = false

diff --git a/src/tests/data/tls-policy/strict.txt b/src/tests/data/tls-policy/strict.txt
@@ -8,7 +8,6 @@ signature_hashes = SHA-512 SHA-384
 signature_methods = ECDSA RSA
 key_exchange_methods = ECDH
 key_exchange_groups = x25519 secp256r1 x25519/Kyber-512-r3/cloudflare x448 secp384r1 secp521r1 brainpool256r1 brainpool384r1 brainpool512r1 ffdhe/ietf/2048 ffdhe/ietf/3072
-key_exchange_groups_to_offer = x25519
 allow_insecure_renegotiation = false
 include_time_in_hello_random = true
 allow_server_initiated_renegotiation = false

diff --git a/src/tests/data/tls-policy/strict_tls13.txt b/src/tests/data/tls-policy/strict_tls13.txt
@@ -8,7 +8,6 @@ signature_hashes = SHA-512 SHA-384
 signature_methods = ECDSA RSA
 key_exchange_methods = ECDH
 key_exchange_groups = x25519 secp256r1 x25519/Kyber-512-r3/cloudflare x448 secp384r1 secp521r1 brainpool256r1 brainpool384r1 brainpool512r1 ffdhe/ietf/2048 ffdhe/ietf/3072
-key_exchange_groups_to_offer = x25519
 allow_insecure_renegotiation = false
 include_time_in_hello_random = true
 allow_server_initiated_renegotiation = false