Bug#805721: jessie-pu: package exim4/4.84-8+deb8u2
On 2015-11-21 Andreas Metzler <ametzler@bebt.de> wrote:
[...]
> I would like to fix 805576 for jessie:
Sorry, I forgot to attach the diff.
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'
File lists identical (after any substitutions)
Control files of package exim4: lines which differ (wdiff format)
-----------------------------------------------------------------
Depends: debconf (>= 0.5) | debconf-2.0, debconf (>= 1.4.69) | cdebconf (>= 0.39), exim4-base (>= [-4.84-8+deb8u1),-] {+4.84-8+deb8u2),+} exim4-base (<< [-4.84-8+deb8u1.1),-] {+4.84-8+deb8u2.1),+} exim4-daemon-light | exim4-daemon-heavy | exim4-daemon-custom
Version: [-4.84-8+deb8u1-] {+4.84-8+deb8u2+}
Control files of package exim4-base: lines which differ (wdiff format)
----------------------------------------------------------------------
Version: [-4.84-8+deb8u1-] {+4.84-8+deb8u2+}
Control files of package exim4-config: lines which differ (wdiff format)
------------------------------------------------------------------------
Version: [-4.84-8+deb8u1-] {+4.84-8+deb8u2+}
Control files of package exim4-daemon-heavy: lines which differ (wdiff format)
------------------------------------------------------------------------------
Installed-Size: [-1482-] {+1483+}
Version: [-4.84-8+deb8u1-] {+4.84-8+deb8u2+}
Control files of package exim4-daemon-heavy-dbg: lines which differ (wdiff format)
----------------------------------------------------------------------------------
Installed-Size: [-2316-] {+2318+}
Version: [-4.84-8+deb8u1-] {+4.84-8+deb8u2+}
Control files of package exim4-daemon-light: lines which differ (wdiff format)
------------------------------------------------------------------------------
Installed-Size: [-1336-] {+1340+}
Version: [-4.84-8+deb8u1-] {+4.84-8+deb8u2+}
Control files of package exim4-daemon-light-dbg: lines which differ (wdiff format)
----------------------------------------------------------------------------------
Installed-Size: [-2079-] {+2080+}
Version: [-4.84-8+deb8u1-] {+4.84-8+deb8u2+}
Control files of package exim4-dbg: lines which differ (wdiff format)
---------------------------------------------------------------------
Version: [-4.84-8+deb8u1-] {+4.84-8+deb8u2+}
Control files of package exim4-dev: lines which differ (wdiff format)
---------------------------------------------------------------------
Version: [-4.84-8+deb8u1-] {+4.84-8+deb8u2+}
Control files of package eximon4: lines which differ (wdiff format)
-------------------------------------------------------------------
Version: [-4.84-8+deb8u1-] {+4.84-8+deb8u2+}
diff -Nru exim4-4.84/debian/changelog exim4-4.84/debian/changelog
--- exim4-4.84/debian/changelog 2015-10-31 13:55:10.000000000 +0100
+++ exim4-4.84/debian/changelog 2015-11-21 11:47:19.000000000 +0100
@@ -1,3 +1,11 @@
+exim4 (4.84-8+deb8u2) jessie; urgency=medium
+
+ * 87_Fix-transport-results-pipe-for-multiple-recipients-c.patch: Pull and
+ unfuzz bd21a78 from upstream GIT, to fix a bug causing duplicate
+ deliveries especially on TLS connections. Closes: #805576
+
+ -- Andreas Metzler <ametzler@debian.org> Sat, 21 Nov 2015 11:24:46 +0100
+
exim4 (4.84-8+deb8u1) jessie; urgency=medium
* Pull 85_Fix-crash-in-mime-acl-when-a-parameter-is-unterminat.patch
diff -Nru exim4-4.84/debian/patches/87_Fix-transport-results-pipe-for-multiple-recipients-c.patch exim4-4.84/debian/patches/87_Fix-transport-results-pipe-for-multiple-recipients-c.patch
--- exim4-4.84/debian/patches/87_Fix-transport-results-pipe-for-multiple-recipients-c.patch 1970-01-01 01:00:00.000000000 +0100
+++ exim4-4.84/debian/patches/87_Fix-transport-results-pipe-for-multiple-recipients-c.patch 2015-11-21 12:52:38.000000000 +0100
@@ -0,0 +1,384 @@
+From bd21a787cdeef803334a6c7bf50d23b2a18cbd6f Mon Sep 17 00:00:00 2001
+From: Wolfgang Breyha <wbreyha@gmx.net>
+Date: Sun, 28 Sep 2014 13:40:45 +0100
+Subject: [PATCH] Fix transport-results pipe for multiple recipients combined
+ with certs.
+
+The previous parsing failed when a result item split over a buffer boundary;
+fix by prefixing sizes to items, and checking enough has been read as the
+initial parsing stage.
+---
+ doc/doc-txt/ChangeLog | 7 +++
+ src/deliver.c | 162 +++++++++++++++++++++++++++++++++++++-------------
+ src/macros.h | 4 ++
+ 3 files changed, 131 insertions(+), 42 deletions(-)
+
+# diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
+# index 46fed6f..76ecc20 100644
+# --- a/doc/doc-txt/ChangeLog
+# +++ b/doc/doc-txt/ChangeLog
+# @@ -37,6 +37,13 @@ TL/04 Bugzilla 1216: Add -M (related messages) option to exigrep.
+# TL/05 GitHub Issue 18: Adjust logic testing for true/false in redis lookups.
+# Merged patch from Sebastian Wiedenroth.
+#
+# +JH/05 Fix results-pipe from transport process. Several recipients, combined
+# + with certificate use, exposed issues where response data items split
+# + over buffer boundaries were not parsed properly. This eventually
+# + resulted in duplicates being sent. This issue only became common enough
+# + to notice due to the introduction of conection certificate information,
+# + the item size being so much larger. Found and fixed by Wolfgang Breyha.
+# +
+# Exim version 4.84
+# -----------------
+# TL/01 Bugzilla 1506: Re-add a 'return NULL' to silence complaints from static
+--- a/src/deliver.c
++++ b/src/deliver.c
+@@ -2823,6 +2823,8 @@ uschar *ptr = endptr;
+ uschar *msg = p->msg;
+ BOOL done = p->done;
+ BOOL unfinished = TRUE;
++/* minimum size to read is header size including id, subid and length */
++int required = PIPE_HEADER_SIZE;
+
+ /* Loop through all items, reading from the pipe when necessary. The pipe
+ is set up to be non-blocking, but there are two different Unix mechanisms in
+@@ -2845,12 +2847,15 @@ while (!done)
+ {
+ retry_item *r, **rp;
+ int remaining = endptr - ptr;
++ uschar header[PIPE_HEADER_SIZE + 1];
++ uschar id, subid;
++ uschar *endc;
+
+ /* Read (first time) or top up the chars in the buffer if necessary.
+ There will be only one read if we get all the available data (i.e. don't
+ fill the buffer completely). */
+
+- if (remaining < 2500 && unfinished)
++ if (remaining < required && unfinished)
+ {
+ int len;
+ int available = big_buffer_size - remaining;
+@@ -2883,17 +2888,64 @@ while (!done)
+ won't read any more, as "unfinished" will get set FALSE. */
+
+ endptr += len;
++ remaining += len;
+ unfinished = len == available;
+ }
+
+ /* If we are at the end of the available data, exit the loop. */
+-
+ if (ptr >= endptr) break;
+
++ /* copy and read header */
++ memcpy(header, ptr, PIPE_HEADER_SIZE);
++ header[PIPE_HEADER_SIZE] = '\0';
++ id = header[0];
++ subid = header[1];
++ required = Ustrtol(header + 2, &endc, 10) + PIPE_HEADER_SIZE; /* header + data */
++ if (*endc)
++ {
++ msg = string_sprintf("failed to read pipe from transport process "
++ "%d for transport %s: error reading size from header", pid, addr->transport->driver_name);
++ done = TRUE;
++ break;
++ }
++
++ DEBUG(D_deliver)
++ debug_printf("header read id:%c,subid:%c,size:%s,required:%d,remaining:%d,unfinished:%d\n",
++ id, subid, header+2, required, remaining, unfinished);
++
++ /* is there room for the dataset we want to read ? */
++ if (required > big_buffer_size - PIPE_HEADER_SIZE)
++ {
++ msg = string_sprintf("failed to read pipe from transport process "
++ "%d for transport %s: big_buffer too small! required size=%d buffer size=%d", pid, addr->transport->driver_name,
++ required, big_buffer_size - PIPE_HEADER_SIZE);
++ done = TRUE;
++ break;
++ }
++
++ /* we wrote all datasets with atomic write() calls
++ remaining < required only happens if big_buffer was too small
++ to get all available data from pipe. unfinished has to be true
++ as well. */
++ if (remaining < required)
++ if (unfinished)
++ continue;
++ else
++ {
++ msg = string_sprintf("failed to read pipe from transport process "
++ "%d for transport %s: required size=%d > remaining size=%d and unfinished=false",
++ pid, addr->transport->driver_name, required, remaining);
++ done = TRUE;
++ break;
++ }
++
++ /* step behind the header */
++ ptr += PIPE_HEADER_SIZE;
++
+ /* Handle each possible type of item, assuming the complete item is
+ available in store. */
+
+- switch (*ptr++)
++ switch (id)
+ {
+ /* Host items exist only if any hosts were marked unusable. Match
+ up by checking the IP address. */
+@@ -2990,7 +3042,7 @@ while (!done)
+ #ifdef SUPPORT_TLS
+ case 'X':
+ if (addr == NULL) goto ADDR_MISMATCH; /* Below, in 'A' handler */
+- switch (*ptr++)
++ switch (subid)
+ {
+ case '1':
+ addr->cipher = NULL;
+@@ -3028,7 +3080,7 @@ while (!done)
+ #endif /*SUPPORT_TLS*/
+
+ case 'C': /* client authenticator information */
+- switch (*ptr++)
++ switch (subid)
+ {
+ case '1':
+ addr->authenticator = (*ptr)? string_copy(ptr) : NULL;
+@@ -3051,7 +3103,7 @@ while (!done)
+
+ #ifdef EXPERIMENTAL_DSN
+ case 'D':
+- if (addr == NULL) break;
++ if (addr == NULL) goto ADDR_MISMATCH;
+ memcpy(&(addr->dsn_aware), ptr, sizeof(addr->dsn_aware));
+ ptr += sizeof(addr->dsn_aware);
+ DEBUG(D_deliver) debug_printf("DSN read: addr->dsn_aware = %d\n", addr->dsn_aware);
+@@ -3119,7 +3171,7 @@ while (!done)
+ continue_hostname = NULL;
+ }
+ done = TRUE;
+- DEBUG(D_deliver) debug_printf("Z%c item read\n", *ptr);
++ DEBUG(D_deliver) debug_printf("Z0%c item read\n", *ptr);
+ break;
+
+ /* Anything else is a disaster. */
+@@ -3572,9 +3624,40 @@ while (parcount > max)
+
+
+ static void
+-rmt_dlv_checked_write(int fd, void * buf, int size)
++rmt_dlv_checked_write(int fd, char id, char subid, void * buf, int size)
+ {
+-int ret = write(fd, buf, size);
++uschar writebuffer[PIPE_HEADER_SIZE + BIG_BUFFER_SIZE];
++int header_length;
++
++/* we assume that size can't get larger then BIG_BUFFER_SIZE which currently is set to 16k */
++/* complain to log if someone tries with buffer sizes we can't handle*/
++
++if (size > 99999)
++{
++ log_write(0, LOG_MAIN|LOG_PANIC_DIE,
++ "Failed writing transport result to pipe: can't handle buffers > 99999 bytes. truncating!\n");
++ size = 99999;
++}
++
++/* to keep the write() atomic we build header in writebuffer and copy buf behind */
++/* two write() calls would increase the complexity of reading from pipe */
++
++/* convert size to human readable string prepended by id and subid */
++header_length = snprintf(writebuffer, PIPE_HEADER_SIZE+1, "%c%c%05d", id, subid, size);
++if (header_length != PIPE_HEADER_SIZE)
++{
++ log_write(0, LOG_MAIN|LOG_PANIC_DIE, "header snprintf failed\n");
++ writebuffer[0] = '\0';
++}
++
++DEBUG(D_deliver) debug_printf("header write id:%c,subid:%c,size:%d,final:%s\n",
++ id, subid, size, writebuffer);
++
++if (buf && size > 0)
++ memcpy(writebuffer + PIPE_HEADER_SIZE, buf, size);
++
++size += PIPE_HEADER_SIZE;
++int ret = write(fd, writebuffer, size);
+ if(ret != size)
+ log_write(0, LOG_MAIN|LOG_PANIC_DIE, "Failed writing transport result to pipe: %s\n",
+ ret == -1 ? strerror(errno) : "short write");
+@@ -4100,8 +4183,8 @@ for (delivery_count = 0; addr_remote !=
+ for (h = addr->host_list; h != NULL; h = h->next)
+ {
+ if (h->address == NULL || h->status < hstatus_unusable) continue;
+- sprintf(CS big_buffer, "H%c%c%s", h->status, h->why, h->address);
+- rmt_dlv_checked_write(fd, big_buffer, Ustrlen(big_buffer+3) + 4);
++ sprintf(CS big_buffer, "%c%c%s", h->status, h->why, h->address);
++ rmt_dlv_checked_write(fd, 'H', '0', big_buffer, Ustrlen(big_buffer+2) + 3);
+ }
+
+ /* The number of bytes written. This is the same for each address. Even
+@@ -4109,9 +4192,8 @@ for (delivery_count = 0; addr_remote !=
+ size of each one is the same, and it's that value we have got because
+ transport_count gets reset before calling transport_write_message(). */
+
+- big_buffer[0] = 'S';
+- memcpy(big_buffer+1, &transport_count, sizeof(transport_count));
+- rmt_dlv_checked_write(fd, big_buffer, sizeof(transport_count) + 1);
++ memcpy(big_buffer, &transport_count, sizeof(transport_count));
++ rmt_dlv_checked_write(fd, 'S', '0', big_buffer, sizeof(transport_count));
+
+ /* Information about what happened to each address. Four item types are
+ used: an optional 'X' item first, for TLS information, then an optional "C"
+@@ -4131,7 +4213,7 @@ for (delivery_count = 0; addr_remote !=
+ if (addr->cipher)
+ {
+ ptr = big_buffer;
+- sprintf(CS ptr, "X1%.128s", addr->cipher);
++ sprintf(CS ptr, "%.128s", addr->cipher);
+ while(*ptr++);
+ if (!addr->peerdn)
+ *ptr++ = 0;
+@@ -4141,35 +4223,33 @@ for (delivery_count = 0; addr_remote !=
+ while(*ptr++);
+ }
+
+- rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
++ rmt_dlv_checked_write(fd, 'X', '1', big_buffer, ptr - big_buffer);
+ }
+ if (addr->peercert)
+ {
+ ptr = big_buffer;
+- *ptr++ = 'X'; *ptr++ = '2';
+ if (!tls_export_cert(ptr, big_buffer_size-2, addr->peercert))
+ while(*ptr++);
+ else
+ *ptr++ = 0;
+- rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
++ rmt_dlv_checked_write(fd, 'X', '2', big_buffer, ptr - big_buffer);
+ }
+ if (addr->ourcert)
+ {
+ ptr = big_buffer;
+- *ptr++ = 'X'; *ptr++ = '3';
+ if (!tls_export_cert(ptr, big_buffer_size-2, addr->ourcert))
+ while(*ptr++);
+ else
+ *ptr++ = 0;
+- rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
++ rmt_dlv_checked_write(fd, 'X', '3', big_buffer, ptr - big_buffer);
+ }
+ #ifndef DISABLE_OCSP
+ if (addr->ocsp > OCSP_NOT_REQ)
+ {
+ ptr = big_buffer;
+- sprintf(CS ptr, "X4%c", addr->ocsp + '0');
++ sprintf(CS ptr, "%c", addr->ocsp + '0');
+ while(*ptr++);
+- rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
++ rmt_dlv_checked_write(fd, 'X', '4', big_buffer, ptr - big_buffer);
+ }
+ # endif
+ #endif /*SUPPORT_TLS*/
+@@ -4177,34 +4257,33 @@ for (delivery_count = 0; addr_remote !=
+ if (client_authenticator)
+ {
+ ptr = big_buffer;
+- sprintf(CS big_buffer, "C1%.64s", client_authenticator);
++ sprintf(CS big_buffer, "%.64s", client_authenticator);
+ while(*ptr++);
+- rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
++ rmt_dlv_checked_write(fd, 'C', '1', big_buffer, ptr - big_buffer);
+ }
+ if (client_authenticated_id)
+ {
+ ptr = big_buffer;
+- sprintf(CS big_buffer, "C2%.64s", client_authenticated_id);
++ sprintf(CS big_buffer, "%.64s", client_authenticated_id);
+ while(*ptr++);
+- rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
++ rmt_dlv_checked_write(fd, 'C', '2', big_buffer, ptr - big_buffer);
+ }
+ if (client_authenticated_sender)
+ {
+ ptr = big_buffer;
+- sprintf(CS big_buffer, "C3%.64s", client_authenticated_sender);
++ sprintf(CS big_buffer, "%.64s", client_authenticated_sender);
+ while(*ptr++);
+- rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
++ rmt_dlv_checked_write(fd, 'C', '3', big_buffer, ptr - big_buffer);
+ }
+
+ #ifndef DISABLE_PRDR
+ if (addr->flags & af_prdr_used)
+- rmt_dlv_checked_write(fd, "P", 1);
++ rmt_dlv_checked_write(fd, 'P', '0', NULL, 0);
+ #endif
+
+ #ifdef EXPERIMENTAL_DSN
+- big_buffer[0] = 'D';
+- memcpy(big_buffer+1, &addr->dsn_aware, sizeof(addr->dsn_aware));
+- rmt_dlv_checked_write(fd, big_buffer, sizeof(addr->dsn_aware) + 1);
++ memcpy(big_buffer, &addr->dsn_aware, sizeof(addr->dsn_aware));
++ rmt_dlv_checked_write(fd, 'D', '0', big_buffer, sizeof(addr->dsn_aware));
+ DEBUG(D_deliver) debug_printf("DSN write: addr->dsn_aware = %d\n", addr->dsn_aware);
+ #endif
+
+@@ -4213,7 +4292,7 @@ for (delivery_count = 0; addr_remote !=
+ for (r = addr->retries; r != NULL; r = r->next)
+ {
+ uschar *ptr;
+- sprintf(CS big_buffer, "R%c%.500s", r->flags, r->key);
++ sprintf(CS big_buffer, "%c%.500s", r->flags, r->key);
+ ptr = big_buffer + Ustrlen(big_buffer+2) + 3;
+ memcpy(ptr, &(r->basic_errno), sizeof(r->basic_errno));
+ ptr += sizeof(r->basic_errno);
+@@ -4224,13 +4303,13 @@ for (delivery_count = 0; addr_remote !=
+ sprintf(CS ptr, "%.512s", r->message);
+ while(*ptr++);
+ }
+- rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
++ rmt_dlv_checked_write(fd, 'R', '0', big_buffer, ptr - big_buffer);
+ }
+
+ /* The rest of the information goes in an 'A' item. */
+
+- ptr = big_buffer + 3;
+- sprintf(CS big_buffer, "A%c%c", addr->transport_return,
++ ptr = big_buffer + 2;
++ sprintf(CS big_buffer, "%c%c", addr->transport_return,
+ addr->special_action);
+ memcpy(ptr, &(addr->basic_errno), sizeof(addr->basic_errno));
+ ptr += sizeof(addr->basic_errno);
+@@ -4265,7 +4344,7 @@ for (delivery_count = 0; addr_remote !=
+ : addr->host_used->dnssec==DS_NO ? '1' : '0';
+
+ }
+- rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
++ rmt_dlv_checked_write(fd, 'A', '0', big_buffer, ptr - big_buffer);
+ }
+
+ /* Add termination flag, close the pipe, and that's it. The character
+@@ -4273,9 +4352,8 @@ for (delivery_count = 0; addr_remote !=
+ A change from non-NULL to NULL indicates a problem with a continuing
+ connection. */
+
+- big_buffer[0] = 'Z';
+- big_buffer[1] = (continue_transport == NULL)? '0' : '1';
+- rmt_dlv_checked_write(fd, big_buffer, 2);
++ big_buffer[0] = (continue_transport == NULL)? '0' : '1';
++ rmt_dlv_checked_write(fd, 'Z', '0', big_buffer, 1);
+ (void)close(fd);
+ exit(EXIT_SUCCESS);
+ }
+--- a/src/macros.h
++++ b/src/macros.h
+@@ -156,6 +156,10 @@ as long as the maximum path length. */
+ #define BIG_BUFFER_SIZE 16384
+ #endif
+
++/* header size of pipe content
++ currently: char id, char subid, char[5] length */
++#define PIPE_HEADER_SIZE 7
++
+ /* This limits the length of data returned by local_scan(). Because it is
+ written on the spool, it gets read into big_buffer. */
+
diff -Nru exim4-4.84/debian/patches/series exim4-4.84/debian/patches/series
--- exim4-4.84/debian/patches/series 2015-10-31 13:50:54.000000000 +0100
+++ exim4-4.84/debian/patches/series 2015-11-21 11:25:42.000000000 +0100
@@ -15,3 +15,4 @@
84_Fix-truncation-of-items-in-headers_remove-lists-this.patch
85_Fix-crash-in-mime-acl-when-a-parameter-is-unterminat.patch
86_Avoid-crash-with-badly-terminated-non-recognised-mim.patch
+87_Fix-transport-results-pipe-for-multiple-recipients-c.patch
Reply to: