- ngIRCd GIT Updates - lists.barton.de

Alexander Barton : Correctly handle return code of Handle_Write()
by alex＠arthur.barton.de 23 Aug '13

23 Aug '13

Module: ngircd.git Branch: branch-20.x Commit: d24df64397015732bc6cc1c36a4710fc4db271f1 URL: http://ngircd.barton.de/cgi-bin/gitweb.cgi?p=ngircd.git&a=commit;h=d24df643… Author: Alexander Barton <alex(a)barton.de> Date: Wed Aug 21 01:28:49 2013 +0200 Correctly handle return code of Handle_Write() There have been code paths that ignored the return code of Handle_Write() when sending "notice auth" messages to new clients connecting to the server. But because Handle_Write() would have closed the client connection again if an error occurred, this would have resulted in new errors and assert()'s later on that could have crashed the server (denial of service). Only setups having the configuration option "NoticeAuth" enabled are affected, which is not the default. CVE-2013-5580. (cherry picked from commit 309122017ebc6fff039a7cab1b82f632853d82d5) --- src/ngircd/conn.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/ngircd/conn.c b/src/ngircd/conn.c index 80b085a..e3921f9 100644 --- a/src/ngircd/conn.c +++ b/src/ngircd/conn.c @@ -1547,7 +1547,11 @@ Conn_StartLogin(CONN_ID Idx) #endif (void)Conn_WriteStr(Idx, "NOTICE AUTH :*** Looking up your hostname"); - (void)Handle_Write(Idx); + /* Send buffered data to the client, but break on errors + * because Handle_Write() would have closed the connection + * again in this case! */ + if (!Handle_Write(Idx)) + return; } Resolve_Addr(&My_Connections[Idx].proc_stat, &My_Connections[Idx].addr, @@ -2339,8 +2343,13 @@ cb_Read_Resolver_Result( int r_fd, UNUSED short events ) } #endif - if (Conf_NoticeAuth) - (void)Handle_Write(i); + if (Conf_NoticeAuth) { + /* Send buffered data to the client, but break on + * errors because Handle_Write() would have closed + * the connection again in this case! */ + if (!Handle_Write(i)) + return; + } Class_HandleServerBans(c); }

1 0

Alexander Barton : ngIRCd Release 20.3
by alex＠arthur.barton.de 23 Aug '13

23 Aug '13

Module: ngircd.git Branch: branch-20.x Commit: bb6e2779636aa6d74bbff474880829f0183a3c94 URL: http://ngircd.barton.de/cgi-bin/gitweb.cgi?p=ngircd.git&a=commit;h=bb6e2779… Author: Alexander Barton <alex(a)barton.de> Date: Fri Aug 23 21:54:40 2013 +0200 ngIRCd Release 20.3 --- ChangeLog | 10 +++++++++- NEWS | 11 ++++++++++- contrib/Debian/changelog | 6 ++++++ contrib/ngircd.spec | 2 +- 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 08d337f..5920316 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,10 +9,18 @@ -- ChangeLog -- +ngIRCd 20.3 (2013-08-23) + + - Security: Fix a denial of service bug (server crash) which could happen + when the configuration option "NoticeAuth" is enabled (which is NOT the + default) and ngIRCd failed to send the "notice auth" messages to new + clients connecting to the server (CVE-2013-5580). + ngIRCd 20.2 (2013-02-15) - Security: Fix a denial of service bug in the function handling KICK - commands that could be used by arbitrary users to to crash the daemon. + commands that could be used by arbitrary users to to crash the daemon + (CVE-2013-1747). - WHO command: Use the currently "displayed hostname" (which can be cloaked!) for hostname matching, not the real one. In other words: don't display all the cloaked users on a specific real hostname! diff --git a/NEWS b/NEWS index 38f6029..d092510 100644 --- a/NEWS +++ b/NEWS @@ -9,11 +9,20 @@ -- NEWS -- +ngIRCd 20.3 (2013-08-23) + + - This release is a bugfix release only, without new features. + - Security: Fix a denial of service bug (server crash) which could happen + when the configuration option "NoticeAuth" is enabled (which is NOT the + default) and ngIRCd failed to send the "notice auth" messages to new + clients connecting to the server (CVE-2013-5580). + ngIRCd 20.2 (2013-02-15) - This release is a bugfix release only, without new features. - Security: Fix a denial of service bug in the function handling KICK - commands that could be used by arbitrary users to to crash the daemon. + commands that could be used by arbitrary users to to crash the daemon + (CVE-2013-1747). ngIRCd 20.1 (2013-01-02) diff --git a/contrib/Debian/changelog b/contrib/Debian/changelog index 2e39af0..396d1d0 100644 --- a/contrib/Debian/changelog +++ b/contrib/Debian/changelog @@ -1,3 +1,9 @@ +ngircd (20.3-0ab1) unstable; urgency=high + + * New "upstream" release, fixing a security related bug: ngIRCd 20.3. + + -- Alexander Barton <alex(a)barton.de> Fri, 23 Aug 2013 21:53:21 +0200 + ngircd (20.2-0ab1) unstable; urgency=high * New "upstream" release, fixing a security related bug: ngIRCd 20.2. diff --git a/contrib/ngircd.spec b/contrib/ngircd.spec index e2448a4..0469313 100644 --- a/contrib/ngircd.spec +++ b/contrib/ngircd.spec @@ -1,5 +1,5 @@ %define name ngircd -%define version 20.2 +%define version 20.3 %define release 1 %define prefix %{_prefix}

