From 321d61b215169346708da3ad2b74711996771635 Mon Sep 17 00:00:00 2001 From: Matthias Andree Date: Sun, 18 Mar 2007 01:24:22 +0000 Subject: 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 --- Makefile.am | 8 ++-- NEWS | 11 +++++ fetchmail.h | 3 ++ fetchmail.man | 19 ++++---- pop3.c | 14 ++++++ rfc822valid.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 183 insertions(+), 12 deletions(-) create mode 100644 rfc822valid.c 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 + * GNU General Public License v2 */ + +/* This works only on ASCII-based computers. */ + +#include "fetchmail.h" +#include + +/* 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 + +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 -- cgit v1.2.3