Fix Python 3 behavioral bugs in handlers and SecurityManager

Mailman/Cgi/create.py

@@ -31,7 +31,7 @@
 from Mailman import i18n
 from Mailman.htmlformat import *
 from Mailman.Logging.Syslog import syslog
-from Mailman.Utils import sha_new
+from Mailman.Utils import sha_new, hash_password
 
 # Set up i18n
 _ = i18n._
@@ -199,7 +199,7 @@ def sigterm_handler(signum, frame, mlist=mlist):
         # Install the emergency shutdown signal handler
         signal.signal(signal.SIGTERM, sigterm_handler)
 
-        pw = sha_new(password).hexdigest()
+        pw = hash_password(password)
         # Guarantee that all newly created files have the proper permission.
         # proper group ownership should be assured by the autoconf script
         # enforcing that all directories have the group sticky bit set

Mailman/Handlers/CookHeaders.py

@@ -456,7 +456,7 @@ def prefix_subject(mlist, msg, msgdata):
     if cset == 'us-ascii':
         try:
             if old_style:
-                h = u' '.join([recolon, prefix, subject])
+                h = u' '.join(x for x in [recolon, prefix, subject] if x)
             else:
                 if recolon:
                     h = u' '.join([prefix, recolon, subject])
@@ -465,7 +465,7 @@ def prefix_subject(mlist, msg, msgdata):
             h = h.encode('us-ascii')
             h = uheader(mlist, h, 'Subject', continuation_ws=ws)
             change_header('Subject', h, mlist, msg, msgdata)
-            ss = u' '.join([recolon, subject])
+            ss = u' '.join(x for x in [recolon, subject] if x)
             ss = ss.encode('us-ascii')
             ss = uheader(mlist, ss, 'Subject', continuation_ws=ws)
             msgdata['stripped_subject'] = ss
@@ -479,11 +479,15 @@ def prefix_subject(mlist, msg, msgdata):
     if recolon:
         recolon += ' '
     if old_style:
-        h = uheader(mlist, recolon, 'Subject', continuation_ws=ws)
-        h.append(prefix)
+        if recolon:
+            h = uheader(mlist, recolon, 'Subject', continuation_ws=ws)
+            h.append(prefix)
+        else:
+            h = uheader(mlist, prefix, 'Subject', continuation_ws=ws)
     else:
         h = uheader(mlist, prefix, 'Subject', continuation_ws=ws)
-        h.append(recolon)
+        if recolon:
+            h.append(recolon)
     # TK: Subject is concatenated and unicode string.
     subject = subject.encode(cset, 'replace')
     h.append(subject, cset)

Mailman/Handlers/Decorate.py

@@ -115,15 +115,17 @@ def process(mlist, msg, msgdata):
                 endsep = u'\n'
             payload = uheader + frontsep + oldpayload + endsep + ufooter
             try:
-                # first, try encode with list charset
-                payload = payload.encode(lcset)
-                newcset = lcset
+                # Prefer the message's own charset to preserve encoding and
+                # avoid unnecessary base64 (e.g. utf-8 charset always
+                # base64-encodes in Python 3's email library).
+                payload.encode(mcset)
+                newcset = mcset
             except UnicodeError:
-                if lcset != mcset:
-                    # if fail, encode with message charset (if different)
-                    payload = payload.encode(mcset)
-                    newcset = mcset
-                    # if this fails, fallback to outer try and wrap=true
+                # Fall back to list charset if message charset can't represent
+                # the decorated content (e.g. non-ASCII header/footer).
+                payload.encode(lcset)
+                newcset = lcset
+                # if this fails, fallback to outer try and wrap=true
             format = msg.get_param('format')
             delsp = msg.get_param('delsp')
             del msg['content-transfer-encoding']
@@ -138,16 +140,23 @@ def process(mlist, msg, msgdata):
             pass
     elif msg.get_content_type() == 'multipart/mixed':
         # The next easiest thing to do is just prepend the header and append
-        # the footer as additional subparts
+        # the footer as additional subparts.  Prefer the message's own charset
+        # to avoid base64-encoding ASCII content when the list charset is utf-8.
+        try:
+            (header or '').encode(mcset)
+            (footer or '').encode(mcset)
+            mime_cset = mcset
+        except (UnicodeError, LookupError):
+            mime_cset = lcset
         payload = msg.get_payload()
         if not isinstance(payload, list):
             payload = [payload]
         if footer:
-            mimeftr = MIMEText(footer, 'plain', lcset)
+            mimeftr = MIMEText(footer, 'plain', mime_cset)
             mimeftr['Content-Disposition'] = 'inline'
             payload.append(mimeftr)
         if header:
