Ticket #1228: 2.4-tls-group.patch

README.ec

@@ -12 +12 @@
 used for authentication, the curve used for the server certificate will be used
 for ECDH too. When autodetection fails (e.g. when using RSA certificates)
 OpenVPN lets the crypto library decide if possible, or falls back to the
-secp384r1 curve. The list of groups/curves that the crypto library will choose
-from can be set with the --tls-groups <grouplist> configuration.
+secp384r1 curve.
 
 An administrator can force an OpenVPN/OpenSSL server to use a specific curve
 using the --ecdh-curve <curvename> option with one of the curves listed as
-available by the --show-groups option. Clients will use the same curve as
+available by the --show-curves option. Clients will use the same curve as
 selected by the server.
 
-Note that not all curves listed by --show-groups are available for use with TLS;
+Note that not all curves listed by --show-curves are available for use with TLS;
 in that case connecting will fail with a 'no shared cipher' TLS error.
 
 Authentication (ECDSA)

doc/openvpn.8

@@ -4948 +4948 @@
 documentation for details on the cipher list interpretation.
 
 For OpenSSL, the
-.B \-\-tls\-cipher
+.B \-\-tls-cipher
 is used for TLS 1.2 and below. For TLS 1.3 and up, the
 .B \-\-tls\-ciphersuites
 setting is used. mbed TLS has no TLS 1.3 support yet and only the
-.B \-\-tls\-cipher
+.B \-\-tls-cipher
 setting is used.
 
 Use
@@ -4960 +4960 @@
 to see a list of TLS ciphers supported by your crypto library.
 
 Warning!
-.B \-\-tls\-groups
-,
 .B \-\-tls\-cipher
 and
 .B \-\-tls\-ciphersuites
@@ -4977 +4975 @@
 The default for \-\-tls\-ciphersuites is to use the crypto library's default.
 .\"*********************************************************
 .TP
-.B \-\-tls\-groups l
-A list
-.B l
-of allowable groups/curves in order of preference.
-
-Set the allowed elictipic curves/groups for the TLS session.
-These groups are  allowed to be used in signatures and key exchange.
-
-mbed TLS currently allows all known curves per default.
-
-OpenSSL 1.1+ restricts the list per default to
-"X25519:secp256r1:X448:secp521r1:secp384r1".
-
-If you use certificates that use non-standard curves, you
-might need to add them here. If you do not force the ecdh curve
-by using
-.B \-\-ecdh\-curve
-, the groups for ecdh will also be picked from this list.
-
-OpenVPN maps the curve name secp256r1 to prime256v1 to allow
-specifying the tls-groups option for mbed TLS and OpenSSL.
-
-Warning: this option not only affects eliptic curve certificates
-but also the key exchange in TLS 1.3 and using this option improperly
-will disable TLS 1.3.
-.\"*********************************************************
-.TP
 .B \-\-tls\-cert\-profile profile
 Set the allowed cryptographic algorithms for certificates according to
 .B profile\fN.
@@ -5698 +5669 @@
 .TP
 .B \-\-show\-curves
 (Standalone)
-Show all available elliptic groups/curves to use with the
+Show all available elliptic curves to use with the
 .B \-\-ecdh\-curve
-and
-.B \-\-tls\-groups
 option.
 .\"*********************************************************
 .SS Generate a random key:

src/openvpn/misc.c

@@ -1634 +1634 @@
     }
 }
 
-int get_num_elements(const char* string, char delimiter)
-{
-  int string_len = strlen(string);
-
-  ASSERT(0 != string_len);
-
-  int element_count = 1;
-  /* Get number of ciphers */
-  for (int i = 0; i < string_len; i++)
-    {
-      if (string[i] == delimiter)
-        {
-          element_count++;
-        }
-    }
-
-  return element_count;
-}
 #endif /* P2MP_SERVER */

src/openvpn/misc.h

@@ -314 +314 @@
 
 #endif /* P2MP_SERVER */
 
-/**
- * Counts the number of delimiter in a string and returns
- * their number +1. This is typically used to find out the
- * number elements in a cipher string or similar that is separated by : like
- *
- *   X25519:secp256r1:X448:secp512r1:secp384r1:brainpoolP384r1
- *
- * @param string        the string to work on
- * @param delimiter     the delimiter to count, typically ':'
- * @return              number of delimiter found + 1
- */
-int
-get_num_elements(const char* string, char delimiter);
 #endif /* ifndef MISC_H */

src/openvpn/options.c

@@ -7644 +7644 @@
         VERIFY_PERMISSION(OPT_P_GENERAL);
         options->show_tls_ciphers = true;
     }
