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: #4877 Author: William Brown <william@blackhats.net.au> Review by: @mreynolds389
389ds · Aug 23, 2021 · 193ae2f · 193ae2f
1 parent bce941e
commit 193ae2f
Show file tree

Hide file tree

Showing 5 changed files with 327 additions and 12 deletions.
diff --git a/dirsrvtests/tests/data/entryuuid/localhost-userRoot-invalid.ldif 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
@@ -10,6 +10,7 @@
 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
@@ -218,7 +219,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)
@@ -243,3 +244,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
@@ -144,7 +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.
-        let raw_filter = format!("(&{}(!(entryuuid=*)))", raw_filter);
+
+        // 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 })
     }
@@ -209,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::Trace,
-            "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<Uuid, _> = (&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
@@ -313,7 +313,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
@@ -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<Self, Self::Error> {
+        (&value.bvr).try_into().map_err(|_| ())
+    }
+}
+
 impl TryFrom<&ValueRef> for Sdn {
     type Error = ();