Full_Name: Tim Bishop Version: 2.4.42 OS: Ubuntu 16.04 URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (2001:630:340:1141::13)
Whilst this issue was discovered on OpenLDAP 2.4.42, the code is unchanged on the latest version, so I believe it is still applicable there.
The high-level overview of the problem is that when using the ppolicy overlay with an external pwdCheckModule, and a password is changed using the LDAPv3 Password Modify (RFC 3062) extended operation (ie. ldappasswd), the message returned by the pwdCheckModule is not passed back to the user. All other failure messages generated in ppolicy are returned correctly.
I've looked into this and discovered the cause of the problem. We can start by looking at where the message is returned:
https://github.com/openldap/openldap/blob/OPENLDAP_REL_ENG_2_4_42/servers/sl...
The check_password_quality function calls out to the pwdCheckModule which sets txt to point at a malloc'd string. Here we set rs->sr_text to point to that, and make a note to free it later. This is the exception, in all other cases, as can be seen a few lines below, rs->sr_text is set to a static string.
Later on, the result is returned to the user:
https://github.com/openldap/openldap/blob/OPENLDAP_REL_ENG_2_4_42/servers/sl...
And then after that, it is free'd.
This all works fine for a normal password modify operation. The problem occurs when we wrap this in an extended password modify:
https://github.com/openldap/openldap/blob/OPENLDAP_REL_ENG_2_4_42/servers/sl...
This line is where the call into ppolicy originates (with a few steps in between), and then on line 237 below we see a call to send_ldap_extended (which returns rs->sr_text to the user) that includes a pointer to rs. In cases where rs->sr_text is a static string this is fine, but where we've set it to a response from pwdCheckModule it has at this point been free'd and set to NULL before send_ldap_extended gets a chance to send it.
The result is that confusingly the response from pwdCheckModule is returned for non-extended password changes, but not for extended ones.
I don't have a fix for this since I'm not familiar enough with the slapd code. But I'd imagine it'd require setting a flag either in rs or op to note when free'ing of rs->sr_text is required, and then move that free call as far out as necessary (back to where rs is created in connection.c?) so that rs->sr_text is always available.