From 0ed5299841733f9e18c5fe8c6a3f9d3fbd49c75a Mon Sep 17 00:00:00 2001 From: Firstyear Date: Fri, 20 Aug 2021 09:18:50 +1000 Subject: [PATCH 3/4] Issue 4877 - RFE - EntryUUID to validate UUIDs on fixup (#4878) Bug Description: Due to changing the syntax of EntryUUID's to string, we may have invalid EntryUUID's imported into the database. Fix Description: To resolve this during a fixup we validate that Uuid's have a valid syntax. If they do not, we regenerate them. fixes: https://github.com/389ds/389-ds-base/issues/4877 Author: William Brown Review by: @mreynolds389 --- .../entryuuid/localhost-userRoot-invalid.ldif | 233 ++++++++++++++++++ .../tests/suites/entryuuid/basic_test.py | 58 ++++- src/plugins/entryuuid/src/lib.rs | 28 ++- src/slapi_r_plugin/src/pblock.rs | 4 +- src/slapi_r_plugin/src/value.rs | 10 +- 5 files changed, 322 insertions(+), 11 deletions(-) create mode 100644 dirsrvtests/tests/data/entryuuid/localhost-userRoot-invalid.ldif diff --git a/dirsrvtests/tests/data/entryuuid/localhost-userRoot-invalid.ldif b/dirsrvtests/tests/data/entryuuid/localhost-userRoot-invalid.ldif new file mode 100644 index 000000000..9703babed --- /dev/null +++ b/dirsrvtests/tests/data/entryuuid/localhost-userRoot-invalid.ldif @@ -0,0 +1,233 @@ +version: 1 + +# entry-id: 1 +dn: dc=example,dc=com +objectClass: top +objectClass: domain +dc: example +description: dc=example,dc=com +creatorsName: cn=Directory Manager +modifiersName: cn=Directory Manager +createTimestamp: 20200325015542Z +modifyTimestamp: 20200325015542Z +nsUniqueId: a2b33229-6e3b11ea-8de0c78c-83e27eda +aci: (targetattr="dc || description || objectClass")(targetfilter="(objectClas + s=domain)")(version 3.0; acl "Enable anyone domain read"; allow (read, search + , compare)(userdn="ldap:///anyone");) +aci: (targetattr="ou || objectClass")(targetfilter="(objectClass=organizationa + lUnit)")(version 3.0; acl "Enable anyone ou read"; allow (read, search, compa + re)(userdn="ldap:///anyone");) + +# entry-id: 2 +dn: cn=389_ds_system,dc=example,dc=com +objectClass: top +objectClass: nscontainer +objectClass: ldapsubentry +cn: 389_ds_system +creatorsName: cn=Directory Manager +modifiersName: cn=Directory Manager +createTimestamp: 20200325015542Z +modifyTimestamp: 20200325015542Z +nsUniqueId: a2b3322a-6e3b11ea-8de0c78c-83e27eda + +# entry-id: 3 +dn: ou=groups,dc=example,dc=com +objectClass: top +objectClass: organizationalunit +ou: groups +aci: (targetattr="cn || member || gidNumber || nsUniqueId || description || ob + jectClass")(targetfilter="(objectClass=groupOfNames)")(version 3.0; acl "Enab + le anyone group read"; allow (read, search, compare)(userdn="ldap:///anyone") + ;) +aci: (targetattr="member")(targetfilter="(objectClass=groupOfNames)")(version + 3.0; acl "Enable group_modify to alter members"; allow (write)(groupdn="ldap: + ///cn=group_modify,ou=permissions,dc=example,dc=com");) +aci: (targetattr="cn || member || gidNumber || description || objectClass")(ta + rgetfilter="(objectClass=groupOfNames)")(version 3.0; acl "Enable group_admin + to manage groups"; allow (write, add, delete)(groupdn="ldap:///cn=group_admi + n,ou=permissions,dc=example,dc=com");) +creatorsName: cn=Directory Manager +modifiersName: cn=Directory Manager +createTimestamp: 20200325015543Z +modifyTimestamp: 20200325015543Z +nsUniqueId: a2b3322b-6e3b11ea-8de0c78c-83e27eda + +# entry-id: 4 +dn: ou=people,dc=example,dc=com +objectClass: top +objectClass: organizationalunit +ou: people +aci: (targetattr="objectClass || description || nsUniqueId || uid || displayNa + me || loginShell || uidNumber || gidNumber || gecos || homeDirectory || cn || + memberOf || mail || nsSshPublicKey || nsAccountLock || userCertificate")(tar + getfilter="(objectClass=posixaccount)")(version 3.0; acl "Enable anyone user + read"; allow (read, search, compare)(userdn="ldap:///anyone");) +aci: (targetattr="displayName || legalName || userPassword || nsSshPublicKey") + (version 3.0; acl "Enable self partial modify"; allow (write)(userdn="ldap:// + /self");) +aci: (targetattr="legalName || telephoneNumber || mobile || sn")(targetfilter= + "(|(objectClass=nsPerson)(objectClass=inetOrgPerson))")(version 3.0; acl "Ena + ble self legalname read"; allow (read, search, compare)(userdn="ldap:///self" + );) +aci: (targetattr="legalName || telephoneNumber")(targetfilter="(objectClass=ns + Person)")(version 3.0; acl "Enable user legalname read"; allow (read, search, + compare)(groupdn="ldap:///cn=user_private_read,ou=permissions,dc=example,dc= + com");) +aci: (targetattr="uid || description || displayName || loginShell || uidNumber + || gidNumber || gecos || homeDirectory || cn || memberOf || mail || legalNam + e || telephoneNumber || mobile")(targetfilter="(&(objectClass=nsPerson)(objec + tClass=nsAccount))")(version 3.0; acl "Enable user admin create"; allow (writ + e, add, delete, read)(groupdn="ldap:///cn=user_admin,ou=permissions,dc=exampl + e,dc=com");) +aci: (targetattr="uid || description || displayName || loginShell || uidNumber + || gidNumber || gecos || homeDirectory || cn || memberOf || mail || legalNam + e || telephoneNumber || mobile")(targetfilter="(&(objectClass=nsPerson)(objec + tClass=nsAccount))")(version 3.0; acl "Enable user modify to change users"; a + llow (write, read)(groupdn="ldap:///cn=user_modify,ou=permissions,dc=example, + dc=com");) +aci: (targetattr="userPassword || nsAccountLock || userCertificate || nsSshPub + licKey")(targetfilter="(objectClass=nsAccount)")(version 3.0; acl "Enable use + r password reset"; allow (write, read)(groupdn="ldap:///cn=user_passwd_reset, + ou=permissions,dc=example,dc=com");) +creatorsName: cn=Directory Manager +modifiersName: cn=Directory Manager +createTimestamp: 20200325015543Z +modifyTimestamp: 20200325015543Z +nsUniqueId: a2b3322c-6e3b11ea-8de0c78c-83e27eda + +# entry-id: 5 +dn: ou=permissions,dc=example,dc=com +objectClass: top +objectClass: organizationalunit +ou: permissions +creatorsName: cn=Directory Manager +modifiersName: cn=Directory Manager +createTimestamp: 20200325015543Z +modifyTimestamp: 20200325015543Z +nsUniqueId: a2b3322d-6e3b11ea-8de0c78c-83e27eda + +# entry-id: 6 +dn: ou=services,dc=example,dc=com +objectClass: top +objectClass: organizationalunit +ou: services +aci: (targetattr="objectClass || description || nsUniqueId || cn || memberOf | + | nsAccountLock ")(targetfilter="(objectClass=netscapeServer)")(version 3.0; + acl "Enable anyone service account read"; allow (read, search, compare)(userd + n="ldap:///anyone");) +creatorsName: cn=Directory Manager +modifiersName: cn=Directory Manager +createTimestamp: 20200325015544Z +modifyTimestamp: 20200325015544Z +nsUniqueId: a2b3322e-6e3b11ea-8de0c78c-83e27eda + +# entry-id: 7 +dn: uid=demo_user,ou=people,dc=example,dc=com +objectClass: top +objectClass: nsPerson +objectClass: nsAccount +objectClass: nsOrgPerson +objectClass: posixAccount +uid: demo_user +cn: Demo User +displayName: Demo User +legalName: Demo User Name +uidNumber: 99998 +gidNumber: 99998 +homeDirectory: /var/empty +loginShell: /bin/false +nsAccountLock: true +creatorsName: cn=Directory Manager +modifiersName: cn=Directory Manager +createTimestamp: 20200325015544Z +modifyTimestamp: 20200325061615Z +nsUniqueId: a2b3322f-6e3b11ea-8de0c78c-83e27eda +entryUUID: INVALID_UUID + +# entry-id: 8 +dn: cn=demo_group,ou=groups,dc=example,dc=com +objectClass: top +objectClass: groupOfNames +objectClass: posixGroup +objectClass: nsMemberOf +cn: demo_group +gidNumber: 99999 +creatorsName: cn=Directory Manager +modifiersName: cn=Directory Manager +createTimestamp: 20200325015544Z +modifyTimestamp: 20200325015544Z +nsUniqueId: a2b33230-6e3b11ea-8de0c78c-83e27eda +entryUUID: f6df8fe9-6b30-46aa-aa13-f0bf755371e8 + +# entry-id: 9 +dn: cn=group_admin,ou=permissions,dc=example,dc=com +objectClass: top +objectClass: groupOfNames +objectClass: nsMemberOf +cn: group_admin +creatorsName: cn=Directory Manager +modifiersName: cn=Directory Manager +createTimestamp: 20200325015545Z +modifyTimestamp: 20200325015545Z +nsUniqueId: a2b33231-6e3b11ea-8de0c78c-83e27eda + +# entry-id: 10 +dn: cn=group_modify,ou=permissions,dc=example,dc=com +objectClass: top +objectClass: groupOfNames +objectClass: nsMemberOf +cn: group_modify +creatorsName: cn=Directory Manager +modifiersName: cn=Directory Manager +createTimestamp: 20200325015545Z +modifyTimestamp: 20200325015545Z +nsUniqueId: a2b33232-6e3b11ea-8de0c78c-83e27eda + +# entry-id: 11 +dn: cn=user_admin,ou=permissions,dc=example,dc=com +objectClass: top +objectClass: groupOfNames +objectClass: nsMemberOf +cn: user_admin +creatorsName: cn=Directory Manager +modifiersName: cn=Directory Manager +createTimestamp: 20200325015545Z +modifyTimestamp: 20200325015545Z +nsUniqueId: a2b33233-6e3b11ea-8de0c78c-83e27eda + +# entry-id: 12 +dn: cn=user_modify,ou=permissions,dc=example,dc=com +objectClass: top +objectClass: groupOfNames +objectClass: nsMemberOf +cn: user_modify +creatorsName: cn=Directory Manager +modifiersName: cn=Directory Manager +createTimestamp: 20200325015546Z +modifyTimestamp: 20200325015546Z +nsUniqueId: a2b33234-6e3b11ea-8de0c78c-83e27eda + +# entry-id: 13 +dn: cn=user_passwd_reset,ou=permissions,dc=example,dc=com +objectClass: top +objectClass: groupOfNames +objectClass: nsMemberOf +cn: user_passwd_reset +creatorsName: cn=Directory Manager +modifiersName: cn=Directory Manager +createTimestamp: 20200325015546Z +modifyTimestamp: 20200325015546Z +nsUniqueId: a2b33235-6e3b11ea-8de0c78c-83e27eda + +# entry-id: 14 +dn: cn=user_private_read,ou=permissions,dc=example,dc=com +objectClass: top +objectClass: groupOfNames +objectClass: nsMemberOf +cn: user_private_read +creatorsName: cn=Directory Manager +modifiersName: cn=Directory Manager +createTimestamp: 20200325015547Z +modifyTimestamp: 20200325015547Z +nsUniqueId: a2b33236-6e3b11ea-8de0c78c-83e27eda + diff --git a/dirsrvtests/tests/suites/entryuuid/basic_test.py b/dirsrvtests/tests/suites/entryuuid/basic_test.py index 4d8a40909..41ffbfe10 100644 --- a/dirsrvtests/tests/suites/entryuuid/basic_test.py +++ b/dirsrvtests/tests/suites/entryuuid/basic_test.py @@ -10,6 +10,7 @@ import ldap import pytest import time import shutil +import uuid from lib389.idm.user import nsUserAccounts, UserAccounts from lib389.idm.account import Accounts from lib389.idm.domain import Domain @@ -217,7 +218,7 @@ def test_entryuuid_fixup_task(topology): # 4. run the fix up # For now set the log level to high! - topology.standalone.config.loglevel(vals=(ErrorLog.DEFAULT,ErrorLog.TRACE)) + topology.standalone.config.loglevel(vals=(ErrorLog.DEFAULT,ErrorLog.PLUGIN)) task = plug.fixup(DEFAULT_SUFFIX) task.wait() assert(task.is_complete() and task.get_exit_code() == 0) @@ -242,3 +243,58 @@ def test_entryuuid_fixup_task(topology): euuid_domain_2 = domain.get_attr_val_utf8('entryUUID') assert(euuid_domain_2 == euuid_domain) +@pytest.mark.skipif(not default_paths.rust_enabled or ds_is_older('1.4.2.0'), reason="Entryuuid is not available in older versions") +def test_entryuuid_import_and_fixup_of_invalid_values(topology): + """ Test that when we import a database with an invalid entryuuid + that it is accepted *and* that subsequently we can fix the invalid + entryuuid during a fixup. + + :id: ec8ef3a7-3cd2-4cbd-b6f1-2449fa17be75 + + :setup: Standalone instance + + :steps: + 1. Import the db from the ldif + 2. Check the entryuuid is invalid + 3. Run the fixup + 4. Check the entryuuid is now valid (regenerated) + + :expectedresults: + 1. Success + 2. The entryuuid is invalid + 3. Success + 4. The entryuuid is valid + """ + + # 1. Import the db + ldif_dir = topology.standalone.get_ldif_dir() + target_ldif = os.path.join(ldif_dir, 'localhost-userRoot-invalid.ldif') + import_ldif = os.path.join(DATADIR1, 'localhost-userRoot-invalid.ldif') + shutil.copyfile(import_ldif, target_ldif) + os.chmod(target_ldif, 0o777) + + be = Backends(topology.standalone).get('userRoot') + task = be.import_ldif([target_ldif]) + task.wait() + assert(task.is_complete() and task.get_exit_code() == 0) + + # 2. Check the entryuuid is invalid + account = nsUserAccounts(topology.standalone, DEFAULT_SUFFIX).get("demo_user") + euuid = account.get_attr_val_utf8('entryUUID') + assert(euuid == "INVALID_UUID") + + # 3. Run the fixup + topology.standalone.config.loglevel(vals=(ErrorLog.DEFAULT,ErrorLog.PLUGIN)) + plug = EntryUUIDPlugin(topology.standalone) + task = plug.fixup(DEFAULT_SUFFIX) + task.wait() + assert(task.is_complete() and task.get_exit_code() == 0) + topology.standalone.config.loglevel(vals=(ErrorLog.DEFAULT,)) + + # 4. Check the entryuuid is valid + euuid = account.get_attr_val_utf8('entryUUID') + print(f"❄️ account entryUUID -> {euuid}"); + assert(euuid != "INVALID_UUID") + # Raises an error if invalid + uuid.UUID(euuid) + diff --git a/src/plugins/entryuuid/src/lib.rs b/src/plugins/entryuuid/src/lib.rs index 29a9f1258..ad3faef4b 100644 --- a/src/plugins/entryuuid/src/lib.rs +++ b/src/plugins/entryuuid/src/lib.rs @@ -144,11 +144,17 @@ impl SlapiPlugin3 for EntryUuid { // Error if the first filter is empty? // Now, to make things faster, we wrap the filter in a exclude term. + + // 2021 - #4877 because we allow entryuuid to be strings, on import these may + // be invalid. As a result, we DO need to allow the fixup to check the entryuuid + // value is correct, so we can not exclude these during the search. + /* let raw_filter = if !raw_filter.starts_with('(') && !raw_filter.ends_with('(') { format!("(&({})(!(entryuuid=*)))", raw_filter) } else { format!("(&{}(!(entryuuid=*)))", raw_filter) }; + */ Ok(FixupData { basedn, raw_filter }) } @@ -213,14 +219,20 @@ pub fn entryuuid_fixup_mapfn(e: &EntryRef, _data: &()) -> Result<(), PluginError /* Supply a modification to the entry. */ let sdn = e.get_sdnref(); - /* Sanity check that entryuuid doesn't already exist */ - if e.contains_attr("entryUUID") { - log_error!( - ErrorLevel::Plugin, - "skipping fixup for -> {}", - sdn.to_dn_string() - ); - return Ok(()); + /* Check that entryuuid doesn't already exist, and is valid */ + if let Some(valueset) = e.get_attr("entryUUID") { + if valueset.iter().all(|v| { + let u: Result = (&v).try_into(); + u.is_ok() + }) { + // All values were valid uuid, move on! + log_error!( + ErrorLevel::Plugin, + "skipping fixup for -> {}", + sdn.to_dn_string() + ); + return Ok(()); + } } // Setup the modifications diff --git a/src/slapi_r_plugin/src/pblock.rs b/src/slapi_r_plugin/src/pblock.rs index 718ff2ca7..ac1b3c33b 100644 --- a/src/slapi_r_plugin/src/pblock.rs +++ b/src/slapi_r_plugin/src/pblock.rs @@ -281,7 +281,9 @@ impl PblockRef { } pub fn get_is_replicated_operation(&mut self) -> bool { - let i = self.get_value_i32(PblockType::IsReplicationOperation).unwrap_or(0); + let i = self + .get_value_i32(PblockType::IsReplicationOperation) + .unwrap_or(0); // Because rust returns the result of the last evaluation, we can // just return if not equal 0. i != 0 diff --git a/src/slapi_r_plugin/src/value.rs b/src/slapi_r_plugin/src/value.rs index 46246837a..cd565295c 100644 --- a/src/slapi_r_plugin/src/value.rs +++ b/src/slapi_r_plugin/src/value.rs @@ -1,6 +1,6 @@ use crate::ber::{ol_berval, BerValRef}; use crate::dn::Sdn; -use std::convert::{From, TryFrom}; +use std::convert::{From, TryFrom, TryInto}; use std::ffi::CString; use std::iter::once; use std::iter::FromIterator; @@ -213,6 +213,14 @@ impl TryFrom<&ValueRef> for String { } } +impl TryFrom<&ValueRef> for Uuid { + type Error = (); + + fn try_from(value: &ValueRef) -> Result { + (&value.bvr).try_into().map_err(|_| ()) + } +} + impl TryFrom<&ValueRef> for Sdn { type Error = (); -- 2.31.1