Out of bounds read error in wildcard_match.c

2015-06-27 Thread Hanno Böck
Hi,

I discovered an out of bounds read error in the file wildcard_match.c.
Here's the code:
   /* find the end of each string */
   while (*(++mask));
   mask--;
   while (*(++data));
   data--;

The problem with this: It will search for the end of the strings
(zero-terminated), but it'll only start at position 1, not at position
0 (because the ++ in front of the variable will first increment and
then return the value). However these strings can be empty.

This can be fixed by changing ++mask to mask++ (and same for data),
then there must be a -=2 instead of -- afterwards. See attached patch.


I found this by compiling dovecot with address sanitizer and running
the test suite.

cu,
-- 
Hanno Böck
http://hboeck.de/

mail/jabber: ha...@hboeck.de
GPG: BBB51E42
--- ./dovecot-2.2.18/src/lib/wildcard-match.c	2014-08-20 11:47:58.0 +0200
+++ ./dovecot-2.2.18-patch/src/lib/wildcard-match.c	2015-06-27 18:01:43.179991109 +0200
@@ -35,10 +35,10 @@
 	  return ma[0] == '\0' ? MATCH : NOMATCH;
   }
   /* find the end of each string */
-  while (*(++mask));
-  mask--;
-  while (*(++data));
-  data--;
+  while (*(mask++));
+  mask-=2;
+  while (*(data++));
+  data-=2;
 
   while (data = na) {
 /* If the mask runs out of chars before the string, fall back on


pgpCLvqAFmxkZ.pgp
Description: OpenPGP digital signature


Re: [patch] TLS Handshake failures can crash imap-login

2015-04-26 Thread Hanno Böck
On Sat, 25 Apr 2015 21:36:25 +0300
Teemu Huovila teemu.huov...@dovecot.fi wrote:

 I was unable to reproduce this nor the first report. Could you
 describe your environment in more detail? What version of openssl do
 you have? What is the crash message you are seeing?

both openssl and dovecot latest (1.0.2a, 2.2.16) on a Gentoo.

Please note that it's not dovecot itself that's crashing but
pop3-login/imap-login. You don't note these if you haven't some kind of
segfault reporting.

-- 
Hanno Böck
http://hboeck.de/

mail/jabber: ha...@hboeck.de
GPG: BBB51E42


pgpJw2zezMDy6.pgp
Description: OpenPGP digital signature


Re: [patch] TLS Handshake failures can crash imap-login

2015-04-26 Thread Hanno Böck
On Sun, 26 Apr 2015 21:51:25 +0300
Teemu Huovila teemu.huov...@dovecot.fi wrote:

 Seems the issue might require a version of libopenssl, that does not
 have support for sslv3 compiled in. I have been made aware, that we
 have a fix for Dovecot in the works.

No that's not true. I have explicitely tried that.
You just need to *disable* SSLv3, but that can be done within the
config file.

-- 
Hanno Böck
http://hboeck.de/

mail/jabber: ha...@hboeck.de
GPG: BBB51E42


pgp6z5bjXKq0y.pgp
Description: OpenPGP digital signature


[patch] TLS Handshake failures can crash imap-login

2015-04-24 Thread Hanno Böck
Hi,

I tracked down a tricky bug in dovecot that can cause the imap-login
and pop3-login processes to crash on handshake failures.
This can be tested by disabling SSLv3 in the dovecot config
(ssl_protocols = !SSLv2 !SSLv3) and trying to connect with openssl and
forced sslv3 (openssl s_client -ssl3 -connect localhost:995). This
would cause a crash.


What was going on is this:
In ssl-proxy-openssl.c in line 545 in the function ssl_step() the
function ssl_handshake() is called. There SSL_accept() is called. If
SSL_accept failes - because a client sent an invalid packet or
something the server doesn't support or any other reason -
ssl_handle_error() will be called.

ssl_handle_error() will call ssl_proxy_destroy().
ssl_proxy_destroy() will then call ssl_proxy_flush(). And
ssl_proxy_flush will call ssl_step() again. Here we have a loop. Now
when SSL_accept() gets called again on the same context this is an
invalid state for OpenSSL and it crashes.

What to do? In essence, if ssl_proxy_destroy is called it shouldn't try
to finish the handshake if the handshake hasn't even started due to an
error. This can be done by a simple if check, see attached patch. I
think this should do it.

I have seen that a bug that is probably rootet in this has been posted
here before regarding ssl3-disabled configs:
http://dovecot.org/pipermail/dovecot/2015-March/100188.html

cu,
-- 
Hanno Böck
http://hboeck.de/

mail/jabber: ha...@hboeck.de
GPG: BBB51E42
--- ./dovecot-2.2.16-vanilla/src/login-common/ssl-proxy-openssl.c	2015-01-29 17:01:15.0 +0100
+++ ./dovecot-2.2.16/src/login-common/ssl-proxy-openssl.c	2015-04-24 23:05:21.988752721 +0200
@@ -822,7 +822,7 @@
 	if (proxy-destroyed || proxy-flushing)
 		return;
 	proxy-flushing = TRUE;
-	ssl_proxy_flush(proxy);
+	if (proxy-handshaked) ssl_proxy_flush(proxy);
 	proxy-destroyed = TRUE;
 
 	ssl_proxy_count--;


pgp3MKl7CIB1L.pgp
Description: OpenPGP digital signature


[patch] enable ECDH auto functions based on feature defines, not on version number

2014-07-22 Thread Hanno Böck
Hello,

I recently tried to build my system with libressl instead of openssl.

In dovecot one issue that popped up was that libressl doesn't have the
ECDH auto functions from openssl 1.0.2 beta versions. However as the
#ifdef's in dovecot's code check for the openssl version and libressl's
version numbers are higher the compilation fails there.

Attached is a patch that will change that checks. Instead of checking
for the version number it checks for the availability of the feature
itself (by checking for the define of SSL_CTRL_SET_ECDH_AUTO). This
should make this check more robust and work independently of the
version number of the used openssl instance.

cu,
-- 
Hanno Böck
http://hboeck.de/

mail/jabber: ha...@hboeck.de
GPG: BBB51E42
diff -Naur dovecot-2.2.13/src/lib-ssl-iostream/iostream-openssl-context.c dovecot-2.2.13-1/src/lib-ssl-iostream/iostream-openssl-context.c
--- dovecot-2.2.13/src/lib-ssl-iostream/iostream-openssl-context.c	2014-02-04 22:28:38.0 +0100
+++ dovecot-2.2.13-1/src/lib-ssl-iostream/iostream-openssl-context.c	2014-07-12 18:42:53.592401159 +0200
@@ -416,7 +416,7 @@
 	return 0;
 }
 
-#if defined(HAVE_ECDH)  OPENSSL_VERSION_NUMBER  0x10002000L
+#if defined(HAVE_ECDH)  !defined(SSL_CTRL_SET_ECDH_AUTO)
 static int
 ssl_proxy_ctx_get_pkey_ec_curve_name(const struct ssl_iostream_settings *set,
  int *nid_r, const char **error_r)
@@ -446,7 +446,7 @@
 const struct ssl_iostream_settings *set ATTR_UNUSED,
 const char **error_r ATTR_UNUSED)
 {
-#if defined(HAVE_ECDH)  OPENSSL_VERSION_NUMBER  0x10002000L
+#if defined(HAVE_ECDH)  !defined(SSL_CTRL_SET_ECDH_AUTO)
 	EC_KEY *ecdh;
 	int nid;
 	const char *curve_name;
@@ -459,7 +459,7 @@
 	   used instead of ECDHE, do not reuse the same ECDH key pair for
 	   different sessions. This option improves forward secrecy. */
 	SSL_CTX_set_options(ssl_ctx, SSL_OP_SINGLE_ECDH_USE);
-#if OPENSSL_VERSION_NUMBER = 0x10002000L
+#ifdef SSL_CTRL_SET_ECDH_AUTO
 	/* OpenSSL = 1.0.2 automatically handles ECDH temporary key parameter
 	   selection. */
 	SSL_CTX_set_ecdh_auto(ssl_ctx, 1);
diff -Naur dovecot-2.2.13/src/login-common/ssl-proxy-openssl.c dovecot-2.2.13-1/src/login-common/ssl-proxy-openssl.c
--- dovecot-2.2.13/src/login-common/ssl-proxy-openssl.c	2014-05-07 16:23:24.0 +0200
+++ dovecot-2.2.13-1/src/login-common/ssl-proxy-openssl.c	2014-07-12 18:47:37.074857141 +0200
@@ -125,7 +125,7 @@
 
 static void ssl_proxy_ctx_set_crypto_params(SSL_CTX *ssl_ctx,
 const struct master_service_ssl_settings *set);
-#if defined(HAVE_ECDH)  OPENSSL_VERSION_NUMBER  0x10002000L
+#if defined(HAVE_ECDH)  !defined(SSL_CTRL_SET_ECDH_AUTO)
 static int ssl_proxy_ctx_get_pkey_ec_curve_name(const struct master_service_ssl_settings *set);
 #endif
 
@@ -1024,7 +1024,7 @@
 ssl_proxy_ctx_set_crypto_params(SSL_CTX *ssl_ctx,
 	const struct master_service_ssl_settings *set ATTR_UNUSED)
 {
-#if defined(HAVE_ECDH)  OPENSSL_VERSION_NUMBER  0x10002000L
+#if defined(HAVE_ECDH)  !defined(SSL_CTRL_SET_ECDH_AUTO)
 	EC_KEY *ecdh;
 	int nid;
 	const char *curve_name;
@@ -1037,7 +1037,7 @@
 	   used instead of ECDHE, do not reuse the same ECDH key pair for
 	   different sessions. This option improves forward secrecy. */
 	SSL_CTX_set_options(ssl_ctx, SSL_OP_SINGLE_ECDH_USE);
-#if OPENSSL_VERSION_NUMBER = 0x10002000L
+#ifdef SSL_CTRL_SET_ECDH_AUTO
 	/* OpenSSL = 1.0.2 automatically handles ECDH temporary key parameter
 	   selection. */
 	SSL_CTX_set_ecdh_auto(ssl_ctx, 1);
@@ -1152,7 +1152,7 @@
 	EVP_PKEY_free(pkey);
 }
 
-#if defined(HAVE_ECDH)  OPENSSL_VERSION_NUMBER  0x10002000L
+#if defined(HAVE_ECDH)  !defined(SSL_CTRL_SET_ECDH_AUTO)
 static int
 ssl_proxy_ctx_get_pkey_ec_curve_name(const struct master_service_ssl_settings *set)
 {


signature.asc
Description: PGP signature


Problems with dovecot 2.2.13 and monit

2014-06-16 Thread Hanno Böck
Hello,

When I upgraded my servers to dovecot 2.2.13 the monitoring tool monit
started to send out warnings that it couldn't reach my imap/pop3
servers through ssl any more.
The same problem didn't happen on non-ssl-connections.

According to people on the monit list this is likely a dovecot issue:
https://lists.gnu.org/archive/html/monit-general/2014-06/msg00031.html
Let me quote:
 the root cause of the error is, that dovecot 2.2.13 closes the
 connection if SSL is used in response to LOGOUT command instead of
 sending usual response. When no SSL is enabled, dovecot responses to
 LOGOUT command normally.
[...]
 According to RFC 3501 (http://tools.ietf.org/html/rfc3501), LOGOUT is
 any-state command, where the server MUST send response before closing
 the connection: http://tools.ietf.org/html/rfc3501#section-3.4
 
 = the problem is caused by dovecot 2.2.13 bug ... its behaviour is
 inconsistent (LOGOUT in non-authenticated state works per RFC
 requirement if no SSL is used and doesn't conform to RFC if SSL is
 used). It is possible that the problem is related to their DoS-attack
 modification, which has most probably unexpected side-effect.


Maybe this is related to the DDoS-protection measures that have been
added in dovecot 2.2.13.

Would apprechiate if someone could have a look.


cu,
-- 
Hanno Böck
http://hboeck.de/

mail/jabber: ha...@hboeck.de
GPG: BBB51E42


signature.asc
Description: PGP signature


[Dovecot] separating logs by port

2014-04-25 Thread Hanno Böck
Hi,

I wanted to ask if there's an easy way to log the port in dovecot.

The background is that, as everyone's probably aware, pop3/imap usually
listen on two ports (110/995 for pop3, 143/993 for imap). One port is
the classic port that allows unencrypted and STARTTLS connections,
the other is the legacy SSL port that allows TLS only connections.

The legacy SSL ports are considered deprecated and I'd like to know if
I can deprecate them on my severs. Therefore I'd like to know how many
users use them, but at the moment I can't see which port my users use.

I haven't found an easy way to detect that. The easiest thing would be
if there'd be a way to add the port number to the pop3-login/imap-login
lines in the log files. Any way to do that?

cu,
-- 
Hanno Böck
http://hboeck.de/

mail/jabber: ha...@hboeck.de
GPG: BBB51E42


signature.asc
Description: PGP signature