1 0

Alexander Barton : Correctly handle return code of Handle_Write()
by alex＠arthur.barton.de 23 Aug '13

23 Aug '13

Module: ngircd.git Branch: master Commit: 309122017ebc6fff039a7cab1b82f632853d82d5 URL: http://ngircd.barton.de/cgi-bin/gitweb.cgi?p=ngircd.git&a=commit;h=30912201… Author: Alexander Barton <alex(a)barton.de> Date: Wed Aug 21 01:28:49 2013 +0200 Correctly handle return code of Handle_Write() There have been code paths that ignored the return code of Handle_Write() when sending "notice auth" messages to new clients connecting to the server. But because Handle_Write() would have closed the client connection again if an error occurred, this would have resulted in new errors and assert()'s later on that could have crashed the server (denial of service). Only setups having the configuration option "NoticeAuth" enabled are affected, which is not the default. CVE-2013-5580. --- src/ngircd/conn.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/ngircd/conn.c b/src/ngircd/conn.c index 30dfd09..8d72c1c 100644 --- a/src/ngircd/conn.c +++ b/src/ngircd/conn.c @@ -1668,7 +1668,11 @@ Conn_StartLogin(CONN_ID Idx) #endif (void)Conn_WriteStr(Idx, "NOTICE AUTH :*** Looking up your hostname"); - (void)Handle_Write(Idx); + /* Send buffered data to the client, but break on errors + * because Handle_Write() would have closed the connection + * again in this case! */ + if (!Handle_Write(Idx)) + return; } Resolve_Addr(&My_Connections[Idx].proc_stat, &My_Connections[Idx].addr, @@ -2458,8 +2462,13 @@ cb_Read_Resolver_Result( int r_fd, UNUSED short events ) } #endif - if (Conf_NoticeAuth) - (void)Handle_Write(i); + if (Conf_NoticeAuth) { + /* Send buffered data to the client, but break on + * errors because Handle_Write() would have closed + * the connection again in this case! */ + if (!Handle_Write(i)) + return; + } Class_HandleServerBans(c); }

1 0

Alexander Barton : Enhance log messages on "recursive" connection errors
by alex＠arthur.barton.de 21 Aug '13

21 Aug '13

