[Openvpn-devel,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 <arne@rfc2549.org>
---
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(-)
  

Hi,

On 10-10-18 17:36, Arne Schwabe wrote:
> 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 <arne@rfc2549.org>
> ---
> 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(-)
> 
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index 1b9f19d0..e5e6e85f 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -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)
> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
> index 4f0af286..0b8ccdd3 100644
> --- a/src/openvpn/ssl.c
> +++ b/src/openvpn/ssl.c
> @@ -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.
> diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
> index 781adb94..0bbe1da7 100644
> --- a/src/openvpn/ssl.h
> +++ b/src/openvpn/ssl.h
> @@ -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 */
> diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h
> index 2ae3bcd7..1c244ece 100644
> --- a/src/openvpn/ssl_backend.h
> +++ b/src/openvpn/ssl_backend.h
> @@ -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
> diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
> index 919ec57c..e5c28508 100644
> --- a/src/openvpn/ssl_common.h
> +++ b/src/openvpn/ssl_common.h
> @@ -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_ */
> diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
> index cb71d55a..3a0b5641 100644
> --- a/src/openvpn/ssl_mbedtls.c
> +++ b/src/openvpn/ssl_mbedtls.c
> @@ -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);
>  }
>  
> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> index ba0d1328..99a8719f 100644
> --- a/src/openvpn/ssl_openssl.c
> +++ b/src/openvpn/ssl_openssl.c
> @@ -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)
> +    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 <steffan@karger.me>

-Steffan
  

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 <arne@rfc2549.org>
     Acked-by: Steffan Karger <steffan.karger@fox-it.com>
     Message-Id: <20181010153624.27957-1-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17723.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering