Factor out convert_tls_list_to_openssl method

PATCH In review Arne Schwabe 3 participants 12 messages Oct 6, 2018 – Oct 11, 2018

lore

Share & tools

lore

Arne Schwabe

author

2018-10-06 08:06 UTC #0 lore raw

PATCH 1/3 Factor out convert_tls_list_to_openssl method

This makes the tls_ctx_restrict_ciphers function more readable and clean ups the code a bit more.

---
 src/openvpn/ssl_openssl.c | 57 +++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 26 deletions(-)

src/openvpn/ssl_openssl.c +31 ~~-26~~

index 3aac216c..1008396d 100644
@@ -323,28 +323,8 @@ tls_ctx_set_options(struct tls_root_ctx *ctx, unsigned int ssl_flags)
 }
 
 void
-tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
+convert_tls_list_to_openssl(char* openssl_ciphers, size_t len,const char *ciphers)
 {
-    if (ciphers == NULL)
-    {
-        /* Use sane default TLS cipher list */
-        if (!SSL_CTX_set_cipher_list(ctx->ctx,
-                                     /* Use openssl's default list as a basis */
-                                     "DEFAULT"
-                                     /* Disable export ciphers and openssl's 'low' and 'medium' ciphers */
-                                     ":!EXP:!LOW:!MEDIUM"
-                                     /* Disable static (EC)DH keys (no forward secrecy) */
-                                     ":!kDH:!kECDH"
-                                     /* Disable DSA private keys */
-                                     ":!DSS"
-                                     /* Disable unsupported TLS modes */
-                                     ":!PSK:!SRP:!kRSA"))
-        {
-            crypto_msg(M_FATAL, "Failed to set default TLS cipher list.");
-        }
-        return;
-    }
-
     /* Parse supplied cipher list and pass on to OpenSSL */
     size_t begin_of_cipher, end_of_cipher;

@@ -353,12 +333,9 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 
     const tls_cipher_name_pair *cipher_pair;
 
-    char openssl_ciphers[4096];
     size_t openssl_ciphers_len = 0;
     openssl_ciphers[0] = '\0';
 
-    ASSERT(NULL != ctx);
-
     /* Translate IANA cipher suite names to OpenSSL names */
     begin_of_cipher = end_of_cipher = 0;
     for (; begin_of_cipher < strlen(ciphers); begin_of_cipher = end_of_cipher)

@@ -395,11 +372,11 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 
         /* Make sure new cipher name fits in cipher string */
         if ((SIZE_MAX - openssl_ciphers_len) < current_cipher_len
-            || ((sizeof(openssl_ciphers)-1) < openssl_ciphers_len + current_cipher_len))
+            || (len - 1) < (openssl_ciphers_len + current_cipher_len))
         {
             msg(M_FATAL,
                 "Failed to set restricted TLS cipher list, too long (>%d).",
-                (int)sizeof(openssl_ciphers)-1);
+                  (int)(len - 1));
         }
 
         /* Concatenate cipher name to OpenSSL cipher string */

@@ -415,6 +392,34 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
     {
         openssl_ciphers[openssl_ciphers_len-1] = '\0';
     }
