ARC signing fails on AR headers with authres-version or comments before resinfo

[Nikolay]

ARC signing doesn't support full spectrum of valid Authentication-Results header values (https://datatracker.ietf.org/doc/html/rfc7601#section-2.2) and instead fails with an error like:

 Traceback (most recent call last):
   File "arc-sign/dkim/__init__.py", line 1075, in sign
     parsed_auth_results = authres.AuthenticationResultsHeader.parse('Authentication-Results: ' + auth_results.decode('utf-8'))
   File "arc-sign/authres/__init__.py", line 206, in parse
     return authres.core.AuthenticationResultsHeader.parse(core_features(), string)
   File "arc-sign/authres/core.py", line 442, in parse
     return self.parse_value(feature_context, string)
   File "arc-sign/authres/core.py", line 454, in parse_value
     header._parse()
   File "arc-sign/authres/core.py", line 517, in _parse
     result = self._parse_resinfo()
   File "arc-sign/authres/core.py", line 550, in _parse_resinfo
     method, version, result = self._parse_methodspec()
   File "arc-sign/authres/core.py", line 573, in _parse_methodspec
     raise SyntaxError('Expected "="', self._parse_text)
 authres.core.SyntaxError: Syntax error: Expected "=" at: ;
   dkim=pass (2048-bit key)
 #011header.d=...

This fails when a header like the following is present in the mail:

 Authentication-Results: example.net (amavisd-new) 1;
         dkim=pass (2048-bit key)
         header.d=domain.tld

Note that either a comment ("(amavisd-new)" above) or authres-version ("1" above). Extra whitespace before the first ';' may also cause issues.

Library version: dkimpy==1.1.5

The following patch fixes the issue at the cost of stripping comments from AR headers for the purposes of AAR header construction. This seems to be allowed by the RFC, since comments are intended to be used by humans.

--- dkim/__init__.py.orig 2024-02-06 12:32:56.324616105 +0000
+++ dkim/__init__.py 2024-02-06 15:20:14.866534604 +0000
@@ -1052,28 +1052,26 @@
     # extract, parse, filter & group AR headers
     ar_headers = [res.strip() for [ar, res] in self.headers if ar == b'Authentication-Results']

- grouped_headers = []
+ parsed_ar_headers = []
     for res in ar_headers:
         try: # see LP: #1884044
- grouped_headers.append((res, authres.AuthenticationResultsHeader.parse('Authentication-Results: ' + res.decode('utf-8'))))
+ # Note: parsing headers currently strips embedded comments
+ parsed_ar_headers.append(authres.AuthenticationResultsHeader.parse('Authentication-Results: ' + res.decode('utf-8')))
         except authres.core.SyntaxError:
             # Skip over invalid AR header fields
             pass
- auth_headers = [res for res in grouped_headers if res[1].authserv_id == srv_id.decode('utf-8')]
+ auth_headers = [header for header in parsed_ar_headers if header.authserv_id == srv_id.decode('utf-8')]

     if len(auth_headers) == 0:
       self.logger.debug("no AR headers found, chain terminated")
       return []

     # consolidate headers
- results_lists = [raw.replace(srv_id + b';', b'', 1).strip() for (raw, parsed) in auth_headers]
- results_lists = [tags.split(b';') for tags in results_lists]
- results = [tag.strip() for sublist in results_lists for tag in sublist]
- auth_results = srv_id + b'; ' + (b';' + self.linesep + b' ').join(results)
+ results = [res for header in auth_headers for res in header.results]
+ auth_results = srv_id + b''.join(b';' + self.linesep + b' ' + str(res).encode('utf-8') for res in results)

     # extract cv
- parsed_auth_results = authres.AuthenticationResultsHeader.parse('Authentication-Results: ' + auth_results.decode('utf-8'))
- arc_results = [res for res in parsed_auth_results.results if res.method == 'arc']
+ arc_results = [res for res in results if res.method == 'arc']
     if len(arc_results) == 0:
       chain_validation_status = CV_None
     elif len(arc_results) != 1:
