From afb6442533dc7475ed61642c3f5b295db1e6f561 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Sun, 9 Jun 2019 23:40:21 -0700 Subject: [PATCH] Fix DTLS bug when lacking deprecated APIs HAVE_DTLS12 is for DTLSv1_method. This causes dtls_method to be NULL and crash. [dwmw2: Rework it quite a bit more] Signed-off-by: Rosen Penev Signed-off-by: David Woodhouse --- configure.ac | 17 ++++++++++++++--- openssl-dtls.c | 49 ++++++++++++++++++++++++++----------------------- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/configure.ac b/configure.ac index 02096c51..f7557933 100644 --- a/configure.ac +++ b/configure.ac @@ -455,11 +455,22 @@ case "$ssl_library" in AC_DEFINE(HAVE_DTLS1_STOP_TIMER, [1], [OpenSSL has dtls1_stop_timer() function])], [AC_MSG_RESULT(no)]) - AC_MSG_CHECKING([for DTLSv1_2_client_method() in OpenSSL]) + # DTLS_client_method() and DTLSv1_2_client_method() were both added between + # OpenSSL v1.0.1 and v1.0.2. DTLSV1.2_client_method() was later deprecated + # in v1.1.0 so we use DTLS_client_method() as our check for DTLSv1.2 support + # and that's what we actually use in openssl-dtls.c too. + AC_MSG_CHECKING([for DTLS_client_method() in OpenSSL]) AC_LINK_IFELSE([AC_LANG_PROGRAM([#include ], - [DTLSv1_2_client_method();])], + [DTLS_client_method();])], [AC_MSG_RESULT(yes) - AC_DEFINE(HAVE_DTLS12, [1], [OpenSSL has DTLSv1_2_client_method() function])], + AC_DEFINE(HAVE_DTLS12, [1], [OpenSSL has DTLS_client_method() function])], + [AC_MSG_RESULT(no)]) + + AC_MSG_CHECKING([for SSL_CTX_set_min_proto_version() in OpenSSL]) + AC_LINK_IFELSE([AC_LANG_PROGRAM([#include ], + [SSL_CTX_set_min_proto_version((void *)0, 0);])], + [AC_MSG_RESULT(yes) + AC_DEFINE(HAVE_SSL_CTX_PROTOVER, [1], [OpenSSL has SSL_CTX_set_min_proto_version() function])], [AC_MSG_RESULT(no)]) AC_CHECK_FUNC(HMAC_CTX_copy, diff --git a/openssl-dtls.c b/openssl-dtls.c index 5086440f..9e3c5d46 100644 --- a/openssl-dtls.c +++ b/openssl-dtls.c @@ -332,6 +332,7 @@ int start_dtls_handshake(struct openconnect_info *vpninfo, int dtls_fd) const char *cipher = vpninfo->dtls_cipher; #ifdef HAVE_DTLS12 + /* These things should never happen unless they're supported */ if (vpninfo->cisco_dtls12) { dtlsver = DTLS1_2_VERSION; } else if (!strcmp(cipher, "OC-DTLS1_2-AES128-GCM")) { @@ -349,16 +350,16 @@ int start_dtls_handshake(struct openconnect_info *vpninfo, int dtls_fd) if (!vpninfo->dtls_ctx) { #ifdef HAVE_DTLS12 + /* If we can use SSL_CTX_set_min_proto_version, do so. */ dtls_method = DTLS_client_method(); #endif -#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) +#ifndef HAVE_SSL_CTX_PROTOVER + /* If !HAVE_DTLS12, dtlsver *MUST* be DTLS1_BAD_VER because it's set + * at the top of the function and nothing can change it. */ if (dtlsver == DTLS1_BAD_VER) dtls_method = DTLSv1_client_method(); -#ifdef HAVE_DTLS12 - else if (dtlsver == DTLS1_2_VERSION) - dtls_method = DTLSv1_2_client_method(); -#endif #endif + vpninfo->dtls_ctx = SSL_CTX_new(dtls_method); if (!vpninfo->dtls_ctx) { vpn_progress(vpninfo, PRG_ERR, @@ -367,24 +368,26 @@ int start_dtls_handshake(struct openconnect_info *vpninfo, int dtls_fd) vpninfo->dtls_attempt_period = 0; return -EINVAL; } - if (dtlsver) { -#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) - if (dtlsver == DTLS1_BAD_VER) - SSL_CTX_set_options(vpninfo->dtls_ctx, SSL_OP_CISCO_ANYCONNECT); -#else - if (!SSL_CTX_set_min_proto_version(vpninfo->dtls_ctx, dtlsver) || - !SSL_CTX_set_max_proto_version(vpninfo->dtls_ctx, dtlsver)) { - vpn_progress(vpninfo, PRG_ERR, - _("Set DTLS CTX version failed\n")); - openconnect_report_ssl_errors(vpninfo); - SSL_CTX_free(vpninfo->dtls_ctx); - vpninfo->dtls_ctx = NULL; - vpninfo->dtls_attempt_period = 0; - return -EINVAL; - } +#ifdef HAVE_SSL_CTX_PROTOVER + if (dtlsver && + (!SSL_CTX_set_min_proto_version(vpninfo->dtls_ctx, dtlsver) || + !SSL_CTX_set_max_proto_version(vpninfo->dtls_ctx, dtlsver))) { + vpn_progress(vpninfo, PRG_ERR, + _("Set DTLS CTX version failed\n")); + openconnect_report_ssl_errors(vpninfo); + SSL_CTX_free(vpninfo->dtls_ctx); + vpninfo->dtls_ctx = NULL; + vpninfo->dtls_attempt_period = 0; + return -EINVAL; + } +#else /* !HAVE_SSL_CTX_PROTOVER */ + /* If we used the legacy version-specific methods, we need the special + * way to make TLSv1_client_method() do DTLS1_BAD_VER. */ + if (dtlsver == DTLS1_BAD_VER) + SSL_CTX_set_options(vpninfo->dtls_ctx, SSL_OP_CISCO_ANYCONNECT); #endif #if defined (HAVE_DTLS12) && !defined(OPENSSL_NO_PSK) - } else { + if (!dtlsver) { SSL_CTX_set_psk_client_callback(vpninfo->dtls_ctx, psk_callback); /* For PSK we override the DTLS master secret with one derived * from the HTTPS session. */ @@ -401,9 +404,9 @@ int start_dtls_handshake(struct openconnect_info *vpninfo, int dtls_fd) } /* For SSL_CTX_set_cipher_list() */ cipher = "PSK"; - -#endif } +#endif /* OPENSSL_NO_PSK */ + /* If we don't readahead, then we do short reads and throw away the tail of data packets. */ SSL_CTX_set_read_ahead(vpninfo->dtls_ctx, 1); -- 2.17.1