+}
+void
+tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
+{
+    if (ciphers == NULL)
+    {
+        /* Use sane default TLS cipher list */
+        if (!SSL_CTX_set_cipher_list(ctx->ctx,
+                                     /* Use openssl's default list as a basis */
+                                     "DEFAULT"
+                                     /* Disable export ciphers and openssl's 'low' and 'medium' ciphers */
+                                     ":!EXP:!LOW:!MEDIUM"
+                                     /* Disable static (EC)DH keys (no forward secrecy) */
+                                     ":!kDH:!kECDH"
+                                     /* Disable DSA private keys */
+                                     ":!DSS"
+                                     /* Disable unsupported TLS modes */
+                                     ":!PSK:!SRP:!kRSA"))
+        {
+            crypto_msg(M_FATAL, "Failed to set default TLS cipher list.");
+        }
+        return;
+    }
+
+    char openssl_ciphers[4096];
+    convert_tls_list_to_openssl(openssl_ciphers, sizeof(openssl_ciphers), ciphers);
+
+    ASSERT(NULL != ctx);
 
     /* Set OpenSSL cipher list */
     if (!SSL_CTX_set_cipher_list(ctx->ctx, openssl_ciphers))

Signature · 1 line

2.19.0

Arne Schwabe

author

2018-10-06 08:06 UTC #1 lore raw

PATCH 2/3 Add support for tls-ciphersuites for TLS 1.3

OpenSSL 1.1.1 introduces a seperate list for TLS 1.3 ciphers. As these interfaces are meant to be user facing or not exposed at all and we expose the tls-cipher interface, we should also expose tls-cipherlist.

Combining both settings into tls-cipher would add a lot of glue logic that needs to be maintained and is error prone. On top of that, users should not set either settings unless absolutely required.

OpenSSL's own s_client/s_server also expose both settings and I believe most other software will too:

-cipher val Specify TLSv1.2 and below cipher list to be used
-ciphersuites val Specify TLSv1.3 ciphersuites to be used

For mbed TLS only the future can tell if we will see a combined or also two seperate lists.

---
 doc/openvpn.8             | 20 +++++++++++++---
 src/openvpn/options.c     |  7 ++++++
 src/openvpn/options.h     |  1 +
 src/openvpn/ssl.c         |  3 ++-
 src/openvpn/ssl_backend.h | 13 ++++++++++-
 src/openvpn/ssl_mbedtls.c | 13 +++++++++++
 src/openvpn/ssl_openssl.c | 48 +++++++++++++++++++++++++++++++++++++++
 7 files changed, 100 insertions(+), 5 deletions(-)

doc/openvpn.8 +17 -3

index 15a10296..ccdb3386 100644
@@ -5001,11 +5001,13 @@ determines the derivation of the tunnel session keys.
 .\"*********************************************************
 .TP
 .B \-\-tls\-cipher l
+.TQ
+.B \-\-tls\-ciphersuites l
 A list
 .B l
 of allowable TLS ciphers delimited by a colon (":").
 
-This setting can be used to ensure that certain cipher suites are used (or
+These setting can be used to ensure that certain cipher suites are used (or
 not used) for the TLS connection.  OpenVPN uses TLS to secure the control
 channel, over which the keys that are used to protect the actual VPN traffic
 are exchanged.

@@ -5014,20 +5016,32 @@ The supplied list of ciphers is (after potential OpenSSL/IANA name translation)
 simply supplied to the crypto library.  Please see the OpenSSL and/or mbed TLS
 documentation for details on the cipher list interpretation.
 
+For OpenSSL, the
+.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
+setting is used.
+
 Use
 .B \-\-show\-tls
 to see a list of TLS ciphers supported by your crypto library.
 
 Warning!
 .B \-\-tls\-cipher
-is an expert feature, which \- if used correcly \- can improve the security of
-your VPN connection.  But it is also easy to unwittingly use it to carefully
+and
+.B \-\-tls\-ciphersuites
+are expert features, which \- if used correcly \- can improve the security of
+your VPN connection.  But it is also easy to unwittingly use them to carefully
 align a gun with your foot, or just break your connection.  Use with care!
 
 The default for \-\-tls\-cipher is to use mbed TLS's default cipher list
 when using mbed TLS or
 "DEFAULT:!EXP:!LOW:!MEDIUM:!kDH:!kECDH:!DSS:!PSK:!SRP:!kRSA" when using
 OpenSSL.
+
+The default for \-\-tls\-ciphersuites is to use the crypto library's default.
 .\"*********************************************************
 .TP
 .B \-\-tls\-cert\-profile profile

src/openvpn/options.c +7 -0

index 03550c1e..b4d3cebe 100644
@@ -1766,6 +1766,7 @@ show_settings(const struct options *o)
 #endif
     SHOW_STR(cipher_list);
     SHOW_STR(tls_cert_profile);
+    SHOW_STR(cipher_list_tls13);
     SHOW_STR(tls_verify);
     SHOW_STR(tls_export_cert);
     SHOW_INT(verify_x509_type);

@@ -2759,6 +2760,7 @@ options_postprocess_verify_ce(const struct options *options, const struct connec
         MUST_BE_UNDEF(pkcs12_file);
 #endif
         MUST_BE_UNDEF(cipher_list);
+        MUST_BE_UNDEF(cipher_list_tls13);
         MUST_BE_UNDEF(tls_cert_profile);
         MUST_BE_UNDEF(tls_verify);
         MUST_BE_UNDEF(tls_export_cert);

@@ -7948,6 +7950,11 @@ add_option(struct options *options,
         VERIFY_PERMISSION(OPT_P_GENERAL);
         options->tls_cert_profile = p[1];
     }
+    else if (streq(p[0], "tls-ciphersuites") && p[1] && !p[2])
+    {
+        VERIFY_PERMISSION(OPT_P_GENERAL);
+        options->cipher_list_tls13 = 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 +1 -0

index 4c3bc4fb..3e7ef4f8 100644
@@ -508,6 +508,7 @@ struct options
     const char *priv_key_file;
     const char *pkcs12_file;
     const char *cipher_list;
+    const char *cipher_list_tls13;
     const char *tls_cert_profile;
     const char *ecdh_curve;
     const char *tls_verify;

src/openvpn/ssl.c +2 -1

index e5e4aac2..616c2696 100644
@@ -626,9 +626,10 @@ init_ssl(const struct options *options, struct tls_root_ctx *new_ctx)
     tls_ctx_set_cert_profile(new_ctx, options->tls_cert_profile);
 
     /* Allowable ciphers */
-    /* Since @SECLEVEL also influces loading of certificates, set the
+    /* Since @SECLEVEL also influences loading of certificates, set the
      * cipher restrictions before loading certificates */
     tls_ctx_restrict_ciphers(new_ctx, options->cipher_list);
+    tls_ctx_restrict_ciphers_tls13(new_ctx, options->cipher_list_tls13);
 
     if (!tls_ctx_set_options(new_ctx, options->ssl_flags))
     {

src/openvpn/ssl_backend.h +12 -1

index 5023c02a..0995bb4c 100644
@@ -169,7 +169,8 @@ bool tls_ctx_initialised(struct tls_root_ctx *ctx);
 bool tls_ctx_set_options(struct tls_root_ctx *ctx, unsigned int ssl_flags);
 
 /**
- * Restrict the list of ciphers that can be used within the TLS context.
+ * Restrict the list of ciphers that can be used within the TLS context for TLS 1.2
+ * and below
  *
  * @param ctx           TLS context to restrict, must be valid.
  * @param ciphers       String containing : delimited cipher names, or NULL to use

@@ -177,6 +178,16 @@ bool tls_ctx_set_options(struct tls_root_ctx *ctx, unsigned int ssl_flags);
  */
 void tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers);
 
+/**
+ * Restrict the list of ciphers that can be used within the TLS context for TLS 1.3
+ * and higher
+ *
+ * @param ctx           TLS context to restrict, must be valid.
+ * @param ciphers       String containing : delimited cipher names, or NULL to use
+ *                                      sane defaults.
+ */
+void tls_ctx_restrict_ciphers_tls13(struct tls_root_ctx *ctx, const char *ciphers);
+
 /**
  * Set the TLS certificate profile.  The profile defines which crypto
  * algorithms may be used in the supplied certificate.

src/openvpn/ssl_mbedtls.c +13 -0

index e4850cb6..f23cd30a 100644
@@ -222,6 +222,19 @@ tls_translate_cipher_name(const char *cipher_name)
     return pair->iana_name;
 }
 
+void
+tls_ctx_restrict_ciphers_tls13(struct tls_root_ctx *ctx, const char *ciphers)
+{
+    if (ciphers == NULL)
+    {
+        /* Nothing to do, return without warning message */
+        return;
+    }
+
+    msg(M_WARN, "mbed TLS does not support setting tls-ciphersuites. "
+                "Ignoring TLS 1.3 cipher list: %s", ciphers);
+}
+
 void
 tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 {

src/openvpn/ssl_openssl.c +48 -0

60 lines not on this page · raw message

Signature · 1 line

2.19.0

Arne Schwabe

author

2018-10-06 08:06 UTC #2 lore raw

PATCH 3/3 Add better support for showing TLS 1.3 ciphersuites in --show-tls

In reply to Arne Schwabe

show-tls shows mixed TLS 1.3 and TLS 1.2 ciphers. The ciphersuites are only valid in tls-cipher or tls-ciphersuites. So this confusing and not really helpful.

This patch modifies show-tls to show separate lists for TLS 1.2 and TLS 1.3.

---
 src/openvpn/init.c        |  1 +
 src/openvpn/ssl_backend.h |  1 +
 src/openvpn/ssl_mbedtls.c |  2 +
 src/openvpn/ssl_openssl.c | 79 ++++++++++++++++++++++++++++++---------
 4 files changed, 65 insertions(+), 18 deletions(-)

src/openvpn/init.c +1 -0

index b2ab2a60..ff863cf8 100644
@@ -1040,6 +1040,7 @@ print_openssl_info(const struct options *options)
         if (options->show_tls_ciphers)
         {
             show_available_tls_ciphers(options->cipher_list,
+                                       options->cipher_list_tls13,
                                        options->tls_cert_profile);
         }
         if (options->show_curves)

src/openvpn/ssl_backend.h +1 -0

index 0995bb4c..42934230 100644
@@ -525,6 +525,7 @@ void print_details(struct key_state_ssl *ks_ssl, const char *prefix);
  */
 void
 show_available_tls_ciphers(const char *cipher_list,
+                           const char *cipher_list_tls13,
                            const char *tls_cert_profile);
 
 /*

src/openvpn/ssl_mbedtls.c +2 -0

index f23cd30a..2c6e54b3 100644
@@ -1341,8 +1341,10 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
 
 void
 show_available_tls_ciphers(const char *cipher_list,
+                           const char *cipher_list_tls13,
                            const char *tls_cert_profile)
 {
+    (void) cipher_list_tls13; // Avoid unused warning
     struct tls_root_ctx tls_ctx;
     const int *ciphers = mbedtls_ssl_list_ciphersuites();

src/openvpn/ssl_openssl.c +61 ~~-18~~

118 lines not on this page · raw message

Signature · 1 line

2.19.0

Steffan Karger

2018-10-10 09:11 UTC #3 lore raw

Re: [Openvpn-devel] [PATCH 3/3] Add better support for showing TLS 1.3 ciphersuites in --show-tls

Hi,

Quoted 6 lines · Arne Schwabe

show-tls shows mixed TLS 1.3 and TLS 1.2 ciphers. The ciphersuites are only valid in tls-cipher or tls-ciphersuites. So this confusing and not really helpful.
This patch modifies show-tls to show separate lists for TLS 1.2 and TLS 1.3.

Feature-ACK.

Quoted 24 lines

---
 src/openvpn/init.c        |  1 +
 src/openvpn/ssl_backend.h |  1 +
 src/openvpn/ssl_mbedtls.c |  2 +
 src/openvpn/ssl_openssl.c | 79 ++++++++++++++++++++++++++++++---------
 4 files changed, 65 insertions(+), 18 deletions(-)

src/openvpn/init.c +1 -0

index b2ab2a60..ff863cf8 100644
@@ -1040,6 +1040,7 @@ print_openssl_info(const struct options *options)
         if (options->show_tls_ciphers)
         {
             show_available_tls_ciphers(options->cipher_list,
+                                       options->cipher_list_tls13,
                                        options->tls_cert_profile);
         }
         if (options->show_curves)

src/openvpn/ssl_backend.h +0 -0

index 0995bb4c..42934230 100644

src/openvpn/ssl_backend.h void print_details(struct key_state_ssl *ks_ssl, const char *prefix); · line 529

Quoted 5 lines

src/openvpn/ssl_backend.h +1 -0

@@ -525,6 +525,7 @@ void print_details(struct key_state_ssl *ks_ssl, const char *prefix);
  */
 void
 show_available_tls_ciphers(const char *cipher_list,
+                           const char *cipher_list_tls13,
                            const char *tls_cert_profile);

Please update to doxygen to include the extra parameter too.

Quoted 30 lines

src/openvpn/ssl_backend.h +0 -0

/*

src/openvpn/ssl_mbedtls.c +2 -0

index f23cd30a..2c6e54b3 100644
@@ -1341,8 +1341,10 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
 
 void
 show_available_tls_ciphers(const char *cipher_list,
+                           const char *cipher_list_tls13,
                            const char *tls_cert_profile)
 {
+    (void) cipher_list_tls13; // Avoid unused warning
     struct tls_root_ctx tls_ctx;
     const int *ciphers = mbedtls_ssl_list_ciphersuites();

src/openvpn/ssl_openssl.c +1 -0

index e3f02d05..044070fe 100644
@@ -60,6 +60,7 @@
 #include <openssl/pkcs12.h>
 #include <openssl/rsa.h>
 #include <openssl/x509.h>
+#include <openssl/ssl.h>
 #ifndef OPENSSL_NO_EC
 #include <openssl/ec.h>
 #endif

src/openvpn/ssl_openssl.c print_details(struct key_state_ssl *ks_ssl, const char *prefix) · line 1985

Quoted 10 lines

src/openvpn/ssl_openssl.c +4 -3

@@ -1978,15 +1979,12 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
     msg(D_HANDSHAKE, "%s%s", s1, s2);
 }
 
-void
-show_available_tls_ciphers(const char *cipher_list,
-                           const char *tls_cert_profile)
+static void
+show_available_tls_ciphers_list(const char *cipher_list,
+                           const char *tls_cert_profile,
+                           const bool tls13)

Argument alignment is now off due to function rename.

Quoted 76 lines

src/openvpn/ssl_openssl.c +33 ~~-15~~

 {
     struct tls_root_ctx tls_ctx;
-    SSL *ssl;
-    const char *cipher_name;
-    const tls_cipher_name_pair *pair;
-    int priority = 0;
 
     tls_ctx.ctx = SSL_CTX_new(SSLv23_method());
     if (!tls_ctx.ctx)

@@ -1994,22 +1992,44 @@ show_available_tls_ciphers(const char *cipher_list,
         crypto_msg(M_FATAL, "Cannot create SSL_CTX object");
     }
 
-    ssl = SSL_new(tls_ctx.ctx);
-    if (!ssl)
+#if (OPENSSL_VERSION_NUMBER >= 0x1010100fL)
+    if (tls13)
     {
-        crypto_msg(M_FATAL, "Cannot create SSL object");
+        SSL_CTX_set_min_proto_version(tls_ctx.ctx, TLS1_3_VERSION);
+    }
+    else
+#endif
+    {
+        SSL_CTX_set_max_proto_version(tls_ctx.ctx, TLS1_2_VERSION);
     }
 
     tls_ctx_set_cert_profile(&tls_ctx, tls_cert_profile);
     tls_ctx_restrict_ciphers(&tls_ctx, cipher_list);
 
-    printf("Available TLS Ciphers,\n");
-    printf("listed in order of preference:\n\n");
-    while ((cipher_name = SSL_get_cipher_list(ssl, priority++)))
+    SSL *ssl = SSL_new(tls_ctx.ctx);
+    if (!ssl)
     {
-        pair = tls_get_cipher_name_pair(cipher_name, strlen(cipher_name));
+        crypto_msg(M_FATAL, "Cannot create SSL object");
+    }
+
+#if (OPENSSL_VERSION_NUMBER < 0x1010000fL)
+    STACK_OF(SSL_CIPHER)* sk = SSL_get_ciphers(ssl);
+#else
+    STACK_OF(SSL_CIPHER)* sk = SSL_get1_supported_ciphers(ssl);
+#endif
+    for (int i=0;i < sk_SSL_CIPHER_num(sk);i++)
+    {
+        const SSL_CIPHER* c = sk_SSL_CIPHER_value(sk, i);
+
+        const char* cipher_name = SSL_CIPHER_get_name(c);
+
+        const tls_cipher_name_pair *pair = tls_get_cipher_name_pair(cipher_name, strlen(cipher_name));
 
-        if (NULL == pair)
+        if (tls13)
+        {
+              printf("%s\n", cipher_name);
+        }
+        else if (NULL == pair)
         {
             /* No translation found, print warning */
             printf("%s (No IANA name known to OpenVPN, use OpenSSL name.)\n", cipher_name);

@@ -2018,14 +2038,37 @@ show_available_tls_ciphers(const char *cipher_list,
         {
             printf("%s\n", pair->iana_name);
         }
-
     }
-    printf("\n" SHOW_TLS_CIPHER_LIST_WARNING);
-
+#if (OPENSSL_VERSION_NUMBER >= 0x1010000fL)
+    sk_SSL_CIPHER_free(sk);
+#endif
     SSL_free(ssl);
     SSL_CTX_free(tls_ctx.ctx);
 }

src/openvpn/ssl_openssl.c show_available_tls_ciphers(const char *cipher_list, · line 2057

Quoted 10 lines

src/openvpn/ssl_openssl.c +9 -0

 
+
+
+void
+show_available_tls_ciphers(const char *cipher_list,
+                           const char *cipher_list_tls13,
+                           const char *tls_cert_profile)
+{
+    printf("Available TLS Ciphers,\n");
+    printf("listed in order of preference:\n");

Since you are changing the output format anyway, can you merge these into a single printf and remove the unneeded extra newline halfway through the sentence?

Quoted 7 lines

src/openvpn/ssl_openssl.c +6 -0

+#if (OPENSSL_VERSION_NUMBER >= 0x1010100fL)
+    printf("\nFor TLS 1.3 and newer (--tls-ciphersuite):\n\n");
+    show_available_tls_ciphers_list(cipher_list_tls13, tls_cert_profile, true);
+#else
+    (void) cipher_list_tls13;  /* Avoid unused warning */
+#endif

src/openvpn/ssl_openssl.c show_available_tls_ciphers(const char *cipher_list, · line 2074

Quoted 10 lines

src/openvpn/ssl_openssl.c +7 -0

+
+    printf("\nFor TLS 1.2 and older (--tls-cipher):\n\n");
+    show_available_tls_ciphers_list(cipher_list, tls_cert_profile, false);
+
+    printf("\n" SHOW_TLS_CIPHER_LIST_WARNING);
+}
+
 /*
  * Show the Elliptic curves that are available for us to use
  * in the OpenSSL library.

Otherwise the patch looks good.

-Steffan

Arne Schwabe

author

2018-10-10 12:01 UTC #4 lore raw

PATCH v2 3/3 Add better support for showing TLS 1.3 ciphersuites in --show-tls

show-tls shows mixed TLS 1.3 and TLS 1.2 ciphers. The ciphersuites are only valid in tls-cipher or tls-ciphersuites. So this confusing and not really helpful.

This patch modifies show-tls to show separate lists for TLS 1.2 and TLS 1.3.

PATCH V2: refactor common code between mbed and OpenSSL into ssl.c,
fix minor style issues.

Signed-off-by Arne Schwabe <[email protected]...>

---
 src/openvpn/init.c        |  1 +
 src/openvpn/ssl.c         | 24 ++++++++++++++++
 src/openvpn/ssl_backend.h | 22 ++++++++++++--
 src/openvpn/ssl_common.h  |  6 ----
 src/openvpn/ssl_mbedtls.c | 15 ++++------
 src/openvpn/ssl_openssl.c | 60 ++++++++++++++++++++++++++-------------
 6 files changed, 92 insertions(+), 36 deletions(-)

src/openvpn/init.c +1 -0

index 52c64da4..8b34ab59 100644
@@ -1034,6 +1034,7 @@ print_openssl_info(const struct options *options)
         if (options->show_tls_ciphers)
         {
             show_available_tls_ciphers(options->cipher_list,
+                                       options->cipher_list_tls13,
                                        options->tls_cert_profile);
         }
         if (options->show_curves)

src/openvpn/ssl.c +24 -0

index dda6bf4e..c8d60e53 100644
@@ -4124,6 +4124,30 @@ tls_check_ncp_cipher_list(const char *list)
     return 0 < strlen(list) && !unsupported_cipher_found;
 }
 
+void
+show_available_tls_ciphers(const char *cipher_list,
+                           const char *cipher_list_tls13,
+                           const char *tls_cert_profile)
+{
+    printf("Available TLS Ciphers, listed in order of preference:\n");
+
+#if (ENABLE_CRYPTO_OPENSSL && OPENSSL_VERSION_NUMBER >= 0x1010100fL)
+    printf("\nFor TLS 1.3 and newer (--tls-ciphersuite):\n\n");
+    show_available_tls_ciphers_list(cipher_list_tls13, tls_cert_profile, true);
+#else
+    (void) cipher_list_tls13;  /* Avoid unused warning */
+#endif
+
+    printf("\nFor TLS 1.2 and older (--tls-cipher):\n\n");
+    show_available_tls_ciphers_list(cipher_list, tls_cert_profile, false);
+
+    printf("\n"
+    "Be aware that that whether a cipher suite in this list can actually work\n"
+    "depends on the specific setup of both peers. See the man page entries of\n"
+    "--tls-cipher and --show-tls for more details.\n\n"
+    );
+}
+
 /*
  * Dump a human-readable rendition of an openvpn packet
  * into a garbage collectable string which is returned.

src/openvpn/ssl_backend.h +20 -2

index 0995bb4c..53eb5d2d 100644
@@ -517,16 +517,34 @@ int key_state_read_plaintext(struct key_state_ssl *ks_ssl, struct buffer *buf,
 void print_details(struct key_state_ssl *ks_ssl, const char *prefix);
 
 /*
- * Show the TLS ciphers that are available for us to use in the OpenSSL
- * library.
+ * Show the TLS ciphers that are available for us to use in the SSL
+ * library with headers hinting their usage and warnings about usage.
  *
  * @param cipher_list       list of allowed TLS cipher, or NULL.
+ * @param cipher_list_tls13 list of allowed TLS 1.3+ cipher, or NULL
  * @param tls_cert_profile  TLS certificate crypto profile name.
  */
 void
 show_available_tls_ciphers(const char *cipher_list,
+                           const char *cipher_list_tls13,
                            const char *tls_cert_profile);
 
+
+/*
+ * Show the TLS ciphers that are available for us to use in the
+ * library depending on the TLS version. This function prints
+ * a list of ciphers without headers/footers.
+ *
+ * @param cipher_list       list of allowed TLS cipher, or NULL.
+ * @param tls_cert_profile  TLS certificate crypto profile name.
+ * @param tls13             Select if <=TLS1.2 or TLS1.3+ ciphers
+ *                          should be shown
+ */
+void
+show_available_tls_ciphers_list(const char *cipher_list,
+                                const char *tls_cert_profile,
+                                bool tls13);
+
 /*
  * Show the available elliptic curves in the crypto library
  */

src/openvpn/ssl_common.h +0 -6

index 08ef6ffa..106d1cd2 100644
@@ -554,10 +554,4 @@ struct tls_multi
      *   sessions with the remote peer. */
 };
 
-
-#define SHOW_TLS_CIPHER_LIST_WARNING \
-    "Be aware that that whether a cipher suite in this list can actually work\n" \
-    "depends on the specific setup of both peers. See the man page entries of\n" \
-    "--tls-cipher and --show-tls for more details.\n\n"
-
 #endif /* SSL_COMMON_H_ */

src/openvpn/ssl_mbedtls.c +6 -9

index f23cd30a..47629cc8 100644
@@ -1340,9 +1340,13 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
 }
 
 void
-show_available_tls_ciphers(const char *cipher_list,
-                           const char *tls_cert_profile)
+show_available_tls_ciphers_list(const char *cipher_list,
+                                const char *tls_cert_profile,
+                                bool tls13)
 {
+    if (tls13)
+        /* mbed TLS has no TLS 1.3 support currently */
+        return;
     struct tls_root_ctx tls_ctx;
     const int *ciphers = mbedtls_ssl_list_ciphersuites();

@@ -1355,18 +1359,11 @@ show_available_tls_ciphers(const char *cipher_list,
         ciphers = tls_ctx.allowed_ciphers;
     }
 
-#ifndef ENABLE_SMALL
-    printf("Available TLS Ciphers,\n");
-    printf("listed in order of preference:\n\n");
-#endif
-
     while (*ciphers != 0)
     {
         printf("%s\n", mbedtls_ssl_get_ciphersuite_name(*ciphers));
         ciphers++;
     }
-    printf("\n" SHOW_TLS_CIPHER_LIST_WARNING);
-
     tls_ctx_free(&tls_ctx);
 }

src/openvpn/ssl_openssl.c +41 ~~-19~~

105 lines not on this page · raw message

Signature · 1 line

2.19.0

Arne Schwabe

author

2018-10-10 12:34 UTC #5 lore raw

PATCH v3 3/3 Add better support for showing TLS 1.3 ciphersuites in --show-tls

In reply to Steffan Karger

show-tls shows mixed TLS 1.3 and TLS 1.2 ciphers. The ciphersuites are only valid in tls-cipher or tls-ciphersuites. So this confusing and not really helpful.

This patch modifies show-tls to show separate lists for TLS 1.2 and TLS 1.3.

PATCH V2: refactor common code between mbed and OpenSSL into ssl.c,
fix minor style issues.

PATCH V3: remove static argument from show_available_tls_ciphers_list
in ssl_openssl.c

Signed-off-by Arne Schwabe <[email protected]...>

---
 src/openvpn/init.c        |  1 +
 src/openvpn/ssl.c         | 24 ++++++++++++++++
 src/openvpn/ssl_backend.h | 22 +++++++++++++--
 src/openvpn/ssl_common.h  |  6 ----
 src/openvpn/ssl_mbedtls.c | 15 ++++------
 src/openvpn/ssl_openssl.c | 58 +++++++++++++++++++++++++++------------
 6 files changed, 91 insertions(+), 35 deletions(-)

src/openvpn/init.c +1 -0

index 52c64da4..8b34ab59 100644
@@ -1034,6 +1034,7 @@ print_openssl_info(const struct options *options)
         if (options->show_tls_ciphers)
         {
             show_available_tls_ciphers(options->cipher_list,
+                                       options->cipher_list_tls13,
                                        options->tls_cert_profile);
         }
         if (options->show_curves)

src/openvpn/ssl.c +24 -0

index dda6bf4e..c8d60e53 100644
@@ -4124,6 +4124,30 @@ tls_check_ncp_cipher_list(const char *list)
     return 0 < strlen(list) && !unsupported_cipher_found;
 }
 
