#260 389 DS does not support multiple paging controls on a single connection
Closed: Fixed None Opened 7 years ago by sgallagh.

The SSSD maintains a single connection to the LDAP server. Each of our requests to LDAP uses a paging control to handle situations where the returned values exceed the single-request limit.

However, when we're under load and performing many concurrent requests, we see "Server is unwilling to perform(53), Simple Paged Results Search already in progress on this connection"

It appears that 389 DS does not maintain paging controls on a per-request basis.


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...

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.

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

Added initial screened field value.

Metadata Update from @sgallagh:
- Issue assigned to nhosoi
- Issue set to the milestone: 1.2.11.a1

2 years ago

Login to comment on this ticket.

Metadata