-            mimehdr = MIMEText(header, 'plain', lcset)
+            mimehdr = MIMEText(header, 'plain', mime_cset)
             mimehdr['Content-Disposition'] = 'inline'
             payload.insert(0, mimehdr)
         msg.set_payload(payload)

Mailman/MTA/Postfix.py

@@ -442,12 +442,12 @@ def checkperms(state):
         if stat and (stat[ST_MODE] & targetmode) != targetmode:
             state.ERRORS += 1
             octmode = oct(stat[ST_MODE])
-            print(C_('%(file)s permissions must be 0664 (got %(octmode)s)'),)
+            print(C_('%(file)s permissions must be 0664 (got %(octmode)s)') % locals())
             if state.FIX:
                 print(C_('(fixing)'))
                 os.chmod(file, stat[ST_MODE] | targetmode)
             else:
-                print
+                print()
         # Make sure the corresponding .db files are owned by the Mailman user.
         # We don't need to check the group ownership of the file, since
         # check_perms checks this itself.
@@ -481,7 +481,7 @@ def checkperms(state):
         if stat and (stat[ST_MODE] & targetmode) != targetmode:
             state.ERRORS += 1
             octmode = oct(stat[ST_MODE])
-            print(C_('%(dbfile)s permissions must be 0664 (got %(octmode)s)'),)
+            print(C_('%(dbfile)s permissions must be 0664 (got %(octmode)s)') % locals())
             if state.FIX:
                 print(C_('(fixing)'))
                 os.chmod(dbfile, stat[ST_MODE] | targetmode)

Mailman/Utils.py

@@ -727,6 +727,11 @@ def hash_password(plaintext):
         binascii.hexlify(dk).decode('ascii'))
 
 
