#50460 Issue 50396 - segfault when using pam passthru and addn plugins together
Closed 14 days ago by spichugi. Opened a year ago by vashirov.
vashirov/389-ds-base addn-tests  into  master

@@ -29,6 +29,7 @@ 

  log = logging.getLogger(__name__)

  

  USER_DN = 'uid=test_user_1001,ou=people,dc=example,dc=com'

+ USER_ADDN = 'test_user_1001'

  USER_PW = 'password'

  GROUP_DN = 'cn=group,' + DEFAULT_SUFFIX

  CONFIG_AREA = 'nsslapd-pluginConfigArea'

@@ -1792,10 +1793,97 @@ 

      return

  

  

+ def test_addn(topo, args=None):

+     """Test AD DN plugin basic functionality

+ 

+     :id: 106ee95e-93ab-11e9-9dd6-3497f624ea11

+     :setup: Standalone Instance

+     :steps:

+         1. Create a test user

+         2. Bind using only the uid

+         3. Bind using uid@example.com

+         4. Bind using uid@example.org (second domain, non-default)

+         5. Bind using uid@nonexistent.domain

+         6. Bind as Root DN

+         7. Configure plugin

+         8. Create a default domain config entry

+         9. Create a second domain config entry

+         10. Enable plugin and restart the instance

+         11. Bind using only the uid

+         12. Bind using uid@example.com

+         13. Bind using uid@example.org (second domain, non-default)

+         14. Bind using uid@nonexistent.domain

+         15. Clean up

+     :expectedresults:

+         1. Success

+         2. Failure

+         3. Failure

+         4. Failure

+         5. Failure

+         6. Success

+         7. Success

+         8. Success

+         9. Success

+         10. Success

+         11. Success

+         12. Success

+         13. Success

+         14. Failure

+         15. Success

+     """

+ 

+     inst = topo[0]

+ 

+     users = UserAccounts(inst, DEFAULT_SUFFIX)

+     user1 = users.create_test_user(uid=1001)

+     user1.replace('userPassword', USER_PW)

+ 

+     with pytest.raises(ldap.LDAPError):

+         assert inst.simple_bind_s(USER_ADDN, USER_PW)

+     with pytest.raises(ldap.LDAPError):

+         assert inst.simple_bind_s(f'{USER_ADDN}@example.com', USER_PW)

+     with pytest.raises(ldap.LDAPError):

+         assert inst.simple_bind_s(f'{USER_ADDN}@example.org', USER_PW)

+     with pytest.raises(ldap.LDAPError):

+         assert inst.simple_bind_s(f'{USER_ADDN}@nonexistent.domain', USER_PW)

+     inst.simple_bind_s(DN_DM, PASSWORD)

+ 

+     # stop the plugin, and start it

+     plugin = AddnPlugin(inst)

+     plugin.create(rdn='addn')

+     plugin.set_default_domain('example.com')

+     addn_domains = AddnDomains(inst)

+     addn_domains.create(

+             properties={'cn': 'example.com',

+                         'addn_base': 'ou=people,dc=example,dc=com',

+                         'addn_filter': '(&(objectClass=account)(uid=%s))',})

+     addn_domains.create(

+             properties={'cn': 'example.org',

+                         'addn_base': 'ou=people,dc=example,dc=com',

+                         'addn_filter': '(&(objectClass=account)(uid=%s))',})

+     plugin.enable()

+ 

+     if args == "restart":

+         return

+ 

+     if args is None:

+         inst.restart()

+ 

+     assert inst.simple_bind_s(USER_ADDN, USER_PW)

+     assert inst.simple_bind_s(f'{USER_ADDN}@example.com', USER_PW)

+     assert inst.simple_bind_s(f'{USER_ADDN}@example.org', USER_PW)

+     with pytest.raises(ldap.LDAPError):

+         assert inst.simple_bind_s(f'{USER_ADDN}@nonexistent.domain', USER_PW)

+ 

+     # Delete user

+     inst.simple_bind_s(DN_DM, PASSWORD)

+     user1.delete()

+ 

+ 

  # Array of test functions

  func_tests = [test_acctpolicy, test_attruniq, test_automember, test_dna,

                test_linkedattrs, test_memberof, test_mep, test_passthru,

-               test_referint, test_retrocl, test_rootdn]

+               test_referint, test_retrocl, test_rootdn, test_addn]

  

  

  def check_all_plugins(topo, args="online"):

