> 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

Reply via email to