First step towards really fixing TLS vuln, CVE-2006-5867, still incomplete.

svn path=/branches/BRANCH_6-3/; revision=4962

5 files changed, 145 insertions, 106 deletions

diff --git a/Makefile.am b/Makefile.am
index 27b302e9..31ac75dd 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -39,7 +39,7 @@ libfm_a_SOURCES=	xmalloc.c base64.c rfc822.c report.c rfc2047e.c \
 			servport.c ntlm.h smbbyteorder.h smbdes.h smbmd4.h \
 			smbencrypt.h smbdes.c smbencrypt.c smbmd4.c smbutil.c \
 			libesmtp/gethostbyname.h libesmtp/gethostbyname.c \
-			smbtypes.h fm_getaddrinfo.c
+			smbtypes.h fm_getaddrinfo.c tls.c
 libfm_a_LIBADD=		$(EXTRAOBJ)
 libfm_a_DEPENDENCIES=	$(EXTRAOBJ)
 LDADD	=		libfm.a @LIBINTL@ $(LIBOBJS)
diff --git a/fetchmail.h b/fetchmail.h
index e5e1dda1..3d6866f7 100644
--- a/fetchmail.h
+++ b/fetchmail.h
@@ -767,5 +767,9 @@ int servport(const char *service);
 int fm_getaddrinfo(const char *node, const char *serv, const struct addrinfo *hints, struct addrinfo **res);
 void fm_freeaddrinfo(struct addrinfo *ai);
 
+/* prototypes from tls.c */
+int maybe_tls(struct query *ctl);
+int must_tls(struct query *ctl);
+
 #endif
 /* fetchmail.h ends here */