+void
+show_available_tls_ciphers(const char *cipher_list,
+                           const char *cipher_list_tls13,
+                           const char *tls_cert_profile)
+{
+    printf("Available TLS Ciphers, listed in order of preference:\n");
+
+#if (ENABLE_CRYPTO_OPENSSL && OPENSSL_VERSION_NUMBER >= 0x1010100fL)
+    printf("\nFor TLS 1.3 and newer (--tls-ciphersuite):\n\n");
+    show_available_tls_ciphers_list(cipher_list_tls13, tls_cert_profile, true);
+#else
+    (void) cipher_list_tls13;  /* Avoid unused warning */
+#endif
+
+    printf("\nFor TLS 1.2 and older (--tls-cipher):\n\n");
+    show_available_tls_ciphers_list(cipher_list, tls_cert_profile, false);
+
+    printf("\n"
+    "Be aware that that whether a cipher suite in this list can actually work\n"
+    "depends on the specific setup of both peers. See the man page entries of\n"
+    "--tls-cipher and --show-tls for more details.\n\n"
+    );
+}
+
 /*
  * Dump a human-readable rendition of an openvpn packet
  * into a garbage collectable string which is returned.

src/openvpn/ssl_backend.h +20 -2

index 0995bb4c..53eb5d2d 100644
@@ -517,16 +517,34 @@ int key_state_read_plaintext(struct key_state_ssl *ks_ssl, struct buffer *buf,
 void print_details(struct key_state_ssl *ks_ssl, const char *prefix);
 
 /*
- * Show the TLS ciphers that are available for us to use in the OpenSSL
- * library.
+ * Show the TLS ciphers that are available for us to use in the SSL
+ * library with headers hinting their usage and warnings about usage.
  *
  * @param cipher_list       list of allowed TLS cipher, or NULL.
+ * @param cipher_list_tls13 list of allowed TLS 1.3+ cipher, or NULL
  * @param tls_cert_profile  TLS certificate crypto profile name.
  */
 void
 show_available_tls_ciphers(const char *cipher_list,
+                           const char *cipher_list_tls13,
                            const char *tls_cert_profile);
 