@@ -0,0 +1,60 @@ 

+ # --- BEGIN COPYRIGHT BLOCK ---

+ # Copyright (C) 2019 Red Hat, Inc.

+ # All rights reserved.

+ #

+ # License: GPL (version 3 or any later version).

+ # See LICENSE for details.

+ # --- END COPYRIGHT BLOCK ---

+ #

+ import ldap

+ import logging

+ import pytest

+ from lib389.plugins import PAMPassThroughAuthPlugin, AddnPlugin, AddnDomains

+ from lib389.topologies import topology_st as topo

+ 

+ pytestmark = pytest.mark.tier1

+ 

+ log = logging.getLogger(__name__)

+ 

+ @pytest.mark.ds50396

+ @pytest.mark.bz1701092

+ def test_crash_in_pam_pta_plugin_when_user_doesnt_exist(topo):

+     """Test that the server doesn't crash when user doesn't exist

+ 

+     :id: cb362b70-9424-11e9-a56a-54e1ad30572c

+     :setup: Standalone Instance

+     :steps:

+         1. Enable PAM PTA plugin

+         2. Enable and configure AD DN plugin

+         3. Restart the instance

+         4. Bind as non-existent user

+     :expectedresults:

+         1. Success

+         2. Success

+         3. Success

+         4. Server shouldn't crash

+     """

+ 

+     inst = topo.standalone

+ 

+     # Enable PAM PTA plugin

+     pampta_plugin = PAMPassThroughAuthPlugin(inst)

+     pampta_plugin.enable()

+ 

+     # Enable and configure AD DN plugin

+     addn_plugin = AddnPlugin(inst)

+     addn_plugin.create(rdn='addn')

+     addn_plugin.set_default_domain('example.com')

+     addn_domains = AddnDomains(inst)

+     addn_domains.create(

+             properties={'cn': 'example.com',

+                         'addn_base': 'ou=people,dc=example,dc=com',

+                         'addn_filter': '(&(objectClass=account)(uid=%s))',})

+     addn_plugin.enable()

+ 

+     #  Restart the instance to enable plugins

+     inst.restart()

+ 

+     # Expect the following bind to fail, but not crash the server (ldap.SERVER_DOWN)

+     with pytest.raises(ldap.OPERATIONS_ERROR):

+         assert inst.simple_bind_s('bob@example.com', 'password')

file modified
+86 -3

@@ -100,17 +100,100 @@ 

  

  

  class AddnPlugin(Plugin):

-     """An instance of addn plugin entry

+     """An instance of ADDN plugin entry

  

      :param instance: An instance

      :type instance: lib389.DirSrv

      :param dn: Entry DN

      :type dn: str

      """

+     _plugin_properties = {

+         'nsslapd-pluginEnabled': 'off',

+         'nsslapd-pluginPath': 'libaddn-plugin',

+         'nsslapd-pluginInitfunc': 'addn_init',

+         'nsslapd-pluginType': 'preoperation',

+         'nsslapd-pluginId': 'addn',

+         'nsslapd-pluginVendor': '389 Project',

+         'nsslapd-pluginVersion': 'none',

+         'nsslapd-pluginDescription': 'Allow AD DN style bind names to LDAP',

+     }

  

      def __init__(self, instance, dn="cn=addn,cn=plugins,cn=config"):

          super(AddnPlugin, self).__init__(instance, dn)

-         # Need to add wrappers to add domains to this.

+         self._create_objectclasses = ['top', 'nsslapdplugin', 'extensibleObject']

+ 

+     def set_default_domain(self, attr):

+         """Add addn_default_domain attribute"""

+ 

+         self.set('addn_default_domain', attr)

+ 

+     def get_default_domain(self):

+         """Get addn_default_domain attribute"""

+ 

+         return self.get_attr_val_utf8('addn_default_domain')

+ 

+ class AddnDomain(DSLdapObject):

+     """A single instance of ADDN plugin domain config entry

+ 

+     :param instance: An instance

+     :type instance: lib389.DirSrv

+     :param dn: Entry DN

+     :type dn: str

+     """

+ 

+     def __init__(self, instance, dn):

+         super(AddnDomain, self).__init__(instance, dn)

+         self._rdn_attribute = 'cn'

+         self._must_attributes = ['cn','addn_base', 'addn_filter']

+         self._create_objectclasses = ['top', 'extensibleObject']

+         self._protected = False

+ 

+ class AddnDomains(DSLdapObjects):

+     """A DSLdapObjects entity which represents ADDN plugin domain config entry

