7-bit check plugin not checking MODRDN operation #978

389-ds-bot · 2020-09-12T22:10:23Z

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/47641

Created at 2013-12-18 23:23:45 by nkinder (@nkinder)
Closed as Fixed
Assigned to mreynolds (@mreynolds389)
Associated bugzillas
- https://bugzilla.redhat.com/show_bug.cgi?id=1034265
- https://bugzilla.redhat.com/show_bug.cgi?id=1048980

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.

The text was updated successfully, but these errors were encountered:

389-ds-bot · 2020-09-12T22:10:25Z

Comment from mreynolds (@mreynolds389) at 2014-01-22 22:15:08

attachment
0001-Ticket-47641-7-bit-check-plugin-not-checking-MODRDN-.patch

389-ds-bot · 2020-09-12T22:10:26Z

Comment from nhosoi (@nhosoi) at 2014-01-23 04:07:24

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

389-ds-bot · 2020-09-12T22:10:27Z

Comment from mreynolds (@mreynolds389) at 2014-01-23 20:35:42

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.

389-ds-bot · 2020-09-12T22:10:29Z

Comment from nhosoi (@nhosoi) at 2014-01-23 23:21:32

Aha, thank you so much for the clear answer, Mark!

389-ds-bot · 2020-09-12T22:10:30Z

Comment from mreynolds (@mreynolds389) at 2014-01-24 02:00:13

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 mreynolds389@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

389-ds-bot · 2020-09-12T22:10:30Z

Comment from mreynolds (@mreynolds389) at 2017-02-11 23:07:45

Metadata Update from @mreynolds389:

Issue assigned to mreynolds389
Issue set to the milestone: 1.2.11.26

389-ds-bot added the closed: fixed Migration flag - Issue label Sep 12, 2020

389-ds-bot added this to the 1.2.11.26 milestone Sep 12, 2020

389-ds-bot closed this as completed Sep 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

7-bit check plugin not checking MODRDN operation #978

7-bit check plugin not checking MODRDN operation #978

389-ds-bot commented Sep 12, 2020

389-ds-bot commented Sep 12, 2020

389-ds-bot commented Sep 12, 2020

389-ds-bot commented Sep 12, 2020

389-ds-bot commented Sep 12, 2020

389-ds-bot commented Sep 12, 2020

389-ds-bot commented Sep 12, 2020

7-bit check plugin not checking MODRDN operation #978

7-bit check plugin not checking MODRDN operation #978

Comments

389-ds-bot commented Sep 12, 2020

389-ds-bot commented Sep 12, 2020

389-ds-bot commented Sep 12, 2020

389-ds-bot commented Sep 12, 2020

389-ds-bot commented Sep 12, 2020

389-ds-bot commented Sep 12, 2020

389-ds-bot commented Sep 12, 2020