+
+/*
+ * Show the TLS ciphers that are available for us to use in the
+ * library depending on the TLS version. This function prints
+ * a list of ciphers without headers/footers.
+ *
+ * @param cipher_list       list of allowed TLS cipher, or NULL.
+ * @param tls_cert_profile  TLS certificate crypto profile name.
+ * @param tls13             Select if <=TLS1.2 or TLS1.3+ ciphers
+ *                          should be shown
+ */
+void
+show_available_tls_ciphers_list(const char *cipher_list,
+                                const char *tls_cert_profile,
+                                bool tls13);
+
 /*
  * Show the available elliptic curves in the crypto library
  */

src/openvpn/ssl_common.h +0 -6

index 08ef6ffa..106d1cd2 100644
@@ -554,10 +554,4 @@ struct tls_multi
      *   sessions with the remote peer. */
 };
 
-
-#define SHOW_TLS_CIPHER_LIST_WARNING \
-    "Be aware that that whether a cipher suite in this list can actually work\n" \
-    "depends on the specific setup of both peers. See the man page entries of\n" \
-    "--tls-cipher and --show-tls for more details.\n\n"
-
 #endif /* SSL_COMMON_H_ */

src/openvpn/ssl_mbedtls.c +6 -9

index f23cd30a..47629cc8 100644
@@ -1340,9 +1340,13 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
 }
 
 void
-show_available_tls_ciphers(const char *cipher_list,
-                           const char *tls_cert_profile)
+show_available_tls_ciphers_list(const char *cipher_list,
+                                const char *tls_cert_profile,
+                                bool tls13)
 {
+    if (tls13)
+        /* mbed TLS has no TLS 1.3 support currently */
+        return;
     struct tls_root_ctx tls_ctx;
     const int *ciphers = mbedtls_ssl_list_ciphersuites();

@@ -1355,18 +1359,11 @@ show_available_tls_ciphers(const char *cipher_list,
         ciphers = tls_ctx.allowed_ciphers;
     }
 
-#ifndef ENABLE_SMALL
-    printf("Available TLS Ciphers,\n");
-    printf("listed in order of preference:\n\n");
-#endif
-
     while (*ciphers != 0)
     {
         printf("%s\n", mbedtls_ssl_get_ciphersuite_name(*ciphers));
         ciphers++;
     }
-    printf("\n" SHOW_TLS_CIPHER_LIST_WARNING);
-
     tls_ctx_free(&tls_ctx);
 }

