Skip to content

Commit e777629

Browse files
jaredmauchjaredmauch
authored
Merge pull request #20 from jaredmauch/hashdigest_fixes
Upgrade password hashing from SHA1 to PBKDF2-SHA256
2 parents c82bcbf + 9b9699b commit e777629

12 files changed

Lines changed: 591 additions & 77 deletions

File tree

‎.gitignore‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ src/roster
7979
src/subscribe
8080
src/vsnprintf.o
8181
templates/Makefile
82+
templates/.converted.stamp
8283
tests/Makefile
8384
tests/bounces/Makefile
8485
tests/msgs/Makefile
86+
messages/.converted.stamp

‎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: 102 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -58,16 +58,12 @@
5858
import urllib.request, urllib.parse, urllib.error
5959
from urllib.parse import urlparse
6060

61-
try:
62-
import crypt
63-
except ImportError:
64-
crypt = None
6561

6662
from Mailman import mm_cfg
6763
from Mailman import Utils
6864
from Mailman import Errors
6965
from Mailman.Logging.Syslog import syslog
70-
from Mailman.Utils import md5_new, sha_new
66+
from Mailman.Utils import sha_new, hash_password, verify_password
7167

7268

7369
class SecurityManager(object):
@@ -142,81 +138,146 @@ def Authenticate(self, authcontexts, response, user=None):
142138