-    else if ((streq(p[0], "show-curves") || streq(p[0], "show-groups")) && !p[1])
+    else if (streq(p[0], "show-curves") && !p[1])
     {
         VERIFY_PERMISSION(OPT_P_GENERAL);
         options->show_curves = true;
@@ -7652 +7652 @@
     else if (streq(p[0], "ecdh-curve") && p[1] && !p[2])
     {
         VERIFY_PERMISSION(OPT_P_GENERAL);
-        msg(M_WARN, "Consider setting groups/curves in preference with "
-            "tls-groups instead of forcing a specific curve with "
-            "ecdh-curve.");
         options->ecdh_curve = p[1];
     }
     else if (streq(p[0], "tls-server") && !p[1])
@@ -7845 +7842 @@
         VERIFY_PERMISSION(OPT_P_GENERAL);
         options->cipher_list_tls13 = p[1];
     }
-    else if (streq(p[0], "tls-groups") && p[1] && !p[2])
-    {
-        VERIFY_PERMISSION(OPT_P_GENERAL);
-        options->tls_groups = p[1];
-    }
     else if (streq(p[0], "crl-verify") && p[1] && ((p[2] && streq(p[2], "dir"))
                                                    || (p[2] && streq(p[1], INLINE_FILE_TAG) ) || !p[2]) && !p[3])
     {

src/openvpn/options.h

@@ -504 +504 @@
     const char *pkcs12_file;
     const char *cipher_list;
     const char *cipher_list_tls13;
-    const char *tls_groups;
     const char *tls_cert_profile;
     const char *ecdh_curve;
     const char *tls_verify;

src/openvpn/ssl.c

@@ -633 +633 @@
     tls_ctx_restrict_ciphers(new_ctx, options->cipher_list);
     tls_ctx_restrict_ciphers_tls13(new_ctx, options->cipher_list_tls13);
 
-    /* Set the allow groups/curves for TLS if we want to override them */
-    if (options->tls_groups)
-    {
-        tls_ctx_set_tls_groups(new_ctx, options->tls_groups);
-    }
-
     if (!tls_ctx_set_options(new_ctx, options->ssl_flags))
     {
         goto err;

src/openvpn/ssl_backend.h

@@ -200 +200 @@
  */
 void tls_ctx_set_cert_profile(struct tls_root_ctx *ctx, const char *profile);
 
-/**
- * Set the allowed (eliptic curve) group allowed for signatures and
- * key exchange.
- *
- * @param ctx       TLS context to restrict, must be valid.
- * @param groups    List of groups that will be allowed, in priority,
- *                  separated by :
- */
-void tls_ctx_set_tls_groups(struct tls_root_ctx *ctx, const char *groups);
-
 /**
  * Check our certificate notBefore and notAfter fields, and warn if the cert is
  * either not yet valid or has expired.  Note that this is a non-fatal error,

src/openvpn/ssl_mbedtls.c

@@ -185 +185 @@
             free(ctx->allowed_ciphers);
         }
 
-        if (ctx->groups)
-        {
-            free(ctx->groups);
-        }
-
         CLEAR(*ctx);
 
         ctx->initialised = false;
@@ -253 +248 @@
 tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 {
     char *tmp_ciphers, *tmp_ciphers_orig, *token;
+    int i, cipher_count;
+    int ciphers_len;
 
     if (NULL == ciphers)
     {
         return; /* Nothing to do */
+
     }
+    ciphers_len = strlen(ciphers);
 
     ASSERT(NULL != ctx);
+    ASSERT(0 != ciphers_len);
 
     /* Get number of ciphers */
-    int cipher_count = get_num_elements(ciphers, ':');
+    for (i = 0, cipher_count = 1; i < ciphers_len; i++)
+    {
+        if (ciphers[i] == ':')
+        {
+            cipher_count++;
+        }
+    }
 
     /* Allocate an array for them */
     ALLOC_ARRAY_CLEAR(ctx->allowed_ciphers, int, cipher_count+1)
 
     /* Parse allowed ciphers, getting IDs */
-    int i = 0;
+    i = 0;
     tmp_ciphers_orig = tmp_ciphers = string_alloc(ciphers, NULL);
 
     token = strtok(tmp_ciphers, ":");
@@ -306 +312 @@
     }
 }
 
