ldap: add 'exop_force' value for ldap_pwmodify_mode

[sumit-bose]

In case the LDAP server allows to run the extended operation to change a
password even if an authenticated bind fails due to missing grace logins
the new option 'exop_force' can be used to run the extended operation to
change the password anyways.

[justin-stephenson]

Sorry for the ignorant question, but how does this differ from SDAP_PPOLICY_PWD_CHANGE_THRESHOLD in https://github.com/SSSD/sssd/blob/master/src/providers/ldap/ldap_auth.c#L243-L249 ?

[sumit-bose]

Sorry for the ignorant question, but how does this differ from SDAP_PPOLICY_PWD_CHANGE_THRESHOLD in https://github.com/SSSD/sssd/blob/master/src/providers/ldap/ldap_auth.c#L243-L249 ?

Hi,

it is related but not the same. The threshold value should make sure that there are a sufficient number of grace logins left to properly change the password. Due to the nature of the two-step PAM flow for password changes and that we do not keep the LDAP connection open between the two steps, a password change requires multiple binds/grace logins. This option is currently only evaluated in the code path where we explicitly check as part of the access control if the password is about to expired, e.g. when using an ssh key for authentication.

The current change affects the code path where we already get the information that the password is expired during authentication from the LDAP server, i.e. when authenticating with a password with auth_provider = ldap. In general checking the threshold value might be a good idea in this code path as well but I didn't want to add this here since the patch will most probably be backported and the option is a new feature. Additionally the check for the threshold wouldn't have helped here because there are no grace logins configured on the server side. So the password is expired without grace logins left. 389ds does not let the user change the password at this state since it requires that the passowrd change extended operation is send after an authenticated bind and since no grace logins are left this is not possible and an admin has to reset the password. It looks like OpenLDAP allows to run the password change exop under certain conditions (so far I wasn't able to figure out the exact details) without an authenticated bind. So even if the bind fails it would be ok for SSSD to continue and send the password change exop to change the password and the password change would be successful on the LDAP server side. This is what the new exop_force value is about.

HTH

bye,
Sumit

[justin-stephenson]

Sorry for the ignorant question, but how does this differ from SDAP_PPOLICY_PWD_CHANGE_THRESHOLD in https://github.com/SSSD/sssd/blob/master/src/providers/ldap/ldap_auth.c#L243-L249 ?

Hi,

it is related but not the same. The threshold value should make sure that there are a sufficient number of grace logins left to properly change the password. Due to the nature of the two-step PAM flow for password changes and that we do not keep the LDAP connection open between the two steps, a password change requires multiple binds/grace logins. This option is currently only evaluated in the code path where we explicitly check as part of the access control if the password is about to expired, e.g. when using an ssh key for authentication.

The current change affects the code path where we already get the information that the password is expired during authentication from the LDAP server, i.e. when authenticating with a password with auth_provider = ldap. In general checking the threshold value might be a good idea in this code path as well but I didn't want to add this here since the patch will most probably be backported and the option is a new feature. Additionally the check for the threshold wouldn't have helped here because there are no grace logins configured on the server side. So the password is expired without grace logins left. 389ds does not let the user change the password at this state since it requires that the passowrd change extended operation is send after an authenticated bind and since no grace logins are left this is not possible and an admin has to reset the password. It looks like OpenLDAP allows to run the password change exop under certain conditions (so far I wasn't able to figure out the exact details) without an authenticated bind. So even if the bind fails it would be ok for SSSD to continue and send the password change exop to change the password and the password change would be successful on the LDAP server side. This is what the new exop_force value is about.

HTH

bye, Sumit

Thank you for the explanation, it makes sense to me now. Ack.

[pbrezina]

Hi Sumit, would it be possible for you to write a system test? There are already few in test_ldap.py that tests "exop".

[sumit-bose]

Hi Sumit, would it be possible for you to write a system test? There are already few in test_ldap.py that tests "exop".

Hi,

sure, I can add exop_force to the parameter list of the existing tests or are you thinking of something different?

bye,
Sumit

[pbrezina]

That's one option, but that won't hit "grace logins exhausted", right? It might be better to add a new test for this case.

[sumit-bose]

That's one option, but that won't hit "grace logins exhausted", right? It might be better to add a new test for this case.

I added a test, please check if I followed all rules and used proper meta-data.

[pbrezina]

I'm sorry, I missed that this is only for OpenLDAP. We could add OpenLDAP instance to the containers and new topology that would run tests against it, do you think it is worth it? Otherwise, we can just drop this test, I suppose.

[sumit-bose]

I'm sorry, I missed that this is only for OpenLDAP. We could add OpenLDAP instance to the containers and new topology that would run tests against it, do you think it is worth it? Otherwise, we can just drop this test, I suppose.

Hi,

so far (after spending quite some time to find the right settings) I'm not able to reproduce the issue with OpenLDAP but I got logs which shows that OpenLDAP can be configured to act in this way (allow password change exop if no grace logins left and bind failed). Having a test which shows that SSSD will ask for a new password with exop_force is imo sufficient to test the SSSD code even if the actual change will fail.

So I think there is no need so far to add an OpenLDAP instance and if you agree we can keep the test.

bye,
Sumit

[pbrezina]

Hi, ok, let's keep the test. But see some suggestions inline.

[pbrezina]

Thank you, this looks great.

Could you run black . and fix pep8 errors?

./tests/test_ldap.py:458:1: E302 expected 2 blank lines, found 1
./tests/test_ldap.py:461:25: E128 continuation line under-indented for visual indent

black is an autoformatter, that will format the code using pep8 rules.

[sumit-bose]

Thank you, this looks great.

Could you run black . and fix pep8 errors?

./tests/test_ldap.py:458:1: E302 expected 2 blank lines, found 1
./tests/test_ldap.py:461:25: E128 continuation line under-indented for visual indent

black is an autoformatter, that will format the code using pep8 rules.

Hi,

new version includes formatted code.

bye,
Sumit

[pbrezina]

Ack. Thank you for working on the test.

I have two minor comments, but you can ignore them if you want to. Let me know, I'll approve afterwards.

[alexey-tikhonov]

@sumit-bose, could you please add :config: release note?

[alexey-tikhonov]

Pushed PR: #7614

• master
  • deefe9a - tests: add 'expo_force' tests
  • 7184541 - ldap: add 'exop_force' value for ldap_pwmodify_mode
• sssd-2-10
  • e609bb6 - tests: add 'expo_force' tests
  • d523261 - ldap: add 'exop_force' value for ldap_pwmodify_mode

[alexey-tikhonov]

@sumit-bose,

Conflict in sssd-2-9

so please open explicit backport PR if C9S needs this.

I didn't check, maybe conflict was in the test only.

[sumit-bose]

@sumit-bose,

Conflict in sssd-2-9

so please open explicit backport PR if C9S needs this.

I didn't check, maybe conflict was in the test only.

Hi,

no. it is mainly about missing ldap_use_ppolicy related patches. I opened #7645 which just the exop_force backport. If you think ldap_use_ppolicy should be backport as well I can add this as well.

bye,
Sumit