[389-devel] 389 DS nightly 2020-02-07 - 97% PASS

2020-02-06 Thread vashirov
https://fedorapeople.org/groups/389ds/ci/nightly/2020/02/07/report-389-ds-base-1.4.2.7-1.fc31.x86_64.html
___
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] Re: [PATCH] prevent slapd from hanging under unlikely circumstances

2020-02-06 Thread William Brown


> On 6 Feb 2020, at 18:40, Ludwig Krispenz  wrote:
> 
> 
> On 02/06/2020 12:57 AM, William Brown wrote:
>> 
>>> On 5 Feb 2020, at 20:08, Ludwig Krispenz  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  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 

[389-devel] please review: PR 50887 - Fix dsconf healthchecks failures related to non-TLS enabled servers

2020-02-06 Thread Mark Reynolds

https://pagure.io/389-ds-base/pull-request/50887

--

389 Directory Server Development Team
___
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] Re: [PATCH] prevent slapd from hanging under unlikely circumstances

2020-02-06 Thread Ludwig Krispenz


On 02/06/2020 12:57 AM, William Brown wrote:



On 5 Feb 2020, at 20:08, Ludwig Krispenz  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  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.



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