#47641 7-bit check plugin not checking MODRDN operation
Closed: wontfix None Opened 10 years ago by nkinder.

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.

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.

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...

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

7 years ago

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.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

3 years ago

Login to comment on this ticket.

Metadata