Skip to content

Commit 8b0024f

Browse files
jared mauchjared mauch
authored andcommitted
Upgrade password hashing from SHA1 to PBKDF2-SHA256
- Add new password hashing functions using PBKDF2-SHA256 with format prefix - Maintain backward compatibility with old SHA1 passwords - Auto-upgrade passwords to PBKDF2 format on successful authentication - Update all password setting locations to use new hashing - Use only Python 3 standard library (hashlib.pbkdf2_hmac, secrets) This addresses GitHub security warnings about SHA1 usage while maintaining full backward compatibility for existing installations. Passwords are automatically upgraded as users authenticate, allowing incremental migration. Format: New passwords use $pbkdf2$<iterations>$<salt>$<hash> prefix Old format: 40 hex character SHA1 hashes (no prefix) still supported
1 parent c82bcbf commit 8b0024f

5 files changed

Lines changed: 220 additions & 37 deletions

File tree

Mailman/Cgi/admin.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ def cmp(a, b):
4141
from Mailman.htmlformat import *
4242
from Mailman.Cgi import Auth
4343
from Mailman.Logging.Syslog import syslog
44-
from Mailman.Utils import sha_new
44+
from Mailman.Utils import sha_new, hash_password
4545
from Mailman.CSRFcheck import csrf_check
4646

4747
# Set up i18n
@@ -1424,7 +1424,8 @@ def safeint(formvar, defaultval=None):
14241424
confirm = cgidata.getfirst('confirmmodpw', '').strip()
14251425
if new or confirm:
14261426
if new == confirm:
1427-
mlist.mod_password = sha_new(new.encode()).hexdigest()
1427+
# Use new PBKDF2 hashing for all new passwords
1428+
mlist.mod_password = hash_password(new)
14281429
# No re-authentication necessary because the moderator's
14291430
# password doesn't get you into these pages.
14301431
else:
@@ -1435,7 +1436,8 @@ def safeint(formvar, defaultval=None):
14351436
confirm = cgidata.getfirst('confirmpostpw', '').strip()
14361437
if new or confirm:
14371438
if new == confirm:
1438-
mlist.post_password = sha_new(new.encode()).hexdigest()
1439+
# Use new PBKDF2 hashing for all new passwords
1440+
mlist.post_password = hash_password(new)
14391441
# No re-authentication necessary because the poster's
14401442
# password doesn't get you into these pages.
14411443
else:
@@ -1445,7 +1447,8 @@ def safeint(formvar, defaultval=None):
14451447
confirm = cgidata.getfirst('confirmpw', '').strip()
14461448
if new or confirm:
14471449
if new == confirm:
1448-
mlist.password = sha_new(new.encode()).hexdigest()
1450+
# Use new PBKDF2 hashing for all new passwords
1451+
mlist.password = hash_password(new)
14491452
# Set new cookie
14501453
print(mlist.MakeCookie(mm_cfg.AuthListAdmin))
14511454
else:

Mailman/SecurityManager.py

Lines changed: 73 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@
6767
from Mailman import Utils
6868
from Mailman import Errors
6969
from Mailman.Logging.Syslog import syslog
70-
from Mailman.Utils import md5_new, sha_new
70+
from Mailman.Utils import md5_new, sha_new, hash_password, verify_password
7171

7272

7373
class SecurityManager(object):
@@ -163,34 +163,47 @@ def cryptmatchp(response, secret):
163163
# know how that can happen (perhaps if a MM2.0 list
164164
# with USE_CRYPT = 0 has been updated? Doubtful.
165165
return False
166-
# The password for the list admin and list moderator are not
167-
# kept as plain text, but instead as an sha hexdigest. The
168-
# response being passed in is plain text, so we need to
169-
# digestify it first. Note however, that for backwards
170-
# compatibility reasons, we'll also check the admin response
171-
# against the crypted and md5'd passwords, and if they match,
172-
# we'll auto-migrate the passwords to sha.
166+
# The password for the list admin is stored as a hash.
167+
# We support multiple formats for backwards compatibility:
168+
# - New format: PBKDF2-SHA256 with $pbkdf2$ prefix
169+
# - Old format: SHA1 hexdigest (40 hex chars)
170+
# - Legacy: MD5 or crypt() (auto-upgrade to PBKDF2)
173171
key, secret = self.AuthContextInfo(ac)
174172
if secret is None:
175173
continue
176174
if isinstance(response, str):
177175
response = response.encode('utf-8')
178176

