Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ACL IP ADDRESS evaluation may corrupt c_isreplication_session connection flags #4797

Closed
progier389 opened this issue Jun 9, 2021 · 4 comments
Closed

ACL IP ADDRESS evaluation may corrupt c_isreplication_session connection flags #4797

progier389 opened this issue Jun 9, 2021 · 4 comments
Assignees
@progier389
Labels
bug Something isn't working easy fix Fix is easy In JIRA ticket is in JIRA priority_high need urgent fix / highly valuable / easy to fix
Milestone
1.3.10

Comments

@progier389
Copy link
Contributor

Issue Description
fix about 4764 issue revealed this issue

break is missing in slapi_pblock_set function between
case SLAPI_CONN_CLIENTNETADDR_ACLIP and
case SLAPI_CONN_IS_REPLICATION_SESSION

Although usually undetected this issue may generates unexpected behavior (operation may be seen as replicated while it should not) because of that for some people, fix 4764 (which is correct) may lead to crash

Package Version and Platform:

  • Platform: any
  • Package and version: latest [e.g. 389-ds-base-1.4.4.4-20200721git5d41dc5a4.fc32.x86_64]
  • Browser [e.g. chrome, safari]

Steps to Reproduce
Steps to reproduce the behavior:

  1. Set up a master instance
  2. Initialize database from ldif in
    tc.zip
  3. Run perl SimpleBindSeveralOps.perl -h localhost -p -D uid=foo,ou=people,dc=example,dc=com -w foo -b dc=example,dc=com -s sub uid=demo_user
  4. Run again the perl command (it fails because server crashed after 3)

Note: for writing a nice test in pytest:
The perl just bind that perform a search then modify the found entry description
(The search trigger the aci evaluation and mark the connection as replicated then the modify is seen as a replicated operation
and since the csn is missing it ended to untested error handling which leads to a crash)
Note: I suspect we may hit the same issue without 4764 issue fix (but we should probably do several modify operation in the same connection )
@progier389 progier389 added needs triage The issue will be triaged during scrum bug Something isn't working easy fix Fix is easy priority_high need urgent fix / highly valuable / easy to fix labels Jun 9, 2021
@tbordaz
Copy link
Contributor

tbordaz commented Jun 10, 2021

@aivanov389 , @progier389 (very) nice finding !!!
The fix for ticket #3764 was broken with a missing break in a switch.

This current ticket applies to all the versions where #3764 was committed. Then milestone is 1.3.10.

@tbordaz tbordaz added Need BZ The ticket needs to be cloned to a BZ and removed needs triage The issue will be triaged during scrum labels Jun 10, 2021
@tbordaz tbordaz added this to the 1.3.10 milestone Jun 10, 2021
@tbordaz tbordaz added In JIRA ticket is in JIRA and removed Need BZ The ticket needs to be cloned to a BZ labels Jun 10, 2021
@tbordaz tbordaz mentioned this issue Jun 10, 2021
Merged
tbordaz added a commit to tbordaz/389-ds-base that referenced this issue Jun 10, 2021
@tbordaz
Issue 4797 - ACL IP ADDRESS evaluation may corrupt c_isreplication_se…
3e02226 
…ssion connection flags

Bug description:
	The fix for ticket 389ds#3764 was broken with a missing break in a
	switch. The consequence is that while setting the client IP
	address in the pblock (SLAPI_CONN_CLIENTNETADDR_ACLIP), the
	connection is erroneously set as replication connection.
        This can lead to crash or failure of testcase
        test_access_from_certain_network_only_ip.
        This bug was quite hidden until the fix for 389ds#4764 is
        showing it more frequently

Fix description:
	Add the missing break

relates: 389ds#4797

Reviewed by: Mark Reynolds

Platforms tested: F33
@tbordaz
Copy link
Contributor

tbordaz commented Jun 10, 2021

Fix verified. Had to install perl-LDAP and update SimpleBindSeveralOps (use ldap rather that ldaps (line66) and supplier port)

