Strange, ticket 47702 apparently does not exist - not sure how that is possible. Anyway, continuing investigation.

William wrote:

Why was the search limit added initially into the aci plugin? By deleting this
code, is there some other edge case that it may cause?

Reply:

It was old code, and it was probably there to keep aci evaluation from taking too long. However, acis should always be fully evaluated.

William wrote:

However, this test is clearly broken, as it still works even without your patch applied.

Reply:

Are you binding as Directory Manager? The "Directory Manager" is not processed by acls. You should binding as a "regular user" from the database if you want to test this fix.

Verification Steps:

[1] Add 10000 users (with passwords)
[2] Add these users to a single group
[3] Add an aci that allows anyone from that group to have all rights to the database.
[4] Bind as the last user listed in the group, and try and add a new entry
[5] If should succeed with the fix, and not without it.

My test sets the sizelimit low (set to N), creates N+1 users, then re-binds with a user and sees if the aci works.

I think the issue is that I'm not testing as the last user in the group. I'll rework the test.

I've attached the work in progress to the ticket.

example test
ticket47703_test.py

Still can't produce this error. I have attached a tweaked version of the test that attempts to bind every possible user to try and fail the aci. I've now also tested with:

MAX_USERS = 10000
SIZELIMIT = '2000'

Is there some specifics around the aci needed to cause the failure or is:

(targetattr ="uniqueMember")(targetfilter ="(cn=target)")(version 3.0;acl "Test ACI";allow (write)(groupdn = "ldap:///cn=testgroup,dc=example,dc=com");)

Followed by a test user in testgroup, writing to cn=target sufficient?

Yeah I'm not sure how to reproduce this. I've cc'ed Ludwig who worked on this initially and discovered the problem.

Ludwig, how do you reproduce this problem? Seems to work fine without any changes to the acl code.

Thanks,
Mark

The patch 0001-Ticket-47703-remove-search-limit-for-aci-group-evalu.patch​ looks good to me.

I'm confused at this note:

This is the more general solution for ticket#47702.

Was #47702 removed? If I try to open the ticket, I get:
Ticket 47702 does not exist.

If "47702" is a typo of some other ticket, was the original issue already fixed by that and that's why we could not reproduce the problem???

Replying to [comment:12 nhosoi]:

The patch 0001-Ticket-47703-remove-search-limit-for-aci-group-evalu.patch​ looks good to me.

I'm confused at this note:

This is the more general solution for ticket#47702.

Was #47702 removed? If I try to open the ticket, I get:
Ticket 47702 does not exist.

I know and I can't find it either. Ludwig said it was deleted somehow. There was a bugzilla about this on the IPA side, but I can't find it.

If "47702" is a typo of some other ticket, was the original issue already fixed by that and that's why we could not reproduce the problem???

Hi Mark, Hi Ludwig,

Do you think the patch 0001-Ticket-47703-remove-search-limit-for-aci-group-evalu.patch should not be pushed until the bug is successfully reproduced?

If so, can we push this bug to 1.3.5?

Thanks,
--noriko

I will look into it and try to reproduce

Ludwig told me that this only applies to search operations, not "modify" operations. I had previously only tested delete operations. I will be testing searches later today...

Replying to [comment:16 mreynolds]:

Ludwig told me that this only applies to search operations, not "modify" operations. I had previously only tested delete operations. I will be testing searches later today...

I still can not reprocuce the problem using searches(or modifies). Here is exactly what I did:

• Set the size limit to 100
• Added a group wih 50k members
• Created a single aci that allows anyone from that group full access
• Bound as the first member of the group and was able to search for a user
• Bound as the last member of the group and was able to search for a user

Ludwig can you look into this and see if you can figure out how to reproduce the issue?

Per triage, push the target milestone to 1.3.6.

There was an error case reported in which sizelimit was set to 1 by some application.
I think we should push Ludwig's patch to the master and 1.3.5.

Replying to [comment:20 nhosoi]:

There was an error case reported in which sizelimit was set to 1 by some application.
I think we should push Ludwig's patch to the master and 1.3.5.

Its my patch, but I couldn't reproduce the issue. So I can't verify if the fix did anything. This simply needs to be revisited to see if there is an issue.

Note: a customer is hoping to have this fix backported to 1.2.11 if it solves their problem.

Input from Hiroko-san.

On 10/13/2016 10:35 AM, Hiroko Miura wrote:

Anyway, I could reproduce the similar issue when client specify sizelimit=1.

Here is my sample ldif/operation log/ access &errors logs.

http://mocha.nrt.redhat.com/dav-readonly/temp/hmiura/case01707132/example.ldif

http://mocha.nrt.redhat.com/dav-readonly/temp/hmiura/case01707132/operation.txt

http://mocha.nrt.redhat.com/dav-readonly/temp/hmiura/case01707132/access

http://mocha.nrt.redhat.com/dav-readonly/temp/hmiura/case01707132/errors

This happens just after restarting server.
It does not happen once client perform search without sizelimit.
I'm not sure if I could reproduce the customer's issue exactly.

But anyway, if client specify too small sizelimit in search operation
like this test case, search result is not correct.
Therefore, I think it would be better to set
aclpb_max_member_sizelimit to more safe value
like SLAPD_DEFAULT_LOOKTHROUGHLIMIT in acl_init_aclpb().

attachment
0001-Ticket-47703-remove-search-limit-for-aci-group-evalu.patch

1e1f6fe..3151648 master -> master
commit 3151648
Author: Mark Reynolds mreynolds@redhat.com
Date: Wed Oct 19 15:50:15 2016 -0400

Set the target milestone to 1.2.11.x.

Mark, could it be possible to backport to the branch?

1.3.5 is likely to have the fix, too.

9982033..99a34b4 389-ds-base-1.3.5 -> 389-ds-base-1.3.5
commit 99a34b4
Author: Mark Reynolds mreynolds@redhat.com
Date: Wed Oct 19 15:50:15 2016 -0400

48ed4c8..68cc036 389-ds-base-1.3.4 -> 389-ds-base-1.3.4
commit 68cc036

c55e708..3fd372e 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit 3fd372e

Hi,

this ticket has been commit in 1.3.5 branch but it's not delivered into RHEL7.3 from what I have checked.

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/1038

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.