src/openvpn/ssl_openssl.c +40 ~~-18~~

103 lines not on this page · raw message

Signature · 1 line

2.19.0

Steffan Karger

2018-10-10 13:38 UTC #6 lore raw

Re: [Openvpn-devel] [PATCH v3 3/3] Add better support for showing TLS 1.3 ciphersuites in --show-tls

Hi,

Thanks, merging these into a single function makes it cleaner. Still a few minor comments though:

Quoted 81 lines · Arne Schwabe

show-tls shows mixed TLS 1.3 and TLS 1.2 ciphers. The ciphersuites are only valid in tls-cipher or tls-ciphersuites. So this confusing and not really helpful.

This patch modifies show-tls to show separate lists for TLS 1.2 and TLS 1.3.

PATCH V2: refactor common code between mbed and OpenSSL into ssl.c,
fix minor style issues.

PATCH V3: remove static argument from show_available_tls_ciphers_list
in ssl_openssl.c

Signed-off-by Arne Schwabe <[email protected]...>

---
 src/openvpn/init.c        |  1 +
 src/openvpn/ssl.c         | 24 ++++++++++++++++
 src/openvpn/ssl_backend.h | 22 +++++++++++++--
 src/openvpn/ssl_common.h  |  6 ----
 src/openvpn/ssl_mbedtls.c | 15 ++++------
 src/openvpn/ssl_openssl.c | 58 +++++++++++++++++++++++++++------------
 6 files changed, 91 insertions(+), 35 deletions(-)

src/openvpn/init.c +1 -0

index 52c64da4..8b34ab59 100644
@@ -1034,6 +1034,7 @@ print_openssl_info(const struct options *options)
         if (options->show_tls_ciphers)
         {
             show_available_tls_ciphers(options->cipher_list,
+                                       options->cipher_list_tls13,
                                        options->tls_cert_profile);
         }
         if (options->show_curves)

src/openvpn/ssl.c +24 -0

index dda6bf4e..c8d60e53 100644
@@ -4124,6 +4124,30 @@ tls_check_ncp_cipher_list(const char *list)
     return 0 < strlen(list) && !unsupported_cipher_found;
 }
 
+void
+show_available_tls_ciphers(const char *cipher_list,
+                           const char *cipher_list_tls13,
+                           const char *tls_cert_profile)
+{
+    printf("Available TLS Ciphers, listed in order of preference:\n");
+
+#if (ENABLE_CRYPTO_OPENSSL && OPENSSL_VERSION_NUMBER >= 0x1010100fL)
+    printf("\nFor TLS 1.3 and newer (--tls-ciphersuite):\n\n");
+    show_available_tls_ciphers_list(cipher_list_tls13, tls_cert_profile, true);
+#else
+    (void) cipher_list_tls13;  /* Avoid unused warning */
+#endif
+
+    printf("\nFor TLS 1.2 and older (--tls-cipher):\n\n");
+    show_available_tls_ciphers_list(cipher_list, tls_cert_profile, false);
+
+    printf("\n"
+    "Be aware that that whether a cipher suite in this list can actually work\n"
+    "depends on the specific setup of both peers. See the man page entries of\n"
+    "--tls-cipher and --show-tls for more details.\n\n"
+    );
+}
+
 /*
  * Dump a human-readable rendition of an openvpn packet
  * into a garbage collectable string which is returned.

src/openvpn/ssl_backend.h +1 -2

index 0995bb4c..53eb5d2d 100644
@@ -517,16 +517,34 @@ int key_state_read_plaintext(struct key_state_ssl *ks_ssl, struct buffer *buf,
 void print_details(struct key_state_ssl *ks_ssl, const char *prefix);
 
 /*
- * Show the TLS ciphers that are available for us to use in the OpenSSL
- * library.
+ * Show the TLS ciphers that are available for us to use in the SSL

src/openvpn/ssl_backend.h int key_state_read_plaintext(struct key_state_ssl *ks_ssl, struct buffer *buf, · line 530

Quoted 10 lines

src/openvpn/ssl_backend.h +3 -0

+ * library with headers hinting their usage and warnings about usage.
  *
  * @param cipher_list       list of allowed TLS cipher, or NULL.
+ * @param cipher_list_tls13 list of allowed TLS 1.3+ cipher, or NULL
  * @param tls_cert_profile  TLS certificate crypto profile name.
  */
 void
 show_available_tls_ciphers(const char *cipher_list,
+                           const char *cipher_list_tls13,
                            const char *tls_cert_profile);

Since the function moved to ssl.c, the prototype should move to ssl.h.

Quoted 40 lines

src/openvpn/ssl_backend.h +15 -0

+/*
+ * Show the TLS ciphers that are available for us to use in the
+ * library depending on the TLS version. This function prints
+ * a list of ciphers without headers/footers.
+ *
+ * @param cipher_list       list of allowed TLS cipher, or NULL.
+ * @param tls_cert_profile  TLS certificate crypto profile name.
+ * @param tls13             Select if <=TLS1.2 or TLS1.3+ ciphers
+ *                          should be shown
+ */
+void
+show_available_tls_ciphers_list(const char *cipher_list,
+                                const char *tls_cert_profile,
+                                bool tls13);
+
 /*
  * Show the available elliptic curves in the crypto library
  */

src/openvpn/ssl_common.h +0 -6

index 08ef6ffa..106d1cd2 100644
@@ -554,10 +554,4 @@ struct tls_multi
      *   sessions with the remote peer. */
 };
 
-
-#define SHOW_TLS_CIPHER_LIST_WARNING \
-    "Be aware that that whether a cipher suite in this list can actually work\n" \
-    "depends on the specific setup of both peers. See the man page entries of\n" \
-    "--tls-cipher and --show-tls for more details.\n\n"
-
 #endif /* SSL_COMMON_H_ */

src/openvpn/ssl_mbedtls.c +0 -0

index f23cd30a..47629cc8 100644
@@ -1340,9 +1340,13 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
 }

src/openvpn/ssl_mbedtls.c print_details(struct key_state_ssl *ks_ssl, const char *prefix) · line 1349

Quoted 10 lines

src/openvpn/ssl_mbedtls.c +6 -2

 void
-show_available_tls_ciphers(const char *cipher_list,
-                           const char *tls_cert_profile)
+show_available_tls_ciphers_list(const char *cipher_list,
+                                const char *tls_cert_profile,
+                                bool tls13)
 {
+    if (tls13)
+        /* mbed TLS has no TLS 1.3 support currently */
+        return;

Please add curly braces (sorry, should have spotted that before).

Quoted 92 lines

src/openvpn/ssl_mbedtls.c +0 -7

     struct tls_root_ctx tls_ctx;
     const int *ciphers = mbedtls_ssl_list_ciphersuites();

@@ -1355,18 +1359,11 @@ show_available_tls_ciphers(const char *cipher_list,
         ciphers = tls_ctx.allowed_ciphers;
     }
 
-#ifndef ENABLE_SMALL
-    printf("Available TLS Ciphers,\n");
-    printf("listed in order of preference:\n\n");
-#endif
-
     while (*ciphers != 0)
     {
         printf("%s\n", mbedtls_ssl_get_ciphersuite_name(*ciphers));
         ciphers++;
     }
-    printf("\n" SHOW_TLS_CIPHER_LIST_WARNING);
-
     tls_ctx_free(&tls_ctx);
 }

src/openvpn/ssl_openssl.c +18 ~~-14~~

index f08d7cae..3c46594c 100644
@@ -60,6 +60,7 @@
 #include <openssl/pkcs12.h>
 #include <openssl/rsa.h>
 #include <openssl/x509.h>
+#include <openssl/ssl.h>
 #ifndef OPENSSL_NO_EC
 #include <openssl/ec.h>
 #endif

@@ -428,7 +429,8 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 }
 
 void
