Allow specification of ECC named curve used in ECDH key exchange

[BenBE]

In recent versions ejabberd allows for the DH parameters to be set in its configuration. But a similar setting for the ECDH parameters is missing. In a quick shot, of which the results are included below, I tried to add this feature in p1_tls, so it can be used in ejabberd.

The intention is to allow the used named curve for ECC to be specified in the ejabberd config by providing its name in a setting like "ecdh_curvename" or "s2s_ecdh_curvename" (for s2s).

diff --git a/c_src/p1_tls_drv.c b/c_src/p1_tls_drv.c
index 30c5e62..010e830 100644
--- a/c_src/p1_tls_drv.c
+++ b/c_src/p1_tls_drv.c
@@ -55,6 +55,7 @@ typedef unsigned __int32 uint32_t;
 #endif

 #define CIPHERS "DEFAULT:!EXPORT:!LOW:!RC4:!SSLv2"
+#define NID_ECDH_DEFAULTCURVE NID_X9_62_prime256v1

 /**
  * Prepare the SSL options flag.
@@ -326,15 +327,23 @@ static int verify_callback(int preverify_ok, X509_STORE_CTX *ctx)
  * for details.
  */
 #ifndef OPENSSL_NO_ECDH
-static void setup_ecdh(SSL_CTX *ctx)
+static void setup_ecdh(SSL_CTX *ctx, char* ecdh_curvename)
 {
+   int nidCurve;
    EC_KEY *ecdh;

    if (SSLeay() < 0x1000005fL) {
       return;
    }

-   ecdh = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1);
+   if (ecdh_curvename && (( nidCurve = OBJ_sn2nid(ecdh_curvename) ))) {
+      ecdh = EC_KEY_new_by_curve_name(nidCurve);
+   }
+
+   if (ecdh == NULL) {
+      ecdh = EC_KEY_new_by_curve_name(NID_ECDH_DEFAULTCURVE);
+   }
+
    SSL_CTX_set_options(ctx, SSL_OP_SINGLE_ECDH_USE);
    SSL_CTX_set_tmp_ecdh(ctx, ecdh);

@@ -537,10 +546,13 @@ static ErlDrvSSizeT tls_drv_control(ErlDrvData handle,
     size_t protocol_options_len = strlen(protocol_options);
     char *dh_file = protocol_options + protocol_options_len + 1;
     size_t dh_file_len = strlen(dh_file);
+    char *ecdh_curvename = dh_file + dh_file_len + 1;
+    size_t ecdh_curvename_len = strlen(ecdh_curvename);
     char *hash_key = (char *)driver_alloc(key_file_len +
                           ciphers_len +
                           protocol_options_len +
-                          dh_file_len + 1);
+                          dh_file_len +
+                          ecdh_curvename + 1);
     long options = 0L;

     if (protocol_options_len != 0) {
@@ -556,13 +568,16 @@ static ErlDrvSSizeT tls_drv_control(ErlDrvData handle,
        free(popts);
     }

-    sprintf(hash_key, "%s%s%s%s", key_file, ciphers, protocol_options,
-        dh_file);
+    sprintf(hash_key, "%s%s%s%s%s", key_file, ciphers, protocol_options,
+        dh_file, ecdh_curvename);
     SSL_CTX *ssl_ctx = hash_table_lookup(hash_key, &key_mtime, &dh_mtime);

     if (dh_file_len == 0)
        dh_file = NULL;

+    if (ecdh_curvename_len == 0)
+       ecdh_curvename = NULL;
+
     if (is_modified(key_file, &key_mtime) ||
         is_modified(dh_file, &dh_mtime) ||
         ssl_ctx == NULL)
@@ -588,7 +603,7 @@ static ErlDrvSSizeT tls_drv_control(ErlDrvData handle,
        SSL_CTX_set_cipher_list(ctx, ciphers);

 #ifndef OPENSSL_NO_ECDH
-       setup_ecdh(ctx);
+       setup_ecdh(ctx, ecdh_curvename);
 #endif
 #ifndef OPENSSL_NO_DH
        res = setup_dh(ctx, dh_file);
diff --git a/src/p1_tls.erl b/src/p1_tls.erl
index 74158cc..73eb1aa 100644
--- a/src/p1_tls.erl
+++ b/src/p1_tls.erl
@@ -145,11 +145,19 @@ tcp_to_tls(TCPSocket, Options) ->
                        false ->
                            <<>>
                    end,
+          ECCurveName = case lists:keysearch(eccurvename, 1, Options) of
+                       {value, {eccurvename, E}} ->
+                           iolist_to_binary(E);
+                       false ->
+                           <<>>
+                   end,
           CertFile1 = iolist_to_binary(CertFile),
      case catch port_control(Port, Command bor Flags,
-                 <<CertFile1/binary, 0, Ciphers/binary,
-                   0, ProtocolOpts/binary, 0, DHFile/binary,
-                   0>>)
+                 <<CertFile1/binary, 0,
+                   Ciphers/binary, 0,
+                   ProtocolOpts/binary, 0,
+                   DHFile/binary, 0,
+                   ECCurveName/binary, 0>>)
          of
        {'EXIT', {badarg, _}} -> {error, einval};
        <<0>> ->

I know this is by far not perfect, but should provide a good starting point for further developments.

Thanks to emias (IRC) for reviewing of and commenting on this initial PoC.

[stef]

this would be nice to be able to configure on a more granular level, so that i can specify multiple curves at least. different servers unfortunately use different curves, like prime256v1 or secp521r1. having different curves prohibits s2s interoperability.

[fancycode]

With OpenSSL 1.1.0 you could even configure a list of curves to support: https://www.openssl.org/docs/man1.1.0/ssl/SSL_CTX_set1_curves_list.html