ARC: enhance debug for signing; explicitly init signing context

7 files changed, 108 insertions, 22 deletions

diff --git a/src/src/arc.c b/src/src/arc.c
index 06ca93736..5f51d614d 100644
--- a/src/src/arc.c
+++ b/src/src/arc.c
@@ -631,6 +631,7 @@ arc_ams_setup_vfy_bodyhash(arc_line * ams)
 int canon_head, canon_body;
 long bodylen;
 
+if (!ams->c.data) ams->c.data = US"simple";	/* RFC 6376 (DKIM) default */
 pdkim_cstring_to_canons(ams->c.data, ams->c.len, &canon_head, &canon_body);
 bodylen = ams->l.data
 	? strtol(CS string_copyn(ams->l.data, ams->l.len), NULL, 10) : -1;
@@ -771,10 +772,10 @@ for(inst = 0; as; as = as->next)
      || arc_cv_match(as->hdr_as, US"fail")
      )
     {
-    arc_state_reason = string_sprintf("i=%d fail"
+    arc_state_reason = string_sprintf("i=%d"
       " (cv, sequence or missing header)", as->instance);
-    DEBUG(D_acl) debug_printf("ARC %s\n", arc_state_reason);
-    ret = US"fail";
+    DEBUG(D_acl) debug_printf("ARC chain fail at %s\n", arc_state_reason);
+    return US"fail";
     }
 
   /* Evaluate the oldest-pass AMS validation while we're here.
@@ -1102,8 +1103,7 @@ return r;
 
 
 /* Walk the given headers strings identifying each header, and construct
-a reverse-order list.  Also parse ARC-chain headers and build the chain
-struct, retaining pointers into the string.
+a reverse-order list.
 */
 
 static hdr_rlist *
@@ -1486,6 +1486,14 @@ return pdkim_set_bodyhash(&dkim_sign_ctx,
 
 
 
+void
+arc_sign_init(void)
+{
+memset(&arc_sign_ctx, 0, sizeof(arc_sign_ctx));
+}
+
+
+
 /* A "normal" header line, identified by DKIM processing.  These arrive before
 the call to arc_sign(), which carries any newly-created DKIM headers - and
 those go textually before the normal ones in the message.
@@ -1551,12 +1559,8 @@ if (*privkey == '/' && !(privkey = expand_file_big_buffer(privkey)))
 
 DEBUG(D_transport) debug_printf("ARC: sign for %s\n", identity);
 
-/*
-- scan headers for existing ARC chain & A-R (with matching system-identfier)
-  - paniclog & skip on problems (no A-R)
-*/
-
-/* Make an rlist of any new DKIM headers, then add the "normals" rlist to it */
+/* Make an rlist of any new DKIM headers, then add the "normals" rlist to it.
+Then scan the list for an A-R header. */
 
 string_from_gstring(sigheaders);
 if ((rheaders = arc_sign_scan_headers(&arc_sign_ctx, sigheaders)))
@@ -1568,16 +1572,15 @@ if ((rheaders = arc_sign_scan_headers(&arc_sign_ctx, sigheaders)))
   }
 else
   rheaders = headers_rlist;
+
 /* Finally, build a normal-order headers list */
 /*XXX only needed for hunt-the-AR? */
-{
-header_line * hnext = NULL;
-for (; rheaders; hnext = rheaders->h, rheaders = rheaders->prev)
-  rheaders->h->next = hnext;
-headers = hnext;
-}
-
-instance = arc_sign_ctx.arcset_chain_last ? arc_sign_ctx.arcset_chain_last->instance + 1 : 1;
+  {
+  header_line * hnext = NULL;
+  for (; rheaders; hnext = rheaders->h, rheaders = rheaders->prev)
+    rheaders->h->next = hnext;
+  headers = hnext;
+  }
 
 if (!(arc_sign_find_ar(headers, identity, &ar)))
   {
@@ -1585,6 +1588,19 @@ if (!(arc_sign_find_ar(headers, identity, &ar)))
   return sigheaders ? sigheaders : string_get(0);
   }
 
+/* We previously built the data-struct for the existing ARC chain, if any, using a headers
+feed from the DKIM module.  Use that to give the instance number for the ARC set we are
+about to build. */
+
+DEBUG(D_transport)
+  if (arc_sign_ctx.arcset_chain_last)
+    debug_printf("ARC: existing chain highest instance: %d\n",
+      arc_sign_ctx.arcset_chain_last->instance);
+  else
+    debug_printf("ARC: no existing chain\n");
+
+instance = arc_sign_ctx.arcset_chain_last ? arc_sign_ctx.arcset_chain_last->instance + 1 : 1;
+
 /*
 - Generate AAR
   - copy the A-R; prepend i= & identity
@@ -1732,6 +1748,8 @@ if (arc_state)
     if (sender_host_address)
       g = string_append(g, 2, US" smtp.client-ip=", sender_host_address);
     }
+  else if (arc_state_reason)
+    g = string_append(g, 3, US" (", arc_state_reason, US")");
   DEBUG(D_acl) debug_printf("ARC:  authres '%.*s'\n",
 		  g->ptr - start - 3, g->s + start + 3);
   }
diff --git a/src/src/dkim_transport.c b/src/src/dkim_transport.c
index 28d567b03..114586803 100644
--- a/src/src/dkim_transport.c
+++ b/src/src/dkim_transport.c
@@ -151,8 +151,11 @@ if (!rc) return FALSE;
 
 /* Get signatures for headers plus spool data file */
 
-dkim->dot_stuffed = !!(save_options & topt_end_dot);
+#ifdef EXPERIMENTAL_ARC
+arc_sign_init();
+#endif
 
+dkim->dot_stuffed = !!(save_options & topt_end_dot);
 if (!(dkim_signature = dkim_exim_sign(deliver_datafile, SPOOL_DATA_START_OFFSET,
 				    hdrs, dkim, &errstr)))
   if (!(rc = dkt_sign_fail(dkim, &errno)))
@@ -264,6 +267,10 @@ if (!rc)
   goto CLEANUP;
   }
 
