Strengthen APOP a bit (validate RFC-822 syntax) in order to fend off

Leurent-style MITM attacks which are based on MD5 and APOP weaknesses.

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

6 files changed, 183 insertions, 12 deletions

diff --git a/Makefile.am b/Makefile.am
index 00c5a3e6..28338b7c 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 tls.c
+			smbtypes.h fm_getaddrinfo.c tls.c rfc822valid.c
 libfm_a_LIBADD=		$(EXTRAOBJ)
 libfm_a_DEPENDENCIES=	$(EXTRAOBJ)
 LDADD	=		libfm.a @LIBINTL@ $(LIBOBJS)
@@ -100,11 +100,13 @@ if NEED_GETADDRINFO
 fetchmail_SOURCES += libesmtp/getaddrinfo.h libesmtp/getaddrinfo.c
 endif
 
-check_PROGRAMS +=	rfc822 unmime netrc rfc2047e mxget
+check_PROGRAMS +=	rfc822 unmime netrc rfc2047e mxget rfc822valid
 
 rfc2047e_CFLAGS=	-DTEST
 
-rfc822_CFLAGS=	-DMAIN
+rfc822valid_CFLAGS=	-DTEST
+
+rfc822_CFLAGS=		-DMAIN
 
 unmime_SOURCES=	unmime.c
 unmime_CFLAGS=	-DSTANDALONE -DHAVE_CONFIG_H -I$(builddir)
diff --git a/NEWS b/NEWS
index de7064b7..4cc6f944 100644
--- a/NEWS
+++ b/NEWS
@@ -44,6 +44,17 @@ be removed from a 6.4.0 or newer release.)
 
 fetchmail 6.3.8 (not yet released):
 
+# SECURITY STRENGTHENING:
+* Make the APOP challenge parser more distrustful and have it reject challenges
+  that do not conform to RFC-822 msg-id format, in the hope to make mounting
+  man-in-the-middle attacks (MITM) against APOP a bit more difficult.
+
+  APOP is claimed insecure by Gaëtan Leurent for MITM scenarios for typical
+  setups: based on MD5 collisions, it is purportedly possible to recover the
+  first three characters of the shared secret (password), which would then make
+  recovery of the shared secret a matter of hours or minutes; this would then
+  enable the attacker to impersonate the client vis-à-vis the server.
+
 # BUG FIXES:
 * Fix pluralization of oversized-message warning mails.
 * Fix manual page: --sslcheck -> --sslcertck, and do not set trailing 
diff --git a/fetchmail.h b/fetchmail.h
index 0893520a..605bcb05 100644
--- a/fetchmail.h
+++ b/fetchmail.h
@@ -770,5 +770,8 @@ void fm_freeaddrinfo(struct addrinfo *ai);
 int maybe_tls(struct query *ctl);
 int must_tls(struct query *ctl);
 
+/* prototype from rfc822valid.c */
+int rfc822_valid_msgid(const unsigned char *);
+
 #endif
 /* fetchmail.h ends here */
diff --git a/fetchmail.man b/fetchmail.man
index 5de1c484..9423ab8e 100644
--- a/fetchmail.man
+++ b/fetchmail.man
@@ -237,6 +237,7 @@ Post Office Protocol 2 (legacy, to be removed from future release)
 Post Office Protocol 3
 .IP APOP
 Use POP3 with old-fashioned MD5-challenge authentication.
+Considered not resistant to man-in-the-middle attacks.
 .IP RPOP
 Use POP3 with RPOP authentication.
 .IP KPOP
@@ -978,15 +979,15 @@ will be removed from a future fetchmail version.  This
 facility was vulnerable to spoofing and was withdrawn in RFC1460.
 .PP
 RFC1460 introduced APOP authentication.  In this variant of POP3,
-you register an APOP password on your server host (the program
-to do this with on the server is probably called \fIpopauth\fR(8)).  You
-put the same password in your
-.I ~/.fetchmailrc
-file.  Each time
-.I fetchmail
-logs in, it sends a cryptographically secure hash of your password and
-the server greeting time to the server, which can verify it by
-checking its authorization database.
+you register an APOP password on your server host (on some servers, the
+program to do this is called \fIpopauth\fR(8)).  You put the same
+password in your \fI~/.fetchmailrc\fP file.  Each time \fIfetchmail\fP
+logs in, it sends an MD5 hash of your password and the server greeting
+time to the server, which can verify it by checking its authorization
+database.
+
+\fBNote that APOP is no longer considered resistant against
+man-in-the-middle attacks.\fP
 .SS RETR or TOP
 .I fetchmail
 makes some efforts to make the server believe messages had not been
diff --git a/pop3.c b/pop3.c
index 3ba6af36..17f66c70 100644
--- a/pop3.c
+++ b/pop3.c
@@ -659,6 +659,20 @@ static int pop3_getauth(int sock, struct query *ctl, char *greeting)
 	else
 	    *++end = '\0';
 
