debian/patches/87_Fix-transport-results-pipe-for-multiple-recipients-c.patch

   1 From bd21a787cdeef803334a6c7bf50d23b2a18cbd6f Mon Sep 17 00:00:00 2001
   2 From: Wolfgang Breyha <wbreyha@gmx.net>
   3 Date: Sun, 28 Sep 2014 13:40:45 +0100
   4 Subject: [PATCH] Fix transport-results pipe for multiple recipients combined
   5  with certs.
   6
   7 The previous parsing failed when a result item split over a buffer boundary;
   8 fix by prefixing sizes to items, and checking enough has been read as the
   9 initial parsing stage.
  10 ---
  11  doc/doc-txt/ChangeLog |   7 +++
  12  src/deliver.c     | 162 +++++++++++++++++++++++++++++++++++++-------------
  13  src/macros.h      |   4 ++
  14  3 files changed, 131 insertions(+), 42 deletions(-)
  15
  16 # diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
  17 # index 46fed6f..76ecc20 100644
  18 # --- a/doc/doc-txt/ChangeLog
  19 # +++ b/doc/doc-txt/ChangeLog
  20 # @@ -37,6 +37,13 @@ TL/04 Bugzilla 1216: Add -M (related messages) option to exigrep.
  21 #  TL/05 GitHub Issue 18: Adjust logic testing for true/false in redis lookups.
  22 #        Merged patch from Sebastian Wiedenroth.
  23 #
  24 # +JH/05 Fix results-pipe from transport process.  Several recipients, combined
  25 # +      with certificate use, exposed issues where response data items split
  26 # +      over buffer boundaries were not parsed properly.  This eventually
  27 # +      resulted in duplicates being sent.  This issue only became common enough
  28 # +      to notice due to the introduction of conection certificate information,
  29 # +      the item size being so much larger.  Found and fixed by Wolfgang Breyha.
  30 # +
  31 #  Exim version 4.84
  32 #  -----------------
  33 #  TL/01 Bugzilla 1506: Re-add a 'return NULL' to silence complaints from static
  34 --- a/src/deliver.c
  35 +++ b/src/deliver.c
  36 @@ -2823,6 +2823,8 @@ uschar *ptr = endptr;
  37  uschar *msg = p->msg;
  38  BOOL done = p->done;
  39  BOOL unfinished = TRUE;
  40 +/* minimum size to read is header size including id, subid and length */
  41 +int required = PIPE_HEADER_SIZE;
  42
  43  /* Loop through all items, reading from the pipe when necessary. The pipe
  44  is set up to be non-blocking, but there are two different Unix mechanisms in
  45 @@ -2845,12 +2847,15 @@ while (!done)
  46    {
  47    retry_item *r, **rp;
  48    int remaining = endptr - ptr;
  49 +  uschar header[PIPE_HEADER_SIZE + 1];
  50 +  uschar id, subid;
  51 +  uschar *endc;
  52
  53    /* Read (first time) or top up the chars in the buffer if necessary.
  54    There will be only one read if we get all the available data (i.e. don't
  55    fill the buffer completely). */
  56
  57 -  if (remaining < 2500 && unfinished)
  58 +  if (remaining < required && unfinished)
  59      {
  60      int len;
  61      int available = big_buffer_size - remaining;
  62 @@ -2883,17 +2888,64 @@ while (!done)
  63      won't read any more, as "unfinished" will get set FALSE. */
  64
  65      endptr += len;
  66 +    remaining += len;
  67      unfinished = len == available;
  68      }
  69
  70    /* If we are at the end of the available data, exit the loop. */
  71 -
  72    if (ptr >= endptr) break;
  73
  74 +  /* copy and read header */
  75 +  memcpy(header, ptr, PIPE_HEADER_SIZE);
  76 +  header[PIPE_HEADER_SIZE] = '\0';
  77 +  id = header[0];
  78 +  subid = header[1];
  79 +  required = Ustrtol(header + 2, &endc, 10) + PIPE_HEADER_SIZE;     /* header + data */
  80 +  if (*endc)
  81 +    {
  82 +    msg = string_sprintf("failed to read pipe from transport process "
  83 +      "%d for transport %s: error reading size from header", pid, addr->transport->driver_name);
  84 +    done = TRUE;
  85 +    break;
  86 +    }
  87 +
  88 +  DEBUG(D_deliver)
  89 +    debug_printf("header read  id:%c,subid:%c,size:%s,required:%d,remaining:%d,unfinished:%d\n",
  90 +                    id, subid, header+2, required, remaining, unfinished);
  91 +
  92 +  /* is there room for the dataset we want to read ? */
  93 +  if (required > big_buffer_size - PIPE_HEADER_SIZE)
  94 +    {
  95 +    msg = string_sprintf("failed to read pipe from transport process "
  96 +      "%d for transport %s: big_buffer too small! required size=%d buffer size=%d", pid, addr->transport->driver_name,
  97 +      required, big_buffer_size - PIPE_HEADER_SIZE);
  98 +    done = TRUE;
  99 +    break;
 100 +    }
 101 +
 102 +  /* we wrote all datasets with atomic write() calls
 103 +     remaining < required only happens if big_buffer was too small
 104 +     to get all available data from pipe. unfinished has to be true
 105 +     as well. */
 106 +  if (remaining < required)
 107 +    if (unfinished)
 108 +      continue;
 109 +    else
 110 +      {
 111 +      msg = string_sprintf("failed to read pipe from transport process "
 112 +        "%d for transport %s: required size=%d > remaining size=%d and unfinished=false",
 113 +        pid, addr->transport->driver_name, required, remaining);
 114 +      done = TRUE;
 115 +      break;
 116 +      }
 117 +
 118 +  /* step behind the header */
 119 +  ptr += PIPE_HEADER_SIZE;
 120 +
 121    /* Handle each possible type of item, assuming the complete item is
 122    available in store. */
 123
 124 -  switch (*ptr++)
 125 +  switch (id)
 126      {
 127      /* Host items exist only if any hosts were marked unusable. Match
 128      up by checking the IP address. */
 129 @@ -2990,7 +3042,7 @@ while (!done)
 130      #ifdef SUPPORT_TLS
 131      case 'X':
 132      if (addr == NULL) goto ADDR_MISMATCH;          /* Below, in 'A' handler */
 133 -    switch (*ptr++)
 134 +    switch (subid)
 135        {
 136        case '1':
 137        addr->cipher = NULL;
 138 @@ -3028,7 +3080,7 @@ while (!done)
 139      #endif     /*SUPPORT_TLS*/
 140
 141      case 'C':  /* client authenticator information */
 142 -    switch (*ptr++)
 143 +    switch (subid)
 144        {
 145        case '1':
 146         addr->authenticator = (*ptr)? string_copy(ptr) : NULL;
 147 @@ -3051,7 +3103,7 @@ while (!done)
 148
 149      #ifdef EXPERIMENTAL_DSN
 150      case 'D':
 151 -    if (addr == NULL) break;
 152 +    if (addr == NULL) goto ADDR_MISMATCH;
 153      memcpy(&(addr->dsn_aware), ptr, sizeof(addr->dsn_aware));
 154      ptr += sizeof(addr->dsn_aware);
 155      DEBUG(D_deliver) debug_printf("DSN read: addr->dsn_aware = %d\n", addr->dsn_aware);
 156 @@ -3119,7 +3171,7 @@ while (!done)
 157        continue_hostname = NULL;
 158        }
 159      done = TRUE;
 160 -    DEBUG(D_deliver) debug_printf("Z%c item read\n", *ptr);
 161 +    DEBUG(D_deliver) debug_printf("Z0%c item read\n", *ptr);
 162      break;
 163
 164      /* Anything else is a disaster. */
 165 @@ -3572,9 +3624,40 @@ while (parcount > max)
 166
 167
 168  static void
 169 -rmt_dlv_checked_write(int fd, void * buf, int size)
 170 +rmt_dlv_checked_write(int fd, char id, char subid, void * buf, int size)
 171  {
 172 -int ret = write(fd, buf, size);
 173 +uschar writebuffer[PIPE_HEADER_SIZE + BIG_BUFFER_SIZE];
 174 +int     header_length;
 175 +
 176 +/* we assume that size can't get larger then BIG_BUFFER_SIZE which currently is set to 16k */
 177 +/* complain to log if someone tries with buffer sizes we can't handle*/
 178 +
 179 +if (size > 99999)
 180 +{
 181 +  log_write(0, LOG_MAIN|LOG_PANIC_DIE,
 182 +    "Failed writing transport result to pipe: can't handle buffers > 99999 bytes. truncating!\n");
 183 +  size = 99999;
 184 +}
 185 +
 186 +/* to keep the write() atomic we build header in writebuffer and copy buf behind */
 187 +/* two write() calls would increase the complexity of reading from pipe */
 188 +
 189 +/* convert size to human readable string prepended by id and subid */
 190 +header_length = snprintf(writebuffer, PIPE_HEADER_SIZE+1, "%c%c%05d", id, subid, size);
 191 +if (header_length != PIPE_HEADER_SIZE)
 192 +{
 193 +  log_write(0, LOG_MAIN|LOG_PANIC_DIE, "header snprintf failed\n");
 194 +  writebuffer[0] = '\0';
 195 +}
 196 +
 197 +DEBUG(D_deliver) debug_printf("header write id:%c,subid:%c,size:%d,final:%s\n",
 198 +                                 id, subid, size, writebuffer);
 199 +
 200 +if (buf && size > 0)
 201 +  memcpy(writebuffer + PIPE_HEADER_SIZE, buf, size);
 202 +
 203 +size += PIPE_HEADER_SIZE;
 204 +int ret = write(fd, writebuffer, size);
 205  if(ret != size)
 206    log_write(0, LOG_MAIN|LOG_PANIC_DIE, "Failed writing transport result to pipe: %s\n",
 207      ret == -1 ? strerror(errno) : "short write");
 208 @@ -4100,8 +4183,8 @@ for (delivery_count = 0; addr_remote !=
 209      for (h = addr->host_list; h != NULL; h = h->next)
 210        {
 211        if (h->address == NULL || h->status < hstatus_unusable) continue;
 212 -      sprintf(CS big_buffer, "H%c%c%s", h->status, h->why, h->address);
 213 -      rmt_dlv_checked_write(fd, big_buffer, Ustrlen(big_buffer+3) + 4);
 214 +      sprintf(CS big_buffer, "%c%c%s", h->status, h->why, h->address);
 215 +      rmt_dlv_checked_write(fd, 'H', '0', big_buffer, Ustrlen(big_buffer+2) + 3);
 216        }
 217
 218      /* The number of bytes written. This is the same for each address. Even
 219 @@ -4109,9 +4192,8 @@ for (delivery_count = 0; addr_remote !=
 220      size of each one is the same, and it's that value we have got because
 221      transport_count gets reset before calling transport_write_message(). */
 222
 223 -    big_buffer[0] = 'S';
 224 -    memcpy(big_buffer+1, &transport_count, sizeof(transport_count));
 225 -    rmt_dlv_checked_write(fd, big_buffer, sizeof(transport_count) + 1);
 226 +    memcpy(big_buffer, &transport_count, sizeof(transport_count));
 227 +    rmt_dlv_checked_write(fd, 'S', '0', big_buffer, sizeof(transport_count));
 228
 229      /* Information about what happened to each address. Four item types are
 230      used: an optional 'X' item first, for TLS information, then an optional "C"
 231 @@ -4131,7 +4213,7 @@ for (delivery_count = 0; addr_remote !=
 232        if (addr->cipher)
 233          {
 234          ptr = big_buffer;
 235 -        sprintf(CS ptr, "X1%.128s", addr->cipher);
 236 +        sprintf(CS ptr, "%.128s", addr->cipher);
 237          while(*ptr++);
 238          if (!addr->peerdn)
 239           *ptr++ = 0;
 240 @@ -4141,35 +4223,33 @@ for (delivery_count = 0; addr_remote !=
 241            while(*ptr++);
 242            }
 243
 244 -        rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
 245 +        rmt_dlv_checked_write(fd, 'X', '1', big_buffer, ptr - big_buffer);
 246          }
 247        if (addr->peercert)
 248         {
 249          ptr = big_buffer;
 250 -       *ptr++ = 'X'; *ptr++ = '2';
 251         if (!tls_export_cert(ptr, big_buffer_size-2, addr->peercert))
 252           while(*ptr++);
 253         else
 254           *ptr++ = 0;
 255 -        rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
 256 +        rmt_dlv_checked_write(fd, 'X', '2', big_buffer, ptr - big_buffer);
 257         }
 258        if (addr->ourcert)
 259         {
 260          ptr = big_buffer;
 261 -       *ptr++ = 'X'; *ptr++ = '3';
 262         if (!tls_export_cert(ptr, big_buffer_size-2, addr->ourcert))
 263           while(*ptr++);
 264         else
 265           *ptr++ = 0;
 266 -        rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
 267 +        rmt_dlv_checked_write(fd, 'X', '3', big_buffer, ptr - big_buffer);
 268         }
 269        #ifndef DISABLE_OCSP
 270        if (addr->ocsp > OCSP_NOT_REQ)
 271         {
 272         ptr = big_buffer;
 273 -       sprintf(CS ptr, "X4%c", addr->ocsp + '0');
 274 +       sprintf(CS ptr, "%c", addr->ocsp + '0');
 275         while(*ptr++);
 276 -        rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
 277 +         rmt_dlv_checked_write(fd, 'X', '4', big_buffer, ptr - big_buffer);
 278         }
 279        # endif
 280        #endif   /*SUPPORT_TLS*/
 281 @@ -4177,34 +4257,33 @@ for (delivery_count = 0; addr_remote !=
 282        if (client_authenticator)
 283          {
 284          ptr = big_buffer;
 285 -       sprintf(CS big_buffer, "C1%.64s", client_authenticator);
 286 +       sprintf(CS big_buffer, "%.64s", client_authenticator);
 287          while(*ptr++);
 288 -        rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
 289 +        rmt_dlv_checked_write(fd, 'C', '1', big_buffer, ptr - big_buffer);
 290         }
 291        if (client_authenticated_id)
 292          {
 293          ptr = big_buffer;
 294 -       sprintf(CS big_buffer, "C2%.64s", client_authenticated_id);
 295 +       sprintf(CS big_buffer, "%.64s", client_authenticated_id);
 296          while(*ptr++);
 297 -        rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
 298 +        rmt_dlv_checked_write(fd, 'C', '2', big_buffer, ptr - big_buffer);
 299         }
 300        if (client_authenticated_sender)
 301          {
 302          ptr = big_buffer;
 303 -       sprintf(CS big_buffer, "C3%.64s", client_authenticated_sender);
 304 +       sprintf(CS big_buffer, "%.64s", client_authenticated_sender);
 305          while(*ptr++);
 306 -        rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
 307 +        rmt_dlv_checked_write(fd, 'C', '3', big_buffer, ptr - big_buffer);
 308         }
 309
 310        #ifndef DISABLE_PRDR
 311        if (addr->flags & af_prdr_used)
 312 -       rmt_dlv_checked_write(fd, "P", 1);
 313 +       rmt_dlv_checked_write(fd, 'P', '0', NULL, 0);
 314        #endif
 315
 316        #ifdef EXPERIMENTAL_DSN
 317 -      big_buffer[0] = 'D';
 318 -      memcpy(big_buffer+1, &addr->dsn_aware, sizeof(addr->dsn_aware));
 319 -      rmt_dlv_checked_write(fd, big_buffer, sizeof(addr->dsn_aware) + 1);
 320 +      memcpy(big_buffer, &addr->dsn_aware, sizeof(addr->dsn_aware));
 321 +      rmt_dlv_checked_write(fd, 'D', '0', big_buffer, sizeof(addr->dsn_aware));
 322        DEBUG(D_deliver) debug_printf("DSN write: addr->dsn_aware = %d\n", addr->dsn_aware);
 323        #endif
 324
 325 @@ -4213,7 +4292,7 @@ for (delivery_count = 0; addr_remote !=
 326        for (r = addr->retries; r != NULL; r = r->next)
 327          {
 328          uschar *ptr;
 329 -        sprintf(CS big_buffer, "R%c%.500s", r->flags, r->key);
 330 +        sprintf(CS big_buffer, "%c%.500s", r->flags, r->key);
 331          ptr = big_buffer + Ustrlen(big_buffer+2) + 3;
 332          memcpy(ptr, &(r->basic_errno), sizeof(r->basic_errno));
 333          ptr += sizeof(r->basic_errno);
 334 @@ -4224,13 +4303,13 @@ for (delivery_count = 0; addr_remote !=
 335            sprintf(CS ptr, "%.512s", r->message);
 336            while(*ptr++);
 337            }
 338 -        rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
 339 +        rmt_dlv_checked_write(fd, 'R', '0', big_buffer, ptr - big_buffer);
 340          }
 341
 342        /* The rest of the information goes in an 'A' item. */
 343
 344 -      ptr = big_buffer + 3;
 345 -      sprintf(CS big_buffer, "A%c%c", addr->transport_return,
 346 +      ptr = big_buffer + 2;
 347 +      sprintf(CS big_buffer, "%c%c", addr->transport_return,
 348          addr->special_action);
 349        memcpy(ptr, &(addr->basic_errno), sizeof(addr->basic_errno));
 350        ptr += sizeof(addr->basic_errno);
 351 @@ -4265,7 +4344,7 @@ for (delivery_count = 0; addr_remote !=
 352                : addr->host_used->dnssec==DS_NO ? '1' : '0';
 353
 354          }
 355 -      rmt_dlv_checked_write(fd, big_buffer, ptr - big_buffer);
 356 +      rmt_dlv_checked_write(fd, 'A', '0', big_buffer, ptr - big_buffer);
 357        }
 358
 359      /* Add termination flag, close the pipe, and that's it. The character
 360 @@ -4273,9 +4352,8 @@ for (delivery_count = 0; addr_remote !=
 361      A change from non-NULL to NULL indicates a problem with a continuing
 362      connection. */
 363
 364 -    big_buffer[0] = 'Z';
 365 -    big_buffer[1] = (continue_transport == NULL)? '0' : '1';
 366 -    rmt_dlv_checked_write(fd, big_buffer, 2);
 367 +    big_buffer[0] = (continue_transport == NULL)? '0' : '1';
 368 +    rmt_dlv_checked_write(fd, 'Z', '0', big_buffer, 1);
 369      (void)close(fd);
 370      exit(EXIT_SUCCESS);
 371      }
 372 --- a/src/macros.h
 373 +++ b/src/macros.h
 374 @@ -156,6 +156,10 @@ as long as the maximum path length. */
 375  #define BIG_BUFFER_SIZE 16384
 376  #endif
 377
 378 +/* header size of pipe content
 379 +   currently: char id, char subid, char[5] length */
 380 +#define PIPE_HEADER_SIZE 7
 381 +
 382  /* This limits the length of data returned by local_scan(). Because it is
 383  written on the spool, it gets read into big_buffer. */
 384