143139
for ac in authcontexts:
144140
if ac == mm_cfg.AuthCreator:
145-
ok = Utils.check_global_password(response, siteadmin=0)
141+
# Auto-upgrade global passwords when used for authentication
142+
ok = Utils.check_global_password(response, siteadmin=0, auto_upgrade=True)
146143
if ok:
147144
return mm_cfg.AuthCreator
148145
elif ac == mm_cfg.AuthSiteAdmin:
149-
ok = Utils.check_global_password(response)
146+
# Auto-upgrade global passwords when used for authentication
147+
ok = Utils.check_global_password(response, auto_upgrade=True)
150148
if ok:
151149
return mm_cfg.AuthSiteAdmin
152150
elif ac == mm_cfg.AuthListAdmin:
153-
def cryptmatchp(response, secret):
154-
try:
155-
salt = secret[:2]
156-
if crypt and crypt.crypt(response, salt) == secret:
157-
return True
158-
return False
159-
except TypeError:
160-
# BAW: Hard to say why we can get a TypeError here.
161-
# SF bug report #585776 says crypt.crypt() can raise
162-
# this if salt contains null bytes, although I don't
163-
# know how that can happen (perhaps if a MM2.0 list
164-
# with USE_CRYPT = 0 has been updated? Doubtful.
165-
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.
151+
# The password for the list admin is stored as a hash.
152+
# We support multiple formats for backwards compatibility:
153+
# - New format: PBKDF2-SHA256 with $pbkdf2$ prefix
154+
# - Old format: SHA1 hexdigest (40 hex chars, auto-upgrade to PBKDF2)
173155
key, secret = self.AuthContextInfo(ac)
174156
if secret is None:
175157
continue
176158
if isinstance(response, str):
177159
response = response.encode('utf-8')
178160

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:
161+
# Try new PBKDF2 or old SHA1 format (verify_password handles both)
162+
ok, needs_upgrade = verify_password(response, secret)
163+
upgrade = needs_upgrade
164+
165+
# Upgrade to new PBKDF2 format if needed
166+
if upgrade and ok:
188167
save_and_unlock = False
189168
if not self.Locked():
190169
self.Lock()
191170
save_and_unlock = True
192171
try:
193-
self.password = sharesponse
172+
# Convert response back to string for hash_password
173+
if isinstance(response, bytes):
174+
response_str = response.decode('utf-8')
175+
else:
176+
response_str = response
177+
self.password = hash_password(response_str)
194178
if save_and_unlock:
195179
self.Save()
180+
except (PermissionError, IOError) as e:
181+
# Log permission error but don't fail authentication
182+
# Get uid/euid/gid/egid for debugging
183+
try:
184+
uid = os.getuid()
185+
euid = os.geteuid()
186+
gid = os.getgid()
187+
egid = os.getegid()
188+
except (AttributeError, OSError):
189+
# Fallback if getuid/geteuid not available
190+
uid = euid = gid = egid = 'unknown'
191+
syslog('error',
192+
'Could not auto-upgrade list admin password for %s: %s (uid=%s euid=%s gid=%s egid=%s)',
193+
self.internal_name(), e, uid, euid, gid, egid)
194+
# Continue - authentication still succeeds even if upgrade fails
196195
finally:
197196
if save_and_unlock:
198197
self.Unlock()
199198
if ok:
200199
return ac
201200
elif ac == mm_cfg.AuthListModerator:
202-
# The list moderator password must be sha'd
201+
# The list moderator password is stored as a hash.
202+
# Supports both new PBKDF2 and old SHA1 formats with auto-upgrade.
203203
key, secret = self.AuthContextInfo(ac)
204204
if secret:
205205
if isinstance(response, str):
206206
response_bytes = response.encode('utf-8')
207207
else:
208208
response_bytes = response
209-
if sha_new(response_bytes).hexdigest() == secret:
209+
ok, needs_upgrade = verify_password(response_bytes, secret)
210+
if ok:
211+
# Upgrade to new format if needed
212+
if needs_upgrade:
213+
save_and_unlock = False
214+
if not self.Locked():
215+
self.Lock()
216+
save_and_unlock = True
217+
try:
218+
if isinstance(response, str):
219+
response_str = response
220+
else:
221+
response_str = response_bytes.decode('utf-8')
222+
self.mod_password = hash_password(response_str)
223+
if save_and_unlock:
224+
self.Save()
225+
except (PermissionError, IOError) as e:
226+
# Log permission error but don't fail authentication
227+
try:
228+
uid = os.getuid()
229+
euid = os.geteuid()
230+
gid = os.getgid()
231+
egid = os.getegid()
232+
except (AttributeError, OSError):
233+
uid = euid = gid = egid = 'unknown'
234+
syslog('error',
235+
'Could not auto-upgrade moderator password for %s: %s (uid=%s euid=%s gid=%s egid=%s)',
236+
self.internal_name(), e, uid, euid, gid, egid)
237+
finally:
238+
if save_and_unlock:
239+
self.Unlock()
210240
return ac
211241
elif ac == mm_cfg.AuthListPoster:
212-
# The list poster password must be sha'd
242+
# The list poster password is stored as a hash.
243+
# Supports both new PBKDF2 and old SHA1 formats with auto-upgrade.
213244
key, secret = self.AuthContextInfo(ac)
214245
if secret:
215246
if isinstance(response, str):
216247
response_bytes = response.encode('utf-8')
217248
else:
218249
response_bytes = response
219-
if sha_new(response_bytes).hexdigest() == secret:
250+
ok, needs_upgrade = verify_password(response_bytes, secret)
251+
if ok:
252+
# Upgrade to new format if needed
253+
if needs_upgrade:
254+
save_and_unlock = False
255+
if not self.Locked():
256+
self.Lock()
257+
save_and_unlock = True
258+
try:
259+
if isinstance(response, str):
260+
response_str = response
261+
else:
262+
response_str = response_bytes.decode('utf-8')
263+
self.post_password = hash_password(response_str)
264+
if save_and_unlock:
265+
self.Save()
266+
except (PermissionError, IOError) as e:
267+
# Log permission error but don't fail authentication
268+
try:
269+
uid = os.getuid()
270+
euid = os.geteuid()
271+
gid = os.getgid()
272+
egid = os.getegid()
273+
except (AttributeError, OSError):
274+
uid = euid = gid = egid = 'unknown'
275+
syslog('error',
276+
'Could not auto-upgrade poster password for %s: %s (uid=%s euid=%s gid=%s egid=%s)',
277+
self.internal_name(), e, uid, euid, gid, egid)
278+
finally:
279+
if save_and_unlock:
280+
self.Unlock()
220281
return ac
221282
elif ac == mm_cfg.AuthUser:
222283
if user is not None:

0 commit comments

Comments
 (0)