+ 

+     :param instance: An instance

+     :type instance: lib389.DirSrv

+     :param basedn: Base DN for all account entries below

+     :type basedn: str

+     """

+ 

+     def __init__(self, instance, basedn=None):

+         super(AddnDomains, self).__init__(instance)

+         self._objectclasses = ['top', 'extensibleObject']

+         self._filterattrs = ['cn']

+         self._childobject = AddnDomain

+         # So we can set the configArea easily

+         if basedn is None:

+             basedn = "cn=addn,cn=plugins,cn=config"

+         self._basedn = basedn

+ 

+     def list(self):

+         """Get a list of children entries (DSLdapObject, Replica, etc.) using a base DN

+         and objectClasses of our object (DSLdapObjects, Replicas, etc.)

+ 

+         :returns: A list of children entries

+         """

+ 

+         # Filter based on the objectclasses and the basedn

+         insts = None

+         # This will yield and & filter for objectClass with as many terms as needed.

+         filterstr = self._get_objectclass_filter()

+         self._log.debug('list filter = %s' % filterstr)

+         try:

+             results = self._instance.search_ext_s(

+                 base=self._basedn,

+                 scope=self._scope,

+                 filterstr=filterstr,

+                 attrlist=self._list_attrlist,

+                 serverctrls=self._server_controls, clientctrls=self._client_controls,

+                 escapehatch='i am sure'

+             )

+             # def __init__(self, instance, dn=None):

+             insts = [self._entry_to_instance(dn=r.dn, entry=r) for r in results]

+         except ldap.NO_SUCH_OBJECT:

+             # There are no objects to select from, se we return an empty array

+             insts = []

+         return insts

  

  

  class AttributeUniquenessPlugin(Plugin):

@@ -1016,7 +1099,7 @@ 

  

  

  class SevenBitCheckPlugin(Plugin):

-     """An instance of addn plugin entry

+     """An instance of 7-bit check plugin entry

  

      :param instance: An instance

      :type instance: lib389.DirSrv

Description:

  • Add missing test for addn plugin in acceptance test module
  • Add regression test for #50396

Relates: https://pagure.io/389-ds-base/issue/50396

Reviewed by: ???

@firstyear, while adding acceptance tests for AD DN plugin, I found that we don't have the default ldif for it in template-dse.ldif. Do you think it should stay that way or we can add it there (off by default, of course)?

@vashirov I'm of two minds here. I really hate template-dse.ldif because it locks us into long-term configuration syntaxes because they aren't upgraded and come from old installs. Older installs also don't have new plugin configs added either so it's non obvious. template-dse.ldif is not documentation, it really should be "the minimal required server to make dscreate work".

I think maybe I'd prefer that dsconf ... plugin addn enable should be the correct way to "add" the required cn=config object, and we have an example ldif in documentation. I think that's the most sustainable approach long term.

@firstyear, I'm fine with the current approach. I was asking because I want to have a more robust and consistent behavior of creating and configuring plugins. Since some plugins are already present, I can configure them right away. And for some, like AD DN, I need to call .create() first to ensure that the DN exists before modifying attributes there. So maybe it's more for lib389 to handle this correctly, since we can't guarantee for sure that the plugin's DN will be present in cn=config by default.

rebased onto 035720d3e5cdf21ffe550bf733133e2bc0058e95

a year ago

rebased onto 28cbbea71bcdf62b53536544dcb4dd8b2d01a68a

a year ago

Dynamic plugins test suite fails because of this PR, investigating.

@vashirov there is a stateful method in lib389, .ensure or something like this. It says "create or modify to make this thing look like this". I think that may be what you want here.

Really I think as a team we need to talk about getting our house in order and how we should be configuring and setting up the server and plugins, because it's such a fragmented and chaotic process today, lib389 has helped a lot, but we need to work out what's best here.

Is there anything outstanding here before we can merge?

rebased onto a4a4d3b66d74884347cb6ba8c5f4e8b9c87e36c3

a year ago

Is there anything outstanding here before we can merge?

There were test failures in plugin test suite with my change. I need to revisit this and fix it.

rebased onto 83e9f05

3 months ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3518

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

14 days ago