Package: release.debian.org
Severity: normal
User: release.debian....@packages.debian.org
Usertags: pu
On 2013-01-12 Andreas Metzler <ametz...@downhill.at.eu.org> wrote:
> On 2013-01-06 "Adam D. Barratt" <a...@adam-barratt.org.uk> wrote:
> [...]
> > In principle the fixes sound okay but a debdiff between stable (well,
> > p-u, as that has +squeeze3) and the proposed package would be
> > appreciated.
> There you go ...
Let's convert this to a bug-report to make sure it does not get lost.
<http://lists.debian.org/debian-release/2013/01/msg00181.html>
----------------------
I would like to push this change to stable:
|------------------------------------------
| http://git.exim.org/exim.git/commit/3f1df0e341c4ddc4add38fa97d9d34972655a6c7
|
| Dovecot: robustness; better msg on missing mech.
|
| If the dovecot protocol response doesn't include the MECH message for
| the SMTP AUTH protocol the client has requested, that's not a protocol
| failure, don't log it as such. Instead, explicitly log that it didn't
| advertise the mechanism we're looking for. This lets administrators
| fix either their Exim or their Dovecot configurations.
|
| Also: make the Dovecot handling more resistant to bad data from the
| auth server; handle too many fields with debug-log message to explain
| what's going on, permit lines of 8192 length per spec and detect if
| the line is too long, so that we can fail auth instead of becoming
| unsynchronised.
|
| Stop using the CUID from the server as the AUTH id counter. They're
| different, by my reading of the spec.
|------------------------------------------
This fixes an exim segfault when accessing a malicious dovecot AUTH
server. I have already talked with the security team, Moritz agrees
that this should be fixed in a point release. Testing already has the
fix since 4.80-6.
On top of this I would like to discuss whether it is acceptable to fix
http://bugs.debian.org/697057 in stable, too. [ I definitily want o
get the fix into testing - #697444.] The Debian configuration
optionally allows to use spfquery to run SPF-checks on incoming mail.
Due to insufficient quoting it is possible to pass on arbitrary
arguments to spfquery and therefore bypass SPF checks. The fix is not
invasive, but it changes dpkg conffiles.
debdiff attached.
Thanks, cu andreas
--
`What a good friend you are to him, Dr. Maturin. His other friends are
so grateful to you.'
`I sew his ears on from time to time, sure'
===============================================================================
exim4
==============================================================================
File lists identical (after any substitutions)
Control files: lines which differ (wdiff format)
------------------------------------------------
Version: [-4.72-6+squeeze3-] {+4.72-6+squeeze4+}
===============================================================================
exim4-base
==============================================================================
File lists identical (after any substitutions)
Control files: lines which differ (wdiff format)
------------------------------------------------
Installed-Size: [-1672-] {+1644+}
Version: [-4.72-6+squeeze3-] {+4.72-6+squeeze4+}
===============================================================================
exim4-config
==============================================================================
File lists identical (after any substitutions)
Control files: lines which differ (wdiff format)
------------------------------------------------
Installed-Size: [-1272-] {+1256+}
Version: [-4.72-6+squeeze3-] {+4.72-6+squeeze4+}
===============================================================================
exim4-daemon-heavy
==============================================================================
File lists identical (after any substitutions)
Control files: lines which differ (wdiff format)
------------------------------------------------
Installed-Size: [-1148-] {+1136+}
Version: [-4.72-6+squeeze3-] {+4.72-6+squeeze4+}
===============================================================================
exim4-daemon-heavy-dbg
==============================================================================
File lists identical (after any substitutions)
Control files: lines which differ (wdiff format)
------------------------------------------------
Installed-Size: [-1936-] {+1924+}
Version: [-4.72-6+squeeze3-] {+4.72-6+squeeze4+}
===============================================================================
exim4-daemon-light
==============================================================================
File lists identical (after any substitutions)
Control files: lines which differ (wdiff format)
------------------------------------------------
Installed-Size: [-1040-] {+1028+}
Version: [-4.72-6+squeeze3-] {+4.72-6+squeeze4+}
===============================================================================
exim4-daemon-light-dbg
==============================================================================
File lists identical (after any substitutions)
Control files: lines which differ (wdiff format)
------------------------------------------------
Installed-Size: [-1736-] {+1724+}
Version: [-4.72-6+squeeze3-] {+4.72-6+squeeze4+}
===============================================================================
exim4-dbg
==============================================================================
File lists identical (after any substitutions)
Control files: lines which differ (wdiff format)
------------------------------------------------
Installed-Size: [-688-] {+664+}
Version: [-4.72-6+squeeze3-] {+4.72-6+squeeze4+}
===============================================================================
exim4-dev
==============================================================================
File lists identical (after any substitutions)
Control files: lines which differ (wdiff format)
------------------------------------------------
Installed-Size: [-244-] {+236+}
Version: [-4.72-6+squeeze3-] {+4.72-6+squeeze4+}
===============================================================================
eximon4
==============================================================================
File lists identical (after any substitutions)
Control files: lines which differ (wdiff format)
------------------------------------------------
Installed-Size: [-304-] {+292+}
Version: [-4.72-6+squeeze3-] {+4.72-6+squeeze4+}
diff -Nru exim4-4.72/debian/changelog exim4-4.72/debian/changelog
--- exim4-4.72/debian/changelog 2012-10-25 19:43:49.000000000 +0200
+++ exim4-4.72/debian/changelog 2013-01-12 11:23:38.000000000 +0100
@@ -1,3 +1,13 @@
+exim4 (4.72-6+squeeze4) stable; urgency=low
+
+ * Cherrypick [86_Dovecot-robustness.diff] from upstream GIT, a
+ robustness fix for the Dovecot authenticator.
+ * Use exim's ${quote:xxx} operator when invoking spfquery to disallow
+ bypassing of SPF validation by using special mailbox names. (Thanks to
+ Lekensteyn for diagnosis and testing.) Closes: #697057
+
+ -- Andreas Metzler <ametz...@debian.org> Sat, 12 Jan 2013 11:22:07 +0100
+
exim4 (4.72-6+squeeze3) stable-security; urgency=high
* Non-maintainer upload by the Security Team.
diff -Nru exim4-4.72/debian/debconf/conf.d/acl/30_exim4-config_check_rcpt exim4-4.72/debian/debconf/conf.d/acl/30_exim4-config_check_rcpt
--- exim4-4.72/debian/debconf/conf.d/acl/30_exim4-config_check_rcpt 2011-01-22 17:01:08.000000000 +0100
+++ exim4-4.72/debian/debconf/conf.d/acl/30_exim4-config_check_rcpt 2013-01-12 11:17:10.000000000 +0100
@@ -254,10 +254,10 @@
log_message = SPF check failed.
!acl = acl_local_deny_exceptions
condition = ${run{/usr/bin/spfquery.mail-spf-perl --ip \
- \"$sender_host_address\" --identity \
+ ${quote:$sender_host_address} --identity \
${if def:sender_address_domain \
- {--scope mfrom --identity \"$sender_address\"}\
- {--scope helo --identity \"$sender_helo_name\"}}}\
+ {--scope mfrom --identity ${quote:$sender_address}}\
+ {--scope helo --identity ${quote:$sender_helo_name}}}}\
{no}{${if eq {$runrc}{1}{yes}{no}}}}
defer
message = Temporary DNS error while checking SPF record. Try again later.
diff -Nru exim4-4.72/debian/patches/86_Dovecot-robustness.diff exim4-4.72/debian/patches/86_Dovecot-robustness.diff
--- exim4-4.72/debian/patches/86_Dovecot-robustness.diff 1970-01-01 01:00:00.000000000 +0100
+++ exim4-4.72/debian/patches/86_Dovecot-robustness.diff 2013-01-12 12:03:59.000000000 +0100
@@ -0,0 +1,308 @@
+From 3f1df0e341c4ddc4add38fa97d9d34972655a6c7 Mon Sep 17 00:00:00 2001
+From: Phil Pennock <p...@exim.org>
+Date: Mon, 19 Nov 2012 23:44:33 -0500
+Subject: [PATCH] Dovecot: robustness; better msg on missing mech.
+
+If the dovecot protocol response doesn't include the MECH message for
+the SMTP AUTH protocol the client has requested, that's not a protocol
+failure, don't log it as such. Instead, explicitly log that it didn't
+advertise the mechanism we're looking for. This lets administrators fix
+either their Exim or their Dovecot configurations.
+
+Also: make the Dovecot handling more resistant to bad data from the auth
+server; handle too many fields with debug-log message to explain what's
+going on, permit lines of 8192 length per spec and detect if the line is
+too long, so that we can fail auth instead of becoming unsynchronised.
+
+Stop using the CUID from the server as the AUTH id counter. They're
+different, by my reading of the spec.
+
+TESTED: works against Dovecot 2.1.10.
+
+Thanks to Brady Catherman for reporting the problem with diagnosis.
+---
+
+diff --git a/src/auths/dovecot.c b/src/auths/dovecot.c
+index 0824240..032a089 100644
+--- a/src/auths/dovecot.c
++++ b/src/auths/dovecot.c
+@@ -12,12 +12,42 @@ commented them specially, but now they are getting quite extensive, so I have
+ ceased doing that. The biggest change is to use unbuffered I/O on the socket
+ because using C buffered I/O gives problems on some operating systems. PH */
+
++/* Protocol specifications:
++ * Dovecot 1, protocol version 1.1
++ * http://wiki.dovecot.org/Authentication%20Protocol
++ *
++ * Dovecot 2, protocol version 1.1
++ * http://wiki2.dovecot.org/Design/AuthProtocol
++ */
++
+ #include "../exim.h"
+ #include "dovecot.h"
+
+ #define VERSION_MAJOR 1
+ #define VERSION_MINOR 0
+
++/* http://wiki.dovecot.org/Authentication%20Protocol
++"The maximum line length isn't defined,
++ but it's currently expected to fit into 8192 bytes"
++*/
++#define DOVECOT_AUTH_MAXLINELEN 8192
++
++/* This was hard-coded as 8.
++AUTH req C->S sends {"AUTH", id, mechanism, service } + params, 5 defined for
++Dovecot 1; Dovecot 2 (same protocol version) defines 9.
++
++Master->Server sends {"USER", id, userid} + params, 6 defined.
++Server->Client only gives {"OK", id} + params, unspecified, only 1 guaranteed.
++
++We only define here to accept S->C; max seen is 3+<unspecified>, plus the two
++for the command and id, where unspecified might include _at least_ user=...
++
++So: allow for more fields than we ever expect to see, while aware that count
++can go up without changing protocol version.
++The cost is the length of an array of pointers on the stack.
++*/
++#define DOVECOT_AUTH_MAXFIELDCOUNT 16
++
+ /* Options specific to the authentication mechanism. */
+ optionlist auth_dovecot_options[] = {
+ {
+@@ -43,7 +73,7 @@ auth_dovecot_options_block auth_dovecot_option_defaults = {
+ /* Static variables for reading from the socket */
+
+ static uschar sbuffer[256];
+-static int sbp;
++static int socket_buffer_left;
+
+
+
+@@ -67,9 +97,28 @@ void auth_dovecot_init(auth_instance *ablock)
+ ablock->client = FALSE;
+ }
+
+-static int strcut(uschar *str, uschar **ptrs, int nptrs)
++/*************************************************
++ * "strcut" to split apart server lines *
++ *************************************************/
++
++/* Dovecot auth protocol uses TAB \t as delimiter; a line consists
++of a command-name, TAB, and then any parameters, each separated by a TAB.
++A parameter can be param=value or a bool, just param.
++
++This function modifies the original str in-place, inserting NUL characters.
++It initialises ptrs entries, setting all to NULL and only setting
++non-NULL N entries, where N is the return value, the number of fields seen
++(one more than the number of tabs).
++
++Note that the return value will always be at least 1, is the count of
++actual fields (so last valid offset into ptrs is one less).
++*/
++
++static int
++strcut(uschar *str, uschar **ptrs, int nptrs)
+ {
+- uschar *tmp = str;
++ uschar *last_sub_start = str;
++ uschar *lastvalid = str + Ustrlen(str);
+ int n;
+
+ for (n = 0; n < nptrs; n++)
+@@ -79,19 +128,44 @@ static int strcut(uschar *str, uschar **ptrs, int nptrs)
+ while (*str) {
+ if (*str == '\t') {
+ if (n <= nptrs) {
+- *ptrs++ = tmp;
+- tmp = str + 1;
+- *str = 0;
++ *ptrs++ = last_sub_start;
++ last_sub_start = str + 1;
++ *str = '\0';
+ }
+ n++;
+ }
+ str++;
+ }
+
+- if (n < nptrs)
+- *ptrs = tmp;
++ if (last_sub_start < lastvalid) {
++ if (n <= nptrs) {
++ *ptrs = last_sub_start;
++ } else {
++ HDEBUG(D_auth) debug_printf("dovecot: warning: too many results from tab-splitting; saw %d fields, room for %d\n", n, nptrs);
++ n = nptrs;
++ }
++ } else {
++ n--;
++ HDEBUG(D_auth) debug_printf("dovecot: warning: ignoring trailing tab\n");
++ }
++
++ return n <= nptrs ? n : nptrs;
++}
+
+- return n;
++static void debug_strcut(uschar **ptrs, int nlen, int alen);
++static void
++debug_strcut(uschar **ptrs, int nlen, int alen)
++{
++ int i;
++ debug_printf("%d read but unreturned bytes; strcut() gave %d results: ",
++ socket_buffer_left, nlen);
++ for (i = 0; i < nlen; i++) {
++ debug_printf(" {%s}", ptrs[i]);
++ }
++ if (nlen < alen)
++ debug_printf(" last is %s\n", ptrs[i] ? ptrs[i] : US"<null>");
++ else
++ debug_printf(" (max for capacity)\n");
+ }
+
+ #define CHECK_COMMAND(str, arg_min, arg_max) do { \
+@@ -125,27 +199,27 @@ int count = 0;
+
+ for (;;)
+ {
+- if (sbp == 0)
++ if (socket_buffer_left == 0)
+ {
+- sbp = read(fd, sbuffer, sizeof(sbuffer));
+- if (sbp == 0) { if (count == 0) return NULL; else break; }
++ socket_buffer_left = read(fd, sbuffer, sizeof(sbuffer));
++ if (socket_buffer_left == 0) { if (count == 0) return NULL; else break; }
+ p = 0;
+ }
+
+- while (p < sbp)
++ while (p < socket_buffer_left)
+ {
+ if (count >= n - 1) break;
+ s[count++] = sbuffer[p];
+ if (sbuffer[p++] == '\n') break;
+ }
+
+- memmove(sbuffer, sbuffer + p, sbp - p);
+- sbp -= p;
++ memmove(sbuffer, sbuffer + p, socket_buffer_left - p);
++ socket_buffer_left -= p;
+
+ if (s[count-1] == '\n' || count >= n - 1) break;
+ }
+
+-s[count] = 0;
++s[count] = '\0';
+ return s;
+ }
+
+@@ -161,12 +235,14 @@ int auth_dovecot_server(auth_instance *ablock, uschar *data)
+ auth_dovecot_options_block *ob =
+ (auth_dovecot_options_block *)(ablock->options_block);
+ struct sockaddr_un sa;
+- uschar buffer[4096];
+- uschar *args[8];
++ uschar buffer[DOVECOT_AUTH_MAXLINELEN];
++ uschar *args[DOVECOT_AUTH_MAXFIELDCOUNT];
+ uschar *auth_command;
+ uschar *auth_extra_data = US"";
++ uschar *p;
+ int nargs, tmp;
+- int cuid = 0, cont = 1, found = 0, fd, ret = DEFER;
++ int crequid = 1, cont = 1, fd, ret = DEFER;
++ BOOL found = FALSE;
+
+ HDEBUG(D_auth) debug_printf("dovecot authentication\n");
+
+@@ -198,37 +274,46 @@ int auth_dovecot_server(auth_instance *ablock, uschar *data)
+
+ auth_defer_msg = US"authentication socket protocol error";
+
+- sbp = 0; /* Socket buffer pointer */
++ socket_buffer_left = 0; /* Global, used to read more than a line but return by line */
+ while (cont) {
+ if (dc_gets(buffer, sizeof(buffer), fd) == NULL)
+ OUT("authentication socket read error or premature eof");
+-
+- buffer[Ustrlen(buffer) - 1] = 0;
++ p = buffer + Ustrlen(buffer) - 1;
++ if (*p != '\n') {
++ OUT("authentication socket protocol line too long");
++ }
++ *p = '\0';
+ HDEBUG(D_auth) debug_printf("received: %s\n", buffer);
+ nargs = strcut(buffer, args, sizeof(args) / sizeof(args[0]));
++ /* HDEBUG(D_auth) debug_strcut(args, nargs, sizeof(args) / sizeof(args[0])); */
+
+ /* Code below rewritten by Kirill Miazine (k...@krot.org). Only check commands that
+ Exim will need. Original code also failed if Dovecot server sent unknown
+ command. E.g. COOKIE in version 1.1 of the protocol would cause troubles. */
+- if (Ustrcmp(args[0], US"CUID") == 0) {
+- CHECK_COMMAND("CUID", 1, 1);
+- cuid = Uatoi(args[1]);
+- } else if (Ustrcmp(args[0], US"VERSION") == 0) {
++ /* pdp: note that CUID is a per-connection identifier sent by the server,
++ which increments at server discretion.
++ By contrast, the "id" field of the protocol is a connection-specific request
++ identifier, which needs to be unique per request from the client and is not
++ connected to the CUID value, so we ignore CUID from server. It's purely for
++ diagnostics. */
++ if (Ustrcmp(args[0], US"VERSION") == 0) {
+ CHECK_COMMAND("VERSION", 2, 2);
+ if (Uatoi(args[1]) != VERSION_MAJOR)
+ OUT("authentication socket protocol version mismatch");
+ } else if (Ustrcmp(args[0], US"MECH") == 0) {
+ CHECK_COMMAND("MECH", 1, INT_MAX);
+ if (strcmpic(US args[1], ablock->public_name) == 0)
+- found = 1;
++ found = TRUE;
+ } else if (Ustrcmp(args[0], US"DONE") == 0) {
+ CHECK_COMMAND("DONE", 0, 0);
+ cont = 0;
+ }
+ }
+
+- if (!found)
++ if (!found) {
++ auth_defer_msg = string_sprintf("Dovecot did not advertise mechanism \"%s\" to us", ablock->public_name);
+ goto out;
++ }
+
+ /* Added by PH: data must not contain tab (as it is
+ b64 it shouldn't, but check for safety). */
+@@ -264,14 +349,11 @@ int auth_dovecot_server(auth_instance *ablock, uschar *data)
+
+ Subsequently, the command was modified to add "secured" and "valid-client-
+ cert" when relevant.
+-
+- The auth protocol is documented here:
+- http://wiki.dovecot.org/Authentication_Protocol
+ ****************************************************************************/
+
+ auth_command = string_sprintf("VERSION\t%d\t%d\nCPID\t%d\n"
+ "AUTH\t%d\t%s\tservice=smtp\t%srip=%s\tlip=%s\tnologin\tresp=%s\n",
+- VERSION_MAJOR, VERSION_MINOR, getpid(), cuid,
++ VERSION_MAJOR, VERSION_MINOR, getpid(), crequid,
+ ablock->public_name, auth_extra_data, sender_host_address,
+ interface_address, data ? (char *) data : "");
+
+@@ -295,7 +377,7 @@ int auth_dovecot_server(auth_instance *ablock, uschar *data)
+ HDEBUG(D_auth) debug_printf("received: %s\n", buffer);
+ nargs = strcut(buffer, args, sizeof(args) / sizeof(args[0]));
+
+- if (Uatoi(args[1]) != cuid)
++ if (Uatoi(args[1]) != crequid)
+ OUT("authentication socket connection id mismatch");
+
+ switch (toupper(*args[0])) {
+@@ -316,7 +398,7 @@ int auth_dovecot_server(auth_instance *ablock, uschar *data)
+ goto out;
+ }
+
+- temp = string_sprintf("CONT\t%d\t%s\n", cuid, data);
++ temp = string_sprintf("CONT\t%d\t%s\n", crequid, data);
+ if (write(fd, temp, Ustrlen(temp)) < 0)
+ OUT("authentication socket write error");
+ break;
+--
+1.7.10.4
+
diff -Nru exim4-4.72/debian/patches/series exim4-4.72/debian/patches/series
--- exim4-4.72/debian/patches/series 2012-10-25 20:00:21.000000000 +0200
+++ exim4-4.72/debian/patches/series 2013-01-12 12:03:36.000000000 +0100
@@ -24,3 +24,4 @@
82_dkimpercent.diff
83_dkimexpand.diff
84_CVE-2012-5671.patch
+86_Dovecot-robustness.diff