+#ifdef EXPERIMENTAL_ARC
+arc_sign_init();
+#endif
+
 /* Feed the file to the goats^W DKIM lib */
 
 dkim->dot_stuffed = !!(options & topt_end_dot);
diff --git a/src/src/functions.h b/src/src/functions.h
index 1b7aff083..9be5c32a4 100644
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -93,6 +93,7 @@ extern void    acl_var_write(uschar *, uschar *, void *);
 extern void   *arc_ams_setup_sign_bodyhash(void);
 extern const uschar *arc_header_feed(gstring *, BOOL);
 extern gstring *arc_sign(const uschar *, gstring *, uschar **);
+extern void     arc_sign_init(void);
 extern const uschar *acl_verify_arc(void);
 #endif
 
diff --git a/test/confs/4561 b/test/confs/4561
new file mode 120000
index 000000000..359a76cee
--- /dev/null
+++ b/test/confs/4561
@@ -0,0 +1 @@
+4560
\ No newline at end of file
diff --git a/test/log/4560 b/test/log/4560
index 88f7ff2f7..a89738027 100644
--- a/test/log/4560
+++ b/test/log/4560
@@ -84,6 +84,7 @@
 1999-03-02 09:44:33 End queue run: pid=pppp
 1999-03-02 09:44:33 Start queue run: pid=pppp
 1999-03-02 09:44:33 10HmbL-0005vi-00 arc_state: <fail>
+1999-03-02 09:44:33 10HmbL-0005vi-00 reason:    <i=3 (cv, sequence or missing header)>
 1999-03-02 09:44:33 10HmbL-0005vi-00 <= CALLER@bloggs.com H=localhost (test.ex) [127.0.0.1] P=esmtp S=sss for a@test.ex
 1999-03-02 09:44:33 10HmbK-0005vi-00 => a@test.ex <za@test.ex> R=fwd T=tsmtp H=127.0.0.1 [127.0.0.1] C="250 OK id=10HmbL-0005vi-00"
 1999-03-02 09:44:33 10HmbK-0005vi-00 Completed
@@ -119,6 +120,7 @@
 1999-03-02 09:44:33 10HmbQ-0005vi-00 DKIM: d=dmarc.org s=clochette c=simple/simple a=rsa-sha256 b=1024 t=1517535263 [verification succeeded]
 1999-03-02 09:44:33 10HmbQ-0005vi-00 DKIM: d=convivian.com s=default c=simple/simple a=rsa-sha256 b=1024 t=1517535248 [verification failed - body hash mismatch (body probably modified in transit)]
 1999-03-02 09:44:33 10HmbQ-0005vi-00 arc_state: <fail>
+1999-03-02 09:44:33 10HmbQ-0005vi-00 reason:    <i=2 (cv, sequence or missing header)>
 1999-03-02 09:44:33 10HmbQ-0005vi-00 <= CALLER@bloggs.com H=localhost (test.ex) [127.0.0.1] P=esmtp S=sss DKIM=dmarc.org id=1426665656.110316.1517535248039.JavaMail.zimbra@convivian.com for a@test.ex
 1999-03-02 09:44:33 10HmbP-0005vi-00 => a@test.ex <za@test.ex> R=fwd T=tsmtp H=127.0.0.1 [127.0.0.1] C="250 OK id=10HmbQ-0005vi-00"
 1999-03-02 09:44:33 10HmbP-0005vi-00 Completed
