> On 6 Feb 2020, at 18:40, Ludwig Krispenz <[email protected]> wrote:
>
>
> On 02/06/2020 12:57 AM, William Brown wrote:
>>
>>> On 5 Feb 2020, at 20:08, Ludwig Krispenz <[email protected]> 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 <[email protected]> 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?
> In fact discussing this yesterday we did get a step further. The call for
> preop plugins is already prevented for tombstone deletions, BUT
>
> the detection if it is a delete of a tombstone flag comes from two sources,
> an operation flag, passed by the internal tombstone purging thread and by
> checking the entry flag of the entry to be deleted, which would be used if an
> external operation deletes a tombstone. Unfortunately this check is after the
> call for preops, so for external deletes the preops are called and the
> postops are not called. Thierry and I agreed that the solution should be to
> do the check of the entry flag earlier and use the already existing bypass
> for pre and post plugins when a tombstone is deleted.
>
> And yes, we need an issue for this.
I'll leave that to you and thierry since you both know more about this at the
moment than I do :)
>>
>>>> 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 -- [email protected]
>>>>> To unsubscribe send an email to [email protected]
>>>>> 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/[email protected]
>>>> _______________________________________________
>>>> 389-devel mailing list -- [email protected]
>>>> To unsubscribe send an email to [email protected]
>>>> 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/[email protected]
>>> --
>>> 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 -- [email protected]
>>> To unsubscribe send an email to [email protected]
>>> 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/[email protected]
>> —
>> Sincerely,
>>
>> William Brown
>>
>> Senior Software Engineer, 389 Directory Server
>> SUSE Labs
>> _______________________________________________
>> 389-devel mailing list -- [email protected]
>> To unsubscribe send an email to [email protected]
>> 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/[email protected]
>
> --
> 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 -- [email protected]
> To unsubscribe send an email to [email protected]
> 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/[email protected]
—
Sincerely,
William Brown
Senior Software Engineer, 389 Directory Server
SUSE Labs
_______________________________________________
389-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
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/[email protected]