-convert_tls13_list_to_openssl(char* openssl_ciphers, size_t len, const char *ciphers)
+convert_tls13_list_to_openssl(char *openssl_ciphers, size_t len,
+                              const char *ciphers)
 {
     /*
      * OpenSSL (and official IANA) cipher names have _ in them. We

@@ -1984,14 +1986,11 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
 }
 
 void
-show_available_tls_ciphers(const char *cipher_list,
-                           const char *tls_cert_profile)
+show_available_tls_ciphers_list(const char *cipher_list,
+                                const char *tls_cert_profile,
+                                const bool tls13)
 {
     struct tls_root_ctx tls_ctx;
-    SSL *ssl;
-    const char *cipher_name;
-    const tls_cipher_name_pair *pair;
-    int priority = 0;
 
     tls_ctx.ctx = SSL_CTX_new(SSLv23_method());
     if (!tls_ctx.ctx)

@@ -1999,22 +1998,44 @@ show_available_tls_ciphers(const char *cipher_list,
         crypto_msg(M_FATAL, "Cannot create SSL_CTX object");
     }
 
-    ssl = SSL_new(tls_ctx.ctx);
-    if (!ssl)
+#if (OPENSSL_VERSION_NUMBER >= 0x1010100fL)
+    if (tls13)
     {
-        crypto_msg(M_FATAL, "Cannot create SSL object");
+        SSL_CTX_set_min_proto_version(tls_ctx.ctx, TLS1_3_VERSION);
+    }
+    else
+#endif
+    {
+        SSL_CTX_set_max_proto_version(tls_ctx.ctx, TLS1_2_VERSION);
     }
 
     tls_ctx_set_cert_profile(&tls_ctx, tls_cert_profile);
     tls_ctx_restrict_ciphers(&tls_ctx, cipher_list);
 
-    printf("Available TLS Ciphers,\n");
-    printf("listed in order of preference:\n\n");
-    while ((cipher_name = SSL_get_cipher_list(ssl, priority++)))
+    SSL *ssl = SSL_new(tls_ctx.ctx);
+    if (!ssl)
     {
-        pair = tls_get_cipher_name_pair(cipher_name, strlen(cipher_name));
+        crypto_msg(M_FATAL, "Cannot create SSL object");
+    }

src/openvpn/ssl_openssl.c show_available_tls_ciphers(const char *cipher_list, · line 2028

Quoted 10 lines

src/openvpn/ssl_openssl.c +8 -1

 
-        if (NULL == pair)
+#if (OPENSSL_VERSION_NUMBER < 0x1010000fL)
+    STACK_OF(SSL_CIPHER)* sk = SSL_get_ciphers(ssl);
+#else
+    STACK_OF(SSL_CIPHER)* sk = SSL_get1_supported_ciphers(ssl);
+#endif
+    for (int i=0;i < sk_SSL_CIPHER_num(sk);i++)
+    {
+        const SSL_CIPHER* c = sk_SSL_CIPHER_value(sk, i);

const SSL_CIPHER *c

src/openvpn/ssl_openssl.c show_available_tls_ciphers(const char *cipher_list, · line 2030

Quoted 1 line

src/openvpn/ssl_openssl.c +1 -0

+        const char* cipher_name = SSL_CIPHER_get_name(c);

const char *cipher_name

src/openvpn/ssl_openssl.c show_available_tls_ciphers(const char *cipher_list, · line 2031

Quoted 2 lines

src/openvpn/ssl_openssl.c +2 -0

+
+        const tls_cipher_name_pair *pair = tls_get_cipher_name_pair(cipher_name, strlen(cipher_name));

Please wrap lines longer than 80 chars.

Quoted 18 lines

src/openvpn/ssl_openssl.c +6 -3

+        if (tls13)
+        {
+              printf("%s\n", cipher_name);
+        }
+        else if (NULL == pair)
         {
             /* No translation found, print warning */
             printf("%s (No IANA name known to OpenVPN, use OpenSSL name.)\n", cipher_name);

@@ -2023,14 +2044,15 @@ show_available_tls_ciphers(const char *cipher_list,
         {
             printf("%s\n", pair->iana_name);
         }
-
     }
-    printf("\n" SHOW_TLS_CIPHER_LIST_WARNING);
-
+#if (OPENSSL_VERSION_NUMBER >= 0x1010000fL)

src/openvpn/ssl_openssl.c show_available_tls_ciphers(const char *cipher_list, · line 2058

Quoted 10 lines

src/openvpn/ssl_openssl.c +3 -0

+    sk_SSL_CIPHER_free(sk);
+#endif
     SSL_free(ssl);
     SSL_CTX_free(tls_ctx.ctx);
 }
 
+
 /*
  * Show the Elliptic curves that are available for us to use
  * in the OpenSSL library.

Logically the patch looks good, and does what it should. But please fix the mentioned minor points.

-Steffan

Arne Schwabe

author

2018-10-10 15:36 UTC #7 lore raw

PATCH v4 3/3 Add better support for showing TLS 1.3 ciphersuites in --show-tls

show-tls shows mixed TLS 1.3 and TLS 1.2 ciphers. The ciphersuites are only valid in tls-cipher or tls-ciphersuites. So this confusing and not really helpful.

This patch modifies show-tls to show separate lists for TLS 1.2 and TLS 1.3.

Signed-off-by Arne Schwabe <[email protected]...>

---
PATCH V2: refactor common code between mbed and OpenSSL into ssl.c,
fix minor style issues.

PATCH V3: remove static argument from show_available_tls_ciphers_list
in ssl_openssl.c

PATCH V4: Style fixes

 src/openvpn/init.c        |  1 +
 src/openvpn/ssl.c         | 24 ++++++++++++++++
 src/openvpn/ssl.h         | 14 ++++++++++
 src/openvpn/ssl_backend.h | 12 +++++---
 src/openvpn/ssl_common.h  |  6 ----
 src/openvpn/ssl_mbedtls.c | 17 ++++++-----
 src/openvpn/ssl_openssl.c | 59 +++++++++++++++++++++++++++------------
 7 files changed, 96 insertions(+), 37 deletions(-)

src/openvpn/init.c +1 -0

index 1b9f19d0..e5e6e85f 100644
@@ -1034,6 +1034,7 @@ print_openssl_info(const struct options *options)
         if (options->show_tls_ciphers)
         {
             show_available_tls_ciphers(options->cipher_list,
+                                       options->cipher_list_tls13,
                                        options->tls_cert_profile);
         }
         if (options->show_curves)

src/openvpn/ssl.c +24 -0

index 4f0af286..0b8ccdd3 100644
@@ -4127,6 +4127,30 @@ tls_check_ncp_cipher_list(const char *list)
     return 0 < strlen(list) && !unsupported_cipher_found;
 }
 
+void
+show_available_tls_ciphers(const char *cipher_list,
+                           const char *cipher_list_tls13,
+                           const char *tls_cert_profile)
+{
+    printf("Available TLS Ciphers, listed in order of preference:\n");
+
+#if (ENABLE_CRYPTO_OPENSSL && OPENSSL_VERSION_NUMBER >= 0x1010100fL)
+    printf("\nFor TLS 1.3 and newer (--tls-ciphersuite):\n\n");
+    show_available_tls_ciphers_list(cipher_list_tls13, tls_cert_profile, true);
+#else
+    (void) cipher_list_tls13;  /* Avoid unused warning */
+#endif
+
+    printf("\nFor TLS 1.2 and older (--tls-cipher):\n\n");
+    show_available_tls_ciphers_list(cipher_list, tls_cert_profile, false);
+
+    printf("\n"
+    "Be aware that that whether a cipher suite in this list can actually work\n"
+    "depends on the specific setup of both peers. See the man page entries of\n"
+    "--tls-cipher and --show-tls for more details.\n\n"
+    );
+}
+
 /*
  * Dump a human-readable rendition of an openvpn packet
  * into a garbage collectable string which is returned.

src/openvpn/ssl.h +14 -0

index 781adb94..0bbe1da7 100644
@@ -601,4 +601,18 @@ bool is_hard_reset(int op, int key_method);
 
 void delayed_auth_pass_purge(void);
 
+
+/*
+ * Show the TLS ciphers that are available for us to use in the SSL
+ * library with headers hinting their usage and warnings about usage.
+ *
+ * @param cipher_list       list of allowed TLS cipher, or NULL.
+ * @param cipher_list_tls13 list of allowed TLS 1.3+ cipher, or NULL
+ * @param tls_cert_profile  TLS certificate crypto profile name.
+ */
+void
+show_available_tls_ciphers(const char *cipher_list,
+                           const char *cipher_list_tls13,
+                           const char *tls_cert_profile);
+
 #endif /* ifndef OPENVPN_SSL_H */

src/openvpn/ssl_backend.h +8 -4

index 2ae3bcd7..1c244ece 100644
@@ -517,15 +517,19 @@ int key_state_read_plaintext(struct key_state_ssl *ks_ssl, struct buffer *buf,
 void print_details(struct key_state_ssl *ks_ssl, const char *prefix);
 
 /*
- * Show the TLS ciphers that are available for us to use in the OpenSSL
- * library.
+ * Show the TLS ciphers that are available for us to use in the
+ * library depending on the TLS version. This function prints
+ * a list of ciphers without headers/footers.
  *
  * @param cipher_list       list of allowed TLS cipher, or NULL.
  * @param tls_cert_profile  TLS certificate crypto profile name.
+ * @param tls13             Select if <=TLS1.2 or TLS1.3+ ciphers
+ *                          should be shown
  */
 void
-show_available_tls_ciphers(const char *cipher_list,
-                           const char *tls_cert_profile);
+show_available_tls_ciphers_list(const char *cipher_list,
+                                const char *tls_cert_profile,
+                                bool tls13);
 
 /*
  * Show the available elliptic curves in the crypto library

src/openvpn/ssl_common.h +0 -6

index 919ec57c..e5c28508 100644
@@ -554,10 +554,4 @@ struct tls_multi
      *   sessions with the remote peer. */
 };
 
-
-#define SHOW_TLS_CIPHER_LIST_WARNING \
-    "Be aware that that whether a cipher suite in this list can actually work\n" \
-    "depends on the specific setup of both peers. See the man page entries of\n" \
-    "--tls-cipher and --show-tls for more details.\n\n"
-
 #endif /* SSL_COMMON_H_ */

src/openvpn/ssl_mbedtls.c +8 -9

index cb71d55a..3a0b5641 100644
@@ -1340,9 +1340,15 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
 }
 
 void
-show_available_tls_ciphers(const char *cipher_list,
-                           const char *tls_cert_profile)
+show_available_tls_ciphers_list(const char *cipher_list,
+                                const char *tls_cert_profile,
+                                bool tls13)
 {
+    if (tls13)
+    {
+        /* mbed TLS has no TLS 1.3 support currently */
+        return;
+    }
     struct tls_root_ctx tls_ctx;
     const int *ciphers = mbedtls_ssl_list_ciphersuites();

@@ -1355,18 +1361,11 @@ show_available_tls_ciphers(const char *cipher_list,
         ciphers = tls_ctx.allowed_ciphers;
     }
 
-#ifndef ENABLE_SMALL
-    printf("Available TLS Ciphers,\n");
-    printf("listed in order of preference:\n\n");
-#endif
-
     while (*ciphers != 0)
     {
         printf("%s\n", mbedtls_ssl_get_ciphersuite_name(*ciphers));
         ciphers++;
     }
-    printf("\n" SHOW_TLS_CIPHER_LIST_WARNING);
-
     tls_ctx_free(&tls_ctx);
 }

src/openvpn/ssl_openssl.c +41 ~~-18~~

104 lines not on this page · raw message

Signature · 1 line

2.19.0

Steffan Karger

Acked-by on v4 3/3

2018-10-10 19:14 UTC #8 lore raw

Re: [Openvpn-devel] [PATCH v4 3/3] Add better support for showing TLS 1.3 ciphersuites in --show-tls

Hi,

Quoted 284 lines · Arne Schwabe

show-tls shows mixed TLS 1.3 and TLS 1.2 ciphers. The ciphersuites are only valid in tls-cipher or tls-ciphersuites. So this confusing and not really helpful.

This patch modifies show-tls to show separate lists for TLS 1.2 and TLS 1.3.

Signed-off-by Arne Schwabe <[email protected]...>

---
PATCH V2: refactor common code between mbed and OpenSSL into ssl.c,
fix minor style issues.

PATCH V3: remove static argument from show_available_tls_ciphers_list
in ssl_openssl.c

PATCH V4: Style fixes

 src/openvpn/init.c        |  1 +
 src/openvpn/ssl.c         | 24 ++++++++++++++++
 src/openvpn/ssl.h         | 14 ++++++++++
 src/openvpn/ssl_backend.h | 12 +++++---
 src/openvpn/ssl_common.h  |  6 ----
 src/openvpn/ssl_mbedtls.c | 17 ++++++-----
 src/openvpn/ssl_openssl.c | 59 +++++++++++++++++++++++++++------------
 7 files changed, 96 insertions(+), 37 deletions(-)

src/openvpn/init.c +1 -0

index 1b9f19d0..e5e6e85f 100644
@@ -1034,6 +1034,7 @@ print_openssl_info(const struct options *options)
         if (options->show_tls_ciphers)
         {
             show_available_tls_ciphers(options->cipher_list,
+                                       options->cipher_list_tls13,
                                        options->tls_cert_profile);
         }
         if (options->show_curves)

src/openvpn/ssl.c +24 -0

index 4f0af286..0b8ccdd3 100644
@@ -4127,6 +4127,30 @@ tls_check_ncp_cipher_list(const char *list)
     return 0 < strlen(list) && !unsupported_cipher_found;
 }
 
+void
+show_available_tls_ciphers(const char *cipher_list,
+                           const char *cipher_list_tls13,
+                           const char *tls_cert_profile)
+{
+    printf("Available TLS Ciphers, listed in order of preference:\n");
+
+#if (ENABLE_CRYPTO_OPENSSL && OPENSSL_VERSION_NUMBER >= 0x1010100fL)
+    printf("\nFor TLS 1.3 and newer (--tls-ciphersuite):\n\n");
+    show_available_tls_ciphers_list(cipher_list_tls13, tls_cert_profile, true);
+#else
+    (void) cipher_list_tls13;  /* Avoid unused warning */
+#endif
+
+    printf("\nFor TLS 1.2 and older (--tls-cipher):\n\n");
+    show_available_tls_ciphers_list(cipher_list, tls_cert_profile, false);
+
+    printf("\n"
+    "Be aware that that whether a cipher suite in this list can actually work\n"
+    "depends on the specific setup of both peers. See the man page entries of\n"
+    "--tls-cipher and --show-tls for more details.\n\n"
+    );
+}
+
 /*
  * Dump a human-readable rendition of an openvpn packet
  * into a garbage collectable string which is returned.

src/openvpn/ssl.h +14 -0

index 781adb94..0bbe1da7 100644
@@ -601,4 +601,18 @@ bool is_hard_reset(int op, int key_method);
 
 void delayed_auth_pass_purge(void);
 
+
+/*
+ * Show the TLS ciphers that are available for us to use in the SSL
+ * library with headers hinting their usage and warnings about usage.
+ *
+ * @param cipher_list       list of allowed TLS cipher, or NULL.
+ * @param cipher_list_tls13 list of allowed TLS 1.3+ cipher, or NULL
+ * @param tls_cert_profile  TLS certificate crypto profile name.
+ */
+void
+show_available_tls_ciphers(const char *cipher_list,
+                           const char *cipher_list_tls13,
+                           const char *tls_cert_profile);
+
 #endif /* ifndef OPENVPN_SSL_H */

src/openvpn/ssl_backend.h +8 -4

index 2ae3bcd7..1c244ece 100644
@@ -517,15 +517,19 @@ int key_state_read_plaintext(struct key_state_ssl *ks_ssl, struct buffer *buf,
 void print_details(struct key_state_ssl *ks_ssl, const char *prefix);
 
 /*
- * Show the TLS ciphers that are available for us to use in the OpenSSL
- * library.
+ * Show the TLS ciphers that are available for us to use in the
+ * library depending on the TLS version. This function prints
+ * a list of ciphers without headers/footers.
  *
  * @param cipher_list       list of allowed TLS cipher, or NULL.
  * @param tls_cert_profile  TLS certificate crypto profile name.
+ * @param tls13             Select if <=TLS1.2 or TLS1.3+ ciphers
+ *                          should be shown
  */
 void
-show_available_tls_ciphers(const char *cipher_list,
-                           const char *tls_cert_profile);
+show_available_tls_ciphers_list(const char *cipher_list,
+                                const char *tls_cert_profile,
+                                bool tls13);
 
 /*
  * Show the available elliptic curves in the crypto library

src/openvpn/ssl_common.h +0 -6

index 919ec57c..e5c28508 100644
@@ -554,10 +554,4 @@ struct tls_multi
      *   sessions with the remote peer. */
 };
 
-
-#define SHOW_TLS_CIPHER_LIST_WARNING \
-    "Be aware that that whether a cipher suite in this list can actually work\n" \
-    "depends on the specific setup of both peers. See the man page entries of\n" \
-    "--tls-cipher and --show-tls for more details.\n\n"
-
 #endif /* SSL_COMMON_H_ */

src/openvpn/ssl_mbedtls.c +8 -9

index cb71d55a..3a0b5641 100644
@@ -1340,9 +1340,15 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
 }
 
 void
