#48026 [Feature] Patch to support uniqueness being enforced over a set of attributes.
Closed: Fixed None Opened 4 years ago by firstyear.

In some directories, it is often seen that objects have a pattern such as:

dn: uid=foo
...
mail: primary@example.com
mailAlternateAddress: secondary@example.com

With the current uniqueness plugin, if a second account were configured such as:

dn: uid=bar
...
mail: secondary@example.com
mailAlternateAddress: primary@example.com

This would not be detected, as the mail attribute is still unique, as is mailAlternateAddress.

The attached patch provides support so that if multiple statements of uniqueness-attribute-name are given to the plugin, each of these will form a set that a value can only exist a single time within. This means that a value of mail could not be duplicated in mail or mailAlternateAddress throughout the directory.

This patch continues to support the legacy syntax format as a single value only, and a single value uniqueness check still forms an identical filter.

On error, the full set of potential violating attributes is returned such as:

additional info: Another entry with the same attribute value already exists (attribute: "mail mailAlternateAddress ")

At this time only a single loophole exists, which is that if you configure an object, it may in a single transaction take the same value twice, IE

dn: uid=foo
...
mail: primary@example.com
mailAlternateAddress: primary@example.com.

This should be resolved in the future, but this patch still represents a significant improvement.

Tests to lib389 still need to be made for it.


Thank you so much for providing us the patch for the new feature.

Let us review and test it.

Thanks!
--noriko

This is an updated version of that patch that fixes operation when an objectClass is set. Additionally, initial tests were added to plugin tests. They currently fail (Indicating there is either a problem with the tests, or the code), but it's a step forwards.

0001-Ticket-48026-Support-for-uniqueness-plugin-to-enforc.patch Fixes the issues highlighted by the tests. Please review.

Replying to [comment:3 firstyear]:

0001-Ticket-48026-Support-for-uniqueness-plugin-to-enforc.patch Fixes the issues highlighted by the tests. Please review.

May we ask to fix these?

  1. Two compiler warnings.
    {{{
    ldap/servers/plugins/uiduniq/uid.c:457:3: warning: statement with no effect [-Wunused-value]
    ldap/servers/plugins/uiduniq/uid.c:1230:9: warning: unused variable 'uniqueAttrCount' [-Wunused-variable]
    }}}
  2. valgrind reports a leak.
    {{{
    ==15698== 24 bytes in 3 blocks are definitely lost in loss record 317 of 1,385
    ==15698== at 0x4A081D4: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==15698== by 0x4C6475A: slapi_ch_calloc (ch_malloc.c:243)
    ==15698== by 0xEC91BC3: create_filter (uid.c:458)
    ==15698== by 0xEC9215D: search_one_berval (uid.c:651)
    ==15698== by 0xEC9206C: search (uid.c:603)
    ==15698== by 0xEC9246C: searchAllSubtrees (uid.c:776)
    ==15698== by 0xEC929E1: preop_add (uid.c:1000)
    ==15698== by 0x4CC5FF3: plugin_call_func (plugin.c:1949)
    ==15698== by 0x4CC5E54: plugin_call_list (plugin.c:1893)
    ==15698== by 0x4CC2CC7: plugin_call_plugins (plugin.c:467)
    ==15698== by 0x109DB032: ldbm_back_add (ldbm_add.c:859)
    ==15698== by 0x4C5958C: op_shared_add (add.c:740)
    ==15698== by 0x4C584DE: do_add (add.c:258)
    ==15698== by 0x120E67: connection_dispatch_operation (connection.c:645)
    ==15698== by 0x122DF2: connection_threadmain (connection.c:2534)
    ==15698== by 0x68EDE3A: _pt_root (ptthread.c:212)
    ==15698== by 0x6F2DEE4: start_thread (in /usr/lib64/libpthread-2.18.so)
    ==15698== by 0x7238B8C: clone (in /usr/lib64/libc-2.18.so)
    }}}

Replaced with new version that fixes compiler warnings and (should) fix for memory leak.

I'm afraid your change crashes the server.
{{{
==19161== Thread 20:
==19161== Invalid free() / delete / delete[] / realloc()
==19161== at 0x4A07577: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==19161== by 0x4C64A49: slapi_ch_free (ch_malloc.c:363)
==19161== by 0xEC91F79: create_filter (uid.c:548)
==19161== by 0xEC9216D: search_one_berval (uid.c:653)
==19161== by 0xEC9207C: search (uid.c:605)
==19161== by 0xEC9247C: searchAllSubtrees (uid.c:778)
==19161== by 0xEC929F1: preop_add (uid.c:1002)
==19161== by 0x4CC5FF3: plugin_call_func (plugin.c:1949)
==19161== by 0x4CC5E54: plugin_call_list (plugin.c:1893)
==19161== by 0x4CC2CC7: plugin_call_plugins (plugin.c:467)
==19161== by 0x109DB032: ldbm_back_add (ldbm_add.c:859)
==19161== by 0x4C5958C: op_shared_add (add.c:740)
==19161== by 0x4C584DE: do_add (add.c:258)
==19161== by 0x120E67: connection_dispatch_operation (connection.c:645)
==19161== by 0x122DF2: connection_threadmain (connection.c:2534)
==19161== by 0x68EDE3A: _pt_root (ptthread.c:212)
==19161== by 0x6F2DEE4: start_thread (in /usr/lib64/libpthread-2.18.so)
==19161== by 0x7238B8C: clone (in /usr/lib64/libc-2.18.so)
==19161== Address 0x1400000004 is not stack'd, malloc'd or (recently) free'd
}}}
For testing, I just added 2 entries that violates the uniqueness condition:
{{{
adding new entry uid=VChalker3339363799,ou=People,dc=example,dc=com
adding new entry uid=DDa3296903592,ou=People,dc=example,dc=com
ldap_add: Constraint violation
ldap_add: additional info: Another entry with the same attribute value already exists (attribute: "mail mailAlternateAddress ")
}}}
If you are interested in, please take a look at this section for running valgrind.
http://www.port389.org/docs/389ds/FAQ/faq.html#memory-growth