-void
-tls_ctx_set_tls_groups(struct tls_root_ctx *ctx, const char *groups)
-{
-    ASSERT(NULL != ctx);
-
-    /* Get number of groups */
-    int groups_count = get_num_elements(groups, ':');
-
-    /* Allocate an array for them */
-    ALLOC_ARRAY_CLEAR(ctx->groups, mbedtls_ecp_group_id, groups_count + 1)
-
-    /* Parse allowed ciphers, getting IDs */
-    int i = 0;
-    char *tmp_groups_orig = string_alloc(groups, NULL);
-    char *tmp_groups = tmp_groups_orig;
-
-    const char *token = strsep(&tmp_groups_orig, ":");
-    while (token)
-    {
-        const mbedtls_ecp_curve_info *ci =
-            mbedtls_ecp_curve_info_from_name(token);
-        if (ci == NULL)
-        {
-            msg(M_WARN, "Warning unknown curve/group specified: %s", token);
-        }
-        else
-        {
-            ctx->groups[i] = ci->grp_id;
-            i++;
-        }
-        token = strsep(&tmp_groups, ":");
-    }
-    ctx->groups[i] = MBEDTLS_ECP_DP_NONE;
-    free(tmp_groups_orig);
-}
-
-
 void
 tls_ctx_check_cert_time(const struct tls_root_ctx *ctx)
 {
@@ -1022 +991 @@
         mbedtls_ssl_conf_ciphersuites(&ks_ssl->ssl_config, ssl_ctx->allowed_ciphers);
     }
 
-    if (ssl_ctx->groups)
-    {
-        mbedtls_ssl_conf_curves(&ks_ssl->ssl_config, ssl_ctx->groups);
-    }
-
     /* Disable record splitting (for now).  OpenVPN assumes records are sent
      * unfragmented, and changing that will require thorough review and
      * testing.  Since OpenVPN is not susceptible to BEAST, we can just

src/openvpn/ssl_mbedtls.h

@@ -82 +82 @@
     struct external_context *external_key; /**< Management external key */
 #endif
     int *allowed_ciphers;       /**< List of allowed ciphers for this connection */
-    mbedtls_ecp_group_id *groups;                /**< List of allowed groups for this connection */
     mbedtls_x509_crt_profile cert_profile; /**< Allowed certificate types */
 };
 

src/openvpn/ssl_openssl.c

@@ -515 +515 @@
 #endif
 }
 
-void
-tls_ctx_set_tls_groups(struct tls_root_ctx *ctx, const char *groups)
-{
-    ASSERT(ctx);
-    /* This method could be as easy as
-     *  SSL_CTX_set1_groups_list(ctx->ctx, groups)
-     * but OpenSSL does not like the name secp256r1 for prime256v1
-     * and as this is one of the more important curve to have
-     * the same name for OpenSSL and mbedTLS, we do this dance
-     */
-
-    int groups_count = get_num_elements(groups, ':');
-
-    int *glist;
-    /* Allocate an array for them */
-    ALLOC_ARRAY_CLEAR(glist, int, groups_count);
-
-    /* Parse allowed ciphers, getting IDs */
-    int glistlen = 0;
-    char *tmp_groups_orig = string_alloc(groups, NULL);
-    char *tmp_groups = tmp_groups_orig;
-
-    const char *token = strsep(&tmp_groups_orig, ":");
-    while (token)
-    {
-        if (streq(token, "secp256r1"))
-        {
-            token = "prime256v1";
-        }
-        int nid = OBJ_sn2nid(token);
-
-        if (nid == 0)
-        {
-            msg(M_WARN, "Warning unknown curve/group specified: %s", token);
-        }
-        else
-        {
-            glist[glistlen] = nid;
-            glistlen++;
-        }
-        token = strsep(&tmp_groups, ":");
-    }
-
-    free(tmp_groups_orig);
-
-    if (!SSL_CTX_set1_groups(ctx->ctx, glist, glistlen))
-    {
-        crypto_msg(M_FATAL, "Failed to set allowed TLS group list: %s",
-                   groups);
-    }
-    free(glist);
-}
-
 void
 tls_ctx_check_cert_time(const struct tls_root_ctx *ctx)
 {
@@ -687 +634 @@
         /* OpenSSL 1.0.2 and newer can automatically handle ECDH parameter
          * loading */
         SSL_CTX_set_ecdh_auto(ctx->ctx, 1);
-
-        /* OpenSSL 1.1.0 and newer have always ecdh auto loading enabled,
-         * so do nothing */
-#endif
         return;
+#endif
 #else
         /* For older OpenSSL we have to extract the curve from key on our own */
         EC_KEY *eckey = NULL;
@@ -1980 +1924 @@
     ALLOC_ARRAY(curves, EC_builtin_curve, crv_len);
     if (EC_get_builtin_curves(curves, crv_len))
     {
-        printf("Consider using openssl ecparam -list_curves as\n"
-               "alternative to running this command to this command.");
-        printf("\nAvailable Elliptic curves/groups:\n");
+        printf("Available Elliptic curves:\n");
         for (n = 0; n < crv_len; n++)
         {
             const char *sname;