Page MenuHomePhabricator

Blocking an account on wikitech should disable LDAP logins
Closed, ResolvedPublic

Description

We had the same issue with mediawiki.org oauth logins, thus blocked wiki users could still log in to phabricator. In fact they could even create new phab accounts while blocked on wiki.

Presumably the same problem exists for wikitech logins and we should probably address this, I'm just not sure WHERE it would be best to implement a block. Most likely in the ldap server or?

Related to Incident: 20170617-Phabricator-spam

See Also: T173463: Have global blocks from any wikimedia project affect Phabricator as well

Details

Related Gerrit Patches:
operations/puppet : productionwmcs: Respect LDAP locks in ssh-key-ldap-lookup
operations/mediawiki-config : masterwikitech: Lock LDAP accounts when users are blocked
mediawiki/extensions/LdapAuthentication : wmf/1.33.0-wmf.23Also set an LDAP password policy on Block
mediawiki/extensions/LdapAuthentication : wmf/1.33.0-wmf.24Also set an LDAP password policy on Block
operations/puppet : productionopenldap: Set default password policy
mediawiki/extensions/LdapAuthentication : masterAlso set an LDAP password policy on Block
mediawiki/extensions/LdapAuthentication : masterAdd support for (un)locking LDAP accounts in response to blocks
mediawiki/vagrant : masterldapauth: support LDAP account locking via blocks

Event Timeline

mmodell created this task.Jun 22 2017, 11:13 PM
Restricted Application added a project: Cloud-Services. · View Herald TranscriptJun 22 2017, 11:13 PM
mmodell updated the task description. (Show Details)Jun 23 2017, 4:28 AM
mmodell lowered the priority of this task from Normal to Low.Jul 10 2017, 4:58 PM
greg lowered the priority of this task from Low to Lowest.Jul 10 2017, 4:58 PM

So instead of disabling an account here, a block on Wikitech will have the effect of blocking the user from Wikitech, Phabricator OAUth/LDAP, Gerrit, etc. right?

Phabricator OAuth is based on blocked at mediawiki.prg isn't it?

demon added a comment.Jul 10 2017, 6:01 PM

So instead of disabling an account here, a block on Wikitech will have the effect of blocking the user from Wikitech, Phabricator OAUth/LDAP, Gerrit, etc. right?

Yep, anything LDAP related (so Logstash, Wikitech, Gerrit, etc etc etc etc etc). As Luke points out Phab OAuth wouldn't be blocked by this. But it at least would unify most accounts in one disable action.

Which is all a good idea, but I'm unsure how much time should be spent on this given T161859...

See also https://wikitech.wikimedia.org/wiki/Help:Disabling_an_account, which documents the afaict current manual procedure of disabling an LDAP account.

There is probably some hook that we can attach code to to make some adjustment to the LDAP object for a user when they are blocked. Whenever T161859: Make Wikitech an SUL wiki happens some other mechanism will be needed.

bd808 merged a task: Restricted Task.Jul 26 2018, 3:10 AM
bd808 added subscribers: chasemp, Bawolff.

A bit of web searching led me to slapo-ppolicy which is a password policy overlay for OpenLDAP. This overlay can do a lot of things, but the most interesting for this feature request is:

pwdLockoutDuration

       This attribute contains the number of seconds during which the password
       cannot be used to authenticate the user to the  directory  due  to  too
       many  consecutive  failed  bind  attempts.   (See  also  pwdLockout and
       pwdMaxFailure.)  If pwdLockoutDuration is not present, or if its  value
       is  zero  (0),  the password cannot be used to authenticate the user to
       the directory again until it is reset by an administrator.

           (  1.3.6.1.4.1.42.2.27.8.1.10
              NAME 'pwdLockoutDuration'
              EQUALITY integerMatch
              SYNTAX 1.3.6.1.4.1.1466.115.121.1.27
              SINGLE-VALUE )

@MoritzMuehlenhoff, do you have thoughts/experience in enabling this overlay for a directory? I'm not sure that we would want many (or even most) of the features it supports, but having an easy way to completely block binding to the directory by a given account seems pretty handy. I think the only other way we have to do this today is to change the account's password which could be problematic in practice (like if an account got locked improperly and we wanted to re-enable it).

MarcoAurelio added a comment.EditedJul 26 2018, 6:33 PM

In case gerrit is also affected:

See also https://wikitech.wikimedia.org/wiki/Help:Disabling_an_account, which documents the afaict current manual procedure of disabling an LDAP account.

AFAIK (@Paladox and @demon will know better) the in v2.15 of gerrit the way gerrit accounts can be disabled has been easened, as per docs this could be:

ssh -p 29418 you@gerrit.wikimedia.org gerrit set-account --full-name "'Vandal <i_liek@vandalizm.com>'" --inactive

Still it requires someone with the appropriate grants to do so of course. The documentation probably needs updating.

A bit of web searching led me to slapo-ppolicy which is a password policy overlay for OpenLDAP.

I was looking at this a bit more today and figured out that our current slapd config loads ppolicy already. We do not however set a ppolicy_default <DN> pointing to a default password policy to be applied to all objects that do not have their own explicit policy configured. As far as I understand from reading various tutorials on this topic we would need to create and apply a default policy in order to be able to apply pwdAccountLockedTime <account-locked-time> lockouts to accounts.

This needs some testing, but I think a policy like this plus a ppolicy_default cn=default,ou=pwpolicies,dc=wikimedia,dc=org setting in slapd.conf would make it possible for us to lock accounts:

dn: ou=pwpolicies,dc=wikimedia,dc=org
objectClass: organizationalUnit
objectClass: top
ou: policies

dn: cn=default,ou=pwpolicies,dc=wikimedia,dc=org
objectClass: pwdPolicy
cn: default
pwdLockout: TRUE
pwdMustChange: FALSE

With this in place we could then code appropriate BlockIp and UnblockUser hooks for wikitech that would set/remove a pwdAccountLockedTime attribute on the backing LDAP entry.

jbond added a subscriber: jbond.Mar 18 2019, 10:13 AM

Change 497684 had a related patch set uploaded (by BryanDavis; owner: Bryan Davis):
[operations/puppet@production] openldap: Set default password policy

https://gerrit.wikimedia.org/r/497684

Change 497864 had a related patch set uploaded (by BryanDavis; owner: Bryan Davis):
[mediawiki/extensions/LdapAuthentication@master] Add support for (un)locking LDAP accounts in response to blocks

https://gerrit.wikimedia.org/r/497864

Change 497866 had a related patch set uploaded (by BryanDavis; owner: Bryan Davis):
[operations/mediawiki-config@master] wikitech: Lock LDAP accounts when users are blocked

https://gerrit.wikimedia.org/r/497866

Change 497928 had a related patch set uploaded (by BryanDavis; owner: Bryan Davis):
[mediawiki/vagrant@master] ldapauth: support LDAP account locking via blocks

https://gerrit.wikimedia.org/r/497928

Change 497928 merged by jenkins-bot:
[mediawiki/vagrant@master] ldapauth: support LDAP account locking via blocks

https://gerrit.wikimedia.org/r/497928

Change 497864 merged by jenkins-bot:
[mediawiki/extensions/LdapAuthentication@master] Add support for (un)locking LDAP accounts in response to blocks

https://gerrit.wikimedia.org/r/497864

jijiki raised the priority of this task from Lowest to High.Mar 22 2019, 9:06 AM
greg removed a subscriber: greg.Mar 27 2019, 11:20 PM
akosiaris added a comment.EditedMar 28 2019, 12:27 PM

Copying from https://gerrit.wikimedia.org/r/497684 (and adding some extra stuff)

Setting pwdAccountLockedTime is not a good idea. That's an operational attribute per https://linux.die.net/man/5/slapo-ppolicy and in it it says

The operational attributes used by the ppolicy module are stored in the user's entry. Most of these attributes are not intended to be changed directly by users; they are there to track user activity.

It is also an attribute that is modified by the overlay code itself, and thus > 1 code paths would adjust it if we wrote code touching it, increasing confusion.

Furthermore, I did some tests and results are below:

  • A password reset from an admin (so effectively https://wikitech.wikimedia.org/wiki/Special:PasswordReset - or any other tool for that matter). I am assuming here we want to revert 4adc89bce4b80 and reenable it) removes that attribute fully reenabling the user. That is not desired in our case, as we don't want that. It's also not obvious that it happens. It also means to use it we would need to scramble the user's email which is fully counterintuitive.
  • The user using LDAP protocol to change their password doesn't work (thankfully)
  • The "account locked" message can be spewed out when trying to do anything, but the ppolicy LDAP extension needs to be added to the LDAP request (-e ppolicy for ldapsearch), meaning some mediawiki code needs to be amended to add it. Also ppolicy_use_lockout needs to be added to slapd.conf, but that's easy.

Overall, this part of the ppolicy overlay's functionality is about locking accounts after failed password attempts and that's ingrained in the code. Abusing it for what we want to do looks like it's going to be with caveats as our changes will interact with the ppolicy changes.

I 've been trying to find an alternative and after some tests and some discussions with @MoritzMuehlenhoff I think I have a better approach, outlined below.

We create the following password policy

dn: cn=blocked,ou=pwpolicies,dc=wikimedia,dc=org
cn: blocked
objectClass: organizationalRole
objectClass: top
objectClass: pwdPolicy
pwdAttribute: userPassword
pwdAllowUserChange: FALSE
pwdMaxAge: 1

And have mediawiki LDAP code set the following operational attribute to user entries we want to disable:

pwdPolicySubentry: cn=blocked,ou=pwpolicies,dc=wikimedia,dc=org

This is in accordance with the overlay's documentation [1] of

Every account that should be subject to password policy control should have a pwdPolicySubentry attribute containing the DN of a valid pwdPolicy entry, or they can simply use the configured default. In this way different users may be managed according to different policies.

So we aren't abusing the ppolicy behavior in any way and thus can expect to have less weird behaviors.

The premise is simple.

  • Expire the offending user's password (the max age is set to 1s)
  • Disallow them from changing their password.

That's it. Note that 1s is required, cause the value 0 is equivalent to the attribute not being there per the docs [1]

Pros:

  • This has been tested and works.
  • Even if the password is reset the user remains blocked. This has been tested.
  • We don't rely on some magic number and some hidden behavior, but rather clearly denote the user is blocked in an LDAP attribute.
  • There is no need for a default ppolicy (not that we can't have one, but it should be a different task), so no need for slapd configuration changes.
  • Our various tools can adapt gradually to the new filter to flush out sleeper accounts, but the moment a user is blocked, LDAP authentication is blocked everywhere.
  • Testing whether a user is blocked is as simple as amending the LDAP search filter to something like (&(cn=<username>)(!(pwdPolicySubentry=cn=blocked,ou=pwpolicies,dc=wikimedia,dc=org)))

Cons:

  • There is the obvious race condition of 1 second after a password change. However, it is really difficult to exploit it (it's just 1 sec after all). The user needs to be really fast in changing the pass and right away trying to login into some other tool. It's also easy to guard against that, with a simple configuration change to all tools changing the LDAP filter to include pwdpolicySubentry
  • All tools will need to update their LDAP filters. However that would happen anyway.
  • The attribute is unfortunately an operational one. That means it either needs to requested specifically to be returned (standard practice for most tools) or the + sigil needs to be passed to the tool e.g. ldapsearch, ldapvi. That might cause some confusion to people using ldap tools.

[1] https://linux.die.net/man/5/slapo-ppolicy

Copying from https://gerrit.wikimedia.org/r/497684 (and adding some extra stuff)
Setting pwdAccountLockedTime is not a good idea. That's an operational attribute per https://linux.die.net/man/5/slapo-ppolicy and in it it says

The operational attributes used by the ppolicy module are stored in the user's entry. Most of these attributes are not intended to be changed directly by users; they are there to track user activity.

That same document also says:

pwdAccountLockedTime
This attribute contains the time that the user's account was locked. If the account has been locked, the password may no longer be used to authenticate the user to the directory. If pwdAccountLockedTime is set to 000001010000Z, the user's account has been permanently locked and may only be unlocked by an administrator. Note that account locking only takes effect when the pwdLockout password policy attribute is set to "TRUE". -- https://linux.die.net/man/5/slapo-ppolicy (emphasis added)

This is very explicitly saying that pwdAccountLockedTime:000001010000Z is meant to signal administrative lockout and also that it restricts reset of the password to admin rights level accounts. This prevents any self-binding password reset in the local testing I did.

It is also an attribute that is modified by the overlay code itself, and thus > 1 code paths would adjust it if we wrote code touching it, increasing confusion.

It is only adjusted by the overlay if the password policy applied includes a pwdLockout:TRUE setting to enforce brute force rate limiting. It seems to be implied in my reading of the docs that pwdAccountLockedTime:000001010000Z would not be changed by any subsequent pwdMaxFailure violations.

Furthermore, I did some tests and results are below:

  • A password reset from an admin (so effectively https://wikitech.wikimedia.org/wiki/Special:PasswordReset - or any other tool for that matter). I am assuming here we want to revert 4adc89bce4b80 and reenable it) removes that attribute fully reenabling the user. That is not desired in our case, as we don't want that. It's also not obvious that it happens. It also means to use it we would need to scramble the user's email which is fully counterintuitive.

Special:PasswordReset will not work as a self-service reset method if the local wiki account is blocked. That leaves the use of Special:PasswordReset as an sysop right to act on an existing account which actually seems reasonable and expected to me.

  • The user using LDAP protocol to change their password doesn't work (thankfully)
  • The "account locked" message can be spewed out when trying to do anything, but the ppolicy LDAP extension needs to be added to the LDAP request (-e ppolicy for ldapsearch), meaning some mediawiki code needs to be amended to add it. Also ppolicy_use_lockout needs to be added to slapd.conf, but that's easy.

I did not have to make any changes to the LDAP binding code or LdapAuthentication to make pwdAccountLockedTime:000001010000Z block LdapAuthentication and direct binds (for example ldapwhoami). Setting ppolicy_use_lockout in the policy is only needed if we want the directory to issue an AccountLocked error code rather than the default InvalidCredentials error code when an account is locked. I guess that's a business logic decision, but I'm not sure that is good one to make:

Note that sending the AccountLocked error code provides useful information to an attacker; sites that are sensitive to security issues should not enable this option. -- https://linux.die.net/man/5/slapo-ppolicy

Overall, this part of the ppolicy overlay's functionality is about locking accounts after failed password attempts and that's ingrained in the code. Abusing it for what we want to do looks like it's going to be with caveats as our changes will interact with the ppolicy changes.

Our readings of the material are different. I see https://linux.die.net/man/5/slapo-ppolicy, https://ldapwiki.com/wiki/PwdAccountLockedTime, and https://docs.oracle.com/cd/E19424-01/820-4813/pwdaccountlockedtime-5dsat/index.html all saying that pwdAccountLockedTime:000001010000Z is meant for administrative lockouts.

I 've been trying to find an alternative and after some tests and some discussions with @MoritzMuehlenhoff I think I have a better approach, outlined below.
We create the following password policy

dn: cn=blocked,ou=pwpolicies,dc=wikimedia,dc=org
cn: blocked
objectClass: organizationalRole
objectClass: top
objectClass: pwdPolicy
pwdAttribute: userPassword
pwdAllowUserChange: FALSE
pwdMaxAge: 1

And have mediawiki LDAP code set the following operational attribute to user entries we want to disable:
pwdPolicySubentry: cn=blocked,ou=pwpolicies,dc=wikimedia,dc=org
This is in accordance with the overlay's documentation [1] of

Every account that should be subject to password policy control should have a pwdPolicySubentry attribute containing the DN of a valid pwdPolicy entry, or they can simply use the configured default. In this way different users may be managed according to different policies.

So we aren't abusing the ppolicy behavior in any way and thus can expect to have less weird behaviors.
The premise is simple.

  • Expire the offending user's password (the max age is set to 1s)
  • Disallow them from changing their password.

That's it. Note that 1s is required, cause the value 0 is equivalent to the attribute not being there per the docs [1]
Pros:

  • This has been tested and works.
  • Even if the password is reset the user remains blocked. This has been tested.
  • We don't rely on some magic number and some hidden behavior, but rather clearly denote the user is blocked in an LDAP attribute.
  • There is no need for a default ppolicy (not that we can't have one, but it should be a different task), so no need for slapd configuration changes.
  • Our various tools can adapt gradually to the new filter to flush out sleeper accounts, but the moment a user is blocked, LDAP authentication is blocked everywhere.
  • Testing whether a user is blocked is as simple as amending the LDAP search filter to something like (&(cn=<username>)(!(pwdPolicySubentry=cn=blocked,ou=pwpolicies,dc=wikimedia,dc=org)))

Cons:

  • There is the obvious race condition of 1 second after a password change. However, it is really difficult to exploit it (it's just 1 sec after all). The user needs to be really fast in changing the pass and right away trying to login into some other tool. It's also easy to guard against that, with a simple configuration change to all tools changing the LDAP filter to include pwdpolicySubentry
  • All tools will need to update their LDAP filters. However that would happen anyway.

Which tools? Wikitech, gerrit, and phabricator all perform LDAP-based authn by performing a bind with the supplied username and password. Either approach would cause these binds to fail and prevent authn.

  • The attribute is unfortunately an operational one. That means it either needs to requested specifically to be returned (standard practice for most tools) or the + sigil needs to be passed to the tool e.g. ldapsearch, ldapvi. That might cause some confusion to people using ldap tools.

This is going to be the case with any lockout method that is based on ppolicy integration. I think my confusion is the same as with the "tools will need to update their LDAP filters" case above; I'm not sure which LDAP interactions you are talking about here. If it is the pure out-of-band lookup case ("who is currently locked") then I agree. If it is the in-band authn case I don't see why the data is needed as long as auth-bind is disabled.

akosiaris added a comment.EditedApr 1 2019, 3:18 PM

In the interest of fully figuring this out I 've updated my testing openldap vagrant env [1] with a Makefile[2] that automates and runs some tests[2]. I 've pasted results for it in P8321. The test suite can (and maybe should) be extended to make sure we got all our bases covered.

The TL;DR is that both solutions have some shortcomings, maybe we should look into using both as they could complement each other.

The pwdAccountLockedTime: 000001010000Z, solution has the following

  • The first, obvious (and documented) drawback that if an admin resets the password, the account is unlocked. Even if documented, this counter-intuitive behavior carries the obvious risk that an account is (mistakenly?) re-enabled if the password is changed outside of the expected process. The expected process here would be https://wikitech.wikimedia.org/wiki/Special:PasswordReset as a sysop. But anyone with access to the admin account could do the password reset and make that mistake. Not only that, it's been standard process for disabling LDAP accounts in the previous weeks to muck the password via standard LDAP procedures. It's even documented in office wiki currently as the way to go. Docs of course can be amended and people retrained, so I categorize this drawback and minor overall, but medium due to our current processes.
  • The second is the operational attribute being changed to magic value. This is less of a drawback and more of a nuisance. Just one of those things that can catch someone off guard.

The max age solution has the following:

  • There is the obvious 1s race condition. The tests actually triggered that, and that's why there's a sleep 2 in there. It's not super worrying as it can be mitigated against by 2 things: a) altering LDAP filters in most services to check for the presence of the pwdPolicySubentry attribute b) By disallowing password change by the locked out users to only allow a 1 second window once per account.
  • More importantly, this doesn't really trigger until the first time the password has been changed and the account actually obtains the pwdChangedTime attribute. That one is actually the gravest of the drawbacks. A quick ldapsearch our data shows that 8356 accounts have it set and 8352 (approximately) don't have it set. This is something that I had failed to catch on my previous, non-automated tests.

That last one is a deal breaker for maxage alone as it would require more extensive changes.

Copying from https://gerrit.wikimedia.org/r/497684 (and adding some extra stuff)
Setting pwdAccountLockedTime is not a good idea. That's an operational attribute per https://linux.die.net/man/5/slapo-ppolicy and in it it says

The operational attributes used by the ppolicy module are stored in the user's entry. Most of these attributes are not intended to be changed directly by users; they are there to track user activity.

That same document also says:

pwdAccountLockedTime
This attribute contains the time that the user's account was locked. If the account has been locked, the password may no longer be used to authenticate the user to the directory. If pwdAccountLockedTime is set to 000001010000Z, the user's account has been permanently locked and may only be unlocked by an administrator. Note that account locking only takes effect when the pwdLockout password policy attribute is set to "TRUE". -- https://linux.die.net/man/5/slapo-ppolicy (emphasis added)

This is very explicitly saying that pwdAccountLockedTime:000001010000Z is meant to signal administrative lockout and also that it restricts reset of the password to admin rights level accounts. This prevents any self-binding password reset in the local testing I did.

Oh it works for sure. But that doesn't stop this from being counter-intuitive and having drawbacks as pointed out above.

It is also an attribute that is modified by the overlay code itself, and thus > 1 code paths would adjust it if we wrote code touching it, increasing confusion.

It is only adjusted by the overlay if the password policy applied includes a pwdLockout:TRUE setting to enforce brute force rate limiting. It seems to be implied in my reading of the docs that pwdAccountLockedTime:000001010000Z would not be changed by any subsequent pwdMaxFailure violations.

Without pwdLockout: TRUE, the lockout doesn't even work (user4 in my tests), so having it around is required. But I concur that no amount of pwdMaxFailure violations would change it.

Furthermore, I did some tests and results are below:

  • A password reset from an admin (so effectively https://wikitech.wikimedia.org/wiki/Special:PasswordReset - or any other tool for that matter). I am assuming here we want to revert 4adc89bce4b80 and reenable it) removes that attribute fully reenabling the user. That is not desired in our case, as we don't want that. It's also not obvious that it happens. It also means to use it we would need to scramble the user's email which is fully counterintuitive.

Special:PasswordReset will not work as a self-service reset method if the local wiki account is blocked. That leaves the use of Special:PasswordReset as an sysop right to act on an existing account which actually seems reasonable and expected to me.

As I point out above it isn't the only way the password can be reset, hence my entire set of second thoughts around this.

  • The user using LDAP protocol to change their password doesn't work (thankfully)
  • The "account locked" message can be spewed out when trying to do anything, but the ppolicy LDAP extension needs to be added to the LDAP request (-e ppolicy for ldapsearch), meaning some mediawiki code needs to be amended to add it. Also ppolicy_use_lockout needs to be added to slapd.conf, but that's easy.

I did not have to make any changes to the LDAP binding code or LdapAuthentication to make pwdAccountLockedTime:000001010000Z block LdapAuthentication and direct binds (for example ldapwhoami).

Yeah, if we had to do changes to the LDAP binding code it would be bad.

Setting ppolicy_use_lockout in the policy is only needed if we want the directory to issue an AccountLocked error code rather than the default InvalidCredentials error code when an account is locked. I guess that's a business logic decision, but I'm not sure that is good one to make:

It's purely informative on the LDAP level. No real reason to do it, just pointing it out.

Note that sending the AccountLocked error code provides useful information to an attacker; sites that are sensitive to security issues should not enable this option. -- https://linux.die.net/man/5/slapo-ppolicy

We are anyway publishing this under https://wikitech.wikimedia.org/wiki/Special:BlockList, so hiding it on the LDAP level protocol is of small use. Publishing it is of small use as well as it requires the LDAP library used to ask for it.

Overall, this part of the ppolicy overlay's functionality is about locking accounts after failed password attempts and that's ingrained in the code. Abusing it for what we want to do looks like it's going to be with caveats as our changes will interact with the ppolicy changes.

Our readings of the material are different. I see https://linux.die.net/man/5/slapo-ppolicy, https://ldapwiki.com/wiki/PwdAccountLockedTime, and https://docs.oracle.com/cd/E19424-01/820-4813/pwdaccountlockedtime-5dsat/index.html all saying that pwdAccountLockedTime:000001010000Z is meant for administrative lockouts.

That's what the draft RFC is saying per https://tools.ietf.org/html/draft-behera-ldap-password-policy-10. It also stipulates other cases (see 7.1) but the implementations differ. See https://www.openldap.org/lists/openldap-technical/201803/msg00006.html for the pwdEndTime and pwdStartTime attributes which are missing in openldap.

I 've been trying to find an alternative and after some tests and some discussions with @MoritzMuehlenhoff I think I have a better approach, outlined below.
We create the following password policy

dn: cn=blocked,ou=pwpolicies,dc=wikimedia,dc=org
cn: blocked
objectClass: organizationalRole
objectClass: top
objectClass: pwdPolicy
pwdAttribute: userPassword
pwdAllowUserChange: FALSE
pwdMaxAge: 1

And have mediawiki LDAP code set the following operational attribute to user entries we want to disable:
pwdPolicySubentry: cn=blocked,ou=pwpolicies,dc=wikimedia,dc=org
This is in accordance with the overlay's documentation [1] of

Every account that should be subject to password policy control should have a pwdPolicySubentry attribute containing the DN of a valid pwdPolicy entry, or they can simply use the configured default. In this way different users may be managed according to different policies.

So we aren't abusing the ppolicy behavior in any way and thus can expect to have less weird behaviors.
The premise is simple.

  • Expire the offending user's password (the max age is set to 1s)
  • Disallow them from changing their password.

That's it. Note that 1s is required, cause the value 0 is equivalent to the attribute not being there per the docs [1]
Pros:

  • This has been tested and works.
  • Even if the password is reset the user remains blocked. This has been tested.
  • We don't rely on some magic number and some hidden behavior, but rather clearly denote the user is blocked in an LDAP attribute.
  • There is no need for a default ppolicy (not that we can't have one, but it should be a different task), so no need for slapd configuration changes.
  • Our various tools can adapt gradually to the new filter to flush out sleeper accounts, but the moment a user is blocked, LDAP authentication is blocked everywhere.
  • Testing whether a user is blocked is as simple as amending the LDAP search filter to something like (&(cn=<username>)(!(pwdPolicySubentry=cn=blocked,ou=pwpolicies,dc=wikimedia,dc=org)))

Cons:

  • There is the obvious race condition of 1 second after a password change. However, it is really difficult to exploit it (it's just 1 sec after all). The user needs to be really fast in changing the pass and right away trying to login into some other tool. It's also easy to guard against that, with a simple configuration change to all tools changing the LDAP filter to include pwdpolicySubentry
  • All tools will need to update their LDAP filters. However that would happen anyway.

Which tools? Wikitech, gerrit, and phabricator all perform LDAP-based authn by performing a bind with the supplied username and password. Either approach would cause these binds to fail and prevent authn.

Those tools also have sessions and users need to be blocked on that level too. Per your suggestion in T218654#5060980, it would help to amend thoses filter for that.

  • The attribute is unfortunately an operational one. That means it either needs to requested specifically to be returned (standard practice for most tools) or the + sigil needs to be passed to the tool e.g. ldapsearch, ldapvi. That might cause some confusion to people using ldap tools.

This is going to be the case with any lockout method that is based on ppolicy integration. I think my confusion is the same as with the "tools will need to update their LDAP filters" case above; I'm not sure which LDAP interactions you are talking about here. If it is the pure out-of-band lookup case ("who is currently locked") then I agree. If it is the in-band authn case I don't see why the data is needed as long as auth-bind is disabled.

It's the out-of-band lookup/manipulation cases.

[1] https://github.com/akosiaris/openldap_vagrant
[2] https://github.com/akosiaris/openldap_vagrant/blob/master/Makefile

akosiaris added a comment.EditedApr 1 2019, 3:44 PM

Now that I 've automated the tests it was easy to combine the 2 approaches and fix the drawbacks. User5 in P8321 is subject to both approaches and the only drawback I could find out is a race of 1s between an admin resets the password (by mistake, or following some old and outdated process) and the user managing to authenticate to a service. This is an attack vector that requires 2 humans to perform an action and thus not something that can be somehow automated to be performed in under 1s. I think we are safe from this one.

crusnov added a subscriber: crusnov.Apr 1 2019, 4:48 PM

Change 500681 had a related patch set uploaded (by Alexandros Kosiaris; owner: Alexandros Kosiaris):
[mediawiki/extensions/LdapAuthentication@master] Also set an LDAP password policy on Block

https://gerrit.wikimedia.org/r/500681

@bd808, I 've submitted https://gerrit.wikimedia.org/r/500681 for review and updated https://gerrit.wikimedia.org/r/#/c/operations/mediawiki-config/+/497866 to match.

On the LDAP servers themselves I 've already added the following entries

dn: ou=ppolicies,dc=wikimedia,dc=org
objectclass: organizationalunit
objectclass: top
ou: ppolicies

dn: cn=disabled,ou=ppolicies,dc=wikimedia,dc=org
cn: disabled
objectclass: organizationalRole
objectclass: top
objectclass: pwdPolicy
pwdAttribute: userPassword
pwdLockout: TRUE
pwdMaxAge: 1

So when the 2 changes are deemed mergeable, we would be a deploy away from enabling this.

https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/497684/ is no longer required after the above are merged.

Names are up for naming bikeshedding 😜

Change 500994 had a related patch set uploaded (by BryanDavis; owner: Alexandros Kosiaris):
[mediawiki/extensions/LdapAuthentication@wmf/1.33.0-wmf.24] Also set an LDAP password policy on Block

https://gerrit.wikimedia.org/r/500994

Change 500994 merged by jenkins-bot:
[mediawiki/extensions/LdapAuthentication@wmf/1.33.0-wmf.24] Also set an LDAP password policy on Block

https://gerrit.wikimedia.org/r/500994

Change 500681 merged by jenkins-bot:
[mediawiki/extensions/LdapAuthentication@master] Also set an LDAP password policy on Block

https://gerrit.wikimedia.org/r/500681

Change 497684 abandoned by BryanDavis:
openldap: Set default password policy

Reason:
We found another method that does not require these changes. See Ib40934155ae040fc48678415d39059d476be871d

https://gerrit.wikimedia.org/r/497684

bd808 added a parent task: Restricted Task.Apr 4 2019, 5:16 PM

Change 501412 had a related patch set uploaded (by BryanDavis; owner: Alexandros Kosiaris):
[mediawiki/extensions/LdapAuthentication@wmf/1.33.0-wmf.23] Also set an LDAP password policy on Block

https://gerrit.wikimedia.org/r/501412

Change 501412 merged by jenkins-bot:
[mediawiki/extensions/LdapAuthentication@wmf/1.33.0-wmf.23] Also set an LDAP password policy on Block

https://gerrit.wikimedia.org/r/501412

Mentioned in SAL (#wikimedia-operations) [2019-04-04T23:52:44Z] <bd808@deploy1001> Synchronized php-1.33.0-wmf.23/extensions/LdapAuthentication: SWAT: [[gerrit:501412|Also set an LDAP password policy on Block]] (T168692) (duration: 01m 01s)

Change 497866 merged by jenkins-bot:
[operations/mediawiki-config@master] wikitech: Lock LDAP accounts when users are blocked

https://gerrit.wikimedia.org/r/497866

Mentioned in SAL (#wikimedia-operations) [2019-04-05T00:07:24Z] <bd808@deploy1001> Synchronized wmf-config/wikitech.php: SWAT: [[gerrit:497866|wikitech: Lock LDAP accounts when users are blocked]], [[gerrit:501123|Disable Phabricator accounts when blocked on wikitech]] (T168692) (duration: 00m 59s)

Mentioned in SAL (#wikimedia-operations) [2019-04-05T00:09:23Z] <bd808@deploy1001> Synchronized wmf-config/InitialiseSettings.php: SWAT: [[gerrit:497866|wikitech: Lock LDAP accounts when users are blocked]], [[gerrit:501123|Disable Phabricator accounts when blocked on wikitech]] (T168692) 2/2 (duration: 00m 57s)

bd808 closed this task as Resolved.Apr 5 2019, 12:21 AM

It works!

https://wikitech.wikimedia.org/wiki/Special:BlockList?wpTarget=Bd20161025&limit=50&wpFormIdentifier=blocklist

$ ldap cn=Bd20161025 \+ \*
dn: uid=bd161025,ou=people,dc=wikimedia,dc=org
uid: bd161025
sn: Bd20161025
cn: Bd20161025
objectClass: inetOrgPerson
objectClass: person
objectClass: ldapPublicKey
objectClass: posixAccount
objectClass: shadowAccount
uidNumber: 15821
gidNumber: 500
homeDirectory: /home/bd161025
loginShell: /bin/bash
structuralObjectClass: inetOrgPerson
entryUUID: aaf1d47a-2f36-1036-8283-01a2f65993d7
creatorsName: uid=novaadmin,ou=people,dc=wikimedia,dc=org
createTimestamp: 20161025194046Z
mail: bd808@wikimedia.org
pwdPolicySubentry: cn=disabled,ou=ppolicies,dc=wikimedia,dc=org
pwdChangedTime: 20190405001611Z
entryCSN: 20190405001611.924798Z#000000#001#000000
modifiersName: uid=novaadmin,ou=people,dc=wikimedia,dc=org
modifyTimestamp: 20190405001611Z
entryDN: uid=bd161025,ou=people,dc=wikimedia,dc=org
subschemaSubentry: cn=Subschema
hasSubordinates: FALSE

Change 505025 had a related patch set uploaded (by BryanDavis; owner: Bryan Davis):
[operations/puppet@production] wmcs: Respect LDAP locks in ssh-key-ldap-lookup

https://gerrit.wikimedia.org/r/505025

Change 505025 merged by Andrew Bogott:
[operations/puppet@production] wmcs: Respect LDAP locks in ssh-key-ldap-lookup

https://gerrit.wikimedia.org/r/505025

sbassett moved this task from Backlog to Done on the Security-Team board.Jun 11 2019, 6:17 PM