openconnect: cannot establish DTLS if built against OpenSSL

[dengqf6]

Maintainer: @nmav
Version: OpenConnect 8.03, OpenSSL 1.1.1c / GNUTLS 3.6.8

Description:
When built against OpenSSL, OpenConnect cannot establish DTLS

daemon.info openconnect[13777]: Got CONNECT response: HTTP/1.1 200 CONNECTED
daemon.info openconnect[13777]: CSTP connected. DPD 45, Keepalive 32400
daemon.notice openconnect[13777]: Initialise DTLSv1 CTX failed
daemon.notice openconnect[13777]: 2010610384:error:140A90C4:SSL routines:SSL_CTX_new:null ssl method passed:ssl/ssl_lib.c:2904:
daemon.info openconnect[13777]: Connected as *.*.*.* , using SSL, with DTLS disabled

GNUTLS is fine

daemon.info openconnect[4659]: Got CONNECT response: HTTP/1.1 200 CONNECTED
daemon.info openconnect[4659]: CSTP connected. DPD 45, Keepalive 32400
daemon.info openconnect[4659]: Connected as *.*.*.* , using SSL, with DTLS in progress
daemon.info openconnect[4659]: Established DTLS connection (using GnuTLS). Ciphersuite (DTLS1.2)-(PSK)-(AES-128-GCM).

[neheb]

Are you compiling without deprecated APIs?

[dengqf6]

No. It doesn't even build without deprecated APIs

[neheb]

Found the error (probably). Try this patch:

--- a/configure.ac
+++ b/configure.ac
@@ -455,11 +455,11 @@ 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])
+	AC_MSG_CHECKING([for DTLS_client_method() in OpenSSL])
 	AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <openssl/ssl.h>],
-					[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_CHECK_FUNC(HMAC_CTX_copy,

Or the full one:

diff --git a/configure.ac b/configure.ac
index 02096c5..443001b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -455,11 +455,11 @@ 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])
+	AC_MSG_CHECKING([for DTLS_client_method() in OpenSSL])
 	AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <openssl/ssl.h>],
-					[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_CHECK_FUNC(HMAC_CTX_copy,
diff --git a/openssl.c b/openssl.c
index 0cc6779..c268a31 100644
--- a/openssl.c
+++ b/openssl.c
@@ -1879,10 +1879,12 @@ int openconnect_init_ssl(void)
 	if (ret)
 		return ret;
 #endif
+#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
 	SSL_library_init();
 	ERR_clear_error();
 	SSL_load_error_strings();
 	OpenSSL_add_all_algorithms();
+#endif
 	return 0;
 }
 
diff --git a/tests/bad_dtls_test.c b/tests/bad_dtls_test.c
index ac8d3f1..c123c8f 100644
--- a/tests/bad_dtls_test.c
+++ b/tests/bad_dtls_test.c
@@ -752,8 +752,10 @@ int main(int argc, char *argv[])
     int ret;
     int i;
 
+#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
     SSL_library_init();
     SSL_load_error_strings();
+#endif
 
     RAND_bytes(session_id, sizeof(session_id));
     RAND_bytes(master_secret, sizeof(master_secret));
@@ -910,8 +912,10 @@ int main(int argc, char *argv[])
         printf("Cisco BadDTLS test: FAILED\n");
     }
 
+#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
     ERR_free_strings();
     EVP_cleanup();
+#endif
 
     return testresult?0:1;
 }

[neheb]

The issue is here: https://github.com/openconnect/openconnect/blob/master/openssl-dtls.c#L352

HAVE_DTLS12 ends up being undefined, which makes https://github.com/openconnect/openconnect/blob/master/openssl-dtls.c#L362 use an uninitialized value, thereby crashing the program.

[nmav]

Could you please open an bug on the upstream project? https://gitlab.com/openconnect/openconnect

[neheb]

https://gitlab.com/openconnect/openconnect/merge_requests/51

[nmav]

Thank you!

[dengqf6]

@neheb What patches should I apply? Would you please open a PR here?

[neheb]

https://gitlab.com/openconnect/openconnect/commit/460c060dda115bc8066bb4b955453c673459b6cc

https://gitlab.com/openconnect/openconnect/commit/afb6442533dc7475ed61642c3f5b295db1e6f561

[dwmw2]

Take https://gitlab.com/openconnect/openconnect/commit/97cafd182f5a5c2d13f57d7faeac8432aea9bbf8 too.

[neheb]

@dwmw2 a bit off topic. I noticed the download URL works with ftp:// but not http:// . HTTP redirects to some spam.

[dwmw2]

Hm? The FTP URL should be just fine as-is. It does indeed only work with ftp at the start, just as it only works with infradead in the middle. Manually change it to go elsewhere... and you'll end up elsewhere :)

[neheb]

Fix was merged and backported to 19.07. @nmav hope you don't mind.

[nmav]

Of course not, thank you!