On Sun, Dec 19, 2021 at 01:31:24PM +0100, Claudio Jeker wrote:
> In LDAP there is two ways to remove an attribute.
> One can remove an attribute by just naming the attribute but it is also
> possible to remove a specific attribute: value combo.
>
> In ldapd the latter is broken if the last attribute is removed because
> the result of ldap_del_values() is an invalid encoding (empty sequence)
> and with that the modification fails because validate_entry() fails.
> The error is LDAP_INVALID_SYNTAX and I have noticed that in tools like
> shelldap multiple times but never really connected the dots until now.
>
> This is the minimal way of solving this. If ldap_del_values()
> removes the last element use ldap_del_attribute() to remove the attribute
> but to make this work the ober_scanf_elements() format has to be relaxed
> since what we remove no longer parses with "{s(". Is this an acceptable
> solution?
I think so, ldapd doesn't seem to check the attribute values consistently
except where it actually has to look at them, so relaxing the check here
seems fine to me.
one note below, but ok jmatthew@
>
> --
> :wq Claudio
>
> Index: attributes.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldapd/attributes.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 attributes.c
> --- attributes.c 24 Oct 2019 12:39:26 -0000 1.6
> +++ attributes.c 19 Dec 2021 12:12:48 -0000
> @@ -181,7 +181,7 @@ ldap_del_attribute(struct ber_element *e
>
> attr = entry->be_sub;
> while (attr) {
> - if (ober_scanf_elements(attr, "{s(", &s) != 0) {
> + if (ober_scanf_elements(attr, "{s", &s) != 0) {
> log_warnx("failed to parse attribute");
> return -1;
> }
> @@ -240,6 +240,9 @@ ldap_del_values(struct ber_element *elm,
> prev = v;
> }
> }
> +
> + if (old_vals->be_sub == NULL)
> + return 1;
>
> return 0;
> }
> Index: modify.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldapd/modify.c,v
> retrieving revision 1.23
> diff -u -p -r1.23 modify.c
> --- modify.c 24 Oct 2019 12:39:26 -0000 1.23
> +++ modify.c 19 Dec 2021 12:20:19 -0000
> @@ -334,7 +334,8 @@ ldap_modify(struct request *req)
> */
> if (vals->be_sub &&
> vals->be_sub->be_type == BER_TYPE_OCTETSTRING) {
> - ldap_del_values(a, vals);
> + if (ldap_del_values(a, vals) == 1)
> + ldap_del_attribute(entry, attr);
> } else {
> ldap_del_attribute(entry, attr);
> }
> Index: validate.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldapd/validate.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 validate.c
> --- validate.c 24 Oct 2019 12:39:26 -0000 1.12
> +++ validate.c 19 Dec 2021 11:42:48 -0000
> @@ -313,6 +313,7 @@ validate_entry(const char *dn, struct be
> objclass = objclass->be_next; /* skip attribute description */
> for (a = objclass->be_sub; a != NULL; a = a->be_next) {
> if (ober_get_string(a, &s) != 0) {
> + log_debug("bad ObjectClass encoding");
> rc = LDAP_INVALID_SYNTAX;
> goto done;
> }
> @@ -396,6 +397,7 @@ validate_entry(const char *dn, struct be
> */
> for (a = entry->be_sub; a != NULL; a = a->be_next) {
> if (ober_scanf_elements(a, "{se{", &s, &vals) != 0) {
> + log_debug("bad attribue encoding");
misspelled 'attribute' here.
> rc = LDAP_INVALID_SYNTAX;
> goto done;
> }
>
