CHUNKING/DKIM: fix handling of lines having a leading dot

9 files changed, 97 insertions, 80 deletions

diff --git a/src/src/dkim.c b/src/src/dkim.c
index f09d438d5..3fa11c800 100644
--- a/src/src/dkim.c
+++ b/src/src/dkim.c
@@ -69,7 +69,7 @@ pdkim_init();
 
 
 void
-dkim_exim_verify_init(void)
+dkim_exim_verify_init(BOOL dot_stuffing)
 {
 /* There is a store-reset between header & body reception
 so cannot use the main pool. Any allocs done by Exim
@@ -85,7 +85,7 @@ if (dkim_verify_ctx)
 
 /* Create new context */
 
-dkim_verify_ctx = pdkim_init_verify(&dkim_exim_query_dns_txt);
+dkim_verify_ctx = pdkim_init_verify(&dkim_exim_query_dns_txt, dot_stuffing);
 dkim_collect_input = !!dkim_verify_ctx;
 
 /* Start feed up with any cached data */
@@ -102,7 +102,7 @@ int rc;
 
 store_pool = POOL_PERM;
 if (  dkim_collect_input
-   && (rc = pdkim_feed(dkim_verify_ctx, (char *)data, len)) != PDKIM_OK)
+   && (rc = pdkim_feed(dkim_verify_ctx, CS data, len)) != PDKIM_OK)
   {
   log_write(0, LOG_MAIN,
 	     "DKIM: validation error: %.100s", pdkim_errstr(rc));
@@ -460,10 +460,9 @@ switch (what)
 
 
 uschar *
-dkim_exim_sign(int dkim_fd, uschar * dkim_private_key,
-		const uschar * dkim_domain, uschar * dkim_selector,
-		uschar * dkim_canon, uschar * dkim_sign_headers)
+dkim_exim_sign(int dkim_fd, struct ob_dkim * dkim)
 {
+const uschar * dkim_domain;
 int sep = 0;
 uschar *seen_items = NULL;
 int seen_items_size = 0;
@@ -487,7 +486,7 @@ int old_pool = store_pool;
 
 store_pool = POOL_MAIN;
 
-if (!(dkim_domain = expand_cstring(dkim_domain)))
+if (!(dkim_domain = expand_cstring(dkim->dkim_domain)))
   {
   /* expansion error, do not send message. */
   log_write(0, LOG_MAIN | LOG_PANIC, "failed to expand "
@@ -525,7 +524,7 @@ while ((dkim_signing_domain = string_nextinlist(&dkim_domain, &sep,
 
   /* Set up $dkim_selector expansion variable. */
 
-  if (!(dkim_signing_selector = expand_string(dkim_selector)))
+  if (!(dkim_signing_selector = expand_string(dkim->dkim_selector)))
     {
     log_write(0, LOG_MAIN | LOG_PANIC, "failed to expand "
 	       "dkim_selector: %s", expand_string_message);
@@ -534,7 +533,8 @@ while ((dkim_signing_domain = string_nextinlist(&dkim_domain, &sep,
 
   /* Get canonicalization to use */
 
-  dkim_canon_expanded = dkim_canon ? expand_string(dkim_canon) : US"relaxed";
+  dkim_canon_expanded = dkim->dkim_canon
+    ? expand_string(dkim->dkim_canon) : US"relaxed";
   if (!dkim_canon_expanded)
     {
     /* expansion error, do not send message. */
@@ -556,8 +556,8 @@ while ((dkim_signing_domain = string_nextinlist(&dkim_domain, &sep,
     }
 
   dkim_sign_headers_expanded = NULL;
-  if (dkim_sign_headers)
-    if (!(dkim_sign_headers_expanded = expand_string(dkim_sign_headers)))
+  if (dkim->dkim_sign_headers)
+    if (!(dkim_sign_headers_expanded = expand_string(dkim->dkim_sign_headers)))
       {
       log_write(0, LOG_MAIN | LOG_PANIC, "failed to expand "
 		 "dkim_sign_headers: %s", expand_string_message);
@@ -567,7 +567,7 @@ while ((dkim_signing_domain = string_nextinlist(&dkim_domain, &sep,
 
   /* Get private key to use. */
 
-  if (!(dkim_private_key_expanded = expand_string(dkim_private_key)))
+  if (!(dkim_private_key_expanded = expand_string(dkim->dkim_private_key)))
     {
     log_write(0, LOG_MAIN | LOG_PANIC, "failed to expand "
 	       "dkim_private_key: %s", expand_string_message);
@@ -587,8 +587,8 @@ while ((dkim_signing_domain = string_nextinlist(&dkim_domain, &sep,
     /* Looks like a filename, load the private key. */
 
     memset(big_buffer, 0, big_buffer_size);
-    privkey_fd = open(CS dkim_private_key_expanded, O_RDONLY);
-    if (privkey_fd < 0)
+
+    if ((privkey_fd = open(CS dkim_private_key_expanded, O_RDONLY)) < 0)
       {
       log_write(0, LOG_MAIN | LOG_PANIC, "unable to open "
 		 "private key file for reading: %s",
@@ -610,16 +610,17 @@ while ((dkim_signing_domain = string_nextinlist(&dkim_domain, &sep,
   ctx = pdkim_init_sign( CS dkim_signing_domain,
 			 CS dkim_signing_selector,
 			 CS dkim_private_key_expanded,
-			 PDKIM_ALGO_RSA_SHA256);
+			 PDKIM_ALGO_RSA_SHA256,
+			 dkim->dot_stuffed);
   pdkim_set_optional(ctx,
-		      (char *) dkim_sign_headers_expanded,
+		      CS dkim_sign_headers_expanded,
 		      NULL,
 		      pdkim_canon,
 		      pdkim_canon, -1, 0, 0);
 
   lseek(dkim_fd, 0, SEEK_SET);
 
-  while ((sread = read(dkim_fd, &buf, 4096)) > 0)
+  while ((sread = read(dkim_fd, &buf, sizeof(buf))) > 0)
     if ((pdkim_rc = pdkim_feed(ctx, buf, sread)) != PDKIM_OK)
       goto pk_bad;
 
diff --git a/src/src/dkim.h b/src/src/dkim.h
index 0dade972f..8474962fc 100644
--- a/src/src/dkim.h
+++ b/src/src/dkim.h
@@ -6,8 +6,8 @@
 /* See the file NOTICE for conditions of use and distribution. */
 
 void    dkim_exim_init(void);
-uschar *dkim_exim_sign(int, uschar *, const uschar *, uschar *, uschar *, uschar *);
-void    dkim_exim_verify_init(void);
+uschar *dkim_exim_sign(int, struct ob_dkim *);
+void    dkim_exim_verify_init(BOOL);
 void    dkim_exim_verify_feed(uschar *, int);
 void    dkim_exim_verify_finish(void);
 void    dkim_exim_acl_setup(uschar *);
diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c
index f468d232b..915b90e5c 100644
--- a/src/src/pdkim/pdkim.c
+++ b/src/src/pdkim/pdkim.c
@@ -794,7 +794,7 @@ for (sig = ctx->sig; sig; sig = sig->next)
     }
 
   /* SIGNING -------------------------------------------------------------- */
-  if (ctx->mode == PDKIM_MODE_SIGN)
+  if (ctx->flags & PDKIM_MODE_SIGN)
     {
     sig->bodyhash = bh;
 
@@ -830,8 +830,32 @@ for (sig = ctx->sig; sig; sig = sig->next)
 
 
 
+static int
+pdkim_body_complete(pdkim_ctx * ctx)
+{
+pdkim_signature * sig = ctx->sig;	/*XXX assumes only one sig */
+
+/* In simple body mode, if any empty lines were buffered,
+replace with one. rfc 4871 3.4.3 */
+/*XXX checking the signed-body-bytes is a gross hack; I think
+it indicates that all linebreaks should be buffered, including
+the one terminating a text line */
+
+if (  sig && sig->canon_body == PDKIM_CANON_SIMPLE
+   && sig->signed_body_bytes == 0
+   && ctx->num_buffered_crlf > 0
+   )
+  pdkim_update_bodyhash(ctx, "\r\n", 2);
+
+ctx->flags |= PDKIM_SEEN_EOD;
+ctx->linebuf_offset = 0;
+return PDKIM_OK;
+}
+
+
+
 /* -------------------------------------------------------------------------- */
-/* Callback from pdkim_feed below for processing complete body lines */
+/* Call from pdkim_feed below for processing complete body lines */
 
 static int
 pdkim_bodyline_complete(pdkim_ctx *ctx)
@@ -841,33 +865,23 @@ int   n = ctx->linebuf_offset;
 pdkim_signature *sig = ctx->sig;	/*XXX assumes only one sig */
 
 /* Ignore extra data if we've seen the end-of-data marker */
-if (ctx->seen_eod) goto BAIL;
+if (ctx->flags & PDKIM_SEEN_EOD) goto BAIL;
 
 /* We've always got one extra byte to stuff a zero ... */
 ctx->linebuf[ctx->linebuf_offset] = '\0';
 
 /* Terminate on EOD marker */
-if (memcmp(p, ".\r\n", 3) == 0)
+if (ctx->flags & PDKIM_DOT_TERM)
   {
-  /* In simple body mode, if any empty lines were buffered,
-  replace with one. rfc 4871 3.4.3 */
-  /*XXX checking the signed-body-bytes is a gross hack; I think
-  it indicates that all linebreaks should be buffered, including
-  the one terminating a text line */
-  if (  sig && sig->canon_body == PDKIM_CANON_SIMPLE
-     && sig->signed_body_bytes == 0
-     && ctx->num_buffered_crlf > 0
-     )
-    pdkim_update_bodyhash(ctx, "\r\n", 2);
+  if ( memcmp(p, ".\r\n", 3) == 0)
+    return pdkim_body_complete(ctx);
 
-  ctx->seen_eod = TRUE;
-  goto BAIL;
-  }
-/* Unstuff dots */
-if (memcmp(p, "..", 2) == 0)
-  {
-  p++;
-  n--;
+  /* Unstuff dots */
+  if (memcmp(p, "..", 2) == 0)
+    {
+    p++;
+    n--;
+    }
   }
 
 /* Empty lines need to be buffered until we find a non-empty line */
@@ -927,7 +941,7 @@ ctx->num_headers++;
 if (ctx->num_headers > PDKIM_MAX_HEADERS) goto BAIL;
 
 /* SIGNING -------------------------------------------------------------- */
-if (ctx->mode == PDKIM_MODE_SIGN)
+if (ctx->flags & PDKIM_MODE_SIGN)
   {
   pdkim_signature *sig;
 
@@ -940,7 +954,7 @@ if (ctx->mode == PDKIM_MODE_SIGN)
 
 /* VERIFICATION ----------------------------------------------------------- */
 /* DKIM-Signature: headers are added to the verification list */
-if (ctx->mode == PDKIM_MODE_VERIFY)
+else
   {
   if (strncasecmp(CCS ctx->cur_header,
 		  DKIM_SIGNATURE_HEADERNAME,
@@ -990,11 +1004,15 @@ pdkim_feed(pdkim_ctx *ctx, char *data, int len)
 {
 int p;
 
+/* Alternate EOD signal, used in non-dotstuffing mode */
+if (!data)
+  pdkim_body_complete(ctx);
+
 for (p = 0; p<len; p++)
   {
   uschar c = data[p];
 
-  if (ctx->past_headers)
+  if (ctx->flags & PDKIM_PAST_HDRS)
     {
     /* Processing body byte */
     ctx->linebuf[ctx->linebuf_offset++] = c;
@@ -1013,28 +1031,27 @@ for (p = 0; p<len; p++)
       {
       if (c == '\n')
         {
-	if (ctx->seen_lf)
+	if (ctx->flags & PDKIM_SEEN_LF)
 	  {
 	  int rc = pdkim_header_complete(ctx); /* Seen last header line */
 	  if (rc != PDKIM_OK) return rc;
 
-	  ctx->past_headers = TRUE;
-	  ctx->seen_lf = 0;
+	  ctx->flags = ctx->flags & ~PDKIM_SEEN_LF | PDKIM_PAST_HDRS;
 	  DEBUG(D_acl) debug_printf(
 	      "PDKIM >> Body data for hash, canonicalized >>>>>>>>>>>>>>>>>>>>>>\n");
 	  continue;
 	  }
 	else
-	  ctx->seen_lf = TRUE;
+	  ctx->flags |= PDKIM_SEEN_LF;
 	}
-      else if (ctx->seen_lf)
+      else if (ctx->flags & PDKIM_SEEN_LF)
         {
 	if (!(c == '\t' || c == ' '))
 	  {
 	  int rc = pdkim_header_complete(ctx); /* End of header */
 	  if (rc != PDKIM_OK) return rc;
 	  }
-	ctx->seen_lf = FALSE;
+	ctx->flags &= ~PDKIM_SEEN_LF;
 	}
       }
 
@@ -1328,7 +1345,7 @@ while (sig)
      Then append to that list any remaining header names for which there was no
      header to sign. */
 
-  if (ctx->mode == PDKIM_MODE_SIGN)
+  if (ctx->flags & PDKIM_MODE_SIGN)
     {
     pdkim_stringlist *p;
     const uschar * l;
@@ -1449,11 +1466,11 @@ while (sig)
     }
 
   /* Remember headers block for signing (when the library cannot do incremental)  */
-  if (ctx->mode == PDKIM_MODE_SIGN)
+  if (ctx->flags & PDKIM_MODE_SIGN)
     (void) exim_rsa_data_append(&hdata, &hdata_alloc, US sig_hdr);
 
   /* SIGNING ---------------------------------------------------------------- */
-  if (ctx->mode == PDKIM_MODE_SIGN)
+  if (ctx->flags & PDKIM_MODE_SIGN)
     {
     es_ctx sctx;
     const uschar * errstr;
@@ -1616,15 +1633,15 @@ return PDKIM_OK;
 /* -------------------------------------------------------------------------- */
 
 DLLEXPORT pdkim_ctx *
-pdkim_init_verify(int(*dns_txt_callback)(char *, char *))
+pdkim_init_verify(int(*dns_txt_callback)(char *, char *), BOOL dot_stuffing)
 {
 pdkim_ctx * ctx;
 
 ctx = store_get(sizeof(pdkim_ctx));
 memset(ctx, 0, sizeof(pdkim_ctx));
 
+if (dot_stuffing) ctx->flags = PDKIM_DOT_TERM;
 ctx->linebuf = store_get(PDKIM_MAX_BODY_LINE_LEN);
-ctx->mode = PDKIM_MODE_VERIFY;
 ctx->dns_txt_callback = dns_txt_callback;
 
 return ctx;
@@ -1634,7 +1651,8 @@ return ctx;
 /* -------------------------------------------------------------------------- */
 
 DLLEXPORT pdkim_ctx *
-pdkim_init_sign(char *domain, char *selector, char *rsa_privkey, int algo)
+pdkim_init_sign(char *domain, char *selector, char *rsa_privkey, int algo,
+  BOOL dot_stuffed)
 {
 pdkim_ctx *ctx;
 pdkim_signature *sig;
@@ -1645,14 +1663,13 @@ if (!domain || !selector || !rsa_privkey)
 ctx = store_get(sizeof(pdkim_ctx));
 memset(ctx, 0, sizeof(pdkim_ctx));
 
+ctx->flags = dot_stuffed ? PDKIM_MODE_SIGN | PDKIM_DOT_TERM : PDKIM_MODE_SIGN;
 ctx->linebuf = store_get(PDKIM_MAX_BODY_LINE_LEN);
 
 sig = store_get(sizeof(pdkim_signature));
 memset(sig, 0, sizeof(pdkim_signature));
 
 sig->bodylength = -1;
-
-ctx->mode = PDKIM_MODE_SIGN;
 ctx->sig = sig;
 
 sig->domain = string_copy(US domain);
diff --git a/src/src/pdkim/pdkim.h b/src/src/pdkim/pdkim.h
index 5d0157e63..536e5af69 100644
--- a/src/src/pdkim/pdkim.h
+++ b/src/src/pdkim/pdkim.h
@@ -245,12 +245,14 @@ typedef struct pdkim_signature {
 
 /* -------------------------------------------------------------------------- */
 /* Context to keep state between all operations. */
-#define PDKIM_MODE_SIGN     0
-#define PDKIM_MODE_VERIFY   1
 typedef struct pdkim_ctx {
 
-  /* PDKIM_MODE_VERIFY or PDKIM_MODE_SIGN */
-  int mode;
+#define PDKIM_MODE_SIGN   BIT(0)	/* if unset, mode==verify */
+#define PDKIM_DOT_TERM	  BIT(1)	/* dot termination and unstuffing */
+#define PDKIM_SEEN_LF	  BIT(2)
+#define PDKIM_SEEN_EOD	  BIT(3)
+#define PDKIM_PAST_HDRS	  BIT(4)
+  unsigned   flags;
 
   /* One (signing) or several chained (verification) signatures */
   pdkim_signature *sig;
@@ -264,9 +266,6 @@ typedef struct pdkim_ctx {
   int        cur_header_len;
   char      *linebuf;
   int        linebuf_offset;
-  BOOL       seen_lf;
-  BOOL       seen_eod;
-  BOOL       past_headers;
   int        num_buffered_crlf;
   int        num_headers;
   pdkim_stringlist *headers; /* Raw headers for verification         */
@@ -285,10 +284,10 @@ extern "C" {
 void	   pdkim_init         (void);
 
 DLLEXPORT
-pdkim_ctx *pdkim_init_sign    (char *, char *, char *, int);
+pdkim_ctx *pdkim_init_sign    (char *, char *, char *, int, BOOL);
 
 DLLEXPORT
-pdkim_ctx *pdkim_init_verify  (int(*)(char *, char *));
+pdkim_ctx *pdkim_init_verify  (int(*)(char *, char *), BOOL);
 
 DLLEXPORT
 int        pdkim_set_optional (pdkim_ctx *, char *, char *,int, int,
diff --git a/src/src/receive.c b/src/src/receive.c
index 6a8ce8841..3351ab1c5 100644
--- a/src/src/receive.c
+++ b/src/src/receive.c
@@ -1615,8 +1615,10 @@ message_linecount = body_linecount = body_zerocount =
   max_received_linelength = 0;
 
 #ifndef DISABLE_DKIM
-/* Call into DKIM to set up the context. */
-if (smtp_input && !smtp_batched_input && !dkim_disable_verify) dkim_exim_verify_init();
+/* Call into DKIM to set up the context.  In CHUNKING mode
+we clear the dot-stuffing flag */
+if (smtp_input && !smtp_batched_input && !dkim_disable_verify)
+  dkim_exim_verify_init(chunking_state <= CHUNKING_OFFERED);
 #endif
 
 #ifdef EXPERIMENTAL_DMARC
diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c
index 6c9afc501..34481aef7 100644
--- a/src/src/smtp_in.c
+++ b/src/src/smtp_in.c
@@ -399,7 +399,7 @@ for(;;)
   if (chunking_state == CHUNKING_LAST)
     {
 #ifndef DISABLE_DKIM
-    dkim_exim_verify_feed(".\r\n", 3);	/* for consistency with .-term MAIL */
+    dkim_exim_verify_feed(NULL, 0);	/* notify EOD */
 #endif
     return EOD;
     }
diff --git a/src/src/structs.h b/src/src/structs.h
index 23c40ea3d..75eba6643 100644
--- a/src/src/structs.h
+++ b/src/src/structs.h
@@ -874,6 +874,7 @@ struct ob_dkim {
   uschar *dkim_canon;
   uschar *dkim_sign_headers;
   uschar *dkim_strict;
-} dkim;
+  BOOL    dot_stuffed;
+};
 
 /* End of structs.h */
diff --git a/src/src/transport.c b/src/src/transport.c
index fdd97822a..8e69fb686 100644
--- a/src/src/transport.c
+++ b/src/src/transport.c
@@ -1081,7 +1081,8 @@ if ((dkim_fd = Uopen(dkim_spool_name, O_RDWR|O_CREAT|O_TRUNC, SPOOL_MODE)) < 0)
   goto CLEANUP;
   }
 
-/* Call original function to write the -K file; does the CRLF expansion */
+/* Call original function to write the -K file; does the CRLF expansion
+(but, in the CHUNKING case, not dot-stuffing and dot-termination). */
 
 options = tctx->options;
 tctx->options &= ~topt_use_bdat;
@@ -1096,14 +1097,9 @@ if (!rc)
   }
 
 /* Rewind file and feed it to the goats^W DKIM lib */
+dkim->dot_stuffed = !!(options & topt_end_dot);
 lseek(dkim_fd, 0, SEEK_SET);
-dkim_signature = dkim_exim_sign(dkim_fd,
-				dkim->dkim_private_key,
-				dkim->dkim_domain,
-				dkim->dkim_selector,
-				dkim->dkim_canon,
-				dkim->dkim_sign_headers);
-if (dkim_signature)
+if ((dkim_signature = dkim_exim_sign(dkim_fd, dkim)))
   siglen = Ustrlen(dkim_signature);
 else if (dkim->dkim_strict)
   {
diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c
index 416ea6297..6b0781096 100644
--- a/src/src/transports/smtp.c
+++ b/src/src/transports/smtp.c
@@ -266,7 +266,8 @@ smtp_transport_options_block smtp_transport_option_defaults = {
     NULL,              /* dkim_private_key */
     NULL,              /* dkim_selector */
     NULL,              /* dkim_sign_headers */
-    NULL}              /* dkim_strict */
+    NULL,              /* dkim_strict */
+    FALSE}	       /* dot_stuffed */
 #endif
 };