diff --git a/imap.c b/imap.c
index c2d36370..23db2188 100644
--- a/imap.c
+++ b/imap.c
@@ -369,6 +369,10 @@ static int imap_getauth(int sock, struct query *ctl, char *greeting)
 /* apply for connection authorization */
 {
     int ok = 0;
+#ifdef SSL_ENABLE
+    int got_tls = 0;
+    char *realhost;
+#endif
     (void)greeting;
 
     /*
@@ -393,64 +397,62 @@ static int imap_getauth(int sock, struct query *ctl, char *greeting)
     }
 
 #ifdef SSL_ENABLE
-    if ((!ctl->sslproto || !strcasecmp(ctl->sslproto,"tls1"))
- 	&& !ctl->use_ssl
-	&& strstr(capabilities, "STARTTLS"))
-    {
-	char *realhost = ctl->server.via ? ctl->server.via : ctl->server.pollname;
-
-	/* Use "tls1" rather than ctl->sslproto because tls1 is the only
-	 * protocol that will work with STARTTLS.  Don't need to worry
-	 * whether TLS is mandatory or opportunistic unless SSLOpen() fails
-	 * (see below). */
-	if (gen_transact(sock, "STARTTLS") == PS_SUCCESS
-	    && SSLOpen(sock, ctl->sslcert, ctl->sslkey, "tls1", ctl->sslcertck,
-		       ctl->sslcertpath, ctl->sslfingerprint, realhost,
-		       ctl->server.pollname, &ctl->remotename) != -1)
+    realhost = ctl->server.via ? ctl->server.via : ctl->server.pollname;
+
+    if (maybe_tls(ctl)) {
+	if (strstr(capabilities, "STARTTLS"))
 	{
-	    /*
-	     * RFC 2595 says this:
-	     *
-	     * "Once TLS has been started, the client MUST discard cached
-	     * information about server capabilities and SHOULD re-issue the
-	     * CAPABILITY command.  This is necessary to protect against
-	     * man-in-the-middle attacks which alter the capabilities list prior
-	     * to STARTTLS.  The server MAY advertise different capabilities
-	     * after STARTTLS."
-	     *
-	     * Now that we're confident in our TLS connection we can
-	     * guarantee a secure capability re-probe.
-	     */
-	    capa_probe(sock, ctl);
-	    if (outlevel >= O_VERBOSE)
+	    /* Use "tls1" rather than ctl->sslproto because tls1 is the only
+	     * protocol that will work with STARTTLS.  Don't need to worry
+	     * whether TLS is mandatory or opportunistic unless SSLOpen() fails
+	     * (see below). */
+	    if (gen_transact(sock, "STARTTLS") == PS_SUCCESS
+		    && SSLOpen(sock, ctl->sslcert, ctl->sslkey, "tls1", ctl->sslcertck,
+			ctl->sslcertpath, ctl->sslfingerprint, realhost,
+			ctl->server.pollname, &ctl->remotename) != -1)
 	    {
-		report(stdout, GT_("%s: upgrade to TLS succeeded.\n"), realhost);
+		/*
+		 * RFC 2595 says this:
+		 *
+		 * "Once TLS has been started, the client MUST discard cached
+		 * information about server capabilities and SHOULD re-issue the
+		 * CAPABILITY command.  This is necessary to protect against
+		 * man-in-the-middle attacks which alter the capabilities list prior
+		 * to STARTTLS.  The server MAY advertise different capabilities
+		 * after STARTTLS."
+		 *
+		 * Now that we're confident in our TLS connection we can
+		 * guarantee a secure capability re-probe.
+		 */
+		got_tls = 1;
+		capa_probe(sock, ctl);
+		if (outlevel >= O_VERBOSE)
+		{
+		    report(stdout, GT_("%s: upgrade to TLS succeeded.\n"), realhost);
+		}
 	    }
 	}
-	else if (ctl->sslfingerprint || ctl->sslcertck
-		 || (ctl->sslproto && !strcasecmp(ctl->sslproto, "tls1")))
-	{
-	    /* Config required TLS but we couldn't guarantee it, so we must
-	     * stop. */
-	    report(stderr, GT_("%s: upgrade to TLS failed.\n"), realhost);
-	    return PS_SOCKET;
-	}
-	else
-	{
-	    if (outlevel >= O_VERBOSE)
-	    {
-		report(stdout, GT_("%s: opportunistic upgrade to TLS failed, trying to continue\n"), realhost);
-	    }
-	    /* We don't know whether the connection is in a working state, so
-	     * test by issuing a NOOP. */
-	    if (gen_transact(sock, "NOOP") != PS_SUCCESS)
-	    {
-		/* Not usable.  Empty sslproto to force an unencrypted
-		 * connection on the next attempt, and repoll. */
-		ctl->sslproto = xstrdup("");
-		return PS_REPOLL;
+
+	if (!got_tls) {
+	    if (must_tls(ctl)) {
+		/* Config required TLS but we couldn't guarantee it, so we must
+		 * stop. */
+		report(stderr, GT_("%s: upgrade to TLS failed.\n"), realhost);
+		return PS_SOCKET;
+	    } else {
+		if (outlevel >= O_VERBOSE) {
+		    report(stdout, GT_("%s: opportunistic upgrade to TLS failed, trying to continue\n"), realhost);
+		}
+		/* We don't know whether the connection is in a working state, so
+		 * test by issuing a NOOP. */
+		if (gen_transact(sock, "NOOP") != PS_SUCCESS) {
+		    /* Not usable.  Empty sslproto to force an unencrypted
+		     * connection on the next attempt, and repoll. */
+		    ctl->sslproto = xstrdup("");
+		    return PS_REPOLL;
+		}
+		/* Usable.  Proceed with authenticating insecurely. */
 	    }
-	    /* Usable.  Proceed with authenticating insecurely. */
 	}
     }
 #endif /* SSL_ENABLE */
diff --git a/pop3.c b/pop3.c
index e637ab0e..b41d4e53 100644
--- a/pop3.c
+++ b/pop3.c
@@ -304,6 +304,7 @@ static int pop3_getauth(int sock, struct query *ctl, char *greeting)
 #ifdef SSL_ENABLE
     char *realhost = ctl->server.via ? ctl->server.via : ctl->server.pollname;
     flag connection_may_have_tls_errors = FALSE;
+    flag got_tls = FALSE;
 #endif /* SSL_ENABLE */
 
 #if defined(GSSAPI)
@@ -440,60 +441,59 @@ static int pop3_getauth(int sock, struct query *ctl, char *greeting)
 	}
 
 #ifdef SSL_ENABLE
-	if (has_stls
-	    && !ctl->use_ssl
-	    /* opportunistic   or    forced TLS */
-	    && (!ctl->sslproto || !strcasecmp(ctl->sslproto, "tls1")))
-	{
-	    /* Use "tls1" rather than ctl->sslproto because tls1 is the only
-	     * protocol that will work with STARTTLS.  Don't need to worry
-	     * whether TLS is mandatory or opportunistic unless SSLOpen() fails
-	     * (see below). */
-	    if (gen_transact(sock, "STLS") == PS_SUCCESS
-		&& SSLOpen(sock, ctl->sslcert, ctl->sslkey, "tls1", ctl->sslcertck,
+	if (maybe_tls(ctl)) {
+	   if (has_stls)
+	   {
+	       /* Use "tls1" rather than ctl->sslproto because tls1 is the only
+		* protocol that will work with STARTTLS.  Don't need to worry
+		* whether TLS is mandatory or opportunistic unless SSLOpen() fails
+		* (see below). */
+	       if (gen_transact(sock, "STLS") == PS_SUCCESS
+		       && SSLOpen(sock, ctl->sslcert, ctl->sslkey, "tls1", ctl->sslcertck,
 			   ctl->sslcertpath, ctl->sslfingerprint, realhost,
 			   ctl->server.pollname, &ctl->remotename) != -1)
-	    {
-		/*
-		 * RFC 2595 says this:
-		 *
-		 * "Once TLS has been started, the client MUST discard cached
-		 * information about server capabilities and SHOULD re-issue the
-		 * CAPABILITY command.  This is necessary to protect against
-		 * man-in-the-middle attacks which alter the capabilities list prior
-		 * to STARTTLS.  The server MAY advertise different capabilities
-		 * after STARTTLS."
-		 *
-		 * Now that we're confident in our TLS connection we can
-		 * guarantee a secure capability re-probe.
-		 */
-		(void)capa_probe(sock);
-		if (outlevel >= O_VERBOSE)
-		{
-		    report(stdout, GT_("%s: upgrade to TLS succeeded.\n"), realhost);
-		}
-	    }
-	    else if (ctl->sslfingerprint || ctl->sslcertck
-		    || (ctl->sslproto && !strcasecmp(ctl->sslproto, "tls1")))
-	    {
-		/* Config required TLS but we couldn't guarantee it, so we must
-		 * stop. */
-		report(stderr, GT_("%s: upgrade to TLS failed.\n"), realhost);
-		return PS_SOCKET;
-	    }
-	    else
-	    {
-		/* We don't know whether the connection is usable, and there's
-		 * no command we can reasonably issue to test it (NOOP isn't
-		 * allowed til post-authentication), so leave it in an unknown
-		 * state, mark it as such, and check more carefully if things
-		 * go wrong when we try to authenticate. */
-		connection_may_have_tls_errors = TRUE;
-		if (outlevel >= O_VERBOSE)
-		{
-		    report(stdout, GT_("%s: opportunistic upgrade to TLS failed, trying to continue.\n"), realhost);
-		}
-	    }
+	       {
+		   /*
+		    * RFC 2595 says this:
+		    *
+		    * "Once TLS has been started, the client MUST discard cached
+		    * information about server capabilities and SHOULD re-issue the
+		    * CAPABILITY command.  This is necessary to protect against
+		    * man-in-the-middle attacks which alter the capabilities list prior
+		    * to STARTTLS.  The server MAY advertise different capabilities
+		    * after STARTTLS."
+		    *
+		    * Now that we're confident in our TLS connection we can
+		    * guarantee a secure capability re-probe.
+		    */
+		   got_tls = TRUE;
+		   (void)capa_probe(sock);
+		   if (outlevel >= O_VERBOSE)
+		   {
+		       report(stdout, GT_("%s: upgrade to TLS succeeded.\n"), realhost);
+		   }
+	       }
+	   }
+
+	   if (!got_tls) {
+	       if (must_tls(ctl)) {
+		   /* Config required TLS but we couldn't guarantee it, so we must
+		    * stop. */
+		   report(stderr, GT_("%s: upgrade to TLS failed.\n"), realhost);
+		   return PS_SOCKET;
+	       } else {
+		   /* We don't know whether the connection is usable, and there's
+		    * no command we can reasonably issue to test it (NOOP isn't
+		    * allowed til post-authentication), so leave it in an unknown
+		    * state, mark it as such, and check more carefully if things
+		    * go wrong when we try to authenticate. */
+		   connection_may_have_tls_errors = TRUE;
+		   if (outlevel >= O_VERBOSE)
+		   {
+		       report(stdout, GT_("%s: opportunistic upgrade to TLS failed, trying to continue.\n"), realhost);
+		   }
+	       }
+	   }
 	}
 #endif /* SSL_ENABLE */
 
diff --git a/tls.c b/tls.c
new file mode 100644
index 00000000..2a1fee3e
--- /dev/null
+++ b/tls.c
@@ -0,0 +1,33 @@
+/** \file tls.c - collect common TLS functionality 
+ * \author Matthias Andree
+ * \year 2006
+ */
+
+#include "fetchmail.h"
+
+#ifdef HAVE_STRINGS_H
+#include <strings.h>
+#endif
+
+/** return true if user allowed TLS */
+int maybe_tls(struct query *ctl) {
+#ifdef SSL_ENABLE
+         /* opportunistic  or forced TLS */
+    return (!ctl->sslproto || !strcasecmp(ctl->sslproto,"tls1"))
+	&& !ctl->use_ssl;
+#else
+    return 0;
+#endif
+}
+
+/** return true if user requires TLS, note though that this code must
+ * always use a logical AND with maybe_tls(). */
+int must_tls(struct query *ctl) {
+#ifdef SSL_ENABLE
+    return maybe_tls(ctl)
+	&& (ctl->sslfingerprint || ctl->sslcertck
+		|| (ctl->sslproto && !strcasecmp(ctl->sslproto, "tls1")));
+#else
+    return 0;
+#endif
+}