> 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?
>>
>> 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]
