TL;DR: OpenBSD's ldapd(8) has issues when deleting individual attribute
values. Patch below.
ZHANG Huangbin reported a misbehaviour in ldapd(8)'s MOD_DELETE
operation when connecting to ldapd(8) with the python-ldap library.
In ldapd(8) The MOD_DELETE operation always deletes all values of an
attribute and not only those specified in the request. (Mails from
Zhang Huangbin to bugs@ on May 18, 2016 and December 30, 2016).
I reproduced this issue connecting to ldapd(8) with the openLDAP client
tools (instead of the pyton-ldap library).
To illustrate the issue, lets take this LDAP entry (take note of the
"memberUID" attribute and its values):
dn: cn=detectives,ou=Group,dc=example,dc=org
objectClass: posixGroup
cn: detectives
gidNumber: 1012
memberUID: dasinger
memberUID: wergard
memberUID: gems
memberUID: amberdon
description: Detectives of the Kyth Interstaller Detective Agency
To delete the memberUID value of "amberdon" from this entry you submit
the following LDIF to the ldapd server:
dn: cn=detectives,ou=group,dc=example,dc=org
changeType: modify
delete: memberUid
memberUid: amberdon
I'm using the openLDAP command line tool "ldapmodify" for this. The
LDIF above is the contents of a file "del_amberdon.ldif":
ldapmodify -x -h $HOST -p 389 -D $BINDDN -w $PASSWD del_amberdon.ldif
The expected result would be a "detectives" group of:
dn: cn=detectives,ou=Group,dc=example,dc=org
objectClass: posixGroup
cn: detectives
gidNumber: 1012
memberUID: dasinger
memberUID: wergard
memberUID: gems
description: Detectives of the Kyth Interstaller Detective Agency
However, ldapd(8) now has removed all values for the "memberUID"
attribute (in LDAP parlance "the entire attribute is removed") and you
get the following entry::
dn: cn=detectives,ou=Group,dc=example,dc=org
objectClass: posixGroup
cn: detectives
gidNumber: 1012
description: Detectives of the Kyth Interstaller Detective Agency
Looking at the source, I found these issues (suggested fixes in
parentheses, tentative patch attached):
- in modify.c:ldap_modify(), lines 298 ff., in case LDAP_MOD_DELETE
there was a check for BER_TYPE_SET, however
1. AttributeValues are always in a set, even if it is empty
(PartialAttribute, see RFC4511, Section 4.1.7), so that check
couldn't have worked, even if the right variable had been checked
--- see next point.
2. The `vals' variable has a value of SET, however the variable
checked, `vals->be_sup' is already an element of the set, that is,
either it has a type of EOC (when there are no attribute values),
or it has a type of OCTETSTRING and contains the first attribute
value. (Look for a type of BER_TYPE_OCTETSTRING instead).
- in attributes.c:ldap_del_values(), lines 222 ff.
1. the elements inspected (variables `vk' and `xk') are not those
containing the attribute values; the attribute values are in `v'
and `x', `xk' and `vk' are (probably) uninitialized. (Use `v' and
`x' instead.)
2. When freeing the element found, current `v' is freed, and
`v->be_next' has no meaning anymore. (Use `next' variable to save
the pointer.)
3. Setting `prev' to `v' is wrong when an element has been
removed. (Set a flag if element is removed and re-set `prev' only
if the flag isn't set.)
- in ber.c:ber_free_elements() the current and all following elements
are freed. (Add ber_free_element() which frees only the current
element and use this function in attributes.c:ldap_del_values().)
Index: attributes.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldapd/attributes.c,v
retrieving revision 1.4
diff -u -p -r1.4 attributes.c
--- attributes.c 20 Jan 2017 11:55:08 -0000 1.4
+++ attributes.c 1 Feb 2017 14:34:42 -0000
@@ -207,9 +207,9 @@ int
ldap_del_values(struct ber_element *elm, struct ber_element *vals)
{
char *attr;
- struct ber_element *old_vals, *v, *x, *vk, *xk, *prev;
+ struct ber_element *old_vals, *v, *x, *prev, *next;
struct ber_element *removed;
-
+ int removed_p;
assert(elm);
assert(vals);
assert(vals->be_sub);
@@ -220,19 +220,25 @@ ldap_del_values(struct ber_element *elm,
}
prev = old_vals;
- for (v = old_vals->be_sub; v; v = v->be_next) {
- vk = v->be_sub;
+ removed_p = 0;
+ for (v = old_vals->be_sub; v; v = next) {
+ next = v->be_next;
+
for (x = vals->be_sub; x; x = x->be_next) {
- xk = x->be_sub;
- if (xk && vk->be_len == xk->be_len &&
- memcmp(vk->be_val, xk->be_val, xk->be_len) == 0) {
+ if (x && v->be_len == x->be_len &&
+ memcmp(v->be_val, x->be_val, x->be_len) == 0) {
removed = ber_unlink_elements(prev);
ber_link_elements(prev, removed->be_next);
- ber_free_elements(removed);
+ ber_free_element(removed);
+ removed_p = 1;
break;
}
}
- prev = v;
+ if (removed_p) {
+ removed_p = 0;
+ } else {
+ prev = v;
+ }
}
return 0;
Index: ber.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldapd/ber.c,v
retrieving revision 1.11
diff -u -p -r1.11 ber.c
--- ber.c 24 Dec 2015 17:47:57 -0000 1.11
+++ ber.c 1 Feb 2017 14:34:42 -0000
@@ -826,6 +826,19 @@ ber_read_elements(struct ber *ber, struc
}
void
+ber_free_element(struct ber_element *root)
+{
+ if (root->be_sub && (root->be_encoding == BER_TYPE_SEQUENCE ||
+ root->be_encoding == BER_TYPE_SET))
+ ber_free_elements(root->be_sub);
+ if (root->be_free && (root->be_encoding == BER_TYPE_OCTETSTRING ||
+ root->be_encoding == BER_TYPE_BITSTRING ||
+ root->be_encoding == BER_TYPE_OBJECT))
+ free(root->be_val);
+ free(root);
+}
+
+void
ber_free_elements(struct ber_element *root)
{
if (root->be_sub && (root->be_encoding == BER_TYPE_SEQUENCE ||
Index: ber.h
===================================================================
RCS file: /cvs/src/usr.sbin/ldapd/ber.h,v
retrieving revision 1.1
diff -u -p -r1.1 ber.h
--- ber.h 31 May 2010 17:36:31 -0000 1.1
+++ ber.h 1 Feb 2017 14:34:43 -0000
@@ -120,6 +120,7 @@ ssize_t ber_get_writebuf(struct ber *
int ber_write_elements(struct ber *, struct ber_element *);
void ber_set_readbuf(struct ber *, void *, size_t);
struct ber_element *ber_read_elements(struct ber *, struct ber_element *);
+void ber_free_element(struct ber_element *);
void ber_free_elements(struct ber_element *);
size_t ber_calc_len(struct ber_element *);
void ber_set_application(struct ber *,
Index: modify.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldapd/modify.c,v
retrieving revision 1.18
diff -u -p -r1.18 modify.c
--- modify.c 20 Jan 2017 11:55:08 -0000 1.18
+++ modify.c 1 Feb 2017 14:34:43 -0000
@@ -296,11 +296,19 @@ ldap_modify(struct request *req)
}
break;
case LDAP_MOD_DELETE:
+ /* we're already in the "SET OF value
+ AttributeValue" (see RFC2411 section 4.1.7)
+ have either EOC, so all values for the
+ attribute gets deleted, or we have a
+ (first) octetstring (there is one for each
+ AttributeValue to be deleted) */
if (vals->be_sub &&
- vals->be_sub->be_type == BER_TYPE_SET)
+ vals->be_sub->be_type == BER_TYPE_OCTETSTRING) {
ldap_del_values(a, vals);
- else
+ }
+ else {
ldap_del_attribute(entry, attr);
+ }
break;
case LDAP_MOD_REPLACE:
if (vals->be_sub != NULL &&
Best Regards
Robert
