From 99ab56234b257791ecedd149fc48dc226826d9c2 Mon Sep 17 00:00:00 2001
From: Jeremy Harris <jgh146exb@wizmail.org>
Date: Sun, 10 Sep 2017 20:23:21 +0100
Subject: [PATCH] DKIM: fix signing bug induced by total size of parameter text
 causing header-line fold between "b=" and terminating ";" of pseudo-header.

---
 doc/doc-txt/ChangeLog |  5 +++++
 src/src/pdkim/pdkim.c | 38 +++++++++++++++++++++++++-------------
 2 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 18a43d292..afd41e40a 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -38,6 +38,11 @@ JH/21 Bug 2151 (partial):
                 Avoid using SIZE on the MAIL for a callout verify, on any but
       the main verify for receipient in uncached-mode.
 
+JH/26 Fix DKIM bug: when the pseudoheader generated for signing was exactly
+      the right size to place the terminating semicolon on its own folded
+      line, the header hash was calculated to an incorrect value thanks to
+      the (relaxed) space the fold became.
+
 
 Exim version 4.89
 -----------------
diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c
index 4c93de70d..7dad1b896 100644
--- a/src/src/pdkim/pdkim.c
+++ b/src/src/pdkim/pdkim.c
@@ -288,7 +288,7 @@ found:
 /* Performs "relaxed" canonicalization of a header. */
 
 static uschar *
-pdkim_relax_header(const uschar * header, int crlf)
+pdkim_relax_header(const uschar * header, BOOL append_crlf)
 {
 BOOL past_field_name = FALSE;
 BOOL seen_wsp = FALSE;
@@ -299,8 +299,8 @@ uschar * q = relaxed;
 for (p = header; *p; p++)
   {
   uschar c = *p;
-  /* Ignore CR & LF */
-  if (c == '\r' || c == '\n')
+
+  if (c == '\r' || c == '\n')	/* Ignore CR & LF */
     continue;
   if (c == '\t' || c == ' ')
     {
@@ -312,8 +312,8 @@ for (p = header; *p; p++)
   else
     if (!past_field_name && c == ':')
       {
-      if (seen_wsp) q--;	/* This removes WSP before the colon */
-      seen_wsp = TRUE;		/* This removes WSP after the colon */
+      if (seen_wsp) q--;	/* This removes WSP immediately before the colon */
+      seen_wsp = TRUE;		/* This removes WSP immediately after the colon */
       past_field_name = TRUE;
       }
     else
@@ -326,7 +326,7 @@ for (p = header; *p; p++)
 
 if (q > relaxed && q[-1] == ' ') q--; /* Squash eventual trailing SP */
 
-if (crlf) { *q++ = '\r'; *q++ = '\n'; }
+if (append_crlf) { *q++ = '\r'; *q++ = '\n'; }
 *q = '\0';
 return relaxed;
 }
@@ -1303,11 +1303,23 @@ if (sig->bodylength >= 0)
   }
 
 /* Preliminary or final version? */
-base64_b = final ? pdkim_encode_base64(&sig->sighash) : US"";
-hdr = pdkim_headcat(&col, hdr, &hdr_size, &hdr_len, US";", US"b=", base64_b);
+if (final)
+  {
+  base64_b = pdkim_encode_base64(&sig->sighash);
+  hdr = pdkim_headcat(&col, hdr, &hdr_size, &hdr_len, US";", US"b=", base64_b);
 
-/* add trailing semicolon: I'm not sure if this is actually needed */
-hdr = pdkim_headcat(&col, hdr, &hdr_size, &hdr_len, NULL, US";", US"");
+  /* add trailing semicolon: I'm not sure if this is actually needed */
+  hdr = pdkim_headcat(&col, hdr, &hdr_size, &hdr_len, NULL, US";", US"");
+  }
+else
+  {
+  /* To satisfy the rule "all surrounding whitespace [...] deleted"
+  ( RFC 6376 section 3.7 ) we ensure there is no whitespace here.  Otherwise
+  the headcat routine could insert a linebreak which the relaxer would reduce
+  to a single space preceding the terminating semicolon, resulting in an
+  incorrect header-hash. */
+  hdr = pdkim_headcat(&col, hdr, &hdr_size, &hdr_len, US";", US"b=;", US"");
+  }
 
 hdr[hdr_len] = '\0';
 return hdr;
@@ -1452,7 +1464,7 @@ while (sig)
 			p->value, (q - US p->value) + (p->next ? 1 : 0));
 
 	rh = sig->canon_headers == PDKIM_CANON_RELAXED
-	  ? pdkim_relax_header(p->value, 1) /* cook header for relaxed canon */
+	  ? pdkim_relax_header(p->value, TRUE) /* cook header for relaxed canon */
 	  : string_copy(CUS p->value);      /* just copy it for simple canon */
 
 	/* Feed header to the hash algorithm */
@@ -1513,7 +1525,7 @@ while (sig)
 	    /* cook header for relaxed canon, or just copy it for simple  */
 
 	    uschar * rh = sig->canon_headers == PDKIM_CANON_RELAXED
-	      ? pdkim_relax_header(hdrs->value, 1)
+	      ? pdkim_relax_header(hdrs->value, TRUE)
 	      : string_copy(CUS hdrs->value);
 
 	    /* Feed header to the hash algorithm */
@@ -1537,7 +1549,7 @@ while (sig)
 
   /* Relax header if necessary */
   if (sig->canon_headers == PDKIM_CANON_RELAXED)
-    sig_hdr = pdkim_relax_header(sig_hdr, 0);
+    sig_hdr = pdkim_relax_header(sig_hdr, FALSE);
 
   DEBUG(D_acl)
     {
-- 
2.30.2