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.

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

A filtered nsrole that specifies an empty nsrole in its nsRoleFilter will result in a segfault. #2181

Closed
389-ds-bot opened this issue Sep 13, 2020 · 20 comments
Labels
closed: fixed Migration flag - Issue
Milestone

Comments

@389-ds-bot
Copy link

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/49122


Adding a filtered role that includes a filter similar to the one below will result in a segfault upon a subsequent search of the directory that requests the nsrole attribute from any entry.

'(!(nsrole=cn=This_Is_An_Empty_Managed_NsRoleDefinition,o=Intra,dc=mycorp,dc=com)'

Example:

[root@98efc6936cc6 ~]# time ldapsearch -x -LLL -h localhost -s sub -b dc=mycorp,dc=com -D"cn=directory manager" -wpassword uid=jfinn nsrole
dn: uid=jfinn,ou=People,o=Intra,dc=mycorp,dc=com
nsrole: cn=adm-approved-email,o=intra,dc=mycorp,dc=com
nsrole: cn=adm-approved-mycorp,o=intra,dc=mycorp,dc=com
nsrole: cn=arb-user,o=intra,dc=mycorp,dc=com
nsrole: cn=ldap-adm-tools,o=intra,dc=mycorp,dc=com
nsrole: cn=ldap-dsts,o=intra,dc=mycorp,dc=com
nsrole: cn=myc-myc-assoc,o=intra,dc=mycorp,dc=com
nsrole: cn=myc-myc-corp,o=intra,dc=mycorp,dc=com
nsrole: cn=myc-sys-it,o=intra,dc=mycorp,dc=com
nsrole: cn=sig-preview,o=intra,dc=mycorp,dc=com
nsrole: cn=sys-has-email,o=intra,dc=mycorp,dc=com
nsrole: cn=sys-has-mycorp,o=intra,dc=mycorp,dc=com
nsrole: cn=unix-admins,o=intra,dc=mycorp,dc=com
nsrole: cn=unix-solaris-admins,o=intra,dc=mycorp,dc=com
nsrole: cn=vpn-remoteaccess,o=intra,dc=mycorp,dc=com

real 0m0.035s
user 0m0.002s
sys 0m0.003s

[root@98efc6936cc6 ~]# cat /tmp/filtered_role_that_includes_empty_role.ldif
dn: cn=Filtered_Role_That_Includes_Empty_Role,o=Intra,dc=mycorp,dc=com
nsRoleFilter: (|(!(nsrole=cn=This_Is_An_Empty_Managed_NsRoleDefinition,o=Intra,dc=mycorp,dc=com)))
description: A filtered role with filter that will crash the server
objectClass: top
objectClass: ldapsubentry
objectClass: nsroledefinition
objectClass: nscomplexroledefinition
objectClass: nsfilteredroledefinition
cn: Filtered_Role_That_Includes_Empty_Role

[root@98efc6936cc6 ~]# ldapadd -x -h localhost -D"cn=directory manager" -wpassword -f /tmp/filtered_role_that_includes_empty_role.ldif
adding new entry "cn=Filtered_Role_That_Includes_Empty_Role,o=Intra,dc=mycorp,dc=com"

[root@98efc6936cc6 ~]# time ldapsearch -x -LLL -h localhost -s sub -b dc=mycorp,dc=com -D"cn=directory manager" -wpassword uid=jfinn nsrole
ldap_result: Can't contact LDAP server (-1)

real 0m12.590s
user 0m13.091s
sys 0m0.184s

This has been tested on versions ranging from 1.2.11.32 up to 1.3.5.10

When the problematic role is written to a master that isn't immediately queried, it has the ability to replicate to all other servers and result in a wide-spread outage.

@389-ds-bot 389-ds-bot added the closed: fixed Migration flag - Issue label Sep 13, 2020
@389-ds-bot 389-ds-bot added this to the 1.2.11.33 milestone Sep 13, 2020
@389-ds-bot
Copy link
Author

Comment from nhosoi (@nhosoi) at 2017-02-04 00:14:20

Linked to Bugzilla bug: https://bugzilla.redhat.com/show_bug.cgi?id=1419162

@389-ds-bot
Copy link
Author

Comment from jamespfinn at 2017-02-04 01:06:51

I have created a Docker container that can be used to easily demo & reproduce the bug.

https://github.com/jamespfinn/389Bug49122

@389-ds-bot
Copy link
Author