Module: ngircd.git Branch: master Commit: 8f530eb3154c7d62201c28a53fac5594a956b447 URL: http://ngircd.barton.de/cgi-bin/gitweb.cgi?p=ngircd.git&a=commit;h=8f530eb3… Author: Alexander Barton <alex(a)barton.de> Date: Wed Aug 21 01:15:19 2013 +0200 Enhance log messages on "recursive" connection errors --- src/ngircd/conn.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/ngircd/conn.c b/src/ngircd/conn.c index 53497e3..30dfd09 100644 --- a/src/ngircd/conn.c +++ b/src/ngircd/conn.c @@ -1181,8 +1181,8 @@ Conn_Close( CONN_ID Idx, const char *LogMsg, const char *FwdMsg, bool InformClie /* Is this link already shutting down? */ if( Conn_OPTION_ISSET( &My_Connections[Idx], CONN_ISCLOSING )) { /* Conn_Close() has been called recursively for this link; - * probabe reason: Handle_Write() failed -- see below. */ - LogDebug("Recursive request to close connection: %d", Idx ); + * probable reason: Handle_Write() failed -- see below. */ + LogDebug("Recursive request to close connection %d!", Idx ); return; } @@ -1450,8 +1450,13 @@ Handle_Write( CONN_ID Idx ) if (errno == EAGAIN || errno == EINTR) return true; - Log(LOG_ERR, "Write error on connection %d (socket %d): %s!", - Idx, My_Connections[Idx].sock, strerror(errno)); + if (!Conn_OPTION_ISSET(&My_Connections[Idx], CONN_ISCLOSING)) + Log(LOG_ERR, + "Write error on connection %d (socket %d): %s!", + Idx, My_Connections[Idx].sock, strerror(errno)); + else + LogDebug("Recursive write error on connection %d (socket %d): %s!", + Idx, My_Connections[Idx].sock, strerror(errno)); Conn_Close(Idx, "Write error", NULL, false); return false; } @@ -2459,7 +2464,8 @@ cb_Read_Resolver_Result( int r_fd, UNUSED short events ) Class_HandleServerBans(c); } #ifdef DEBUG - else Log( LOG_DEBUG, "Resolver: discarding result for already registered connection %d.", i ); + else + LogDebug("Resolver: discarding result for already registered connection %d.", i); #endif } /* cb_Read_Resolver_Result */

1 0

Alexander Barton : Add some assert() calls to ng_ipaddr library
by alex＠arthur.barton.de 21 Aug '13

21 Aug '13

Module: ngircd.git Branch: master Commit: d56341c77b19b3e1d4cf13c2f95ec1612e8d52c9 URL: http://ngircd.barton.de/cgi-bin/gitweb.cgi?p=ngircd.git&a=commit;h=d56341c7… Author: Alexander Barton <alex(a)barton.de> Date: Wed Aug 21 00:22:55 2013 +0200 Add some assert() calls to ng_ipaddr library --- src/ipaddr/ng_ipaddr.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/ipaddr/ng_ipaddr.h b/src/ipaddr/ng_ipaddr.h index 1f209ef..8f73760 100644 --- a/src/ipaddr/ng_ipaddr.h +++ b/src/ipaddr/ng_ipaddr.h @@ -47,6 +47,7 @@ typedef struct NG_IP_ADDR_DONTUSE ng_ipaddr_t; static inline int ng_ipaddr_af(const ng_ipaddr_t *a) { + assert(a != NULL); #ifdef WANT_IPV6 return a->sa.sa_family; #else @@ -59,6 +60,7 @@ ng_ipaddr_af(const ng_ipaddr_t *a) static inline socklen_t ng_ipaddr_salen(const ng_ipaddr_t *a) { + assert(a != NULL); #ifdef WANT_IPV6 assert(a->sa.sa_family == AF_INET || a->sa.sa_family == AF_INET6); if (a->sa.sa_family == AF_INET6) @@ -75,11 +77,14 @@ ng_ipaddr_getport(const ng_ipaddr_t *a) #ifdef WANT_IPV6 int af = a->sa.sa_family; + assert(a != NULL); assert(af == AF_INET || af == AF_INET6); if (af == AF_INET6) return ntohs(a->sin6.sin6_port); #endif /* WANT_IPV6 */ + + assert(a != NULL); assert(a->sin4.sin_family == AF_INET); return ntohs(a->sin4.sin_port); } @@ -109,12 +114,15 @@ GLOBAL bool ng_ipaddr_tostr_r PARAMS((const ng_ipaddr_t *addr, char *dest)); static inline const char* ng_ipaddr_tostr(const ng_ipaddr_t *addr) { + assert(addr != NULL); return inet_ntoa(addr->sin4.sin_addr); } static inline bool ng_ipaddr_tostr_r(const ng_ipaddr_t *addr, char *d) { + assert(addr != NULL); + assert(d != NULL); strlcpy(d, inet_ntoa(addr->sin4.sin_addr), NG_INET_ADDRSTRLEN); return true; }

1 0

Alexander Barton : Update ChangeLog file
by alex＠arthur.barton.de 20 Aug '13

20 Aug '13

