#344 Review kerberos 1.9 password policy code
Closed: Fixed None Opened 13 years ago by rcritten.

Once it is available, review the kerberos 1.9 password policy enforcement code so we can understand how lockout works and ensure that we are replicating the correct attribures, updating the right ones, etc.

We will probably want to deny binds to LDAP when an account is locked out and this code review will tell us what to examine.

The goal is also to lock out all replicated KDCs as well. This is not possible according to the kerberos design doc but since we use MMR it may work for us if we replicate the right attributes.


Some notes from my inspection:
I checked out kerberos svn repo (trunk, last revision 24590).

the lockout file is: src/plugins/kdb/ldap/libkdb_ldap/lockout.c (the main function is locked_check_p on line 93)

Basically what is needed are these attributes (on the left side there are variables in lockout code, corresponding LDAP attributes are on the right side):

unlock_time - (per principal) krbLastAdminUnlock
entry->last_failed - (per principal) krbLastFailedAuth
max_fail - (per pw policy) krbpwdmaxfailure
entry->fail_auth_count - (per principal) krbLoginFailedCount
lockout_duration - (per pw policy) krbpwdlockoutduration

Per principal attributes are populated in function populate_krb5_db_entry (plugins/kdb/ldap/libkdb_ldap/ldap_misc.c:1830). Per pw policy attributes are populated in function populate_policy (plugins/kdb/ldap/libkdb_ldap/ldap_pwd_policy.c:194).

Sorry for the bad formatting, here is the attribute list again:

unlock_time            - (per principal) krbLastAdminUnlock
entry->last_failed     - (per principal) krbLastFailedAuth
max_fail               - (per pw policy) krbpwdmaxfailure
entry->fail_auth_count - (per principal) krbLoginFailedCount
lockout_duration       - (per pw policy) krbpwdlockoutduration

The other piece needed not explicitly mentioned in the description is the ability to unlock a user. MIT has said that zeroing krbLoginFailedCount is enough and using ldapmodify in krb 1.8 works. We also need to set the krbLastAdminUnlock time (for reference see what 'modprinc -unlock' does).

This is why we renamed the user-lock and user-unlock commands to user-enable and user-disable. We will need a new user-lock that will do these changes (and an aci to grant write permissions on these attributes).

Well, from the code I can tell that setting krbLastAdminUnlock to current time should be enough. There is a simple test which checks if krbLastAdminUnlock is greater than krbLastFailedAuth. If yes, the account is not considered locked any more.

Of course if we skip setting krbLoginFailedCount to zero, the account will be locked again if the next attempt fails (i.e. instead of for example 3 login attempts the user will be given only one).

I added a proof of concept patch. It still need some work though:
- the FIXME has to be decided (it's rather a matter of our policy)
- OID for krbLastAdminUnlock should be either verified or changed. I chose this just for testing purposes
- ACI: allowing all access is probably wrong, but when I tried allowing only add, delete and write, the operation failed because of insufficient access

There is one issue with Kerberos using LDAP. When unlocking the account, it partially works - the user can try to log again. What fails though is updating krbLastFailedAuth when authentication fails again - that leads to the situation in which unlocked user has no limit of bad authentications - he can try as many passwords as he wants to.

Replying to [comment:9 jzeleny]:

There is one issue with Kerberos using LDAP. When unlocking the account, it partially works - the user can try to log again. What fails though is updating krbLastFailedAuth when authentication fails again - that leads to the situation in which unlocked user has no limit of bad authentications - he can try as many passwords as he wants to.

Is this something we can fix or it is a bug in the MIT code?

Replying to [comment:10 dpal]:

Replying to [comment:9 jzeleny]:

There is one issue with Kerberos using LDAP. When unlocking the account, it partially works - the user can try to log again. What fails though is updating krbLastFailedAuth when authentication fails again - that leads to the situation in which unlocked user has no limit of bad authentications - he can try as many passwords as he wants to.

Is this something we can fix or it is a bug in the MIT code?

I believe this is in MIT code, because Kerberos is responsible for updating LDAP attributes when authenticating user.

The OID for krbLastAdminUnlock should be 1.3.6.1.4.1.5322.21.2.5

There is a typo in the krbLastAdminUnlock attribute in user_lock()

Yes, we need to set krbLoginFailedCount to 0. The KDC does this (see unlock_princ() in kadmin/cli/kadmin.c)

A copule of notes about the aci.

The aci should grant only write permissions and you'll need to add krbLoginFailedCount as well.

I think this is the type of thing that users will want to delegate so we need to create a permission and a privilege for this.

The permission will look something like:

ipa permission-add --desc='Unlock user accounts' --attrs=krblastadminunlock,krbloginfailedcount --permissions=write --type=user unlock_user

ipa privilege-add --desc='Unlock user accounts 'Unlock user'

ipa privilege-add-permission --permission=unlock_user 'Unlock user'

Note that the attributes need to be in the schema before you can create the permission.

Once you create these you can show them in raw format so you can see what you need to add to install/share/delegation.ldif

To find the aci use ldapsearch: ldapsearch -x -s base -b 'dc=example,dc=com' aci

It should be the last one.

Then you'll probably want to add cn=admins as a member of the privilege.

Thank you very much for your review, I really appreciate it

Replying to [comment:12 rcritten]:

The OID for krbLastAdminUnlock should be 1.3.6.1.4.1.5322.21.2.5
I already found that in the original kerberos patch. But thanks anyway[[BR]][[BR]]

There is a typo in the krbLastAdminUnlock attribute in user_lock()
I'm not sure I see it, but it doesn't matter[[BR]][[BR]]

Yes, we need to set krbLoginFailedCount to 0. The KDC does this (see unlock_princ() in kadmin/cli/kadmin.c)

Thanks for that reference in the code, I was just looking for it.[[BR]][[BR]]

The aci should grant only write permissions and you'll need to add krbLoginFailedCount as well.

Yes, that's what I thought. I made another patch and all your ACI-related comments are taken care of. Will be sending it in couple minutes.

What remains though is the issue with krbLastAdminUnlock attribute. I was thinking that we even don't have to use this attribute - it is present because of LDAP replication when Kerberos is used with LDAP (some details can be found here), but from what I understand, we have much better replication mechanisms and replication of krbLoginFailedCount shouldn't be a problem. Am I correct?

Yeah, I goofed on the typo, seeing krblastadminunlock made my eyes cross :-)

I can see your point about krbLastAdminUnlock and our much faster replication but I think we should still set it. It at least reflects the fact that account was reset by an administrator.

Metadata Update from @rcritten:
- Issue assigned to jzeleny
- Issue set to the milestone: FreeIPA 2.0 - 2011/01 (cleanup)

7 years ago

Login to comment on this ticket.

Metadata