Comment from jamespfinn at 2017-02-11 22:50:13

Metadata Update from @jamespfinn:

  • Issue set to the milestone: 1.2.11.33

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2017-02-24 14:01:08

Metadata Update from @mreynolds389:

  • Issue assigned to mreynolds389

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2017-02-24 16:52:53

@jamespfinn Are you successfully using other filtered roles that use nsrole in the filter?

The problem is that since nsrole is in the filter its causing a stack overflow as it keeps re-evaluating nsrole over and over and over.

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2017-02-24 16:53:02

Metadata Update from @mreynolds389:

  • Custom field component reset
  • Issue close_status updated to: None

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2017-02-24 18:30:59

Turns out virtual attributes (like COS attributes) are supposed to be ignored in role filters. This includes nsrole as it is a virtual attribute as well.

The fix is to ignore filters that use nsrole, and log a message to the errors log

0001-Issue-49122-Filtered-nsrole-that-uses-nsrole-crashes.patch

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2017-02-24 18:31:08

Metadata Update from @mreynolds389:

  • Custom field reviewstatus adjusted to review

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2017-02-24 18:33:29

CI Test

0001-Issue-49122-add-ci-test.patch

@389-ds-bot
Copy link
Author

Comment from nhosoi (@nhosoi) at 2017-02-24 23:44:11

Looks good to me.

Just wondering this change (replacing a tab with white spaces) does not break the indentation? ;)

56 static int roles_cache_create_object_from_entry(Slapi_Entry *role_entry, role_object **result, int hint)
57 {
58 -» int rc = 0;
59 -» int type = 0;
60 -» role_object *this_role = NULL;
61 - char *rolescopeDN = NULL;
62 + int rc = 0;
63 + int type = 0;
64 + role_object *this_role = NULL;
65 + char *rolescopeDN = NULL;
66
67 » slapi_log_err(SLAPI_LOG_PLUGIN, ROLES_PLUGIN_SUBSYSTEM,

@389-ds-bot
Copy link
Author

Comment from nhosoi (@nhosoi) at 2017-02-24 23:44:20

Metadata Update from @nhosoi:

  • Custom field reviewstatus adjusted to ack (was: review)

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2017-02-25 13:18:13

Actually I started fixing indentation then realized the entire function is completely mismatched. So I stopped . So yes there are still existing mismatches. I'll fix the entire function for the final push.

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2017-02-27 14:18:58

c46822a..a95889d master -> master

8fcda3d..d589950 389-ds-base-1.3.5 -> 389-ds-base-1.3.5

56a24df..75cde2c 389-ds-base-1.3.4 -> 389-ds-base-1.3.4

960ad68..a96eee0 389-ds-base-1.2.11 -> 389-ds-base-1.2.11

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2017-02-27 14:19:06

Metadata Update from @mreynolds389:

  • Issue close_status updated to: fixed
  • Issue status updated to: Closed (was: Open)

@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2017-03-02 11:19:55

Bytecompiling .py files below /builddir/build/BUILDROOT/389-ds-base-1.3.6.1-20170301gitb2c6545.fc25.x86_64/usr/lib/python3.5 using /usr/bin/python3.5
*** Error compiling '/builddir/build/BUILDROOT/389-ds-base-1.3.6.1-20170301gitb2c6545.fc25.x86_64/usr/lib/python3.5/site-packages/dirsrvtests/tests/tickets/ticket49122_test.py'...
  File "/usr/lib/python3.5/ticket49122_test.py", line 55
    print "Attach gdb"
                     ^
SyntaxError: Missing parentheses in call to 'print'

0001-Issue-49122-Fix-rpm-build.patch

@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2017-03-02 11:20:11

Metadata Update from @vashirov:

  • Custom field reviewstatus adjusted to review (was: ack)
  • Issue status updated to: Open (was: Closed)

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2017-03-02 12:40:50

ack

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2017-03-02 12:40:59

Metadata Update from @mreynolds389:

  • Custom field reviewstatus adjusted to ack (was: review)

@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2017-03-02 13:32:41

commit 15ddba9
To ssh://pagure.io/389-ds-base.git
b2c6545..15ddba9 master -> master

@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2017-03-02 13:32:57

Metadata Update from @vashirov:

  • Issue close_status updated to: fixed
  • Issue status updated to: Closed (was: Open)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed: fixed Migration flag - Issue
Projects
None yet
Development

No branches or pull requests

1 participant