Module: ngircd.git Branch: master Commit: 212d99146d4a3681976450b5ff0dfa57e1d2e44f URL: http://ngircd.barton.de/cgi-bin/gitweb.cgi?p=ngircd.git&a=commit;h=212d9914… Author: Alexander Barton <alex(a)barton.de> Date: Tue Aug 20 13:08:43 2013 +0200 Update ChangeLog file --- ChangeLog | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index af4bc08..9fe53c7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -11,6 +11,9 @@ ngIRCd 21 + - Enforce "penalty times" on error conditions more consistently and in + more places. Now most error codes sent back from the IRC server to the + client should result in a 2 second "penalty". - Implement a new configuration option "AllowedChannelTypes" that lists all allowed channel types (channel prefixes) for newly created channels on the local server. By default, all supported channel types are allowed. @@ -85,8 +88,8 @@ ngIRCd 21 of an IRC service id displayed in the output. - Exit message: use singular & plural :-) - autogen.sh: Check for autoconf/automake wrapper scripts - - Add missing punctuation marks in log messages and adjust some - severity levels. + - Add missing punctuation marks in log messages, adjust some severity + levels, and make SSL-related messages more readable. - AUTHORS file: Update list of contributors. - Update systemd(8) example configuration files in ./contrib/ directory: the "ngircd.service" file now uses the "forking" service type which

1 0

Alexander Barton : Debian init script: test for binary after reading defaults
by alex＠arthur.barton.de 19 Aug '13

19 Aug '13

Module: ngircd.git Branch: master Commit: e2f09213bcef479e7b3a35d67b1cc6b76f2205fb URL: http://ngircd.barton.de/cgi-bin/gitweb.cgi?p=ngircd.git&a=commit;h=e2f09213… Author: Alexander Barton <alex(a)barton.de> Date: Mon Aug 19 23:31:10 2013 +0200 Debian init script: test for binary after reading defaults This allows the system administrator to overwrite the DAEMON variable in /etc/defaults/<name> and to use this init script even when the default "/usr/sbin/ngircd" doesn't exist on the system. --- contrib/Debian/ngircd.default | 2 -- contrib/Debian/ngircd.init | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/contrib/Debian/ngircd.default b/contrib/Debian/ngircd.default index 5b6c93f..add278f 100644 --- a/contrib/Debian/ngircd.default +++ b/contrib/Debian/ngircd.default @@ -1,8 +1,6 @@ # # Defaults for ngIRCd start and stop script # -# $Id: ngircd.default,v 1.1 2003/12/31 17:20:11 alex Exp $ -# # Parameters to pass to the ngircd daemon on startup, see ngircd(8) for # possible options (default: empty). diff --git a/contrib/Debian/ngircd.init b/contrib/Debian/ngircd.init index 6a418d7..8639e22 100755 --- a/contrib/Debian/ngircd.init +++ b/contrib/Debian/ngircd.init @@ -1,7 +1,7 @@ #!/bin/sh # # ngIRCd start and stop script for Debian-based systems -# Copyright 2008-2010 Alexander Barton <alex(a)barton.de> +# Copyright 2008-2013 Alexander Barton <alex(a)barton.de> # ### BEGIN INIT INFO @@ -24,13 +24,13 @@ PARAMS="" STARTTIME=1 DIETIME=10 -test -x $DAEMON || exit 5 - test -h "$0" && me=`readlink $0` || me="$0" BASENAME=`basename $me` test -r /etc/default/$BASENAME && . /etc/default/$BASENAME +test -x $DAEMON || exit 5 + # LSB compatibility functions that become used if there is no local # include file available. log_daemon_msg() {

1 0

Alexander Barton : Make SSL-related log messages more readable
by alex＠arthur.barton.de 17 Aug '13

17 Aug '13

Module: ngircd.git Branch: master Commit: a919e02ba1670277fd5a501e8c112b7c5e9771ac URL: http://ngircd.barton.de/cgi-bin/gitweb.cgi?p=ngircd.git&a=commit;h=a919e02b… Author: Alexander Barton <alex(a)barton.de> Date: Wed Aug 14 10:56:09 2013 +0200 Make SSL-related log messages more readable - Don't use internal function names but describe the error. - Streamline wording, use "SSL" for SSL and TLS. - Streamline punctuation. --- src/ngircd/conn-ssl.c | 108 ++++++++++++++++++++++++++++++------------------- 1 file changed, 66 insertions(+), 42 deletions(-) Diff: http://ngircd.barton.de/cgi-bin/gitweb.cgi?p=ngircd.git&a=commitdiff;h=a919…

1 0

Alexander Barton : ConnSSL_HandleError: Code cleanup, more documentation
by alex＠arthur.barton.de 17 Aug '13

17 Aug '13

