On Tue, May 15, 2018 at 02:10:47PM +0200, Christophe Simon wrote:
> >Synopsis: The search time/size limit bounds seem badly evaluated in ldapd
> >Category: system
> >Environment:
> System : OpenBSD 6.3
> Details : OpenBSD 6.3 (GENERIC.MP) #2: Sun May 6 20:07:33 CEST 2018
>
> [email protected]:/usr/src/sys/arch/amd64/compile/GENERIC.MP
>
> Architecture: OpenBSD.amd64
> Machine : amd64
> >Description:
> The client search request time and size limits in ldapd seem to be badly
> evaluated. An error is risen when the time or the size limit is reached:
>
>
> * Without size limit
>
> $ ldapsearch -x -D xxx -W "cn=Christophe Simon" dn
> # extended LDIF
> #
> # LDAPv3
> # base <dc=pipeless,dc=org> (default) with scope subtree
> # filter: cn=Christophe Simon
> # requesting: dn
> #
>
> # christophe simon, users, pipeless.org
> dn: cn=christophe simon,xxx
>
> # search result
> search: 2
> result: 0 Success
>
> # numResponses: 2
> # numEntries: 1
>
>
> * With size limit
>
> $ ldapsearch -x -D xxx -W -z 1 "cn=Christophe Simon" dn
> # extended LDIF
> #
> # LDAPv3
> # base <dc=pipeless,dc=org> (default) with scope subtree
> # filter: cn=Christophe Simon
> # requesting: dn
> #
>
> # christophe simon, users, pipeless.org
> dn: cn=christophe simon,ou=users,dc=pipeless,dc=org
>
> # search result
> search: 2
> result: 4 Size limit exceeded
>
> # numResponses: 2
> # numEntries: 1
>
>
> The same query ran against OpenLDAP returns the correct response code.
>
> RFC 4511 in section A.2 tells that this error should be risen if the
> limit is exceeded (not only reached):
>
> timeLimitExceeded (3)
> Indicates that the time limit specified by the client was
> exceeded before the operation could be completed.
>
> sizeLimitExceeded (4)
> Indicates that the size limit specified by the client was
> exceeded before the operation could be completed.
>
> This breaks software such as Cyrus saslauthd that specify LDAP search
> limit to 1 when trying to find an user using the ldap plugin.
> >How-To-Repeat:
> Do an LDAP search request with a size limit equal to the number of
> matching entries.
> >Fix:
> Here's a patch fixing the entries/limit time/limit comparison.
>
> Is it OK for you, or is there a particular reason explaining the
> current behaviour ?
>
Makes sense, the logic and diff look OK.
(the affected lines are from initial import version 1.1 so it is very
likely that it was not intended this way)
Reyk
>
> Index: usr.sbin/ldapd/search.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldapd/search.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 search.c
> --- usr.sbin/ldapd/search.c 20 Jan 2017 11:55:08 -0000 1.18
> +++ usr.sbin/ldapd/search.c 15 May 2018 11:12:47 -0000
> @@ -437,8 +437,8 @@ conn_search(struct search *search)
>
> /* Check if we have passed the size limit. */
> if (rc == BT_SUCCESS && search->szlim > 0 &&
> - search->nmatched >= search->szlim) {
> - log_debug("search %d/%lld has reached size limit
> (%u)",
> + search->nmatched > search->szlim) {
> + log_debug("search %d/%lld has exceeded size limit
> (%u)",
> search->conn->fd, search->req->msgid,
> search->szlim);
> reason = LDAP_SIZELIMIT_EXCEEDED;
> @@ -449,8 +449,8 @@ conn_search(struct search *search)
> /* Check if we have passed the time limit. */
> now = time(0);
> if (rc == 0 && search->tmlim > 0 &&
> - search->started_at + search->tmlim <= now) {
> - log_debug("search %d/%lld has reached time limit (%u)",
> + search->started_at + search->tmlim < now) {
> + log_debug("search %d/%lld has exceeded time limit (%u)",
> search->conn->fd, search->req->msgid,
> search->tmlim);
> reason = LDAP_TIMELIMIT_EXCEEDED;
>
--
