Bug 1420960

Summary:	Need a way for plugin to modify the request during ADD op. after the ACIs are evaluated
Product:	Red Hat Enterprise Linux 7	Reporter:	Noriko Hosoi <nhosoi>
Component:	389-ds-base	Assignee:	wibrown <wibrown>
Status:	CLOSED ERRATA	QA Contact:	Viktor Ashirov <vashirov>
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	7.4	CC:	mreynolds, nkinder, rmeggins, spichugi, wibrown
Target Milestone:	rc
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	389-ds-base-1.3.6.1-3.el7	Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2017-08-01 21:14:10 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Noriko Hosoi 2017-02-10 00:31:22 UTC

Description of problem:
 IPA has a plugin which replaces a known value (ipaUniqueID=autogenerate) with a randomly generated UUID (ipaUniqueID=autogenerate). It currently executes in preoperation and internalpreoperation.

In the case of preoperation, the access controls are evaluated *after* the UUID plugin makes the change. This means that the access controls are useless for any attribute randomly generated by the UUID plugin. You should be able to create an ACI that restricts access to the known value only.

Here is a real world case. To allow users to create tokens, we have an ACI like this:

aci: (target = "ldap:///ipatokenuniqueid=*,cn=otp,$SUFFIX")(targetfilter
"(objectClass=ipaToken)")(version 3.0; acl "Users can create

self-managed tokens"; allow (add) userattr = "ipatokenOwner#SELFDN" and
userattr = "managedBy#SELFDN";)

The problem is that this permits a user to create an entry with any value. If ACIs were evaluated before the UUID plugin and not afterward, we could use this ACI:

aci: (target = "ldap:///ipatokenuniqueid=autogenerate,cn=otp,
$SUFFIX")(targetfilter = "(objectClass=ipaToken)")(version 3.0; acl
"Users can create self-managed tokens"; allow (add) userattr =
"ipatokenOwner#SELFDN" and userattr = "managedBy#SELFDN";)

For add operations, ACIs are currently evaluated after bepreop but before betxnpreop. Unfortunately, we can't change DN/ipaTokenUniqueID in betxnpreop. For modify operations, the ACIs are currently evaluated before bepreop. One possible solution would be to move the add operation behavior to be the same as the modify operation.

Comment 2 Simon Pichugin 2017-05-31 17:36:53 UTC

After a small investigation, I have a question regarding verification steps.

Is there an easy way to verify the BZ with the user operations?

Or the only way to verify it is to run ACI (and plugins) test suites for a check for regressions and also to do the sanity testing?

Comment 3 wibrown@redhat.com 2017-05-31 23:49:52 UTC

I have *no idea* if there is an easy way to test. I would test aci's still evaluate correctly, no resource leaks etc. It was IPA who raised this request so maybe they know what do here .... ? Sorry I can't help more, I wish I could.

Comment 4 Simon Pichugin 2017-06-02 09:32:31 UTC

According to William's commentary above and source code investigation, we can't reproduce the problem properly. The change is about a change of internal operations order.
Through our CI we check it for regressions. Also sanity testing.

[root@qeos-67 ds]# py.test -v dirsrvtests/tests/suites/acl/
=================== test session starts ===================
platform linux2 -- Python 2.7.5, pytest-3.1.1, py-1.4.33, pluggy-0.4.0 -- /usr/bin/python
cachedir: .cache
metadata: {'Python': '2.7.5', 'Platform': 'Linux-3.10.0-671.el7.x86_64-x86_64-with-redhat-7.4-Maipo', 'Packages': {'py': '1.4.33', 'pytest': '3.1.1', 'pluggy': '0.4.0'}, 'Plugins': {'beakerlib': '0.7.1', 'html': '1.14.2', 'cov': '2.5.1', 'metadata': '1.5.0'}}
DS build: 1.3.6.1
389-ds-base: 1.3.6.1-15.el7
nss: 3.28.4-8.el7
nspr: 4.13.1-1.0.el7_3
openldap: 2.4.44-4.el7
svrcore: 4.1.3-2.el7

rootdir: /mnt/tests/rhds/tests/upstream/ds, inifile:
plugins: metadata-1.5.0, html-1.14.2, cov-2.5.1, beakerlib-0.7.1
collected 26 items

dirsrvtests/tests/suites/acl/acl_test.py::test_aci_attr_subtype_targetattr[lang-ja] PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_aci_attr_subtype_targetattr[binary] PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_aci_attr_subtype_targetattr[phonetic] PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_mode_default_add_deny PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_mode_default_delete_deny PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_moddn_staging_prod[0-cn=staged user,dc=example,dc=com-cn=accounts,dc=example,dc=com-False] PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_moddn_staging_prod[1-cn=staged user,dc=example,dc=com-cn=accounts,dc=example,dc=com-False] PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_moddn_staging_prod[2-cn=staged user,dc=example,dc=com-cn=bad*,dc=example,dc=com-True] PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_moddn_staging_prod[3-cn=st*,dc=example,dc=com-cn=accounts,dc=example,dc=com-False] PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_moddn_staging_prod[4-cn=bad*,dc=example,dc=com-cn=accounts,dc=example,dc=com-True] PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_moddn_staging_prod[5-cn=st*,dc=example,dc=com-cn=ac*,dc=example,dc=com-False] PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_moddn_staging_prod[6-None-cn=ac*,dc=example,dc=com-False] PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_moddn_staging_prod[7-cn=st*,dc=example,dc=com-None-False] PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_moddn_staging_prod[8-None-None-False] PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_moddn_staging_prod_9 PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_moddn_prod_staging PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_check_repl_M2_to_M1 PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_moddn_staging_prod_except PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_mode_default_ger_no_moddn PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_mode_default_ger_with_moddn PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_mode_switch_default_to_legacy PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_mode_legacy_ger_no_moddn1 PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_mode_legacy_ger_no_moddn2 PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_mode_legacy_ger_with_moddn PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_rdn_write_get_ger PASSED
dirsrvtests/tests/suites/acl/acl_test.py::test_rdn_write_modrdn_anonymous PASSED

=================== 26 passed in 21.97 seconds ===================

Comment 5 errata-xmlrpc 2017-08-01 21:14:10 UTC

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2017:2086