+def is_old_password_format(pw):
+    """Return True if pw is a legacy password that needs upgrading to PBKDF2."""
+    return bool(pw) and not pw.startswith(_PBKDF2_SHA256_PREFIX)
+
+
 def verify_password(response, stored):
     """Verify password against stored hash.
 
@@ -746,6 +751,11 @@ def verify_password(response, stored):
     else:
         response_bytes = bytes(response)
     if not isinstance(stored, str):
+        # Legacy md5 binary digest: 16 raw bytes (pre-SHA1 era).
+        if isinstance(stored, (bytes, bytearray)) and len(stored) == 16:
+            if hmac.compare_digest(md5_new(response_bytes).digest(), bytes(stored)):
+                return True, True
+            return False, False
         try:
             stored = stored.decode('ascii', errors='strict')
         except Exception:
@@ -775,6 +785,14 @@ def verify_password(response, stored):
         if hmac.compare_digest(digest.lower(), stored.lower()):
             return True, True
         return False, False
+    # Legacy crypt(3) format — last resort fallback.
+    try:
+        import crypt as _crypt
+        resp_str = response_bytes.decode('utf-8', errors='replace')
+        if _crypt.crypt(resp_str, stored) == stored:
+            return True, True
+    except (ImportError, AttributeError, OSError):
+        pass
     return False, False
 
 

bin/check_perms

@@ -141,7 +141,7 @@ def checkwalk(arg, dirname, names):
         octperms = oct(targetperms)
         if S_ISDIR(mode) and (mode & targetperms) != targetperms:
             arg.ERRORS += 1
-            print(C_('directory permissions must be %(octperms)s: %(path)s'),)
+            print(C_('directory permissions must be %(octperms)s: %(path)s') % locals())
             if STATE.FIX:
                 print(C_('(fixing)'))
                 os.chmod(path, mode | targetperms)
@@ -150,7 +150,7 @@ def checkwalk(arg, dirname, names):
         elif os.path.splitext(path)[1] in ('.py', '.pyc', '.pyo'):
             octperms = oct(PYFILEPERMS)
             if mode & PYFILEPERMS != PYFILEPERMS:
-                print(C_('source perms must be %(octperms)s: %(path)s'),)
+                print(C_('source perms must be %(octperms)s: %(path)s') % locals())
                 arg.ERRORS += 1
                 if STATE.FIX:
                     print(C_('(fixing)'))
@@ -161,7 +161,7 @@ def checkwalk(arg, dirname, names):
             # Article files must be group writeable
             octperms = oct(ARTICLEFILEPERMS)
             if mode & ARTICLEFILEPERMS != ARTICLEFILEPERMS:
-                print(C_('article db files must be %(octperms)s: %(path)s'),)
+                print(C_('article db files must be %(octperms)s: %(path)s') % locals())
                 arg.ERRORS += 1
                 if STATE.FIX:
                     print(C_('(fixing)'))
@@ -187,7 +187,7 @@ def checkall():
             continue
         if (mode & DIRPERMS) != DIRPERMS:
             STATE.ERRORS += 1
-            print(C_('directory must be at least 02775: %(d)s'),)
+            print(C_('directory must be at least 02775: %(d)s') % locals())
             if STATE.FIX:
                 print(C_('(fixing)'))
                 os.chmod(d, mode | DIRPERMS)
@@ -204,7 +204,7 @@ def checkarchives():
     mode = statmode(private)
     if mode & S_IROTH:
         STATE.ERRORS += 1
-        print(C_('%(private)s must not be other-readable'),)
+        print(C_('%(private)s must not be other-readable') % locals())
         if STATE.FIX:
             print(C_('(fixing)'))
             os.chmod(private, mode & ~S_IROTH)
@@ -253,7 +253,7 @@ def checkarchivedbs():
             continue
         if mode & S_IRWXO:
             STATE.ERRORS += 1
-            print(C_('%(dbdir)s "other" perms must be 000'),)
+            print(C_('%(dbdir)s "other" perms must be 000') % locals())
             if STATE.FIX:
                 print(C_('(fixing)'))
                 os.chmod(dbdir, mode & ~S_IRWXO)
@@ -272,7 +272,7 @@ def checkcgi():
         mode = statmode(path)
         if mode & S_IXGRP and not mode & S_ISGID:
             STATE.ERRORS += 1
-            print(C_('%(path)s must be set-gid'),)
+            print(C_('%(path)s must be set-gid') % locals())
             if STATE.FIX:
                 print(C_('(fixing)'))
                 os.chmod(path, mode | S_ISGID)
@@ -286,7 +286,7 @@ def checkmail():
     mode = statmode(wrapper)
     if not mode & S_ISGID:
         STATE.ERRORS += 1
-        print(C_('%(wrapper)s must be set-gid'),)
+        print(C_('%(wrapper)s must be set-gid') % locals())
         if STATE.FIX:
             print(C_('(fixing)'))
             os.chmod(wrapper, mode | S_ISGID)
@@ -346,7 +346,7 @@ def checkdata():
                 continue
             if (mode & targetmode) != targetmode:
                 STATE.ERRORS += 1
-                print(C_('file permissions must be at least 660: %(path)s'),)
+                print(C_('file permissions must be at least 660: %(path)s') % locals())
                 if STATE.FIX:
                     print(C_('(fixing)'))
                     os.chmod(path, mode | targetmode)

bin/newlist

@@ -241,8 +241,7 @@ def main():
 
     # And send the notice to the list owner
     if not quiet and not automate:
-        print('Hit enter to notify %(listname)s owner...'),
-        sys.stdin.readline()
+        input('Hit enter to notify %s owner...' % listname)
     if not quiet:
         siteowner = Utils.get_site_email(mlist.host_name, 'owner')
         text = Utils.maketext(

bin/update

@@ -814,27 +814,27 @@ def send_password_upgrade_notification(mlist):
         listaddr = mlist.GetListEmail()
         siteowner = Utils.get_site_email(mlist.host_name, 'owner')
 
-        text = _("""\
+        text = C_("""\
 This is an automated message from your Mailman installation.
 
-Your mailing list "%(listname)s" (%(listaddr)s) is using an older password 
-hashing format. For security reasons, we have upgraded Mailman to use a more 
+Your mailing list "%(listname)s" (%(listaddr)s) is using an older password
+hashing format. For security reasons, we have upgraded Mailman to use a more
 secure password hashing method (PBKDF2-SHA256 instead of SHA1).
 
 To complete the upgrade, please log in to your list administration page:
 
     %(admin_url)s
 
-Simply logging in with your current list administrator password will 
-automatically upgrade your password to the new secure format. You don't 
-need to change your password - just log in once and the upgrade will happen 
+Simply logging in with your current list administrator password will
+automatically upgrade your password to the new secure format. You don't
+need to change your password - just log in once and the upgrade will happen
 automatically.
 
-This is a one-time upgrade. After you log in, your password will be 
-automatically converted to the new format and you won't need to do anything 
+This is a one-time upgrade. After you log in, your password will be
+automatically converted to the new format and you won't need to do anything
 else.
 
-If you have any questions or concerns, please contact the site administrator 
+If you have any questions or concerns, please contact the site administrator
 at %(siteowner)s.
 
 Thank you for your attention to this important security upgrade.
@@ -844,8 +844,8 @@ Thank you for your attention to this important security upgrade.
             'admin_url': admin_url,
             'siteowner': siteowner,
         }
-        
-        subject = _('Action Required: Password Upgrade for %(listname)s') % {
+
+        subject = C_('Action Required: Password Upgrade for %(listname)s') % {
             'listname': listname
         }