DKIM: fix signing bug induced by total size of parameter text

[exim.git] / src / src / pdkim / pdkim.c

diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c
index 0ae075f712c4f3c1ad4a4f13b9e465a8d6ad9a86..7dad1b896b292b6534b6554834da4cca07cf02d0 100644 (file)
--- 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;
 }
@@ -582,8 +582,12 @@ DEBUG(D_acl)
          "PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
   }
 
-exim_sha_init(&sig->body_hash_ctx,
-              sig->algo == PDKIM_ALGO_RSA_SHA1 ? HASH_SHA1 : HASH_SHA256);
+if (!exim_sha_init(&sig->body_hash_ctx,
+              sig->algo == PDKIM_ALGO_RSA_SHA1 ? HASH_SHA1 : HASH_SHA256))
+  {
+  DEBUG(D_acl) debug_printf("PDKIM: hash init internal error\n");
+  return NULL;
+  }
 return sig;
 }
 
@@ -954,11 +958,13 @@ if (ctx->flags & PDKIM_MODE_SIGN)
 /* DKIM-Signature: headers are added to the verification list */
 else
   {
+#ifdef notdef
   DEBUG(D_acl)
     {
     debug_printf("PDKIM >> raw hdr: ");
     pdkim_quoteprint(CUS ctx->cur_header, Ustrlen(ctx->cur_header));
     }
+#endif
   if (strncasecmp(CCS ctx->cur_header,
                  DKIM_SIGNATURE_HEADERNAME,
                  Ustrlen(DKIM_SIGNATURE_HEADERNAME)) == 0)
@@ -1050,13 +1056,13 @@ else for (p = 0; p<len; p++)
        if ((rc = pdkim_header_complete(ctx)) != PDKIM_OK)
          return rc;
 
-       ctx->flags = ctx->flags & ~(PDKIM_SEEN_LF|PDKIM_SEEN_CR) | PDKIM_PAST_HDRS;
+       ctx->flags = (ctx->flags & ~(PDKIM_SEEN_LF|PDKIM_SEEN_CR)) | PDKIM_PAST_HDRS;
        DEBUG(D_acl) debug_printf(
            "PDKIM >> Body data for hash, canonicalized >>>>>>>>>>>>>>>>>>>>>>>>>>>>\n");
        continue;
        }
       else
-       ctx->flags = ctx->flags & ~PDKIM_SEEN_CR | PDKIM_SEEN_LF;
+       ctx->flags = (ctx->flags & ~PDKIM_SEEN_CR) | PDKIM_SEEN_LF;
       }
     else if (ctx->flags & PDKIM_SEEN_LF)
       {
@@ -1297,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;
@@ -1411,7 +1429,11 @@ while (sig)
   hdata.data = NULL;
   hdata.len = 0;
 
-  exim_sha_init(&hhash_ctx, is_sha1 ? HASH_SHA1 : HASH_SHA256);
+  if (!exim_sha_init(&hhash_ctx, is_sha1 ? HASH_SHA1 : HASH_SHA256))
+    {
+    DEBUG(D_acl) debug_printf("PDKIM: hask setup internal error\n");
+    break;
+    }
 
   DEBUG(D_acl) debug_printf(
       "PDKIM >> Header data for hash, canonicalized, in sequence >>>>>>>>>>>>>>\n");
@@ -1442,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 */
@@ -1503,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 */
@@ -1527,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)
     {
@@ -1717,8 +1739,13 @@ sig->selector = string_copy(US selector);
 sig->rsa_privkey = string_copy(US rsa_privkey);
 sig->algo = algo;
 
-exim_sha_init(&sig->body_hash_ctx,
-              algo == PDKIM_ALGO_RSA_SHA1 ? HASH_SHA1 : HASH_SHA256);
+if (!exim_sha_init(&sig->body_hash_ctx,
+              algo == PDKIM_ALGO_RSA_SHA1 ? HASH_SHA1 : HASH_SHA256))
+  {
+  DEBUG(D_acl) debug_printf("PDKIM: hash setup internal error\n");
+  return NULL;
+  }
+
 DEBUG(D_acl)
   {
   pdkim_signature s = *sig;