--- dkim/tests/test_arc.py.orig 2024-02-06 14:48:53.902445378 +0000
+++ dkim/tests/test_arc.py 2024-02-06 15:20:44.687648362 +0000
@@ -74,7 +74,7 @@
         sig_lines = dkim.arc_sign(
             self.message, b"test", b"example.com", self.key, b"lists.example.org", timestamp="12345")

- expected_sig = [b'ARC-Seal: i=1; cv=none; a=rsa-sha256; d=example.com; s=test; t=12345;\r\n b=MBw2+L1/4PuYWJlt1tZlDtbOvyfbyH2t2N6DinFV/BIaB2LqbDKTYjXXk9HuuK1/qEkTd\r\n TxCYScIrtVO7pFbGiSawMuLatVzHNCqTURa1zBTXr2mKW1hgdmrtMMUcMVCYxr1AJpu6IYX\r\n VMIoOAn7tIDdO0VLokK6FnIXTWEAplQ=\r\n', b'ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed;\r\n d=example.com; s=test; t=12345; h=message-id : date : from : to :\r\n subject : from; bh=wE7NXSkgnx9PGiavN4OZhJztvkqPDlemV3OGuEnLwNo=;\r\n b=a0f6qc3k9eECTSR155A0TQS+LjqPFWfI/brQBA83EUz00SNxj1wmWykvs1hhBVeM0r1kE\r\n Qc6CKbzRYaBNSiFj4q8JBpRIujLz1qLyGmPuAI6ddu/Z/1hQxgpVcp/odmI1UMV2R+d+yQ7\r\n tUp3EQxF/GYNt22rV4rNmDmANZVqJ90=\r\n', b'ARC-Authentication-Results: i=1; lists.example.org; arc=none;\r\n spf=pass <email address hidden>;\r\n dkim=pass (1024-bit key) <email address hidden>;\r\n dmarc=pass\r\n']
+ expected_sig = [b'ARC-Seal: i=1; cv=none; a=rsa-sha256; d=example.com; s=test; t=12345;\r\n b=iSKjTQ93xUC6gt4yutHrOf0F/qb4E5voEeuucd66VhM4n/7ifBMMHqYwncgz9sefduM6C\r\n UthuUSzqE2YamkGXQgKPIG9t4ZCOrx1OXGE34WF9ZeI/E0csrN+wK7sq/RjgN3z4qxLPMsp\r\n lW+BUUHCNuCIvxcZ55Ky6evIb/Saj2o=\r\n', b'ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed;\r\n d=example.com; s=test; t=12345; h=message-id : date : from : to :\r\n subject : from; bh=wE7NXSkgnx9PGiavN4OZhJztvkqPDlemV3OGuEnLwNo=;\r\n b=a0f6qc3k9eECTSR155A0TQS+LjqPFWfI/brQBA83EUz00SNxj1wmWykvs1hhBVeM0r1kE\r\n Qc6CKbzRYaBNSiFj4q8JBpRIujLz1qLyGmPuAI6ddu/Z/1hQxgpVcp/odmI1UMV2R+d+yQ7\r\n tUp3EQxF/GYNt22rV4rNmDmANZVqJ90=\r\n', b'ARC-Authentication-Results: i=1; lists.example.org;\r\n arc=none;\r\n spf=pass <email address hidden>;\r\n dkim=pass <email address hidden>;\r\n dmarc=pass\r\n']
         self.assertEqual(expected_sig, sig_lines)

         (cv, res, reason) = dkim.arc_verify(b''.join(sig_lines) + self.message, dnsfunc=self.dnsfunc)

[Scott Kitterman (kitterman)]

Thank you. In the future, patches attached in a text file are much more useful since the web U/I strips leading spaced. I plan to include this in the next release.