The Drupal superuser account (uid 1) should be covered by password expiration by default. Having the superuser password expire mitigates password compromise. Exempting the superuser from expiration does not, so is an insecure default.

This change would only affect new installations. Existing installations would retain the existing setting so there is no unexpected change in behavior.

CommentFileSizeAuthor
#11 password_policy-7.x-1.x-change_admin_expiration_default-2476113-11.patch5.17 KBaohrvetpv
#10 password_policy-7.x-1.x-change_admin_expiration_default-2476113-10.patch5.27 KBaohrvetpv
#8 password_policy-7.x-1.x-change_admin_expiration_default-2476113-8.patch4.7 KBaohrvetpv
#6 password_policy-7.x-1.x-change_admin_expiration_default-2476113-6.patch3.13 KBaohrvetpv
#2 password_policy-7.x-1.x-change_admin_expiration_default-2476113-2.patch2.87 KBaohrvetpv

Comments

aohrvetpv’s picture

aohrvetpv commented

This may be the default to prevent administrators from locking themselves out of their sites. However, it is worse for an attacker to compromise the superuser account than administrator lock-out, the latter of which should always be remediable without any harm to website users.

aohrvetpv’s picture

aohrvetpv commented
Status: Active » Needs review
StatusFileSize
new password_policy-7.x-1.x-change_admin_expiration_default-2476113-2.patch2.87 KB

This patch changes the default to expire the superuser (uid 1) password if it is covered by an expiration policy.

The default value should be set in one place, not three, but that is perhaps a separate problem.

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
commented

Looks okay to me.

aohrvetpv’s picture

aohrvetpv commented

Retesting patch to check that newly committed test methods work as expected.

There should be failures because the new methods expect "Admin (UID = 1) expires" to be unchecked by default.

AohRveTPV queued 2: password_policy-7.x-1.x-change_admin_expiration_default-2476113-2.patch for re-testing.

aohrvetpv’s picture

aohrvetpv commented
StatusFileSize
new password_policy-7.x-1.x-change_admin_expiration_default-2476113-6.patch3.13 KB

The tests passed (unexpected). It looks like I missed changing the default in one place.

This patch will hopefully fail testing.

Status: Needs review » Needs work

The last submitted patch, 6: password_policy-7.x-1.x-change_admin_expiration_default-2476113-6.patch, failed testing.

aohrvetpv’s picture

aohrvetpv commented
Status: Needs work » Needs review
StatusFileSize
new password_policy-7.x-1.x-change_admin_expiration_default-2476113-8.patch4.7 KB

Updated test methods per change in default.

Status: Needs review » Needs work

The last submitted patch, 8: password_policy-7.x-1.x-change_admin_expiration_default-2476113-8.patch, failed testing.

aohrvetpv’s picture

aohrvetpv commented
Status: Needs work » Needs review
StatusFileSize
new password_policy-7.x-1.x-change_admin_expiration_default-2476113-10.patch5.27 KB

Fixed mistake in test method changes (hopefully).

aohrvetpv’s picture

aohrvetpv commented
StatusFileSize
new password_policy-7.x-1.x-change_admin_expiration_default-2476113-11.patch5.17 KB

Removed redundant sentence from hook_update_N() comment.

  • AohRveTPV committed e16c8d5 on 7.x-1.x 
    Issue #2476113 by AohRveTPV: Exempting uid 1 from expiration is insecure...
aohrvetpv’s picture

aohrvetpv commented
Version: 7.x-1.x-dev » 6.x-1.x-dev
Status: Needs review » Patch (to be ported)
aohrvetpv’s picture

aohrvetpv commented
Status: Patch (to be ported) » Closed (won't fix)

6.x-1.x is no longer supported.