Skip to content

Commit a16da9c

Browse files
jared mauchjared mauch
authored andcommitted
Handle permission errors gracefully during password auto-upgrade
- Catch PermissionError/IOError when attempting to write password upgrades - Log uid/euid/gid/egid for debugging permission issues - Continue authentication even if upgrade fails (non-fatal) - Apply to global passwords, list admin, moderator, and poster passwords This prevents authentication failures when the process doesn't have write permissions to update password files, while still logging the issue for administrator review.
1 parent 66e84d9 commit a16da9c

2 files changed

Lines changed: 130 additions & 23 deletions

File tree

Mailman/SecurityManager.py

Lines changed: 112 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):
@@ -165,60 +165,150 @@ def cryptmatchp(response, secret):
165165
# know how that can happen (perhaps if a MM2.0 list
166166
# with USE_CRYPT = 0 has been updated? Doubtful.
167167
return False
168-
# The password for the list admin and list moderator are not
169-
# kept as plain text, but instead as an sha hexdigest. The
170-
# response being passed in is plain text, so we need to
171-
# digestify it first. Note however, that for backwards
172-
# compatibility reasons, we'll also check the admin response
173-
# against the crypted and md5'd passwords, and if they match,
174-
# we'll auto-migrate the passwords to sha.
168+
# The password for the list admin is stored as a hash.
169+
# We support multiple formats for backwards compatibility:
170+
# - New format: PBKDF2-SHA256 with $pbkdf2$ prefix
171+
# - Old format: SHA1 hexdigest (40 hex chars)
172+
# - Legacy: MD5 or crypt() (auto-upgrade to PBKDF2)
175173
key, secret = self.AuthContextInfo(ac)
176174
if secret is None:
177175
continue
178176
if isinstance(response, str):
179177
response = response.encode('utf-8')
180178