Please note that this is the prototype of slapi_ch_free.
{{{
void slapi_ch_free(void **ptr);
}}}

Thanks.

Fixed slapi_ch_free call, tested with valgrind and did not receive errors or crash.

Thank you for your contribution, William.

Pushed to master:
d33676f..430410d master -> master
commit 430410d
Author: William B william.e.brown@adelaide.edu.au
Date: Sun Feb 8 09:42:59 2015 +1030

I have reviewed this, and all tests still pass with the second patch.

Replying to [comment:11 firstyear]:

I have reviewed this, and all tests still pass with the second patch.

Thank you for reviewing and testing the patch 0001-Ticket-48026-Support-for-uniqueness-plugin-to-enforc.2.patch, William.

Pushed to master:
24c303d..1aeaf34 master -> master
commit 1aeaf34

I've verified the test successfully passed. Thanks a lot, William!

Attachment 0001-Tests-to-support-ticket-48026.patch​ added
Update of tests that now pass.

Pushed on behalf of William E Brown william.e.brown@adelaide.edu.au.
8309fd0..d61a073 master -> master
commit d61a073

There is still an invalid write caused by these patches. See https://fedorahosted.org/389/ticket/48167 I will close the above ticket as aduplicate of this ticket

{{{
==10418== Invalid write of size 2
==10418== at 0xEF64C0B: uniqueness_entry_to_config (uid.c:375)
==10418== by 0xEF667EB: uiduniq_start (uid.c:1383)
==10418== by 0x4EED8E0: plugin_call_func (plugin.c:1949)
==10418== by 0x4EED76E: plugin_call_one (plugin.c:1899)
==10418== by 0x4EEBE63: plugin_start (plugin.c:1282)
==10418== by 0x4EEFE64: plugin_add (plugin.c:3032)
==10418== by 0x4E9B73D: dse_add_plugin (dse.c:2182)
==10418== by 0x4E9C050: dse_add (dse.c:2403)
==10418== by 0x4E7ECB3: op_shared_add (add.c:740)
==10418== by 0x4E7DBC7: do_add (add.c:258)
==10418== by 0x4163EC: connection_dispatch_operation (connection.c:645)
==10418== by 0x4183BC: connection_threadmain (connection.c:2534)
==10418== by 0x6B1ACAA: ??? (in /usr/lib64/libnspr4.so)
==10418== by 0x7159529: start_thread (in /usr/lib64/libpthread-2.20.so)
==10418== by 0x746E77C: clone (in /usr/lib64/libc-2.20.so)
==10418== Address 0x141ddab0 is 16 bytes inside a block of size 17 alloc'd
==10418== at 0x4C2B946: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==10418== by 0x4E8A136: slapi_ch_calloc (ch_malloc.c:243)
==10418== by 0xEF64BA4: uniqueness_entry_to_config (uid.c:370)
==10418== by 0xEF667EB: uiduniq_start (uid.c:1383)
==10418== by 0x4EED8E0: plugin_call_func (plugin.c:1949)
==10418== by 0x4EED76E: plugin_call_one (plugin.c:1899)
==10418== by 0x4EEBE63: plugin_start (plugin.c:1282)
==10418== by 0x4EEFE64: plugin_add (plugin.c:3032)
==10418== by 0x4E9B73D: dse_add_plugin (dse.c:2182)
==10418== by 0x4E9C050: dse_add (dse.c:2403)
==10418== by 0x4E7ECB3: op_shared_add (add.c:740)
==10418== by 0x4E7DBC7: do_add (add.c:258)
==10418== by 0x4163EC: connection_dispatch_operation (connection.c:645)
==10418== by 0x4183BC: connection_threadmain (connection.c:2534)
==10418== by 0x6B1ACAA: ??? (in /usr/lib64/libnspr4.so)
==10418== by 0x7159529: start_thread (in /usr/lib64/libpthread-2.20.so)
==10418== by 0x746E77C: clone (in /usr/lib64/libc-2.20.so)
}}}

1bf67a4..48a9d16 master -> master
commit 48a9d16
Author: Mark Reynolds mreynolds@redhat.com
Date: Tue May 12 10:43:26 2015 -0400

memory leak patch coming next...

48a9d16..c9220fe master -> master
commit c9220fe
Author: Mark Reynolds mreynolds@redhat.com
Date: Tue May 12 11:47:52 2015 -0400

My tests show this as working. Thanks for your time in picking these up!

Metadata Update from @nhosoi:
- Issue set to the milestone: 1.3.4 backlog

2 years ago

Login to comment on this ticket.

Metadata