Ticket was cloned from Red Hat Bugzilla (product Red Hat Enterprise Linux 6): Bug 1034265
Description of problem: 7-bit check plugin should deny all operations, that would change tracked attribute to value that is not 7-bit clean. However, MODRDN operation is not checked, which allows us to change the tracked attribute to value, that would normally be refused. Version-Release number of selected component (if applicable): 389-ds-base-1.2.11.15-29.el6.x86_64 How reproducible: always Steps to Reproduce: ldapmodify -h $HOST -p $PORT -D "cn=directory manager" -w Secret123 -a <<EOF dn: uid=tuser,ou=people,dc=example,dc=com objectclass: person objectclass: inetOrgPerson objectclass: top cn: tuser sn: tuser uid: tuser EOF ldapmodify -h $HOST -p $PORT -D "cn=directory manager" -w Secret123 <<EOF dn: cn=7-bit check,cn=plugins,cn=config changetype: modify replace: nsslapd-pluginEnabled nsslapd-pluginEnabled: on EOF sudo service dirsrv restart ldapmodify -h $HOST -p $PORT -D "cn=directory manager" -w Secret123 <<EOF dn: uid=tuser,ou=people,dc=example,dc=com changetype: modrdn newrdn: uid=tuser???? deleteoldrdn: true EOF ldapsearch -h $HOST -p $PORT -LLL -D "cn=directory manager" -w Secret123 -b "uid=tuser????,ou=people,dc=example,dc=com" dn:: dWlkPXR1c2VyxaHEvsSNxaUsb3U9UGVvcGxlLGRjPWV4YW1wbGUsZGM9Y29t objectClass: person objectClass: inetOrgPerson objectClass: top objectClass: organizationalPerson cn: tuser sn: tuser uid:: dHVzZXLFocS+xI3FpQ== ldapsearch -h $HOST -p $PORT -LLL -D "cn=directory manager" -w Secret123 -b "cn=7-bit check,cn=plugins,cn=config " dn: cn=7-bit check,cn=plugins,cn=config objectClass: top objectClass: nsSlapdPlugin objectClass: extensibleObject cn: 7-bit check nsslapd-pluginPath: libattr-unique-plugin nsslapd-pluginInitfunc: NS7bitAttr_Init nsslapd-pluginType: preoperation nsslapd-pluginEnabled: on nsslapd-pluginarg0: uid nsslapd-pluginarg1: mail nsslapd-pluginarg2: userpassword nsslapd-pluginarg3: , nsslapd-pluginarg4: dc=example,dc=com nsslapd-plugin-depends-on-type: database nsslapd-pluginId: NS7bitAttr nsslapd-pluginVersion: 1.2.11.15 nsslapd-pluginVendor: 389 Project nsslapd-pluginDescription: Enforce 7-bit clean attribute values Actual results: Uid attribute contains value which is not 7-bit clean. Uid attribute is tracked by 7-bit check plugin.
attachment 0001-Ticket-47641-7-bit-check-plugin-not-checking-MODRDN-.patch
Hi Mark, Sorry, I need your help to understand why your patch fixes the problem...
In preop_modrdn, superior is retrieved from pblock, which is NULL if newsuperior is not given in modrdn. In the case, target_sdn is set. {{{ "7bit.c" 543 preop_modrdn(Slapi_PBlock pb) 595 / Get superior value - unimplemented in 3.0 DS / 596 err = slapi_pblock_get(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, &superior); 597 if (err) { result = op_error(20); break; } 598 599 / 600 * No superior means the entry is just renamed at 601 * its current level in the tree. Use the target DN for 602 * determining which managed tree this belongs to 603 */ 604 if (!superior) superior = target_sdn; }}}
The newsuperior is set in do_modrdn. If newsuperior is not given in the modrdn operation, snewsuperior is NULL. So, preop_modrdn in 7bit.c is getting NULL at the line 596 above... {{{ 71 do_modrdn( Slapi_PBlock pb ) 253 slapi_pblock_set( pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, (void )snewsuperior ); }}}
Your patch looks there is a case newsuperior is give but it is empty? {{{ ldapmodify << EOF dn: uid=tuser,ou=people,dc=example,dc=com changetype: modrdn newrdn: uid=tuser<8bit> deleteoldrdn: true newsuperior: EOF }}}
But it's supposed to be rejected in do_modrdn... {{{ 184 if (rawnewsuperior) { 185 if (config_get_dn_validate_strict()) { 186 / check that the dn is formatted correctly / 187 err = slapi_dn_syntax_check(pb, rawnewsuperior, 1); 188 if (err) { / syntax check failed / 189 op_shared_log_error_access(pb, "MODRDN", rawnewsuperior, 190 "strict: invalid new superior"); 191 send_ldap_result(pb, LDAP_INVALID_DN_SYNTAX, 192 NULL, "invalid new superior", 0, NULL); 193 slapi_ch_free_string( &rawnewsuperior ); 194 goto free_and_return; 195 } 196 } 197 snewsuperior = slapi_sdn_new_dn_passin(rawnewsuperior); 198 newsuperior = slapi_sdn_get_dn(snewsuperior); 199 } }}}
I must be missing something...
Replying to [comment:5 nhosoi]:
Hi Mark, Sorry, I need your help to understand why your patch fixes the problem... In preop_modrdn, superior is retrieved from pblock, which is NULL if newsuperior is not given in modrdn.
In preop_modrdn, superior is retrieved from pblock, which is NULL if newsuperior is not given in modrdn.
That's what I would of thought, but it returns an empty Slapi_DN. Note I am doing:
dn: uid=tuser,ou=people,dc=example,dc=com changetype: modrdn newrdn: uid=tuser???? -> this is 8 bit characters deleteoldrdn: true
In the case, target_sdn is set. {{{ "7bit.c" 543 preop_modrdn(Slapi_PBlock pb) 595 / Get superior value - unimplemented in 3.0 DS / 596 err = slapi_pblock_get(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, &superior); 597 if (err) { result = op_error(20); break; } 598 599 / 600 * No superior means the entry is just renamed at 601 * its current level in the tree. Use the target DN for 602 * determining which managed tree this belongs to 603 */ 604 if (!superior) superior = target_sdn; }}} The newsuperior is set in do_modrdn. If newsuperior is not given in the modrdn operation, snewsuperior is NULL. So, preop_modrdn in 7bit.c is getting NULL at the line 596 above... {{{ 71 do_modrdn( Slapi_PBlock pb ) 253 slapi_pblock_set( pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, (void )snewsuperior ); }}} Your patch looks there is a case newsuperior is give but it is empty? {{{ ldapmodify << EOF dn: uid=tuser,ou=people,dc=example,dc=com changetype: modrdn newrdn: uid=tuser<8bit> deleteoldrdn: true newsuperior: EOF }}} But it's supposed to be rejected in do_modrdn... {{{ 184 if (rawnewsuperior) { 185 if (config_get_dn_validate_strict()) { 186 / check that the dn is formatted correctly / 187 err = slapi_dn_syntax_check(pb, rawnewsuperior, 1); 188 if (err) { / syntax check failed / 189 op_shared_log_error_access(pb, "MODRDN", rawnewsuperior, 190 "strict: invalid new superior"); 191 send_ldap_result(pb, LDAP_INVALID_DN_SYNTAX, 192 NULL, "invalid new superior", 0, NULL); 193 slapi_ch_free_string( &rawnewsuperior ); 194 goto free_and_return; 195 } 196 } 197 snewsuperior = slapi_sdn_new_dn_passin(rawnewsuperior); 198 newsuperior = slapi_sdn_get_dn(snewsuperior); 199 } }}} I must be missing something...
In the case, target_sdn is set. {{{ "7bit.c" 543 preop_modrdn(Slapi_PBlock pb) 595 / Get superior value - unimplemented in 3.0 DS / 596 err = slapi_pblock_get(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, &superior); 597 if (err) { result = op_error(20); break; } 598 599 / 600 * No superior means the entry is just renamed at 601 * its current level in the tree. Use the target DN for 602 * determining which managed tree this belongs to 603 */ 604 if (!superior) superior = target_sdn; }}}
Turns out the pblock value will never be null, as its actually a Slapi_DN struct, and not a pointer to one:
3017: case SLAPI_MODRDN_NEWSUPERIOR_SDN: if(pblock->pb_op!=NULL) { pblock->pb_op->o_params.p.p_modrdn.modrdn_newsuperior_address.sdn = (Slapi_DN *) value; }
So its either set or not set, but it's never NULL.
Aha, thank you so much for the clear answer, Mark!
git merge ticket47641 Updating 3fee1fc..ddbec8c Fast-forward ldap/servers/plugins/uiduniq/7bit.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
git push origin master 3fee1fc..ddbec8c master -> master
commit ddbec8c Author: Mark Reynolds mreynolds@redhat.com Date: Wed Jan 22 11:08:18 2014 -0500
1.3.2
fe75b11..628cb90 389-ds-base-1.3.2 -> 389-ds-base-1.3.2
1.3.1
aec2050..3cfd994 389-ds-base-1.3.1 -> 389-ds-base-1.3.1
1.3.0
b51a57b..a696448 389-ds-base-1.3.0 -> 389-ds-base-1.3.0
1.2.11
a3b6e22..7a5f2e7 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
Metadata Update from @mreynolds: - Issue assigned to mreynolds - Issue set to the milestone: 1.2.11.26
389-ds-base is moving from Pagure to Github. This means that new issues and pull requests will be accepted only in 389-ds-base's github repository.
This issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/978
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: Fixed)
Login to comment on this ticket.