> On 5 Feb 2020, at 20:08, Ludwig Krispenz <lkris...@redhat.com> wrote: > > > On 02/05/2020 10:53 AM, thierry bordaz wrote: >> >> >> On 2/5/20 2:30 AM, William Brown wrote: >>> >>>> On 5 Feb 2020, at 03:10, Ludwig Krispenz <lkris...@redhat.com> wrote: >>>> >>>> I think I can agree with 1-8, 9 is one solution to fix the problem you >>>> reported, but not yet validate that there are no other side effects, there >>>> are potential postop plugins which should NOT be called for tombstone >>>> delete, eg retro cl, we need to investigate side effects of the patch and >>>> evaluate other solutions, I suggested one in anotehr reply. >>>> >>>> for 10, the claim that not crying hurrah and merging a patch without >>>> further investigation and testing is "doctrinal objection" is very strong >>>> and I have to reject this. >>>> >>>> Regards, >>>> Ludwig >>>> >>>> On 02/04/2020 05:00 PM, Jay Fenlason wrote: >>>>> Ok, let's take this from the top: >>>>> >>>>> 1: Defects that cause a server to become unresponsive are bad and must >>>>> be repaired as soon as possible. >>> I'm not objecting to this. >>> >>>>> 2: Some #1 class defects are exploitable and require a CVE. As far as >>>>> I can tell, this one does not, but you should keep an eye out for the >>>>> possibility. >>>>> >>>>> 3: The #1 class defect I have found can be triggered in at least two >>>>> ways. One requires ipa-replica-install and is hard to reproduce in a >>>>> test environment. The other requires ldapdelete and is easy to >>>>> reproduce in an isolated VM. >>> Ipa replica install and ldapdelete of a tombstone/conflict both require >>> cn=directory manager, which would automatically cause any reported CVE to >>> be void - cn=directory manager is game over. >>> >>>>> 4: The defect is a mismatch between the plugin ABI as implemented by >>>>> 389-ds, and the behavior the NIS plugin expects. >>>>> >>>>> 5: I have found no specification or documentation on said ABI, so I >>>>> can only guess what the correct behavior is here. >>>>> >>>>> 6: The ABI includes two functions in the plugin: pre_delete and >>>>> post_delete. >>>>> >>>>> 7: The NIS plugin expects that every call to pre_delete will be >>>>> followed by a call to post_delete. It takes a lock in pre_delete and >>>>> releases it in post_delete. >>> But you didn't report an issue in NIS? You reported an issue with >>> ldapdelete on tombstones and conflicts ... the slapi-nis plugin is >>> maintained by freeipa and if an issue in it exists, please report it to >>> them with reproduction steps. >> >> Hi Jay, >> >> Thanks for your great investigations/patch/summary. >> >> I made the change in NIS plugin to acquire/release map lock in pre/post ops. >> At that time I was assuming the pre/post callback were balanced. >> I was wrong and already hit similar issue in #1694263. I updated NIS plugin >> to workaround that bug. >> >> The new bug you are reporting is a new example that pre/post are sometime >> not balanced :( >> The proposed fixes (prevent preop call on tombstone, force postop call even >> on tombstone) looks valid but will have wider impact and need additional >> investigation. >> An other approach would be to make NIS plugin ignore operations on >> tombstone. It also require additional investigation. > I think that for tombstone deletions we should also prevent the call of the > preop plugins, no plugin should deal with these operations
That sounds reasonable. Can we create an issue for this? >> >> best regards >> thierry >> >> >>>>> 8: Under some circumstances 389-ds can call pre_delete but fail to >>>>> call post_delete. Because of #5, there is no way to tell if this is >>>>> expected behavior, but the NIS plugin clearly does not expect it. >>>>> >>>>> 9: My patch ensures that every call to pre_delete is followed by a >>>>> corresponding call to post_delete. V1 of the patch also ensures that >>>>> every call to post_delete is after a corresponding pre_delete call. >>>>> V2 relaxes the second requirement, allowing post_delete calls without >>>>> a corresponding pre_delete call because someone expressed worry about >>>>> potential regressions. >>>>> >>>>> 10: You are refusing to merge my patch because of a doctrinal >>>>> objection to the use of ldapdelete in the simple reproducer. >>> It's not really a doctrinal objection. Replication is a really complex >>> topic, and difficult to get correct. Ludwig knows a lot in this area, and >>> has really good comments to make on the topic. I understand that you have >>> an issue you have found in your setup, and you want to resolve it, but we >>> have to consider the deployments of many thousands of other users and their >>> replication experience too. For us it's important to be able to reproduce, >>> and see the issue, to understand the consequences, and to make these >>> changes carefully. Accepting the patch without clear understanding of it's >>> consequences is not something we do. >>> >>> At this time we still don't have a clear reproducer from you on "how" you >>> constructed the invalid directory state. You reported the following: >>> >>>> I found the bug by doing a series of "ipa-client-install" (with lots >>>> of arguments, followed by >>>> echo ca_host = {a not-firewalled IPA CA} >> /etc/ipa/default.conf >>>> echo [global] > /etc/ipa/installer.conf >>>> echo ca_host = {ditto} >> /etc/ipa/installer.conf >>>> echo {password} | kinit admin >>>> ipa hostgroup-add-member ipaservers --hosts $(hostname -f) >>>> ipa-relica-install --setup-ca --setup-dns --forwarder={ip addr} >>>> >>>> followed by the replica install failing due to network issues, >>>> misconfigured firewalls, etc, then >>>> ipa-server-install --uninstall on the host >>>> and ipa-replica-manage del {failed install host} >>>> elsewhere in the mesh, sometimes with ldapdelete of the initial >>>> replication agreement that ipa-replica-manage did not remove. >>> I'm certainly not able to produce a reproduction case from these steps ... >>> This is why I have questioned what ipa-replica-install is doing in a >>> previous email, and I think that we need a better series of steps to >>> attempt to reproduce, as well as better and clearer information about >>> "what" has crashed/hung/failed in the directories in these states. >>> >>> We still want to help, but please be patient with us as we want to ensure >>> our fixes are the highest possible quality. >>> >>> Thanks, >>> >>> — >>> Sincerely, >>> >>> William Brown >>> >>> Senior Software Engineer, 389 Directory Server >>> SUSE Labs >>> _______________________________________________ >>> 389-devel mailing list -- 389-devel@lists.fedoraproject.org >>> To unsubscribe send an email to 389-devel-le...@lists.fedoraproject.org >>> Fedora Code of Conduct: >>> https://docs.fedoraproject.org/en-US/project/code-of-conduct/ >>> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines >>> List Archives: >>> https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.org >> _______________________________________________ >> 389-devel mailing list -- 389-devel@lists.fedoraproject.org >> To unsubscribe send an email to 389-devel-le...@lists.fedoraproject.org >> Fedora Code of Conduct: >> https://docs.fedoraproject.org/en-US/project/code-of-conduct/ >> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines >> List Archives: >> https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.org > > -- > Red Hat GmbH, http://www.de.redhat.com/, Sitz: Grasbrunn, > Handelsregister: Amtsgericht München, HRB 153243, > Geschäftsführer: Charles Cachera, Laurie Krebs, Michael O'Neill, Thomas Savage > _______________________________________________ > 389-devel mailing list -- 389-devel@lists.fedoraproject.org > To unsubscribe send an email to 389-devel-le...@lists.fedoraproject.org > Fedora Code of Conduct: > https://docs.fedoraproject.org/en-US/project/code-of-conduct/ > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines > List Archives: > https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.org — Sincerely, William Brown Senior Software Engineer, 389 Directory Server SUSE Labs _______________________________________________ 389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.org