Module: ngircd.git Branch: master Commit: dd4c60cf392ecabeeadd616c7fc561af87a7cef5 URL: http://ngircd.barton.de/cgi-bin/gitweb.cgi?p=ngircd.git&a=commit;h=dd4c60cf… Author: Alexander Barton <alex(a)barton.de> Date: Mon Aug 12 23:12:37 2013 +0200 ConnSSL_HandleError: Code cleanup, more documentation --- src/ngircd/conn-ssl.c | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/src/ngircd/conn-ssl.c b/src/ngircd/conn-ssl.c index 0bca933..1721aac 100644 --- a/src/ngircd/conn-ssl.c +++ b/src/ngircd/conn-ssl.c @@ -488,16 +488,23 @@ ConnSSL_PrepareConnect(CONNECTION *c, UNUSED CONF_SERVER *s) } -/* - Check an Handle Error return code after failed calls to ssl/tls functions. - OpenSSL: - SSL_connect(), SSL_accept(), SSL_do_handshake(), SSL_read(), SSL_peek(), or SSL_write() on ssl. - GNUTLS: - gnutlsssl_read(), gnutls_write() or gnutls_handshake(). - Return: -1 on fatal error, 0 if we can try again later. +/** + * Check and handle error return codes after failed calls to SSL/TLS functions. + * + * OpenSSL: + * SSL_connect(), SSL_accept(), SSL_do_handshake(), SSL_read(), SSL_peek(), or + * SSL_write() on ssl. + * + * GNUTLS: + * gnutlsssl_read(), gnutls_write() or gnutls_handshake(). + * + * @param c The connection handle. + * @prarm code The return code. + * @param fname The name of the function in which the error occurred. + * @return -1 on fatal errors, 0 if we can try again later. */ static int -ConnSSL_HandleError( CONNECTION *c, const int code, const char *fname ) +ConnSSL_HandleError(CONNECTION * c, const int code, const char *fname) { #ifdef HAVE_LIBSSL int ret = SSL_ERROR_SYSCALL; @@ -518,21 +525,22 @@ ConnSSL_HandleError( CONNECTION *c, const int code, const char *fname ) case SSL_ERROR_ZERO_RETURN: LogDebug("TLS/SSL connection shut down normally"); break; - /* - SSL_ERROR_WANT_CONNECT, SSL_ERROR_WANT_ACCEPT, SSL_ERROR_WANT_X509_LOOKUP - */ case SSL_ERROR_SYSCALL: + /* SSL_ERROR_WANT_CONNECT, SSL_ERROR_WANT_ACCEPT, + * and SSL_ERROR_WANT_X509_LOOKUP */ sslerr = ERR_get_error(); if (sslerr) { - Log( LOG_ERR, "%s: %s", fname, ERR_error_string(sslerr, NULL )); + Log(LOG_ERR, "%s: %s", fname, + ERR_error_string(sslerr, NULL)); } else { switch (code) { /* EOF that violated protocol */ case 0: - Log(LOG_ERR, "%s: Client Disconnected", fname ); + Log(LOG_ERR, "%s: Client Disconnected", fname); break; - case -1: /* low level socket I/O error, check errno */ - Log(LOG_ERR, "%s: %s", fname, strerror(real_errno)); + case -1: /* low level socket I/O error, check errno */ + Log(LOG_ERR, "%s: %s", fname, + strerror(real_errno)); } } break; @@ -540,7 +548,7 @@ ConnSSL_HandleError( CONNECTION *c, const int code, const char *fname ) LogOpenSSLError("TLS/SSL Protocol Error", fname); break; default: - Log( LOG_ERR, "%s: Unknown error %d!", fname, ret); + Log(LOG_ERR, "%s: Unknown error %d!", fname, ret); } ConnSSL_Free(c); return -1;

1 0

Alexander Barton : Merge branch 'ssl-log-messages'
by alex＠arthur.barton.de 17 Aug '13

17 Aug '13

Module: ngircd.git Branch: master Commit: c8b12af1d2d155ec79dc2044a4ff177cf07de4fe URL: http://ngircd.barton.de/cgi-bin/gitweb.cgi?p=ngircd.git&a=commit;h=c8b12af1… Author: Alexander Barton <alex(a)barton.de> Date: Sat Aug 17 22:05:59 2013 +0200 Merge branch 'ssl-log-messages' * ssl-log-messages: Make SSL-related log messages more readable ConnSSL_HandleError: Code cleanup, more documentation ---

1 0