Does openldap support multiple simple paged results per connection?
How urgent is this?

For the record, there is a related ticket now open for SSSD: https://fedorahosted.org/sssd/ticket/1202

We are going to need to fix this ticket quickly to handle existing DS deployments, so I don't know if this impacts your prioritization of this ticket.

Replying to [comment:5 sgallagh]:

For the record, there is a related ticket now open for SSSD: https://fedorahosted.org/sssd/ticket/1202

We are going to need to fix this ticket quickly to handle existing DS deployments, so I don't know if this impacts your prioritization of this ticket.

Thanks for the link, Stephen. The milestone of this ticket is 1.2.11.a1. Hope it's okay for SSSD...

git patch file (master)
0001-Trac-Ticket-260-389-DS-does-not-support-multiple.patch

Fix description:
1. "Connection" object holds the paged results related values.
Now they are packaged in one PagedResults object. And the
array of PagedResults with its length and used count are
placed in PagedResultList, which is stashed in Connection
(slap.h).
2. The pagedresults APIs are extended to take the index of Paged-
Results array. The index is set to the cookie in the Paged-
Results control before returning it to the client. When a
client sends a paged results request, the cookie field in the
first request control is supposed to be empty. The result
control is returned with the search results. The result
control stores the index in the cookie and the client sets
it to the following request control to get the next pages.
When the paged search is done, empty cookie is returned.
3. The array grows if multiple simple paged results requests
over a single connection come in. The array does not get
released every time, but it is when the server is shutdown.
4. Simple paged results connection is timed out when it exeeds
the timelimit. If multiple requests are served, it won't
be disconnected until all the requests are timed out.

Nice! One thing - in pagedresults_parse_control_value since you now have the conn, you might want to check the given index against the max size of the prlist in the conn, to check against malicious input (e.g. someone sending a bogus index value trying to crash the server), and at least log an error. I don't know if we do that for other control values. I see that you have protected against a bogus index value everywhere it is used.

Replying to [comment:8 rmeggins]:

Nice! One thing - in pagedresults_parse_control_value since you now have the conn, you might want to check the given index against the max size of the prlist in the conn, to check against malicious input (e.g. someone sending a bogus index value trying to crash the server), and at least log an error. I don't know if we do that for other control values. I see that you have protected against a bogus index value everywhere it is used.

Thanks a lot, Rich. Yes, the check is really important for the security. I've added it. (The new patch is tested and being attached next.)

Additional fix description:
If a passed cookie is less than 0 or greater than the array
size and the request control is "critical", the request fails
with LDAP_PROTOCOL_ERROR and the error is logged in the error
log. If the control is not "critical", the paged results
request is disabled.

revised git patch file (master)
0001-Trac-Ticket-260-389-DS-does-not-support-multiple.2.patch

Reviewed by Rich (Thank you!!!)

Pushed to master.

$ git merge trac260
Updating 7028076..add880a
Fast-forward
ldap/servers/slapd/abandon.c | 11 +-
ldap/servers/slapd/back-ldbm/filterindex.c | 27 +-
ldap/servers/slapd/back-ldbm/ldbm_search.c | 29 +-
ldap/servers/slapd/back-ldbm/vlv.c | 4 +-
ldap/servers/slapd/connection.c | 8 +-
ldap/servers/slapd/daemon.c | 18 +-
ldap/servers/slapd/opshared.c | 83 +++--
ldap/servers/slapd/pagedresults.c | 528 +++++++++++++++++++++++----
ldap/servers/slapd/pblock.c | 12 +
ldap/servers/slapd/proto-slap.h | 52 ++-
ldap/servers/slapd/result.c | 3 +-
ldap/servers/slapd/slap.h | 30 ++-
ldap/servers/slapd/slapi-plugin.h | 3 +
ldap/servers/slapd/sort.c | 9 +-
14 files changed, 635 insertions(+), 182 deletions(-)

$ git push
Counting objects: 39, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (20/20), done.
Writing objects: 100% (20/20), 9.57 KiB, done.
Total 20 (delta 17), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
7028076..add880a master -> master

Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=830347

Added initial screened field value.

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/260

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.