tbordaz added a commit that referenced this issue Jun 10, 2021
@tbordaz
Issue 4797 - ACL IP ADDRESS evaluation may corrupt c_isreplication_se…
551b5a9 
…ssion connection flags (#4799)

Bug description:
	The fix for ticket #3764 was broken with a missing break in a
	switch. The consequence is that while setting the client IP
	address in the pblock (SLAPI_CONN_CLIENTNETADDR_ACLIP), the
	connection is erroneously set as replication connection.
        This can lead to crash or failure of testcase
        test_access_from_certain_network_only_ip.
        This bug was quite hidden until the fix for #4764 is
        showing it more frequently

Fix description:
	Add the missing break

relates: #4797

Reviewed by: Mark Reynolds

Platforms tested: F33
@tbordaz
Copy link
Contributor

tbordaz commented Jun 10, 2021

@progier389, sorry for having "stolen" that ticket after you did all the hard work. I had to fix it rapidly for a release.

tbordaz added a commit that referenced this issue Jun 10, 2021
@tbordaz
Issue 4797 - ACL IP ADDRESS evaluation may corrupt c_isreplication_se…
02ca55d 
…ssion connection flags (#4799)

Bug description:
	The fix for ticket #3764 was broken with a missing break in a
	switch. The consequence is that while setting the client IP
	address in the pblock (SLAPI_CONN_CLIENTNETADDR_ACLIP), the
	connection is erroneously set as replication connection.
        This can lead to crash or failure of testcase
        test_access_from_certain_network_only_ip.
        This bug was quite hidden until the fix for #4764 is
        showing it more frequently

Fix description:
	Add the missing break

relates: #4797

Reviewed by: Mark Reynolds

Platforms tested: F33
tbordaz added a commit that referenced this issue Jun 10, 2021
@tbordaz
Issue 4797 - ACL IP ADDRESS evaluation may corrupt c_isreplication_se…
05c07a6 
…ssion connection flags (#4799)

Bug description:
	The fix for ticket #3764 was broken with a missing break in a
	switch. The consequence is that while setting the client IP
	address in the pblock (SLAPI_CONN_CLIENTNETADDR_ACLIP), the
	connection is erroneously set as replication connection.
        This can lead to crash or failure of testcase
        test_access_from_certain_network_only_ip.
        This bug was quite hidden until the fix for #4764 is
        showing it more frequently

Fix description:
	Add the missing break

relates: #4797

Reviewed by: Mark Reynolds

Platforms tested: F33
tbordaz added a commit that referenced this issue Jun 10, 2021
@tbordaz
Issue 4797 - ACL IP ADDRESS evaluation may corrupt c_isreplication_se…
a7b67cf 
…ssion connection flags (#4799)

Bug description:
    The fix for ticket #3764 was broken with a missing break in a
    switch. The consequence is that while setting the client IP
    address in the pblock (SLAPI_CONN_CLIENTNETADDR_ACLIP), the
    connection is erroneously set as replication connection.
    This can lead to crash or failure of testcase
    test_access_from_certain_network_only_ip.
    This bug was quite hidden until the fix for #4764 is
    showing it more frequently

Fix description:
    Add the missing break

relates: #4797

Reviewed by: Mark Reynolds

Platforms tested: F33
@tbordaz
Copy link
Contributor

tbordaz commented Jun 10, 2021

2437047..551b5a9 master
f31010e..02ca55d 389-ds-base-1.4.4
506dce2..05c07a6 389-ds-base-1.4.3
f3fc9ef..a7b67cf 389-ds-base-1.3.10

@tbordaz tbordaz closed this as completed Jun 10, 2021
mreynolds389 pushed a commit that referenced this issue Aug 12, 2021
@tbordaz @mreynolds389
Issue 4797 - ACL IP ADDRESS evaluation may corrupt c_isreplication_se…
9f33827 
…ssion connection flags (#4799)

Bug description:
	The fix for ticket #3764 was broken with a missing break in a
	switch. The consequence is that while setting the client IP
	address in the pblock (SLAPI_CONN_CLIENTNETADDR_ACLIP), the
	connection is erroneously set as replication connection.
        This can lead to crash or failure of testcase
        test_access_from_certain_network_only_ip.
        This bug was quite hidden until the fix for #4764 is
        showing it more frequently

Fix description:
	Add the missing break

relates: #4797

Reviewed by: Mark Reynolds

Platforms tested: F33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@progier389 progier389

Labels
bug Something isn't working easy fix Fix is easy In JIRA ticket is in JIRA priority_high need urgent fix / highly valuable / easy to fix
Projects
None yet
Milestone
1.3.10
Development

No branches or pull requests
2 participants
@tbordaz @progier389