179-
sharesponse = sha_new(response).hexdigest()
180-
upgrade = ok = False
181-
if sharesponse == secret:
182-
ok = True
183-
elif md5_new(response).digest() == secret:
184-
ok = upgrade = True
185-
elif cryptmatchp(response, secret):
186-
ok = upgrade = True
187-
if upgrade:
177+
# Try new PBKDF2 or old SHA1 format first
178+
ok, needs_upgrade = verify_password(response, secret)
179+
upgrade = needs_upgrade
180+
181+
# If that didn't work, try legacy MD5 and crypt() formats
182+
if not ok:
183+
sharesponse = sha_new(response).hexdigest()
184+
if sharesponse == secret:
185+
ok = True
186+
upgrade = True
187+
elif md5_new(response).digest() == secret:
188+
ok = True
189+
upgrade = True
190+
elif cryptmatchp(response, secret):
191+
ok = True
192+
upgrade = True
193+
194+
# Upgrade to new PBKDF2 format if needed
195+
if upgrade and ok:
188196
save_and_unlock = False
189197
if not self.Locked():
190198
self.Lock()
191199
save_and_unlock = True
192200
try:
193-
self.password = sharesponse
201+
# Convert response back to string for hash_password
202+
if isinstance(response, bytes):
203+
response_str = response.decode('utf-8')
204+
else:
205+
response_str = response
206+
self.password = hash_password(response_str)
194207
if save_and_unlock:
195208
self.Save()
196209
finally:
@@ -199,24 +212,62 @@ def cryptmatchp(response, secret):
199212
if ok:
200213
return ac
201214
elif ac == mm_cfg.AuthListModerator:
202-
# The list moderator password must be sha'd
215+
# The list moderator password is stored as a hash.
216+
# Supports both new PBKDF2 and old SHA1 formats with auto-upgrade.
203217
key, secret = self.AuthContextInfo(ac)
204218
if secret:
205219
if isinstance(response, str):
206220
response_bytes = response.encode('utf-8')
207221
else:
208222
response_bytes = response
209-
if sha_new(response_bytes).hexdigest() == secret:
223+
ok, needs_upgrade = verify_password(response_bytes, secret)
224+
if ok:
225+
# Upgrade to new format if needed
226+
if needs_upgrade:
227+
save_and_unlock = False
228+
if not self.Locked():
229+
self.Lock()
230+
save_and_unlock = True
231+
try:
232+
if isinstance(response, str):
233+
response_str = response
234+
else:
235+
response_str = response_bytes.decode('utf-8')
236+
self.mod_password = hash_password(response_str)
237+
if save_and_unlock:
238+
self.Save()
239+
finally:
240+
if save_and_unlock:
241+
self.Unlock()
210242
return ac
211243
elif ac == mm_cfg.AuthListPoster:
212-
# The list poster password must be sha'd
244+
# The list poster password is stored as a hash.
245+
# Supports both new PBKDF2 and old SHA1 formats with auto-upgrade.
213246
key, secret = self.AuthContextInfo(ac)
214247
if secret:
215248
if isinstance(response, str):
216249
response_bytes = response.encode('utf-8')
217250
else:
218251
response_bytes = response
219-
if sha_new(response_bytes).hexdigest() == secret:
252+
ok, needs_upgrade = verify_password(response_bytes, secret)
253+
if ok:
254+
# Upgrade to new format if needed
255+
if needs_upgrade:
256+
save_and_unlock = False
257+
if not self.Locked():
258+
self.Lock()
259+
save_and_unlock = True
260+
try:
261+
if isinstance(response, str):
262+
response_str = response
263+
else:
264+
response_str = response_bytes.decode('utf-8')
265+
self.post_password = hash_password(response_str)
266+
if save_and_unlock:
267+
self.Save()
268+
finally:
269+
if save_and_unlock:
270+
self.Unlock()
220271
return ac
221272
elif ac == mm_cfg.AuthUser:
222273
if user is not None:

