tls: support for ECDSA P-384 and P-521 certificates (envoyproxy#36369)

Commercial National Security Algorithm Suite
(CNSA) requires ECDSA keys be specified with P-384 curves. The assertion
that there are [no security benefits to curves higher than
P-256](envoyproxy#5224 (comment)) is
no longer true. This change is intended to limit the adoptable curves to
P-384 and P-521.

Risk Level: Medium - removal of limitation of curves to be used for
ECDSA certificates, with [potential misconfiguration and DoS
risks](envoyproxy#10855 (comment))
mentioned in previous discourse on the issue. This risk is mitigated in
this PR, however, by continuing to expressly limit the type of EC keys
accepted to those associated with the P-256, P-384 or P-521 curves and
no others.

Fixes envoyproxy#10855

Signed-off-by: anitabyte <anita@anitabyte.xyz>

Signed-off-by: anitabyte <111812252+anitabyte@users.noreply.github.com>

changelogs/current.yaml

@@ -13,6 +13,9 @@ removed_config_or_runtime:
 # *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`
 
 new_features:
+- area: tls
+  change: |
+    Added support for P-384 and P-521 curves for TLS server certificates.
 - area: stats ads
   change: |
     Fixed metric for ADS disconnection counters, extracting the tag from `grpc.ads_cluster.*` counter stats.

docs/root/intro/arch_overview/security/ssl.rst

@@ -114,15 +114,15 @@ Certificate selection
 ---------------------
 
 :ref:`DownstreamTlsContexts <envoy_v3_api_msg_extensions.transport_sockets.tls.v3.DownstreamTlsContext>` support multiple TLS
-certificates. These may be a mix of RSA and P-256 ECDSA certificates for multiple server name patterns.
+certificates. These may be a mix of RSA and ECDSA certificates for multiple server name patterns.
 
 Certificate config/loading rules:
 
 * DNS SANs or Subject Common Name is extracted as server name pattern to match SNI during handshake. Subject Common Name is not used if DNS SANs are present in the certificate.
 * FQDN like "test.example.com" and wildcard like "\*.example.com" are valid at the same time, which will be loaded
   as two different server name patterns.
 * If multiple certificates of a particular type (RSA or ECDSA) are specified for the same name or name pattern, the first one loaded is used for that name.
-* Non-P-256 server ECDSA certificates are rejected.
+* Non-P-256, P-384 or P-521 server ECDSA certificates are rejected.
 * Static and SDS certificates may not be mixed in a given :ref:`DownstreamTlsContext
   <envoy_v3_api_msg_extensions.transport_sockets.tls.v3.DownstreamTlsContext>`.
 
@@ -144,7 +144,7 @@ Certificate selection rules:
   is false or true.
 * Full scan execuates OCSP and key type checking on each cert which is the same as described above in exact SNI matching.
   It falls back to the first cert in the whole list if there is no cert selected.
-* Currently only two kinds of key type are supported, RSA or ECDSA. If the client supports P-256 ECDSA, the P-256 ECDSA certificate
+* Currently only two kinds of key type are supported, RSA or ECDSA. If the client supports P-256, P-384 or P-521 ECDSA, the P-256, P-384 or P-521 ECDSA certificate
   is preferred over RSA. The certificate that it falls back to might result in a failed handshake. For instance, a client only supports
   RSA certificates and the certificate only support ECDSA.
 * The final selected certificate must adhere to the OCSP policy. If no such certificate is found, the connection is refused.

envoy/ssl/handshaker.h

@@ -9,6 +9,7 @@
 #include "envoy/server/options.h"
 #include "envoy/singleton/manager.h"
 
+#include "absl/container/inlined_vector.h"
 #include "openssl/ssl.h"
 
 namespace Envoy {
@@ -26,6 +27,11 @@ struct TlsContext;
 
 class ServerContextConfig;
 
+using CurveNID = int;
+// Currently this type only ever holds 3 values: P-256, P-384, and P-521, so optimize by using
+// `InlinedVector`.
+using CurveNIDVector = absl::InlinedVector<int, 3>;
+
 class HandshakeCallbacks {
 public:
   virtual ~HandshakeCallbacks() = default;
@@ -231,8 +237,8 @@ class TlsCertificateSelector {
    * @return context will have the same lifetime as ``ServerContextImpl``.
    */
   virtual std::pair<const Ssl::TlsContext&, OcspStapleAction>
-  findTlsContext(absl::string_view sni, bool client_ecdsa_capable, bool client_ocsp_capable,
-                 bool* cert_matched_sni) PURE;
+  findTlsContext(absl::string_view sni, const CurveNIDVector& client_ecdsa_capabilities,
+                 bool client_ocsp_capable, bool* cert_matched_sni) PURE;
 };
 
 using TlsCertificateSelectorPtr = std::unique_ptr<TlsCertificateSelector>;

source/common/quic/quic_server_transport_socket_factory.cc

@@ -171,8 +171,9 @@ QuicServerTransportSocketFactory::getTlsCertificateAndKey(absl::string_view sni,
   }
   auto ctx =
       std::dynamic_pointer_cast<Extensions::TransportSockets::Tls::ServerContextImpl>(ssl_ctx);
-  auto [tls_context, ocsp_staple_action] = ctx->findTlsContext(
-      sni, true /* TODO: ecdsa_capable */, false /* TODO: ocsp_capable */, cert_matched_sni);
+  auto [tls_context, ocsp_staple_action] =
+      ctx->findTlsContext(sni, Ssl::CurveNIDVector{NID_X9_62_prime256v1} /* TODO: ecdsa_capable */,
+                          false /* TODO: ocsp_capable */, cert_matched_sni);
 
   // Thread safety note: accessing the tls_context requires holding a shared_ptr to the ``ssl_ctx``.
   // Both of these members are themselves reference counted, so it is safe to use them after

source/common/quic/quic_server_transport_socket_factory.h

@@ -4,6 +4,7 @@
 #include "envoy/network/transport_socket.h"
 #include "envoy/server/transport_socket_config.h"
 #include "envoy/ssl/context_config.h"
+#include "envoy/ssl/handshaker.h"
 
 #include "source/common/common/assert.h"
 #include "source/common/network/transport_socket_options_impl.h"

source/common/tls/context_impl.cc

@@ -218,23 +218,24 @@ ContextImpl::ContextImpl(Stats::Scope& scope, const Envoy::Ssl::ContextConfig& c
 
       bssl::UniquePtr<EVP_PKEY> public_key(X509_get_pubkey(ctx.cert_chain_.get()));
       const int pkey_id = EVP_PKEY_id(public_key.get());
-      ctx.is_ecdsa_ = pkey_id == EVP_PKEY_EC;
       switch (pkey_id) {
       case EVP_PKEY_EC: {
-        // We only support P-256 ECDSA today.
+        // We only support P-256, P-384 or P-521 ECDSA today.
         const EC_KEY* ecdsa_public_key = EVP_PKEY_get0_EC_KEY(public_key.get());
         // Since we checked the key type above, this should be valid.
         ASSERT(ecdsa_public_key != nullptr);
         const EC_GROUP* ecdsa_group = EC_KEY_get0_group(ecdsa_public_key);
+        const int ec_group_curve_name = EC_GROUP_get_curve_name(ecdsa_group);
         if (ecdsa_group == nullptr ||
-            EC_GROUP_get_curve_name(ecdsa_group) != NID_X9_62_prime256v1) {
+            (ec_group_curve_name != NID_X9_62_prime256v1 && ec_group_curve_name != NID_secp384r1 &&
+             ec_group_curve_name != NID_secp521r1)) {
           creation_status = absl::InvalidArgumentError(
-              fmt::format("Failed to load certificate chain from {}, only P-256 "
-                          "ECDSA certificates are supported",
+              fmt::format("Failed to load certificate chain from {}, only P-256, "
+                          "P-384 or P-521 ECDSA certificates are supported",
                           ctx.cert_chain_file_path_));
           return;
         }
-        ctx.is_ecdsa_ = true;
+        ctx.ec_group_curve_name_ = ec_group_curve_name;
       } break;
       case EVP_PKEY_RSA: {
         // We require RSA certificates with 2048-bit or larger keys.

source/common/tls/context_impl.h

@@ -38,6 +38,8 @@ namespace Envoy {
 
 namespace Ssl {
 
+const int EC_CURVE_INVALID_NID = -1;
+
 struct TlsContext {
   // Each certificate specified for the context has its own SSL_CTX. `SSL_CTXs`
   // are identical with the exception of certificate material, and can be
@@ -47,7 +49,9 @@ struct TlsContext {
   bssl::UniquePtr<X509> cert_chain_;
   std::string cert_chain_file_path_;
   std::unique_ptr<OcspResponseWrapper> ocsp_response_;
-  bool is_ecdsa_{};
+  // We initialize the curve name variable to EC_CURVE_INVALID_NID which is used as a sentinel value
+  // for "not an ECDSA context".
+  CurveNID ec_group_curve_name_ = EC_CURVE_INVALID_NID;
   bool is_must_staple_{};
   Ssl::PrivateKeyMethodProviderSharedPtr private_key_method_provider_{};
 

source/common/tls/default_tls_certificate_selector.cc

@@ -89,11 +89,12 @@ DefaultTlsCertificateSelector::selectTlsContext(const SSL_CLIENT_HELLO& ssl_clie
                                                 Ssl::CertificateSelectionCallbackPtr) {
   absl::string_view sni =
       absl::NullSafeStringView(SSL_get_servername(ssl_client_hello.ssl, TLSEXT_NAMETYPE_host_name));
-  const bool client_ecdsa_capable = server_ctx_.isClientEcdsaCapable(ssl_client_hello);
+  const Ssl::CurveNIDVector client_ecdsa_capabilities =
+      server_ctx_.getClientEcdsaCapabilities(ssl_client_hello);
   const bool client_ocsp_capable = server_ctx_.isClientOcspCapable(ssl_client_hello);
 
   auto [selected_ctx, ocsp_staple_action] =
-      findTlsContext(sni, client_ecdsa_capable, client_ocsp_capable, nullptr);
+      findTlsContext(sni, client_ecdsa_capabilities, client_ocsp_capable, nullptr);
 
   auto stats = server_ctx_.stats();
   if (client_ocsp_capable) {
@@ -162,7 +163,8 @@ Ssl::OcspStapleAction DefaultTlsCertificateSelector::ocspStapleAction(const Ssl:
 }
 
 std::pair<const Ssl::TlsContext&, Ssl::OcspStapleAction>
-DefaultTlsCertificateSelector::findTlsContext(absl::string_view sni, bool client_ecdsa_capable,
+DefaultTlsCertificateSelector::findTlsContext(absl::string_view sni,
+                                              const Ssl::CurveNIDVector& client_ecdsa_capabilities,
                                               bool client_ocsp_capable, bool* cert_matched_sni) {
   bool unused = false;
   if (cert_matched_sni == nullptr) {
@@ -176,20 +178,36 @@ DefaultTlsCertificateSelector::findTlsContext(absl::string_view sni, bool client
   const Ssl::TlsContext* candidate_ctx = nullptr;
   Ssl::OcspStapleAction ocsp_staple_action;
 
+  // If the capabilities list vector is not empty, then the client is ECDSA-capable.
+  const bool client_ecdsa_capable = !client_ecdsa_capabilities.empty();
+
   auto selected = [&](const Ssl::TlsContext& ctx) -> bool {
     auto action = ocspStapleAction(ctx, client_ocsp_capable);
     if (action == Ssl::OcspStapleAction::Fail) {
       // The selected ctx must adhere to OCSP policy
       return false;
     }
+    // If the client is ECDSA-capable and the context is ECDSA, we check if it is capable of
+    // handling the curves in the cert in a given TlsContext. If the client is not ECDSA-capable,
+    // we will not std::find anything here and move on. If the context is RSA, we will not find
+    // the value of `EC_CURVE_INVALID_NID` in an vector of ECDSA capabilities.
+    // If we have a matching curve NID in our client capabilities, return `true`.
+    if (std::find(client_ecdsa_capabilities.begin(), client_ecdsa_capabilities.end(),
+                  ctx.ec_group_curve_name_) != client_ecdsa_capabilities.end()) {
+      selected_ctx = &ctx;
+      ocsp_staple_action = action;
+      return true;
+    }
 
-    if (client_ecdsa_capable == ctx.is_ecdsa_) {
+    // If the client is not ECDSA-capable and the `ctx` is non-ECDSA, then select this `ctx`.
+    if (!client_ecdsa_capable && ctx.ec_group_curve_name_ == Ssl::EC_CURVE_INVALID_NID) {
       selected_ctx = &ctx;
       ocsp_staple_action = action;
       return true;
     }
 
-    if (client_ecdsa_capable && !ctx.is_ecdsa_ && candidate_ctx == nullptr) {
+    if (client_ecdsa_capable && ctx.ec_group_curve_name_ == Ssl::EC_CURVE_INVALID_NID &&
+        candidate_ctx == nullptr) {
       // ECDSA cert is preferred if client is ECDSA capable, so RSA cert is marked as a candidate,
       // searching will continue until exhausting all certs or find a exact match.
       candidate_ctx = &ctx;

source/common/tls/default_tls_certificate_selector.h

@@ -31,8 +31,8 @@ class DefaultTlsCertificateSelector : public Ssl::TlsCertificateSelector,
   // Finds the best matching context. The returned context will have the same lifetime as
   // ``ServerContextImpl``.
   std::pair<const Ssl::TlsContext&, Ssl::OcspStapleAction>
-  findTlsContext(absl::string_view sni, bool client_ecdsa_capable, bool client_ocsp_capable,
-                 bool* cert_matched_sni) override;
+  findTlsContext(absl::string_view sni, const Ssl::CurveNIDVector& client_ecdsa_capabilities,
+                 bool client_ocsp_capable, bool* cert_matched_sni) override;
 
 private:
   // Currently, at most one certificate of a given key type may be specified for each exact

source/common/tls/server_context_impl.cc

@@ -40,28 +40,34 @@
 #include "openssl/rand.h"
 
 namespace Envoy {
-namespace {
 
-bool cbsContainsU16(CBS& cbs, uint16_t n) {
+namespace Extensions {
+namespace TransportSockets {
+namespace Tls {
+
+Ssl::CurveNIDVector getClientCurveNIDSupported(CBS& cbs) {
+  Ssl::CurveNIDVector cnsv{};
   while (CBS_len(&cbs) > 0) {
     uint16_t v;
     if (!CBS_get_u16(&cbs, &v)) {
-      return false;
+      break;
     }
-    if (v == n) {
-      return true;
+    // Check for values that refer to the `sigalgs` used in TLSv1.3 negotiation (left)
+    // or their equivalent curve expressed in TLSv1.2 `TLSEXT_TYPE_supported_groups`
+    // present in ClientHello (right).
+    if (v == SSL_SIGN_ECDSA_SECP256R1_SHA256 || v == SSL_CURVE_SECP256R1) {
+      cnsv.push_back(NID_X9_62_prime256v1);
+    }
+    if (v == SSL_SIGN_ECDSA_SECP384R1_SHA384 || v == SSL_CURVE_SECP384R1) {
+      cnsv.push_back(NID_secp384r1);
+    }
+    if (v == SSL_SIGN_ECDSA_SECP521R1_SHA512 || v == SSL_CURVE_SECP521R1) {
+      cnsv.push_back(NID_secp521r1);
     }
   }
-
-  return false;
+  return cnsv;
 }
 
-} // namespace
-
-namespace Extensions {
-namespace TransportSockets {
-namespace Tls {
-
 int ServerContextImpl::alpnSelectCallback(const unsigned char** out, unsigned char* outlen,
                                           const unsigned char* in, unsigned int inlen) {
   // Currently this uses the standard selection algorithm in priority order.
@@ -376,7 +382,10 @@ int ServerContextImpl::sessionTicketProcess(SSL*, uint8_t* key_name, uint8_t* iv
   }
 }
 
-bool ServerContextImpl::isClientEcdsaCapable(const SSL_CLIENT_HELLO& ssl_client_hello) const {
+// Returns a list of client capabilities for ECDSA curves as NIDs. An empty vector indicates
+// a client that is unable to handle ECDSA.
+Ssl::CurveNIDVector
+ServerContextImpl::getClientEcdsaCapabilities(const SSL_CLIENT_HELLO& ssl_client_hello) const {
   CBS client_hello;
   CBS_init(&client_hello, ssl_client_hello.client_hello, ssl_client_hello.client_hello_len);
 
@@ -399,14 +408,12 @@ bool ServerContextImpl::isClientEcdsaCapable(const SSL_CLIENT_HELLO& ssl_client_
         CBS_init(&signature_algorithms_ext, signature_algorithms_data, signature_algorithms_len);
         if (!CBS_get_u16_length_prefixed(&signature_algorithms_ext, &signature_algorithms) ||
             CBS_len(&signature_algorithms_ext) != 0) {
-          return false;
-        }
-        if (cbsContainsU16(signature_algorithms, SSL_SIGN_ECDSA_SECP256R1_SHA256)) {
-          return true;
+          return Ssl::CurveNIDVector{};
         }
+        return getClientCurveNIDSupported(signature_algorithms);
       }
 
-      return false;
+      return Ssl::CurveNIDVector{};
     }
   }
 
@@ -416,15 +423,16 @@ bool ServerContextImpl::isClientEcdsaCapable(const SSL_CLIENT_HELLO& ssl_client_
   size_t curvelist_len;
   if (!SSL_early_callback_ctx_extension_get(&ssl_client_hello, TLSEXT_TYPE_supported_groups,
                                             &curvelist_data, &curvelist_len)) {
-    return false;
+    return Ssl::CurveNIDVector{};
   }
 
   CBS curvelist;
   CBS_init(&curvelist, curvelist_data, curvelist_len);
 
-  // We only support P256 ECDSA curves today.
-  if (!cbsContainsU16(curvelist, SSL_CURVE_SECP256R1)) {
-    return false;
+  Ssl::CurveNIDVector client_capabilities = getClientCurveNIDSupported(curvelist);
+  // if we haven't got any curves in common with the client, return empty CurveNIDVector.
+  if (client_capabilities.empty()) {
+    return Ssl::CurveNIDVector{};
   }
 
   // The client must have offered an ECDSA ciphersuite that we like.
@@ -434,16 +442,16 @@ bool ServerContextImpl::isClientEcdsaCapable(const SSL_CLIENT_HELLO& ssl_client_
   while (CBS_len(&cipher_suites) > 0) {
     uint16_t cipher_id;
     if (!CBS_get_u16(&cipher_suites, &cipher_id)) {
-      return false;
+      return Ssl::CurveNIDVector{};
     }
     // All tls_context_ share the same set of enabled ciphers, so we can just look at the base
     // context.
     if (tls_contexts_[0].isCipherEnabled(cipher_id, client_version)) {
-      return true;
+      return client_capabilities;
     }
   }
 
-  return false;
+  return Ssl::CurveNIDVector{};
 }
 
 bool ServerContextImpl::isClientOcspCapable(const SSL_CLIENT_HELLO& ssl_client_hello) const {
@@ -458,10 +466,11 @@ bool ServerContextImpl::isClientOcspCapable(const SSL_CLIENT_HELLO& ssl_client_h
 }
 
 std::pair<const Ssl::TlsContext&, Ssl::OcspStapleAction>
-ServerContextImpl::findTlsContext(absl::string_view sni, bool client_ecdsa_capable,
+ServerContextImpl::findTlsContext(absl::string_view sni,
+                                  const Ssl::CurveNIDVector& client_ecdsa_capabilities,
                                   bool client_ocsp_capable, bool* cert_matched_sni) {
-  return tls_certificate_selector_->findTlsContext(sni, client_ecdsa_capable, client_ocsp_capable,
-                                                   cert_matched_sni);
+  return tls_certificate_selector_->findTlsContext(sni, client_ecdsa_capabilities,
+                                                   client_ocsp_capable, cert_matched_sni);
 }
 
 enum ssl_select_cert_result_t

source/common/tls/server_context_impl.h

@@ -12,6 +12,7 @@
 #include "envoy/network/transport_socket.h"
 #include "envoy/ssl/context.h"
 #include "envoy/ssl/context_config.h"
+#include "envoy/ssl/handshaker.h"
 #include "envoy/ssl/private_key/private_key.h"
 #include "envoy/ssl/ssl_socket_extended_info.h"
 #include "envoy/stats/scope.h"
@@ -35,10 +36,13 @@
 #endif
 
 namespace Envoy {
+
 namespace Extensions {
 namespace TransportSockets {
 namespace Tls {
 
+Ssl::CurveNIDVector getClientCurveNIDSupported(CBS& cbs);
+
 class ServerContextImpl : public ContextImpl,
                           public Envoy::Ssl::ServerContext,
                           public Envoy::Ssl::TlsCertificateSelectorContext {
@@ -60,11 +64,11 @@ class ServerContextImpl : public ContextImpl,
 
   // Finds the best matching context. The returned context will have the same lifetime as
   // this ``ServerContextImpl``.
-  std::pair<const Ssl::TlsContext&, Ssl::OcspStapleAction> findTlsContext(absl::string_view sni,
-                                                                          bool client_ecdsa_capable,
-                                                                          bool client_ocsp_capable,
-                                                                          bool* cert_matched_sni);
-  bool isClientEcdsaCapable(const SSL_CLIENT_HELLO& ssl_client_hello) const;
+  std::pair<const Ssl::TlsContext&, Ssl::OcspStapleAction>
+  findTlsContext(absl::string_view sni, const Ssl::CurveNIDVector& client_ecdsa_capable,
+                 bool client_ocsp_capable, bool* cert_matched_sni);
+
+  Ssl::CurveNIDVector getClientEcdsaCapabilities(const SSL_CLIENT_HELLO& ssl_client_hello) const;
   bool isClientOcspCapable(const SSL_CLIENT_HELLO& ssl_client_hello) const;
 
 private:

test/common/quic/envoy_quic_proof_source_test.cc

@@ -195,6 +195,7 @@ class EnvoyQuicProofSourceTest : public ::testing::Test {
     auto factory = Extensions::TransportSockets::Tls::TlsCertificateSelectorConfigFactoryImpl::
         getDefaultTlsCertificateSelectorConfigFactory();
     ASSERT_TRUE(factory);
+    ASSERT_EQ("envoy.tls.certificate_selectors.default", factory->name());
     const ProtobufWkt::Any any;
     absl::Status creation_status = absl::OkStatus();
     auto tls_certificate_selector_factory_cb = factory->createTlsCertificateSelectorFactory(