Fix DKIM signing to always ;-terminate. Bug 2295

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 7372ac24400535226b96c7d80713727a8b601019..8d368c8f3c49f445a9c60d8c96d69cf6c696cf8a 100644 (file)
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -81,6 +81,11 @@ JH/16 Bug 2615: Fix pause during message reception, on systems that have been
       post-message next-tick-wait.  Also change to using CLOCK_BOOTTIME if it
       exists, just to get a clock slightly more aligned to reality.
 
       post-message next-tick-wait.  Also change to using CLOCK_BOOTTIME if it
       exists, just to get a clock slightly more aligned to reality.
 

+JH/17 Bug 2295: Fix DKIM signing to always semicolon-terminate.  Although the
+      RFC says it is optional some validators care.  The missing char was not
+      intended but triggered by a line-wrap alignement.  Discovery and fix by
+      Guillaume Outters, hacked on by JH.
+

 
 Exim version 4.94
 -----------------
 
 Exim version 4.94
 -----------------

diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c
index 7712409b55fece810d4dca371b908690118cb75c..3a6ca4e91b9955c726d9eb18243b830dde2e138b 100644 (file)
--- a/src/src/pdkim/pdkim.c
+++ b/src/src/pdkim/pdkim.c
@@ -1111,14 +1111,14 @@ return string_catn(str, US"\r\n\t", 3);
 
 /*
  * RFC 5322 specifies that header line length SHOULD be no more than 78
 
 /*
  * RFC 5322 specifies that header line length SHOULD be no more than 78

- * lets make it so!

  *  pdkim_headcat
  *
  *  pdkim_headcat
  *

- * returns uschar * (not nul-terminated)
+ * Returns gstring (not nul-terminated) appending to one supplied

  *
  * col: this int holds and receives column number (octets since last '\n')
  * str: partial string to append to
  *
  * col: this int holds and receives column number (octets since last '\n')
  * str: partial string to append to

- * pad: padding, split line or space after before or after eg: ";"
+ * pad: padding, split line or space after before or after eg: ";".
+ *               Only the initial charater is used.

  * intro: - must join to payload eg "h=", usually the tag name
  * payload: eg base64 data - long data can be split arbitrarily.
  *
  * intro: - must join to payload eg "h=", usually the tag name
  * payload: eg base64 data - long data can be split arbitrarily.
  *


@@ -1127,7 +1127,7 @@ return string_catn(str, US"\r\n\t", 3);
  * pairs and inside long values. it also always spaces or breaks after the
  * "pad"
  *
  * pairs and inside long values. it also always spaces or breaks after the
  * "pad"
  *

- * no guarantees are made for output given out-of range input. like tag
+ * No guarantees are made for output given out-of range input. like tag

  * names longer than 78, or bogus col. Input is assumed to be free of line breaks.
  */
 
  * names longer than 78, or bogus col. Input is assumed to be free of line breaks.
  */
 


@@ -1135,92 +1135,64 @@ static gstring *
 pdkim_headcat(int * col, gstring * str,
   const uschar * pad, const uschar * intro, const uschar * payload)
 {
 pdkim_headcat(int * col, gstring * str,
   const uschar * pad, const uschar * intro, const uschar * payload)
 {

-size_t l;
+int len, chomp, padded = 0;

 
 

-if (pad)
-  {
-  l = Ustrlen(pad);
-  if (*col + l > 78)
-    str = pdkim_hdr_cont(str, col);
-  str = string_catn(str, pad, l);
-  *col += l;
-  }
-
-l = (pad?1:0) + (intro?Ustrlen(intro):0);
+/* If we can fit at least the pad at the end of current line, do it now.
+Otherwise, wrap if there is a pad. */

 
 

-if (*col + l > 78)
-  { /*can't fit intro - start a new line to make room.*/
-  str = pdkim_hdr_cont(str, col);
-  l = intro?Ustrlen(intro):0;
-  }
-
-l += payload ? Ustrlen(payload):0 ;
-
-while (l>77)
-  { /* this fragment will not fit on a single line */
-  if (pad)
-    {
-    str = string_catn(str, US" ", 1);
-    *col += 1;
-    pad = NULL; /* only want this once */
-    l--;
-    }
-
-  if (intro)
-    {
-    size_t sl = Ustrlen(intro);
-
-    str = string_catn(str, intro, sl);
-    *col += sl;
-    l -= sl;
-    intro = NULL; /* only want this once */
-    }
-
-  if (payload)
+if (pad)
+  if (*col + 1 <= 78)

     {
     {

-    size_t sl = Ustrlen(payload);
-    size_t chomp = *col+sl < 77 ? sl : 78-*col;
-
-    str = string_catn(str, payload, chomp);
-    *col += chomp;
-    payload += chomp;
-    l -= chomp-1;
+    str = string_catn(str, pad, 1);
+    (*col)++;
+    pad = NULL;
+    padded = 1;

     }
     }

+  else
+    str = pdkim_hdr_cont(str, col);

 
 

-  /* the while precondition tells us it didn't fit. */
-  str = pdkim_hdr_cont(str, col);
-  }
+/* Special case: if the whole addition does not fit at the end of the current
+line, but could fit on a new line, wrap to give it its full, dedicated line.  */

 
 

-if (*col + l > 78)
+len = (pad ? 2 : padded)
+    + (intro ? Ustrlen(intro) : 0)
+    + (payload ? Ustrlen(payload) : 0);
+if (len <= 77 && *col+len > 78)

   {
   str = pdkim_hdr_cont(str, col);
   {
   str = pdkim_hdr_cont(str, col);

-  pad = NULL;
+  padded = 0;

   }
 
   }
 

+/* Either we already dealt with the pad or we know there is room */
+

 if (pad)
   {
 if (pad)
   {

+  str = string_catn(str, pad, 1);

   str = string_catn(str, US" ", 1);
   str = string_catn(str, US" ", 1);

-  *col += 1;
-  pad = NULL;
+  *col += 2;

   }
   }

-
-if (intro)
+else if (padded && *col < 78)

   {
   {

-  size_t sl = Ustrlen(intro);
-
-  str = string_catn(str, intro, sl);
-  *col += sl;
-  l -= sl;
-  intro = NULL;
+  str = string_catn(str, US" ", 1);
+  (*col)++;

   }
 
   }
 

-if (payload)
-  {
-  size_t sl = Ustrlen(payload);
+/* Call recursively with intro as payload: it gets the same, special treatment
+(that is, not split if < 78).  */

 
 

-  str = string_catn(str, payload, sl);
-  *col += sl;
-  }
+if (intro)
+  str = pdkim_headcat(col, str, NULL, NULL, intro);
+
+if (payload)
+  for (len = Ustrlen(payload); len; len -= chomp)
+    {
+    if (*col >= 78)
+      str = pdkim_hdr_cont(str, col);
+    chomp = *col+len > 78 ? 78 - *col : len;
+    str = string_catn(str, payload, chomp);
+    *col += chomp;
+    payload += chomp;
+    }

 
 return str;
 }
 
 return str;
 }