Mailman/Utils.py

Lines changed: 136 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import errno
3232
import base64
3333
import random
34+
import secrets
3435
import urllib
3536
import urllib.request, urllib.error
3637
import html.entities
@@ -663,6 +664,134 @@ def mkletter(c):
663664
return "%c%c" % tuple(map(mkletter, (chr1, chr2)))
664665

665666

667+
668+
# Password hashing functions for secure password storage
669+
# Format: $pbkdf2$<iterations>$<salt>$<hash>
670+
# Old format (SHA1): 40 hex characters, no prefix
671+
672+
# PBKDF2 iterations - should be high enough to be secure but not too slow
673+
PBKDF2_ITERATIONS = 100000
674+
PBKDF2_SALT_LENGTH = 16 # 128 bits
675+
676+
def hash_password(password):
677+
"""Hash a password using PBKDF2-SHA256.
678+
679+
Returns a string in the format: $pbkdf2$<iterations>$<salt>$<hash>
680+
where salt and hash are base64-encoded.
681+
682+
Args:
683+
password: The password to hash (str or bytes)
684+
685+
Returns:
686+
str: The hashed password with format prefix
687+
"""
688+
if isinstance(password, str):
689+
password = password.encode('utf-8')
690+
691+
# Generate a random salt
692+
salt = secrets.token_bytes(PBKDF2_SALT_LENGTH)
693+
694+
# Hash using PBKDF2-SHA256
695+
# hashlib.pbkdf2_hmac is available in Python 3.4+
696+
dk = hashlib.pbkdf2_hmac('sha256', password, salt, PBKDF2_ITERATIONS)
697+
698+
# Encode salt and hash as base64
699+
salt_b64 = base64.b64encode(salt).decode('ascii')
700+
hash_b64 = base64.b64encode(dk).decode('ascii')
701+
702+
return f'$pbkdf2${PBKDF2_ITERATIONS}${salt_b64}${hash_b64}'
703+
704+
705+
def verify_password(password, stored_hash):
706+
"""Verify a password against a stored hash.
707+
708+
Supports both old SHA1 format (40 hex chars) and new PBKDF2 format.
709+
710+
Args:
711+
password: The password to verify (str or bytes)
712+
stored_hash: The stored hash to verify against (str)
713+
714+
Returns:
715+
tuple: (bool, bool) - (is_valid, needs_upgrade)
716+
is_valid: True if password matches
717+
needs_upgrade: True if password is valid but in old format
718+
"""
719+
if isinstance(password, str):
720+
password = password.encode('utf-8')
721+
722+
# Check if it's the new format (starts with $pbkdf2$)
723+
if stored_hash.startswith('$pbkdf2$'):
724+
return _verify_pbkdf2(password, stored_hash), False
725+
726+
# Old format: SHA1 hexdigest (40 hex characters)
727+
# Check if it looks like a SHA1 hash (40 hex chars)
728+
if len(stored_hash) == 40 and all(c in '0123456789abcdef' for c in stored_hash.lower()):
729+
sha1_hash = sha_new(password).hexdigest()
730+
if sha1_hash == stored_hash:
731+
return True, True # Valid but needs upgrade
732+
return False, False
733+
734+
# Fallback: try SHA1 comparison for backwards compatibility
735+
sha1_hash = sha_new(password).hexdigest()
736+
if sha1_hash == stored_hash:
737+
return True, True # Valid but needs upgrade
738+
return False, False
739+
740+
741+
def _verify_pbkdf2(password, stored_hash):
742+
"""Verify a password against a PBKDF2 hash.
743+
744+
Args:
745+
password: The password to verify (bytes)
746+
stored_hash: The stored hash in format $pbkdf2$<iterations>$<salt>$<hash>
747+
748+
Returns:
749+
bool: True if password matches
750+
"""
751+
try:
752+
# Parse the hash format: $pbkdf2$<iterations>$<salt>$<hash>
753+
parts = stored_hash.split('$')
754+
if len(parts) != 5 or parts[1] != 'pbkdf2':
755+
return False
756+
757+
iterations = int(parts[2])
758+
salt_b64 = parts[3]
759+
hash_b64 = parts[4]
760+
761+
# Decode salt and hash
762+
salt = base64.b64decode(salt_b64)
763+
stored_dk = base64.b64decode(hash_b64)
764+
765+
# Compute hash with same parameters
766+
dk = hashlib.pbkdf2_hmac('sha256', password, salt, iterations)
767+
768+
# Constant-time comparison to prevent timing attacks
769+
return secrets.compare_digest(dk, stored_dk)
770+
except (ValueError, TypeError, IndexError):
771+
return False
772+
773+
774+
def is_old_password_format(stored_hash):
775+
"""Check if a stored hash is in the old SHA1 format.
776+
777+
Args:
778+
stored_hash: The stored hash to check (str)
779+
780+
Returns:
781+
bool: True if the hash is in old format (needs upgrade)
782+
"""
783+
# New format starts with $pbkdf2$
784+
if stored_hash.startswith('$pbkdf2$'):
785+
return False
786+
787+
# Old format: 40 hex characters
788+
if len(stored_hash) == 40 and all(c in '0123456789abcdef' for c in stored_hash.lower()):
789+
return True
790+
791+
# If it doesn't match either format, assume old for backwards compatibility
792+
return True
793+
794+
666795

