[svn-commits] mjordan: trunk r426003 - in /trunk: ./ main/tcptls.c res/res_xmpp.c

Mon Oct 20 09:20:18 CDT 2014

Author: mjordan
Date: Mon Oct 20 09:20:15 2014
New Revision: 426003

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=426003
Log:
AST-2014-011: Fix POODLE security issues

There are two aspects to the vulnerability:
(1) res_jabber/res_xmpp use SSLv3 only. This patch updates the module to use
    TLSv1+. At this time, it does not refactor res_jabber/res_xmpp to use the
    TCP/TLS core, which should be done as an improvement at a latter date.
(2) The TCP/TLS core, when tlsclientmethod/sslclientmethod is left unspecified,
    will default to the OpenSSL SSLv23_method. This method allows for all
    ecnryption methods, including SSLv2/SSLv3. A MITM can exploit this by
    forcing a fallback to SSLv3, which leaves the server vulnerable to POODLE.
    This patch adds WARNINGS if a user uses SSLv2/SSLv3 in their configuration,
    and explicitly disables SSLv2/SSLv3 if using SSLv23_method.

For TLS clients, Asterisk will default to TLSv1+ and WARN if SSLv2 or SSLv3 is
explicitly chosen. For TLS servers, Asterisk will no longer support SSLv2 or
SSLv3.

Much thanks to abelbeck for reporting the vulnerability and providing a patch
for the res_jabber/res_xmpp modules.

Review: https://reviewboard.asterisk.org/r/4096/

ASTERISK-24425 #close
Reported by: abelbeck
Tested by: abelbeck, opsmonitor, gtjoseph
patches:
  asterisk-1.8-jabber-tls.patch uploaded by abelbeck (License 5903)
  asterisk-11-jabber-xmpp-tls.patch uploaded by abelbeck (License 5903)
  AST-2014-011-1.8.diff uploaded by mjordan (License 6283)
  AST-2014-011-11.diff uploaded by mjordan (License 6283)
........

Merged revisions 425987 from http://svn.asterisk.org/svn/asterisk/branches/12
........

Merged revisions 425991 from http://svn.asterisk.org/svn/asterisk/branches/13

Modified:
    trunk/   (props changed)
    trunk/main/tcptls.c
    trunk/res/res_xmpp.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-13-merged' - no diff available.

Modified: trunk/main/tcptls.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/tcptls.c?view=diff&rev=426003&r1=426002&r2=426003
==============================================================================

--- trunk/main/tcptls.c (original)
+++ trunk/main/tcptls.c Mon Oct 20 09:20:15 2014
@@ -747,6 +747,8 @@
 	cfg->enabled = 0;
 	return 0;
 #else
+	int disable_ssl = 0;
+ 
 	if (!cfg->enabled) {
 		return 0;
 	}
@@ -762,22 +764,21 @@
 	if (client) {
 #ifndef OPENSSL_NO_SSL2
 		if (ast_test_flag(&cfg->flags, AST_SSL_SSLV2_CLIENT)) {
+			ast_log(LOG_WARNING, "Usage of SSLv2 is discouraged due to known vulnerabilities. Please use 'tlsv1' or leave the TLS method unspecified!\n");
 			cfg->ssl_ctx = SSL_CTX_new(SSLv2_client_method());
 		} else
 #endif
 		if (ast_test_flag(&cfg->flags, AST_SSL_SSLV3_CLIENT)) {
+			ast_log(LOG_WARNING, "Usage of SSLv3 is discouraged due to known vulnerabilities. Please use 'tlsv1' or leave the TLS method unspecified!\n");
 			cfg->ssl_ctx = SSL_CTX_new(SSLv3_client_method());
 		} else if (ast_test_flag(&cfg->flags, AST_SSL_TLSV1_CLIENT)) {
 			cfg->ssl_ctx = SSL_CTX_new(TLSv1_client_method());
 		} else {
-			/* SSLv23_client_method() sends SSLv2, this was the original
-			 * default for ssl clients before the option was given to
-			 * pick what protocol a client should use.  In order not
-			 * to break expected behavior it remains the default. */
+			disable_ssl = 1;
 			cfg->ssl_ctx = SSL_CTX_new(SSLv23_client_method());
 		}
 	} else {
-		/* SSLv23_server_method() supports TLSv1, SSLv2, and SSLv3 inbound connections. */
+		disable_ssl = 1;
 		cfg->ssl_ctx = SSL_CTX_new(SSLv23_server_method());
 	}
 
@@ -785,6 +786,17 @@
 		ast_debug(1, "Sorry, SSL_CTX_new call returned null...\n");
 		cfg->enabled = 0;
 		return 0;
+	}
+
+	/* Due to the POODLE vulnerability, completely disable
+	 * SSLv2 and SSLv3 if we are not explicitly told to use
+	 * them. SSLv23_*_method supports TLSv1+.
+	 */
+	if (disable_ssl) {
+		long ssl_opts;
+
+		ssl_opts = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3;
+		SSL_CTX_set_options(cfg->ssl_ctx, ssl_opts);
 	}
 
 	SSL_CTX_set_verify(cfg->ssl_ctx,

Modified: trunk/res/res_xmpp.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/res_xmpp.c?view=diff&rev=426003&r1=426002&r2=426003
==============================================================================
--- trunk/res/res_xmpp.c (original)
+++ trunk/res/res_xmpp.c Mon Oct 20 09:20:15 2014
@@ -2637,6 +2637,7 @@
 {
 #ifdef HAVE_OPENSSL
 	int sock;
+	long ssl_opts;
 #endif
 
 	if (!strcmp(iks_name(node), "success")) {
@@ -2655,10 +2656,13 @@
 	ast_log(LOG_ERROR, "Somehow we managed to try to start TLS negotiation on client '%s' without OpenSSL support, disconnecting\n", client->name);
 	return -1;
 #else
-	client->ssl_method = SSLv3_method();
+	client->ssl_method = SSLv23_method();
 	if (!(client->ssl_context = SSL_CTX_new((SSL_METHOD *) client->ssl_method))) {
 		goto failure;
 	}
+
+	ssl_opts = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3;
+	SSL_CTX_set_options(client->ssl_context, ssl_opts);
 
 	if (!(client->ssl_session = SSL_new(client->ssl_context))) {
 		goto failure;