diff --git a/test/mail/4560.a b/test/mail/4560.a
index 4d3d3bde0..dd38aa6da 100644
--- a/test/mail/4560.a
+++ b/test/mail/4560.a
@@ -192,7 +192,7 @@ This is a generic mailinglist footer
 From CALLER@bloggs.com Tue Mar 02 09:44:33 1999
 Authentication-Results: test.ex;
 	iprev=pass (localhost);
-	arc=fail (i=3) header.s=sel arc.oldest-pass=0 smtp.client-ip=127.0.0.1
+	arc=fail (i=3 (cv, sequence or missing header))
 Received: from localhost ([127.0.0.1] helo=test.ex)
 	by test.ex with esmtp (Exim x.yz)
 	(envelope-from <CALLER@bloggs.com>)
@@ -321,7 +321,7 @@ Authentication-Results: test.ex;
 	dkim=pass header.d=dmarc.org header.s=clochette header.a=rsa-sha256;
 	dkim=fail (body hash mismatch; body probably modified in transit)
 		 header.d=convivian.com header.s=default header.a=rsa-sha256;
-	arc=fail (i=2) header.s=sel arc.oldest-pass=0 smtp.client-ip=127.0.0.1
+	arc=fail (i=2 (cv, sequence or missing header))
 Received: from localhost ([127.0.0.1] helo=test.ex)
 	by test.ex with esmtp (Exim x.yz)
 	(envelope-from <CALLER@bloggs.com>)
diff --git a/test/scripts/4560-ARC/4561 b/test/scripts/4560-ARC/4561
new file mode 100644
index 000000000..76f7f375a
--- /dev/null
+++ b/test/scripts/4560-ARC/4561
@@ -0,0 +1,57 @@
+# ARC verify, bad chain
+#
+exim -DSERVER=server -bd -oX PORT_D
+****
+#
+# This should fail verify (not a complete chain).
+client 127.0.0.1 PORT_D
+??? 220
+HELO xxx
+??? 250
+MAIL FROM:<CALLER@bloggs.com>
+??? 250
+RCPT TO:<a@test.ex>
+??? 250
+DATA
+??? 354
+ARC-Seal: i=2; cv=none; a=rsa-sha256; d=test.ex; s=r201803;
+      b=HxjMzNcj7OX+I9Vr1Xr14AGgAci/CI8JxspaeoNT7TBsiOAtZ+YDBBSqXe6fqX3mHQEwpnXrdz
+       PCMIU1SF3ZiBtqWaLBPhStfuNQl5cw+TWXC60rOwCD2bxuBqubM/3AZLMPzIpm62MUYUUGaxwi
+       +LssT4F237WN88Lu4g5vqi8=;
+ARC-Authentication-Results: i=2; test.ex;
+     iprev=fail;
+     auth=pass (PLAIN) smtp.auth=fred@test.ex
+ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed; d=test.ex; s=r201803;
+     bh=52LTOZoDLUGP5RZMMrrjLkwTKuJ59wx4n3rL9RKBtgg=;
+     h=Content-Transfer-Encoding:Content-Type:MIME-Version:Date:Message-ID:Subject:
+       From:To:Content-Transfer-Encoding:Content-Type:MIME-Version:Date:Message-ID:
+       Subject:From:To;
+     b=LYAs/k3m790qXfftAVQxqHFCUkqcavgcpKnbEje3MgCmpCiYzeeusloVSjyFx6Mdo0XkN0GSZb
+       HXOCRGaJVnpU9V1QzdIqvz/I7DAyWl53XsKxl9YhsuaeuMMgPpqWpYkp0mOIo3Mtg+VdbF2DKd
+       O8BRJnqfkZkGUqMUJzdaYMU=;
+Authentication-Results: test.ex;
+     iprev=fail;
+     auth=pass (PLAIN) smtp.auth=fred@test.ex
+Received: from [127.0.0.1]
+To: a@test.ex
+From: Jeremy Harris <b@test.ex>
+Subject: another test
+Message-ID: <3885245d-3bae-66a2-7a1e-0dbceae2fb50@test.ex>
+Date: Mon, 26 Mar 2018 10:55:18 +0100
+
+This is a simple test.
+.
+??? 250
+QUIT
+??? 221
+****
+exim -DSERVER=server -DNOTDAEMON -q
+****
+#
+#
+#
+#
+#
+killdaemon
+no_stdout_check
+no_msglog_check