667796
def set_global_password(pw, siteadmin=True):
668797
if siteadmin:
@@ -673,10 +802,8 @@ def set_global_password(pw, siteadmin=True):
673802
omask = os.umask(0o026)
674803
try:
675804
fp = open(filename, 'w')
676-
if isinstance(pw, bytes):
677-
fp.write(sha_new(pw).hexdigest() + '\n')
678-
else:
679-
fp.write(sha_new(pw.encode()).hexdigest() + '\n')
805+
# Use new PBKDF2 hashing for all new passwords
806+
fp.write(hash_password(pw) + '\n')
680807
fp.close()
681808
finally:
682809
os.umask(omask)
@@ -702,10 +829,11 @@ def check_global_password(response, siteadmin=True):
702829
challenge = get_global_password(siteadmin)
703830
if challenge is None:
704831
return None
705-
if isinstance(response, bytes):
706-
return challenge == sha_new(response).hexdigest()
707-
else:
708-
return challenge == sha_new(response.encode()).hexdigest()
832+
# Use verify_password which handles both old SHA1 and new PBKDF2 formats
833+
is_valid, needs_upgrade = verify_password(response, challenge)
834+
# Note: We don't auto-upgrade global passwords here since they're in files
835+
# and we'd need to write back to the file. This could be added if needed.
836+
return is_valid
709837

710838

711839

bin/change_pw

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ def main():
147147
if password is not None:
148148
if not password:
149149
usage(1, C_('Empty list passwords are not allowed'))
150-
shapassword = Utils.sha_new(password.encode()).hexdigest()
150+
shapassword = Utils.hash_password(password)
151151

152152
if domains:
153153
for name in Utils.list_names():
@@ -167,7 +167,7 @@ def main():
167167
if password is None:
168168
randompw = Utils.MakeRandomPassword(
169169
mm_cfg.ADMIN_PASSWORD_LENGTH)
170-
shapassword = Utils.sha_new(randompw.encode('utf-8')).hexdigest()
170+
shapassword = Utils.hash_password(randompw)
171171
notifypassword = randompw
172172
else:
173173
notifypassword = password

bin/newlist

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,8 @@ def main():
198198

199199
mlist = MailList.MailList()
200200
try:
201-
pw = Utils.sha_new(listpasswd.encode()).hexdigest()
201+
# Use new PBKDF2 hashing for all new passwords
202+
pw = Utils.hash_password(listpasswd)
202203
# Guarantee that all newly created files have the proper permission.
203204
# proper group ownership should be assured by the autoconf script
204205
# enforcing that all directories have the group sticky bit set

0 commit comments

Comments
 (0)