Hi,

the SSSD should be fixed to skip entries for which we only receive a DN, but it would be nice to have a server-side fix as well. Some deployments that upgrade to IPA 4.x might not be able to upgrade all their clients to the fixed SSSD version...

A bit more context -- such entries are very common on an IPA server starting with 4.0 where IPA reworked their ACIs.

attachment
0001-Ticket-47885-deref-plugin-should-not-return-referenc.patch

commited to
master:
ldap/servers/plugins/deref/deref.c | 46 ++++++++++++++++++-------------------
1 file changed, 23 insertions(+), 23 deletions(-)

New commits:
commit 61dc5e1

1.3.3
ldap/servers/plugins/deref/deref.c | 46 ++++++++++++++++++-------------------
1 file changed, 23 insertions(+), 23 deletions(-)

New commits:
commit 9eae60a

1.3.2
ldap/servers/plugins/deref/deref.c | 46 ++++++++++++++++++-------------------
1 file changed, 23 insertions(+), 23 deletions(-)

New commits:
commit c4a3563

1112702 was flagged as blocker, so did backport to 1.2.11

New commits:
commit 39f44c5

Ludwig, I guess you could close this ticket now?

The fix did not return a deref response control if the list of dereferenced entries was empty, this caused new issues with SSSD which always expects a response control if a request was sent.

I think it is ok to always retrun the control, in the spec it says:
controlValue ::= SEQUENCE OF derefSpec DerefSpec

and sequence can contain 0 elements.

A new fix will be attached.

attachment
0001-fix-for-47885-did-not-always-return-a-response-contr.patch

Replying to [comment:14 lkrispen]:

The fix did not return a deref response control if the list of dereferenced entries was empty, this caused new issues with SSSD which always expects a response control if a request was sent.

I think it is ok to always retrun the control, in the spec it says:
controlValue ::= SEQUENCE OF derefSpec DerefSpec

and sequence can contain 0 elements.

A new fix will be attached.

Thank you very much for looking into this, Ludwig!

Would you prefer if we removed the check from SSSD? I still think we need to "fix" the DS side in order to avoid breaking clients, but I guess it's a good idea to make the client do the right thing.

Replying to [comment:16 jhrozek]:

Would you prefer if we removed the check from SSSD? I still think we need to "fix" the DS side in order to avoid breaking clients, but I guess it's a good idea to make the client do the right thing.

I think we are balancing a bit on what has to be done and what could be done. I also think it is better to keep the change in DS at a minimum (stripping deref entries without access and sending the control always) but the clients should also try to me more tolerant in handling the responses

Replying to [comment:17 lkrispen]:

Replying to [comment:16 jhrozek]:

Would you prefer if we removed the check from SSSD? I still think we need to "fix" the DS side in order to avoid breaking clients, but I guess it's a good idea to make the client do the right thing.

I think we are balancing a bit on what has to be done and what could be done. I also think it is better to keep the change in DS at a minimum (stripping deref entries without access and sending the control always) but the clients should also try to me more tolerant in handling the responses

I agree completely. I will send a patch upstream. Frankly I don't remember why I added such a strict check -- the code dates back to 2011..

$ git merge ticket47885-2
Updating 1279f0e..de57632
Fast-forward
ldap/servers/plugins/deref/deref.c | 30 ++++++++++++------------------
1 file changed, 12 insertions(+), 18 deletions(-)
$ git push origin master
Counting objects: 13, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (7/7), done.
Writing objects: 100% (7/7), 790 bytes, done.
Total 7 (delta 4), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
1279f0e..de57632 master -> master

$ git checkout 389-ds-base-1.3.3
Switched to branch '389-ds-base-1.3.3'
$ git cherry-pick de57632
[389-ds-base-1.3.3 55e317f] fix for 47885 did not always return a response control
1 file changed, 12 insertions(+), 18 deletions(-)
$ git push origin 389-ds-base-1.3.3
Counting objects: 13, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (7/7), done.
Writing objects: 100% (7/7), 789 bytes, done.
Total 7 (delta 4), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
aa8ff4b..55e317f 389-ds-base-1.3.3 -> 389-ds-base-1.3.3

$ git checkout 389-ds-base-1.3.2
Switched to branch '389-ds-base-1.3.2'
$ git cherry-pick de57632
[389-ds-base-1.3.2 0056b61] fix for 47885 did not always return a response control
1 file changed, 12 insertions(+), 18 deletions(-)
$ git push origin 389-ds-base-1.3.2
Counting objects: 13, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (7/7), done.
Writing objects: 100% (7/7), 779 bytes, done.
Total 7 (delta 4), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
6eec63f..0056b61 389-ds-base-1.3.2 -> 389-ds-base-1.3.2

$ git checkout 389-ds-base-1.2.11
Switched to branch '389-ds-base-1.2.11'
$ git cherry-pick de57632
[389-ds-base-1.2.11 67e4eed] fix for 47885 did not always return a response control
1 file changed, 12 insertions(+), 18 deletions(-)
$ git push origin 389-ds-base-1.2.11
Counting objects: 13, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (7/7), done.
Writing objects: 100% (7/7), 764 bytes, done.
Total 7 (delta 4), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
aa935c9..67e4eed 389-ds-base-1.2.11 -> 389-ds-base-1.2.11

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

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.