181-
sharesponse = sha_new(response).hexdigest()
182-
upgrade = ok = False
183-
if sharesponse == secret:
184-
ok = True
185-
elif md5_new(response).digest() == secret:
186-
ok = upgrade = True
187-
elif cryptmatchp(response, secret):
188-
ok = upgrade = True
189-
if upgrade:
179+
# Try new PBKDF2 or old SHA1 format first
180+
ok, needs_upgrade = verify_password(response, secret)
181+
upgrade = needs_upgrade
182+
183+
# If that didn't work, try legacy MD5 and crypt() formats
184+
if not ok:
185+
sharesponse = sha_new(response).hexdigest()
186+
if sharesponse == secret:
187+
ok = True
188+
upgrade = True
189+
elif md5_new(response).digest() == secret:
190+
ok = True
191+
upgrade = True
192+
elif cryptmatchp(response, secret):
193+
ok = True
194+
upgrade = True
195+
196+
# Upgrade to new PBKDF2 format if needed
197+
if upgrade and ok:
190198
save_and_unlock = False
191199
if not self.Locked():
192200
self.Lock()
193201
save_and_unlock = True
194202
try:
195-
self.password = sharesponse
203+
# Convert response back to string for hash_password
204+
if isinstance(response, bytes):
205+
response_str = response.decode('utf-8')
206+
else:
207+
response_str = response
208+
self.password = hash_password(response_str)
196209
if save_and_unlock:
197210
self.Save()
211+
except (PermissionError, IOError) as e:
212+
# Log permission error but don't fail authentication
213+
# Get uid/euid/gid/egid for debugging
214+
try:
215+
uid = os.getuid()
216+
euid = os.geteuid()
217+
gid = os.getgid()
218+
egid = os.getegid()
219+
except (AttributeError, OSError):
220+
# Fallback if getuid/geteuid not available
221+
uid = euid = gid = egid = 'unknown'
222+
syslog('error',
223+
'Could not auto-upgrade list admin password for %s: %s (uid=%s euid=%s gid=%s egid=%s)',
224+
self.internal_name(), e, uid, euid, gid, egid)
225+
# Continue - authentication still succeeds even if upgrade fails
198226
finally:
199227
if save_and_unlock:
200228
self.Unlock()
201229
if ok:
202230
return ac
203231
elif ac == mm_cfg.AuthListModerator:
204-
# The list moderator password must be sha'd
232+
# The list moderator password is stored as a hash.
233+
# Supports both new PBKDF2 and old SHA1 formats with auto-upgrade.
205234
key, secret = self.AuthContextInfo(ac)
206235
if secret:
207236
if isinstance(response, str):
208237
response_bytes = response.encode('utf-8')
209238
else:
210239
response_bytes = response
211-
if sha_new(response_bytes).hexdigest() == secret:
240+
ok, needs_upgrade = verify_password(response_bytes, secret)
241+
if ok:
242+
# Upgrade to new format if needed
243+
if needs_upgrade:
244+
save_and_unlock = False
245+
if not self.Locked():
246+
self.Lock()
247+
save_and_unlock = True
248+
try:
249+
if isinstance(response, str):
250+
response_str = response
251+
else:
252+
response_str = response_bytes.decode('utf-8')
253+
self.mod_password = hash_password(response_str)
254+
if save_and_unlock:
255+
self.Save()
256+
except (PermissionError, IOError) as e:
257+
# Log permission error but don't fail authentication
258+
try:
259+
uid = os.getuid()
260+
euid = os.geteuid()
261+
gid = os.getgid()
262+
egid = os.getegid()
263+
except (AttributeError, OSError):
264+
uid = euid = gid = egid = 'unknown'
265+
syslog('error',
266+
'Could not auto-upgrade moderator password for %s: %s (uid=%s euid=%s gid=%s egid=%s)',
267+
self.internal_name(), e, uid, euid, gid, egid)
268+
finally:
269+
if save_and_unlock:
270+
self.Unlock()
212271
return ac
213272
elif ac == mm_cfg.AuthListPoster:
214-
# The list poster password must be sha'd
273+
# The list poster password is stored as a hash.
274+
# Supports both new PBKDF2 and old SHA1 formats with auto-upgrade.
215275
key, secret = self.AuthContextInfo(ac)
216276
if secret:
217277
if isinstance(response, str):
218278
response_bytes = response.encode('utf-8')
219279
else:
220280
response_bytes = response
221-
if sha_new(response_bytes).hexdigest() == secret:
281+
ok, needs_upgrade = verify_password(response_bytes, secret)
282+
if ok:
283+
# Upgrade to new format if needed
284+
if needs_upgrade:
285+
save_and_unlock = False
286+
if not self.Locked():
287+
self.Lock()
288+
save_and_unlock = True
289+
try:
290+
if isinstance(response, str):
291+
response_str = response
292+
else:
293+
response_str = response_bytes.decode('utf-8')
294+
self.post_password = hash_password(response_str)
295+
if save_and_unlock:
296+
self.Save()
297+
except (PermissionError, IOError) as e:
298+
# Log permission error but don't fail authentication
299+
try:
300+
uid = os.getuid()
301+
euid = os.geteuid()
302+
gid = os.getgid()
303+
egid = os.getegid()
304+
except (AttributeError, OSError):
305+
uid = euid = gid = egid = 'unknown'
306+
syslog('error',
307+
'Could not auto-upgrade poster password for %s: %s (uid=%s euid=%s gid=%s egid=%s)',
308+
self.internal_name(), e, uid, euid, gid, egid)
309+
finally:
310+
if save_and_unlock:
311+
self.Unlock()
222312
return ac
223313
elif ac == mm_cfg.AuthUser:
224314
if user is not None:

Mailman/Utils.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -716,7 +716,24 @@ def check_global_password(response, siteadmin=True, auto_upgrade=False):
716716
is_valid, needs_upgrade = verify_password(response, challenge)
717717
# Auto-upgrade if requested and password is valid but in old format
718718
if is_valid and needs_upgrade and auto_upgrade:
719-
set_global_password(response, siteadmin)
719+
try:
720+
set_global_password(response, siteadmin)
721+
except (PermissionError, IOError) as e:
722+
# Log permission error but don't fail authentication
723+
# Get uid/euid/gid/egid for debugging
724+
try:
725+
uid = os.getuid()
726+
euid = os.geteuid()
727+
gid = os.getgid()
728+
egid = os.getegid()
729+
except (AttributeError, OSError):
730+
# Fallback if getuid/geteuid not available
731+
uid = euid = gid = egid = 'unknown'
732+
pw_type = 'site admin' if siteadmin else 'list creator'
733+
syslog('error',
734+
'Could not auto-upgrade %s password: %s (uid=%s euid=%s gid=%s egid=%s)',
735+
pw_type, e, uid, euid, gid, egid)
736+
# Continue - authentication still succeeds even if upgrade fails
720737
return is_valid
721738

722739

0 commit comments

Comments
 (0)