+	/* SECURITY: 2007-03-17
+	 * Strictly validating the presented challenge for RFC-822
+	 * conformity (it must be a msg-id in terms of that standard) is
+	 * supposed to make attacks against the MD5 implementation
+	 * harder[1]
+	 *
+	 * [1] "Security vulnerability in APOP authentication",
+	 *     Gaëtan Leurent, fetchmail-devel, 2007-03-17 */
+	if (!rfc822_valid_msgid((unsigned char *)start)) {
+	    report(stderr,
+		    GT_("Invalid APOP timestamp.\n"));
+	    return PS_AUTHFAIL;
+	}
+
 	/* copy timestamp and password into digestion buffer */
 	msg = xmalloc((end-start+1) + strlen(ctl->password) + 1);
 	strcpy(msg,start);
diff --git a/rfc822valid.c b/rfc822valid.c
new file mode 100644
index 00000000..0fabe1ef
--- /dev/null
+++ b/rfc822valid.c
@@ -0,0 +1,140 @@
+/* rfc822valid.c -- validators for RFC-822 syntax
+ * (C) Copyright 2007 Matthias Andree <matthias.andree@gmx.de>
+ * GNU General Public License v2 */
+
+/* This works only on ASCII-based computers. */
+
+#include "fetchmail.h"
+#include <string.h>
+
+/* CHAR except specials, SPACE, CTLs */
+static const char *atomchar = "!#$%&'*+-/0123456789=?ABCDEFGHIJKLMNOPQRSTUVWXYZ^_`abcdefghijklmnopqrstuvwxyz{|}~";
+
+static int quotedpair(unsigned char const **x) {
+    if (**x != '\\') return 0;
+    ++ *x;
+    if ((int)* *x > 127 || * *x == '\0')
+	/* XXX FIXME: 0 is a legal CHAR, so the == '\0' is sort of bogus
+	 * above, but fetchmail does not currently deal with NUL inputs
+	 * so we don't need to make the distinction between
+	 * end-of-string and quoted NUL. */
+	return 0;
+    ++ *x;
+    return 1;
+}
+
+
+static int quotedstring(unsigned char const **x) {
+    if (* *x != '"') return 0;
+    ++ *x;
+    for(;;) {
+	switch (* *x) {
+	    case '"':
+		++ *x;
+		return 1;
+	    case '\\':
+		if (quotedpair(x) == 0) return 0;
+		continue;
+	    case '\r':
+	    case '\0':
+		return 0;
+	}
+	if ((int)* *x >= 128) {
+	    return 0;
+	}
+	++ *x;
+    }
+}
+
+static int atom(unsigned char const **x) {
+    /* atom */
+    if (strchr(atomchar, (const char)**x)) {
+	*x += strspn((const char *)*x, atomchar);
+	return 1;
+    }
+    /* invalid character */
+    return 0;
+}
+
+static int word(unsigned char const **x) {
+    if (**x == '"')
+	return quotedstring(x);
+    return atom(x);
+}
+
+static int domain_literal(unsigned char const **x) {
+    if (**x != '[') return 0;
+    ++ *x;
+    for(;;) {
+	switch (* *x) {
+	    case '\0':
+	    case '\r':
+	    case '[':
+		return 0;
+	    case ']':
+		++ *x;
+		return 1;
+	    case '\\':
+		if (quotedpair(x) == 0) return 0;
+		continue;
+	}
+	if ((int)* *x > 127) return 0;
+	++ *x;
+    }
+}
+
+static int subdomain(unsigned char const **x) {
+    if (* *x == '[') return domain_literal(x);
+    return atom(x);
+}
+
+int rfc822_valid_msgid(const unsigned char *x) {
+    /* expect "<" */
+    if (*x != '<') return 0;
+    ++ x;
+
+    /* expect local-part = word *("." word)
+     * where
+     * word = atom/quoted-string
+     * atom = 1*ATOMCHAR
+     * quoted-string = <"> *(qtext/quoted-pair) <">
+     * qtext = CHAR except ", \, CR
+     * quoted-pair = "\" CHAR
+     */
+    for(;;) {
+	if (word(&x) == 0) return 0;
+	if (*x == '.') { ++x; continue; }
+	if (*x == '@') break;
+	return 0;
+    }
+
+    /* expect "@" */
+    if (*x != '@') return 0;
+    ++ x;
+
+    /* expect domain = sub-domain *("." sub-domain)
+     * sub-domain = domain-ref/domain-literal
+     * domain-ref = atom
+     * domain-literal = "[" *(dtext/quoted-pair) "]" */
+    for(;;) {
+	if (subdomain(&x) == 0) return 0;
+	if (*x == '.') { ++x; continue; }
+	if (*x == '>') break;
+	return 0;
+    }
+
+    if (*x != '>') return 0;
+    return 1;
+}
+
+#ifdef TEST
+#include <stdio.h>
+
+int main(int argc, char **argv) {
+    int i;
+    for (i = 1; i < argc; i++) {
+	printf("%s: %s\n", argv[i], rfc822_valid_msgid((unsigned char *)argv[i]) ? "OK" : "INVALID");
+    }
+    return 0;
+}
+#endif