X-Git-Url: https://git.hcoop.net/hcoop/debian/exim4.git/blobdiff_plain/493d55f6840d04ef186778724fc67530b1600113..0baa7b9df9e8d0188307c635776394b0db691e7d:/debian/patches/87_Fix-transport-results-pipe-for-multiple-recipients-c.patch diff --git a/debian/patches/87_Fix-transport-results-pipe-for-multiple-recipients-c.patch b/debian/patches/87_Fix-transport-results-pipe-for-multiple-recipients-c.patch deleted file mode 100644 index 8da9b4f..0000000 --- a/debian/patches/87_Fix-transport-results-pipe-for-multiple-recipients-c.patch +++ /dev/null @@ -1,384 +0,0 @@ -From bd21a787cdeef803334a6c7bf50d23b2a18cbd6f Mon Sep 17 00:00:00 2001 -From: Wolfgang Breyha -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. */ -