diff --git a/test/mail/4520.b12 b/test/mail/4520.b12
index e327a0deb4ee32295e6bac47a8752deb8de2a138..392cee332ef271217faf48f272364365a8116506 100644 (file)
--- a/test/mail/4520.b12
+++ b/test/mail/4520.b12
@@ -6,9 +6,9 @@ Received: from the.local.host.name ([ip4.ip4.ip4.ip4] helo=myhost.test.ex)
        for b12@test.ex; Tue, 2 Mar 1999 09:44:33 +0000
 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=test.ex;
        s=sel; h=X-mine:X-mine:From; bh=/Ab0giHZitYQbDhFszoqQRUkgqueaX9zatJttIU/plc=;
        for b12@test.ex; Tue, 2 Mar 1999 09:44:33 +0000
 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=test.ex;
        s=sel; h=X-mine:X-mine:From; bh=/Ab0giHZitYQbDhFszoqQRUkgqueaX9zatJttIU/plc=;

-        b=VSF3Fmf7fHLpqKWgaG0t/RJdd+7JJNRN/+q7Gcz0xtIOjt5tL4Pfc4VDSeQYPq6e1pCcjvj79k
-       57of1wr0KIfk5FAvUZXPu3OtRm71y1X2rUaQB8c2Y2SROSMyaT3tsPzaEigiptIeeOkULYkFl2Hln
-       15ssCcfufIlOx4EQ9fQA=;
+       b=VSF3Fmf7fHLpqKWgaG0t/RJdd+7JJNRN/+q7Gcz0xtIOjt5tL4Pfc4VDSeQYPq6e1pCcjvj79k5
+       7of1wr0KIfk5FAvUZXPu3OtRm71y1X2rUaQB8c2Y2SROSMyaT3tsPzaEigiptIeeOkULYkFl2Hln1
+       5ssCcfufIlOx4EQ9fQA=;

 Received: from CALLER by myhost.test.ex with local (Exim x.yz)
        (envelope-from <CALLER@myhost.test.ex>)
        id 10HmbE-0005vi-00
 Received: from CALLER by myhost.test.ex with local (Exim x.yz)
        (envelope-from <CALLER@myhost.test.ex>)
        id 10HmbE-0005vi-00

diff --git a/test/mail/4545.a b/test/mail/4545.a
index b4a931fa1f80f8202108b7b5325afaa204e786dc..18752a7785c474d2f4e7b576bca3113581ea085c 100644 (file)
--- a/test/mail/4545.a
+++ b/test/mail/4545.a
@@ -5,9 +5,9 @@ Received: from the.local.host.name ([ip4.ip4.ip4.ip4] helo=myhost.test.ex)
        id 10HmaY-0005vi-00
        for a@test.ex; Tue, 2 Mar 1999 09:44:33 +0000
 DKIM-Signature: v=1; a=ed25519-sha256; q=dns/txt; c=relaxed/relaxed; d=test.ex
        id 10HmaY-0005vi-00
        for a@test.ex; Tue, 2 Mar 1999 09:44:33 +0000
 DKIM-Signature: v=1; a=ed25519-sha256; q=dns/txt; c=relaxed/relaxed; d=test.ex

-       ; s=sed; h=From:To:Subject; bh=/Ab0giHZitYQbDhFszoqQRUkgqueaX9zatJttIU/plc=;
-        b=5fhyD3EILDrnL4DnkD4hDaeis7+GSzL9GMHrhIDZJjuJ00WD5iI8SQ1q9rDfzFL/Kdw0VIyB4R
-       Dq0a4H6HI+Bw==;
+       ; s=sed; h=From:To:Subject; bh=/Ab0giHZitYQbDhFszoqQRUkgqueaX9zatJttIU/plc=; 
+       b=5fhyD3EILDrnL4DnkD4hDaeis7+GSzL9GMHrhIDZJjuJ00WD5iI8SQ1q9rDfzFL/Kdw0VIyB4RD
+       q0a4H6HI+Bw==;

 Received: from CALLER by myhost.test.ex with local (Exim x.yz)
        (envelope-from <CALLER@myhost.test.ex>)
        id 10HmaX-0005vi-00
 Received: from CALLER by myhost.test.ex with local (Exim x.yz)
        (envelope-from <CALLER@myhost.test.ex>)
        id 10HmaX-0005vi-00