-show_available_tls_ciphers(const char *cipher_list,
-                           const char *tls_cert_profile)
+show_available_tls_ciphers_list(const char *cipher_list,
+                                const char *tls_cert_profile,
+                                bool tls13)
 {
+    if (tls13)
+    {
+        /* mbed TLS has no TLS 1.3 support currently */
+        return;
+    }
     struct tls_root_ctx tls_ctx;
     const int *ciphers = mbedtls_ssl_list_ciphersuites();

@@ -1355,18 +1361,11 @@ show_available_tls_ciphers(const char *cipher_list,
         ciphers = tls_ctx.allowed_ciphers;
     }
 
-#ifndef ENABLE_SMALL
-    printf("Available TLS Ciphers,\n");
-    printf("listed in order of preference:\n\n");
-#endif
-
     while (*ciphers != 0)
     {
         printf("%s\n", mbedtls_ssl_get_ciphersuite_name(*ciphers));
         ciphers++;
     }
-    printf("\n" SHOW_TLS_CIPHER_LIST_WARNING);
-
     tls_ctx_free(&tls_ctx);
 }

src/openvpn/ssl_openssl.c +38 ~~-18~~

index ba0d1328..99a8719f 100644
@@ -60,6 +60,7 @@
 #include <openssl/pkcs12.h>
 #include <openssl/rsa.h>
 #include <openssl/x509.h>
+#include <openssl/ssl.h>
 #ifndef OPENSSL_NO_EC
 #include <openssl/ec.h>
 #endif

@@ -428,7 +429,8 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 }
 
 void
-convert_tls13_list_to_openssl(char* openssl_ciphers, size_t len, const char *ciphers)
+convert_tls13_list_to_openssl(char *openssl_ciphers, size_t len,
+                              const char *ciphers)
 {
     /*
      * OpenSSL (and official IANA) cipher names have _ in them. We

@@ -1984,14 +1986,11 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
 }
 
 void
-show_available_tls_ciphers(const char *cipher_list,
-                           const char *tls_cert_profile)
+show_available_tls_ciphers_list(const char *cipher_list,
+                                const char *tls_cert_profile,
+                                const bool tls13)
 {
     struct tls_root_ctx tls_ctx;
-    SSL *ssl;
-    const char *cipher_name;
-    const tls_cipher_name_pair *pair;
-    int priority = 0;
 
     tls_ctx.ctx = SSL_CTX_new(SSLv23_method());
     if (!tls_ctx.ctx)

@@ -1999,22 +1998,45 @@ show_available_tls_ciphers(const char *cipher_list,
         crypto_msg(M_FATAL, "Cannot create SSL_CTX object");
     }
 
-    ssl = SSL_new(tls_ctx.ctx);
-    if (!ssl)
+#if (OPENSSL_VERSION_NUMBER >= 0x1010100fL)
+    if (tls13)
     {
-        crypto_msg(M_FATAL, "Cannot create SSL object");
+        SSL_CTX_set_min_proto_version(tls_ctx.ctx, TLS1_3_VERSION);
+    }
+    else
+#endif
+    {
+        SSL_CTX_set_max_proto_version(tls_ctx.ctx, TLS1_2_VERSION);
     }
 
     tls_ctx_set_cert_profile(&tls_ctx, tls_cert_profile);
     tls_ctx_restrict_ciphers(&tls_ctx, cipher_list);
 
-    printf("Available TLS Ciphers,\n");
-    printf("listed in order of preference:\n\n");
-    while ((cipher_name = SSL_get_cipher_list(ssl, priority++)))
+    SSL *ssl = SSL_new(tls_ctx.ctx);
+    if (!ssl)
     {
-        pair = tls_get_cipher_name_pair(cipher_name, strlen(cipher_name));
+        crypto_msg(M_FATAL, "Cannot create SSL object");
+    }
 
-        if (NULL == pair)
+#if (OPENSSL_VERSION_NUMBER < 0x1010000fL)
+    STACK_OF(SSL_CIPHER) *sk = SSL_get_ciphers(ssl);
+#else
+    STACK_OF(SSL_CIPHER) *sk = SSL_get1_supported_ciphers(ssl);
+#endif
+    for (int i=0;i < sk_SSL_CIPHER_num(sk);i++)
+    {
+        const SSL_CIPHER *c = sk_SSL_CIPHER_value(sk, i);
+
+        const char *cipher_name = SSL_CIPHER_get_name(c);
+
+        const tls_cipher_name_pair *pair =
+            tls_get_cipher_name_pair(cipher_name, strlen(cipher_name));
+
+        if (tls13)
+        {
+              printf("%s\n", cipher_name);
+        }
+        else if (NULL == pair)
         {
             /* No translation found, print warning */
             printf("%s (No IANA name known to OpenVPN, use OpenSSL name.)\n", cipher_name);

@@ -2023,14 +2045,15 @@ show_available_tls_ciphers(const char *cipher_list,
         {
             printf("%s\n", pair->iana_name);
         }
-
     }
-    printf("\n" SHOW_TLS_CIPHER_LIST_WARNING);
-
+#if (OPENSSL_VERSION_NUMBER >= 0x1010000fL)

src/openvpn/ssl_openssl.c show_available_tls_ciphers(const char *cipher_list, · line 2059

Quoted 10 lines

src/openvpn/ssl_openssl.c +3 -0

+    sk_SSL_CIPHER_free(sk);
+#endif
     SSL_free(ssl);
     SSL_CTX_free(tls_ctx.ctx);
 }
 
+
 /*
  * Show the Elliptic curves that are available for us to use
  * in the OpenSSL library.

Looks all good now, thanks!

Acked-by Steffan Karger <[email protected]...>

-Steffan

Gert Doering

2018-10-11 18:52 UTC #9 lore raw

[Openvpn-devel] [PATCH applied] Re: Add better support for showing TLS 1.3 ciphersuites in --show-tls

In reply to Arne Schwabe

Your patch has been applied to the master and release/2.4 branch (long term compatibility).

I had to fix one minor bit - it prints

For TLS 1.3 and newer (--tls-ciphersuite):

but that option is called "--tls-ciphersuites"...

I am not really able to make sense of it, though. If I try to specify a TLS 1.3 ciphersuite, to see if it has any effect, all I get is errors in the log ("no valid translation"), and no effect... so maybe something with the --tls-ciphersuites patch is wrong, and this one is only exposing it?

Arne, Steffan, could you have a look? This is "master" with OpenSSL 1.1.1:

   $ src/openvpn/openvpn --verb 4 --tls-ciphersuites TLS_AES_256_GCM_SHA384 --show-tls --tls-cipher TLS-DHE-RSA-WITH-AES-256-CBC-SHA256
   Available TLS Ciphers, listed in order of preference:

For TLS 1.3 and newer (--tls-ciphersuite):

Thu Oct 11 20:32:44 2018 us=310005 No valid translation found for TLS cipher 'TLS_AES_256_GCM_SHA384' TLS_AES_256_GCM_SHA384
TLS_CHACHA20_POLY1305_SHA256
TLS_AES_128_GCM_SHA256

For TLS 1.2 and older (--tls-cipher):

TLS-DHE-RSA-WITH-AES-256-CBC-SHA256

.. so "--tls-cipher" is definitely sticking, while "--tls-ciphersuites" leads to a confusing warning (if --verb is high enough), listing the very same cipher(suite) in the next line...?

commit 7aeabadd69fca0071152c42d58fee0b565f01eb3 (master) commit e8467c8648f699b221004a9f15d48b8e558049f0 (release/2.4)

Author: Arne Schwabe
Date:   Wed Oct 10 17:36:24 2018 +0200

     Add better support for showing TLS 1.3 ciphersuites in --show-tls

     Signed-off-by: Arne Schwabe <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg17723.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

Steffan Karger

Acked-by on v1 1/3

2018-10-06 09:04 UTC #10 lore raw

In reply to Arne Schwabe

Hi,

Two minor nits:

Quoted 2 lines · Arne Schwabe

This makes the tls_ctx_restrict_ciphers function more readable and clean ups the code a bit more.

The signed-off-by tag is missing.

src/openvpn/ssl_openssl.c tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers) · line 379

Quoted 3 lines

"Failed to set restricted TLS cipher list, too long (>%d).",
src/openvpn/ssl_openssl.c +1 -1
-                (int)sizeof(openssl_ciphers)-1);
+                  (int)(len - 1));

This breaks the indenting alignment, there should be two less spaces.

Otherwise this makes sense and does not introduce any logical changes.

Acked-by Steffan Karger <[email protected]...>

-Steffan

Gert Doering

2018-10-06 12:48 UTC #11 lore raw

In reply to Arne Schwabe

Your patch has been applied to the master branch.

Whitespace and signed-off-by line have been adjusted as instructed.

commit 3b9d4d2a9aa89f9c21870a97bcdb42bb007e3ac0

Author: Arne Schwabe
Date:   Sat Oct 6 10:06:15 2018 +0200

     Factor out convert_tls_list_to_openssl method

     Signed-off-by: Arne Schwabe <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/search?l=mid&[email protected]...
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering