Disabled users are allowed to login, make ldap filter configurable

lwillek commented 5 years ago

Hi. This is both, a feature request and a (very small) security issue. I did not created 2 separated issues because I think both is related.

Let's start with the security issue. The current ldap filter (wekanActivated=TRUE) also allows to login for users administratively deactivated or locked, so this is imho a (tiny) security issue.

In addition to the current check if the account is wekanActivated=TRUE, it should be also checked if the account is actually a real person, has a primary mail address set, is enabled, not deactivated and in addition also not locked out.

Therefore, I suggest to change the current default filter:

(&(wekanActivated=TRUE)(uid=<some_user_name>))

to this new default filter:

(&(objectClass=person)(mailPrimaryAddress=*)(!(shadowExpire=*))(sambaBadPasswordTime=0)(wekanActivated=TRUE)(uid=<some_user_name>))

And this is the related feature request: It is not uncommon that a Ldap search filter needs to be adjusted by the local Univention Administrator, for example because additional site specific criteria have to be fulfilled, e.g. membership in a certain group. An example of such a request: See https://help.univention.com/t/howto-enable-wekan-app-for-a-group/13052

Therefore I suggest to make the ldap search filter configurable for the users, as already accomplished with ROOT_URL and ACCOUNTS_LOCKOUT_KNOWN_USERS_FAILURES_BEFORE.

spaceone commented 5 years ago

I think it's not easily possible with an LDAP filter to ensure that the user is enabled. FYI: objectClass=person includes also computer objects. But computers don't have mailPrimaryAddress afaik. A user is disabled when one of the following is true:

sambaAcctFlags contains D
krb5KDCFlags & 128 == 128
shadowExpire is 1 or smaller than time() / 3600 / 24
userPassword hash starts with ! (after {crypt}, {LANMAN}, any other {etc}) A user is locked when one of:
sambaAcctFlags contains L
krb5KDCFlags &131072 == 131072

A filter can check the set bits e.g. via: (!(krb5KDCFlags:1.2.840.113556.1.4.803:=128))

xet7 commented 5 years ago

Thanks! I sent question about these to tech person at Univention, he knows about these more.

lwillek commented 4 years ago

Thank you so much for making the ldap search filter configurable for the users with the new Wekan version available within Univention!

Users are now able to configure this with ease, simply by using the "Univention management console": UMC --> "Installed Applications" --> "Wekan" --> "APP SETTINGS" --> "LDAP Settings"

I successfully tested the following filter:

(objectClass=person)(mailPrimaryAddress=*)(wekanActivated=TRUE)(!(krb5KDCFlags:1.2.840.113556.1.4.803:=128))(!(shadowExpire=*))(!(sambaAcctFlags=[UL       ]))(!(sambaAcctFlags=[UD       ]))

How to test your own filters

command line

Testing specific filters is possible (and recommended) via command line before applying. This example uses exactly the same filter shown above for an user with UID=lutz:

# univention-ldapsearch -LLL '(&(objectClass=person)(mailPrimaryAddress=*)(wekanActivated=TRUE)(!(krb5KDCFlags:1.2.840.113556.1.4.803:=128))(!(shadowExpire=*))(!(sambaAcctFlags=[UL       ]))(!(sambaAcctFlags=[UD       ]))(uid=lutz))' uid | tail -2
uid: lutz

This filter does not match anymore if the user is disabled or locked. This can be set via command line too. This example again uses an user with UID=lutz:

### this enables the ucr variables for the interactive shell (eg. $ldap_base)
# eval $(ucr shell)

### this will unlock and enable the user
# udm users/user modify --dn uid=lutz,cn=users,$ldap_base --set locked=0 --set disabled=0

### this disables the user
# udm users/user modify --dn uid=lutz,cn=users,$ldap_base  --set disabled=1

### this will lock the user account
# udm users/user modify --dn uid=lutz,cn=users,$ldap_base --set locked=1

### this will set the user to both, disabled and locked:
# udm users/user modify --dn uid=lutz,cn=users,$ldap_base --set locked=1 --set disabled=1

Wekan Log File

To test the filters for the Wekan docker container, simply look into the log, after the filter is applied:

# docker logs wekan-app | less

If the user is disabled or locked, the log will show:

[INFO] Search returned
[ERROR] Error: User not Found

If the user is not disabled or locked, the log will show:

[INFO] Search result count 1
[INFO] Authenticating "uid=lutz,cn=users,***removed***"
[INFO] Authenticated "uid=lutz,cn=users,***removed***"
...

lwillek commented 4 years ago

Currently, the default filter is still (wekanActivated=TRUE), but with the help of the comment above, administrators should be able to test and apply their own filters if needed.

Maybe its a good idea to talk to the tech person at Univention, most likely the filter mentioned above could added as default filter. However, I am not sure if this filter is valid for all possible environments, and maybe the filter could be still improved. (I just tested within one environment: works for me)

Thanks again 👍 Lutz

spaceone commented 1 year ago

On a UCS 5 system you can lookup which filter UDM uses via:

# python3
>>> import univention.admin.modules
>>> univention.admin.modules.update()
>>> U = univention.admin.modules.get('users/user').object
>>> str(U.lookup_filter('disabled=1'))
'(&(univentionObjectType=users/user)(!(uidNumber=0))(!(univentionObjectFlag=functional))(&(shadowExpire=1)(krb5KDCFlags:1.2.840.113556.1.4.803:=128)(|(sambaAcctFlags=[UD       ])(sambaAcctFlags=[ULD       ]))))'
>>> str(U.lookup_filter('locked=1'))
'(&(univentionObjectType=users/user)(!(uidNumber=0))(!(univentionObjectFlag=functional))(|(krb5KDCFlags:1.2.840.113556.1.4.803:=131072)(sambaAcctFlags=[UL       ])(sambaAcctFlags=[ULD       ])))'
>>> str(U.lookup_filter('(|(disabled=1)(locked=1))'))
'(&(univentionObjectType=users/user)(!(uidNumber=0))(!(univentionObjectFlag=functional))(|(&(shadowExpire=1)(krb5KDCFlags:1.2.840.113556.1.4.803:=128)(|(sambaAcctFlags=[UD       ])(sambaAcctFlags=[ULD       ])))(|(krb5KDCFlags:1.2.840.113556.1.4.803:=131072)(sambaAcctFlags=[UL       ])(sambaAcctFlags=[ULD       ]))))'

so that reveals that @lwillek filter is not completely accurate for users which are locked and disabled at the same time.

Please note, that we have a bug in that filter as well: https://forge.univention.org/bugzilla/show_bug.cgi?id=55633

wekan / univention