Logging fixups: Don't traceback for non-UTF-8 data in mail headers and don't put byte string markers in logs (some remain, but are from dkimpy and should be fixed there), related to LP: #1980821

CHANGES

@@ -1,22 +1,7 @@
-1.2.3 2023-02-26
- - Improve support for non-ASCII email messages.  Anything UTF-8 should work
-   (including correct signing/verification).  For messages that contain header
-   fields with non-ASCII or UTF-8 content, signatures are likely fail
-   verification, but the milter should continue to run.  (Thanks to Casper
-   Bruun for help with this)
- - Set minimum pymilter and dkimpy versions in setup.py to those that will
-   work reliably with non-ASCII content.
- - Fixed support for percent in KeyTable - Thanks to Mika Tiainen
- - Fix formatting for MinimumKeyBits in dkimpy-milter.conf(5)
-   (Closes: #995335)
- - Reset the i= signature identity in get_identities_sign() (Closes: #981157)
- - Improve documentation of inter-relationship between Mode, InternalHosts,
-   MacroList, and MacroListVerify options in dkimpy-milter.conf.5 (Closes:
-   #969215)
- - Fix subdomain signing with top-level organizational domain (LP: #1999434)
-   - Thanks to Matthias Hunstock for the report and the fix
- - Fix comma separated list processing in dkimpy_milter/config.py
-   (LP: #1901445)
+1.2.3
+ - Logging fixups: Don't traceback for non-UTF-8 data in mail headers and
+   don't put byte string markers in logs (some remain, but are from dkimpy
+   and should be fixed there), related to LP: #1980821.
 
 1.2.2 2020-08-09
  - Improve README.md formating for markdown display on pypi

README.md

@@ -336,8 +336,3 @@ later support Ed25519 signing and verification.  RFC 8301 removed rsa-sha1
 from DKIM.  dkimpy-milter does not sign with rsa-sha1, but still considers
 rsa-sha1 signatures as valid for verification because they are still in
 common use and are not known to be cryptographically broken.
-
-Support for non-ASCII email messages:  Anything UTF-8 should work (including
-correct signing/verification).  For messages that contain header fields with
-non-ASCII or UTF-8 content, signatures are likely fail verification, but the
-milter should continue to run.  RFC 8616 is not supported.

dkimpy_milter/__init__.py

@@ -39,7 +39,7 @@ from dkimpy_milter.util import write_pid
 from dkimpy_milter.util import get_keys
 from dkimpy_milter.util import fold
 
-__version__ = "1.2.3"
+__version__ = "1.2.0"
 FWS = re.compile(r'\r?\n[ \t]+')
 
 
@@ -101,28 +101,16 @@ class dkimMilter(Milter.Base):
         if self.conf.get('Syslog') and self.conf.get('debugLevel') >= 1:
             syslog.syslog("connect from {0} at {1} {2}"
                           .format(hostname, hostaddr, connecttype))
-        if self.conf.get('Syslog') and self.conf.get('debugLevel') >= 3:
-            syslog.syslog("internal_conn: {0}, external_conn: {1}"
-                          .format(self.internal_connection, self.external_connection))
-
         return Milter.CONTINUE
 
     # multiple messages can be received on a single connection
     # envfrom (MAIL FROM in the SMTP protocol) seems to mark the start
     # of each message.
     @Milter.noreply
-    def envfrom(self, f, *moredata):
-        try:
-            f = str(codecs.encode(f, 'UTF-8', 'replace'), 'UTF-8', 'ignore')
-        except TypeError:
-            f = codecs.encode(f, 'UTF-8', 'replace').decode()
-        try:
-            moredata = str(codecs.encode(str(moredata), 'UTF-8', 'replace'), 'UTF-8', 'ignore')
-        except TypeError:
-            moredata = codecs.encode(str(moredata), 'UTF-8', 'replace').decode()
-
+    def envfrom(self, f, *stri):
         if self.conf.get('Syslog') and self.conf.get('debugLevel') >= 2:
-            syslog.syslog("mail from: {0} {1}".format(f, moredata))
+            f = str(bytes(f, encoding='utf-8', errors='replace'))[2:-1]
+            syslog.syslog("mail from: {0} {1}".format(f, stri))
         self.fp = io.BytesIO()
         self.mailfrom = f
         t = parse_addr(f)
@@ -146,7 +134,8 @@ class dkimMilter(Milter.Base):
         if lname == 'dkim-signature':
             if (self.conf.get('Syslog') and
                     self.conf.get('debugLevel') >= 1):
-                syslog.syslog("{0}: {1}".format(name, val))
+                val2 = str(bytes(val, encoding='utf-8', errors='replace'))[2:-1]
+                syslog.syslog("{0}: {1}".format(name, val2))
             self.has_dkim += 1
         if lname == 'from':
             fname, self.author = parseaddr(val)
@@ -154,23 +143,15 @@ class dkimMilter(Milter.Base):
                 self.fdomain = self.author.split('@')[1].lower()
             except IndexError as er:
                 pass # self.author was not a proper email address
-            # This keeps non-ascii characters out of the From domain
-            try:
-                self.fdomain = str(codecs.encode(self.fdomain, 'ascii', 'replace'), 'ascii', 'ignore')
-            except TypeError:
-                self.fdomain = codecs.encode(self.fdomain, 'ascii', 'replace').decode('ascii','ignore')
             if (self.conf.get('Syslog') and
                     self.conf.get('debugLevel') >= 1):
-                syslog.syslog("{0}: {1}".format(name, val))
+                val2 = str(bytes(val, encoding='utf-8', errors='replace'))[2:-1]
+                syslog.syslog("{0}: {1}".format(name, val2))
         elif lname == 'authentication-results':
             self.arheaders.append(val)
         if self.fp:
             try:
-                if lname == 'from':
-                    # Non-ascii in email address localpart is legal, so this is a special case
-                    self.fp.write(b"%s: %s\n" % (codecs.encode(name, 'ascii'), codecs.encode(val, 'UTF-8', 'replace')))
-                else:
-                    self.fp.write(b"%s: %s\n" % (codecs.encode(name, 'ascii'), codecs.encode(val, 'ascii')))
+                self.fp.write(b"%s: %s\n" % (codecs.encode(name, 'ascii'), codecs.encode(val, 'ascii')))
             except:
                 # Don't choke on header fields with non-ascii garbage in them.
                 pass
@@ -203,7 +184,8 @@ class dkimMilter(Milter.Base):
                     self.chgheader('authentication-results', i, '')
                     if (self.conf.get('Syslog') and
                             self.conf.get('debugLevel') >= 1):
-                        syslog.syslog('REMOVE: {0}'.format(val))
+                        val2 = str(bytes(val, encoding='utf-8', errors='replace'))[2:-1]
+                        syslog.syslog('REMOVE: {0}'.format(val2))
             except:
                 # Don't error out on unparseable AR header fiels
                 pass
@@ -217,8 +199,6 @@ class dkimMilter(Milter.Base):
             syslog.syslog('self.domain: {0}, self.fdomain: {1}, self.iequals: {2}'.format(self.domain, self.fdomain, self.iequals))
         if ((self.fdomain in self.domain) and not self.conf.get('Mode') == 'v'
                 and not self.external_connection):
-            if (self.conf.get('Syslog') and self.conf.get('debugLevel') >= 3):
-                syslog.syslog("Signing DKIM")
             self.sign_dkim(txt)
         if ((self.has_dkim) and (not self.internal_connection) and
             (self.conf.get('Mode') == 'v' or
@@ -251,7 +231,7 @@ class dkimMilter(Milter.Base):
     def get_identities_sign(self):
         """Determine d= and i= identiies for signature"""
         self.domain = []
-        self.iequals = None
+        iequals = None
         try:
             self.privkeyRSA = self.conf.get('privateRSA')
         except:
@@ -304,7 +284,6 @@ class dkimMilter(Milter.Base):
                         keytabledata = self.conf.get('privateRSATable')[keytablekey]
                         try:
                             self.fdomain = keytabledata[0]
-                            self.domain.append(self.fdomain)
                             self.selectorRSA = keytabledata[1]
                             self.privkeyRSA = keytabledata[2]
                         except:
@@ -315,14 +294,11 @@ class dkimMilter(Milter.Base):
                         keytabledata = self.conf.get('privateEd25519Table')[keytablekey]
                         try:
                             self.fdomain = keytabledata[0]
-                            self.domain.append(self.fdomain)
                             self.selectorEd25519 = keytabledata[1]
                             self.privkeyEd25519 = keytabledata[2]
                         except:
                             if (self.conf.get('Syslog')):
                                     syslog.syslog('Error: Invalid KeyTable data {0}'.format(keytabledata))
-                    if (self.fdomain == '%'):
-                        self.fdomain = self.author.split('@')[1].lower()
                     break
 
     def sign_dkim(self, txt):
@@ -456,7 +432,8 @@ class dkimMilter(Milter.Base):
                     if self.conf.get('Syslog'):
                         if d.domain:
                             syslog.syslog('DKIM: Fail ({0})'
-                                          .format(d.domain.lower()))
+                                .format(str(d.domain.lower(), 'UTF-8',
+                                errors='replace')))
                         else:
                             syslog.syslog('DKIM: Fail, unextractable domain')
             if res:
@@ -466,13 +443,13 @@ class dkimMilter(Milter.Base):
             res = False
             if self.header_d:
                 self.arresults.append(
-                    authres.DKIMAuthenticationResult(result=result,
+                    authres.DKIMAuthenticationResult(result=result[2:-1],
                                                  header_i=self.header_i,
                                                  header_d=self.header_d,
                                                  header_a=self.header_a,
                                                  result_comment=
                                                  self.dkim_comment)
-            )
+                )
             self.header_a = None
         return
 

dkimpy_milter/config.py

@@ -89,23 +89,16 @@ class HostsDataset(object):
                 self.item = item[1:]
                 self.negative = True
             try:
-                try:
-                    self.item = ipaddress.ip_address(str(self.item, "utf-8"))
-                except TypeError:
-                    self.item = ipaddress.ip_address(self.item)
+                self.item = ipaddress.ip_address(str(self.item, "utf-8"))
                 if isinstance(self.item, ipaddress.IPv4Address):
                     self.isipv4 = True
                 elif isinstance(self.item, ipaddress.IPv6Address):
                     self.isipv6 = True
             except ValueError as e:
                 try:
-                    try:
-                        self.item = ipaddress.ip_network(str
-                                                         (self.item, "utf-8"),
-                                                         strict=False)
-                    except TypeError:
-                        self.item = ipaddress.ip_network(self.item,
-                                                         strict=False)
+                    self.item = ipaddress.ip_network(str
+                                                     (self.item, "utf-8"),
+                                                     strict=False)
                     if isinstance(self.item, ipaddress.IPv4Network):
                         self.isipv4cidr = True
                     elif isinstance(self.item, ipaddress.IPv6Network):
@@ -121,10 +114,7 @@ class HostsDataset(object):
 
     def match(self, connectip):
         '''Check if the connect IP is part of the dataset'''
-        try:
-            source = ipaddress.ip_address(str(connectip, "utf-8"))
-        except TypeError:
-            source = ipaddress.ip_address(connectip)
+        source = ipaddress.ip_address(str(connectip, "utf-8"))
         for item in self.dataset:
             if item.isdomain or item.ishostname:
                 result = self.matchname(source)   # Match host/domains first
@@ -174,19 +164,13 @@ class HostsDataset(object):
             if isinstance(source, ipaddress.IPv4Address):
                 ips = s.dns(name, 'A')
                 for ip in ips:
-                    try:
-                        ip = ipaddress.IPv4Address(str(ip, 'UTF-8'))
-                    except TypeError:
-                        ip = ipaddress.IPv4Address(ip)
+                    ip = ipaddress.IPv4Address(str(ip, 'UTF-8'))
                     if ip == source:
                         results.append(name)
             if isinstance(source, ipaddress.IPv6Address):
                 ips = s.dns(name, 'AAAA')
                 for ip in ips:
-                    try:
-                        ip = ipaddress.IPv6Address(str(ip, 'UTF-8'))
-                    except TypeError:
-                        ip = ipaddress.IPv6Address(ip)
+                    ip = ipaddress.IPv6Address(str(ip, 'UTF-8'))
                 if ip == source:
                     results.append(name)
         return results
@@ -322,15 +306,9 @@ def _dataset_to_list(dataset):
                 return dsd
         # If it's a str and csl, it has one value and we return a list
         if dataset[:4] == 'csl:':
-            datalist = dataset[4:].split(',')
-            for item in datalist:
-                datalist[datalist.index(item)] = item.strip().strip(',')
-            return datalist
+            return [dataset[4:].strip().strip(',')]
         else:
-            datalist = dataset.split(',')
-            for item in datalist:
-                datalist[datalist.index(item)] = item.strip().strip(',')
-            return datalist
+            return [dataset.strip().strip(',')]
         if dataset[-3:] == '.db' or dataset[:3] == 'db:':
             #  This is a Sleepycat (Oracle) DB  dataset, which we dont support
             raise dkim.ParameterError('Unsupported dataset db datase: {0}'
@@ -461,14 +439,8 @@ def _readConfigFile(path, configData=None, configGlobal={}):
     fp.close()
     try:
         configData['AuthservID'] = _make_authserv_id(configData.get('AuthservID', 'HOSTNAME'))
-    except Exception as e:
-        syslog.syslog("Could not make AuthservID: {}".format(e))
-        pass
-
-    try:
         configData['IntHosts'] = HostsDataset(configData['InternalHosts'])
-    except Exception as e:
-        syslog.syslog("Could not make HostDataset from InternalHosts: {}".format(e))
+    except:
         pass
 
     return(configData)

man/dkimpy-milter.conf.5

@@ -244,10 +244,6 @@ Naturally, providing a value here overrides the default, so if mail from
 127.0.0.1 should be signed, the list provided here should include that
 address explicitly. [PeerList NOT IMPLEMENTED]
 
-Mail sent via connections from InternalHosts will not have any existing DKIM
-signatures verified.  This is not overridden by MacroList or Mode.  If the
-Mode is 'v', then no actions will be performed.
-
 .TP
 .I KeyFile (string)
 Gives the location of a PEM-formatted private key to be used for RSA signing
@@ -302,10 +298,6 @@ at the time the filter receives a connection from the MTA and its availability
 depends upon the version of milter used to compile the filter and the version
 of the MTA making the connection.
 
-Mail sent via connections where macros that are in MacroList are provided
-will not have any existing DKIM signatures verified.  If the Mode is 'v', then
-no actions will be performed.
-
 .TP
 .I MacroListVerify (dataset)
 Defines a set of MTA-provided
@@ -316,10 +308,6 @@ Entries in this data set follow the same form as those of the
 .I MacroList
 option above.  [this option is not inhereted from OpenDKIM]  
 
-Mail sent via connections where macros that are in MacroListVerify are
-provided will be not DKIM signed.  If the Mode is 's', then no actions will
-be performed.
-
 .TP
 .I Mode (string)
 Selects operating modes.  The string is a concatenation of characters that
@@ -339,12 +327,7 @@ be set:
 (a) Domain, KeyFile, Selector, no KeyTable, no SigningTable;
 (b) KeyTable, SigningTable, no Domain, no KeyFile, no Selector;
 
-The action to sign or verify is also affected by the InternalHosts, MacroList,
-and MacroListVerify options.  Those options may preclude signing or
-verification in some cases, but will not enable signing or verifying if not
-allowed by Mode.
-
-.TP
+TP
 .I MinimumKeyBits (integer)
 Establishes a minimum key size for acceptable RSA signatures.  Signatures with
 smaller key sizes, even if they otherwise pass DKIM validation, will me marked

man/dkimpy-milter.conf.5.in

@@ -244,10 +244,6 @@ Naturally, providing a value here overrides the default, so if mail from
 127.0.0.1 should be signed, the list provided here should include that
 address explicitly. [PeerList NOT IMPLEMENTED]
 
-Mail sent via connections from InternalHosts will not have any existing DKIM
-signatures verified.  This is not overridden by MacroList or Mode.  If the
-Mode is 'v', then no actions will be performed.
-
 .TP
 .I KeyFile (string)
 Gives the location of a PEM-formatted private key to be used for RSA signing
@@ -302,10 +298,6 @@ at the time the filter receives a connection from the MTA and its availability
 depends upon the version of milter used to compile the filter and the version
 of the MTA making the connection.
 
-Mail sent via connections where macros that are in MacroList are provided
-will not have any existing DKIM signatures verified.  If the Mode is 'v', then
-no actions will be performed.
-
 .TP
 .I MacroListVerify (dataset)
 Defines a set of MTA-provided
@@ -316,10 +308,6 @@ Entries in this data set follow the same form as those of the
 .I MacroList
 option above.  [this option is not inhereted from OpenDKIM]  
 
-Mail sent via connections where macros that are in MacroListVerify are
-provided will be not DKIM signed.  If the Mode is 's', then no actions will
-be performed.
-
 .TP
 .I Mode (string)
 Selects operating modes.  The string is a concatenation of characters that
@@ -339,12 +327,7 @@ be set:
 (a) Domain, KeyFile, Selector, no KeyTable, no SigningTable;
 (b) KeyTable, SigningTable, no Domain, no KeyFile, no Selector;
 
-The action to sign or verify is also affected by the InternalHosts, MacroList,
-and MacroListVerify options.  Those options may preclude signing or
-verification in some cases, but will not enable signing or verifying if not
-allowed by Mode.
-
-.TP
+TP
 .I MinimumKeyBits (integer)
 Establishes a minimum key size for acceptable RSA signatures.  Signatures with
 smaller key sizes, even if they otherwise pass DKIM validation, will me marked

setup.py

@@ -83,13 +83,13 @@ class FileMacroExpand(distutils.cmd.Command):
 kw = {}  # Work-around for lack of 'or' requires in setuptools.
 try:
     import dns
-    kw['install_requires'] = ['dkimpy>=1.1.0', 'pymilter>=1.0.5', 'authres>=1.1.0', 'PyNaCl', 'dnspython>=1.16.0']
+    kw['install_requires'] = ['dkimpy>=1.0', 'pymilter', 'authres>=1.1.0', 'PyNaCl', 'dnspython>=1.16.0']
 except ImportError:  # If PyDNS is not installed, prefer dnspython
-    kw['install_requires'] = ['dkimpy>=1.1.0', 'pymilter>=1.0.5', 'authres>=1.1.0', 'PyNaCl', 'Py3DNS']
+    kw['install_requires'] = ['dkimpy>=1.0', 'pymilter', 'authres>=1.1.0', 'PyNaCl', 'Py3DNS']
 
 setup(
     name='dkimpy-milter',
-    version='1.2.3',
+    version='1.2.1',
     author='Scott Kitterman',
     author_email='scott@kitterman.com',
     url='https://launchpad.net/dkimpy-milter',

tests/02_sign_message.miltertest

@@ -26,7 +26,7 @@ function connect_and_send (sockname, headers, body)
    end
 
    -- mt.macro(conn, SMFIC_MAIL, "i", "simple-message")
-   if mt.mailfrom(conn, "<alicüe@example.net> (Alicþþÿÿe)") ~= nil then
+   if mt.mailfrom(conn, "<alice@example.net>") ~= nil then
       error "mt.mailfrom() failed"
    end
    if mt.getreply(conn) ~= SMFIR_CONTINUE then