#51171 EntryUUID vs nsUniqueId can confuse syncrepl
Closed: wontfix 3 years ago by spichugi. Opened 3 years ago by firstyear.

Issue Description

Depending on the client, due to EntryUUID plugin now existing, this can cause the sent entryuuid in syncrepl (derived from nsunique id) to diverge from the entryuuid attribute of the entry. Especially when replicating to openldap, this causes some excellent mayhem, where entries can be deleted or not added.

When EntryUUID is available, this is used as the uuid for the syncrepl item, if it is not available, we use nsuniqueid.


Metadata Update from @firstyear:
- Custom field origin adjusted to None
- Custom field reviewstatus adjusted to None
- Issue marked as blocking: #50544

3 years ago

Metadata Update from @mreynolds:
- Issue priority set to: major
- Issue set to the milestone: 1.4.4

3 years ago

Metadata Update from @firstyear:
- Issue marked as blocking: #51174

3 years ago

@mreynolds this is the issue blocking rust by default. I'm going to work on it shortly :)

So I'm wondering about what the approach should be here with this. When adding entryuuid, it all worked just find except for the fact I didn't consider syncrepl.

The possibilities for entryuuid generation were:

  • nsUniqueId always generates entryuuid (imported entryuuid is ignored)
  • entryuuid always defines nsuniqueid (existing nsuniqueid ignored/reset)
  • nsUniqueId may generate entryuuid (imported entryuuid diverges from nsuniqueid)
  • entryuuid may define nsuniqueid (imported nsuniqueid diverges from entryuuid)
  • nsUniqueId and entryuuid are unrelated/divergent

I choose to implement the last option, where these ID's diverge and are unrelated. This is for a few reasons, mainly around data import/migration.

For example, a site with 389-ds and openldap wishes to merge their directories. In this case if cn=user was present in both, applications that primary-key a user from nsuniqueid/entryuuid must continue to work, so we import the entryuuid to the user. This eliminate the first two options where one id must directly define the other.

We can also imagine a situation where during testing a user is added/deleted to test a certain code path. I've certainly seen and done this before. Because 389-ds replication uses the nsuniqueid as an immutable primarykey of the entry to associate changes in replication we could imagine that if we added and deleted a user with an imported entryuuid, that this could affect the nsuniqueid if we generated the nsuniqueid from entryuuid. In the case of entryuuid "may" define nsunique, we'd need to account for a situation where when the entry was deleted, the tombstone containing UUID=X still exists, but then the subsequent add tries to add another entry with the nsuniqueid of X generated from the entryuuid. And I don't feel like adding that code because it could go sideways, fast.

Regardless, we still have a situation where it must be the case that entryuuid and nsuniqueid must be able to diverge and be unrelated. This leaves two options for implementation.

  • nsUniqueId may generate entryUUID if no entryUUID is provided on import
  • nsUniqueId and EntryUUID are always divergent

Regardless of what was chosen, we must and have to account for these values diverging. This makes the simplest option to always have these values seperate and divergent because then we always have to handle that situation.

The would normally not be a problem, because it's generally up to applications to interpret these. The majority of applications use entryuuid as the primary key in the wild. But an issue was already somewhat hinted at in that nsuniqueid is 389's primary key to an object for replication. This is true of entryuuid and SyncRepl for OpenLDAP.

This is where SyncRepl gets things to be a little bit messy for us. SyncRepl as a protocol places the entryuuid into the ldap messages that are sent. But in addition the entryuuid attribute is also in the entry that is sent in the search results. As our SyncRepl uses nsuniqueid in the ldap msg uuid, once we add an entryuuid entry attribute some syncrepl clients can become confused about the divergence of these ID's.

That means we have to account for this in syncrepl.

I've thought about this a lot and I think the solution is that in syncrepl, we do:

if entry has entryuuid {
    uuid = entryuuid
} else {
    uuid = nsuniqueid
}

An alternate option is to strip the entryuuid from the entries before we send them, leaving only the entryuuid in the ldap msg. This way clients that care about the entryuuid attribute could re-add it into the entries from the value in the ldapmsg. I'd want to check that with 389-ds to openldap syncrepl to be sure, as openldap as a syncrepl client has shown to be very sensitive to these changes so far.

This seems like the most consistent way to resolve this. Thoughts? ping @tbordaz @mreynolds. Thanks!

It would help if you describe some precise use case that you want to address.

as long as nsuniqueid and entryuuid are unique I do not see a concern if they diverge. My concern is rather if entryuuid exists or not (assuming nsuniqueid always exists).

sync repl is based on retroCL that logs, for each update, the unique identifier of the entry. It can be nsuniqueid or entryuuid but it can not be both. In your case, I understand that sync repl client expect 'entryuuid' and retroCL can be configured with this attribute. Of course it requires that entryuuid is always present . In your use case, does it exists 389ds entries without entryuuid ?

In sync_repl, I found a couple of cases where there is an assumption that the retroCL key is nsuniqueid (sync_send_entry_from_changelog and sync_deleted_entry_from_changelog) this can be fixed but likely it exists others cases.

Regarding your suggested fix, it could work but requires a similar fix in retroCL to know which key to record.

It would help if you describe some precise use case that you want to address.

Not breaking sync repl seems like the precise use case :)

as long as nsuniqueid and entryuuid are unique I do not see a concern if they diverge. My concern is rather if entryuuid exists or not (assuming nsuniqueid always exists).

That's also something I thought about too ... but I think it won't be a problem either. So the scenarios are:

  • entryuuid plugin is disabled - behaves like everything is today with nsuniqueid
  • entryuuid plugin, every entry has a uuid - every client application of syncrepl uses the entryuuid as the objectidentifier.

The tricky case is when you enable entryuuid later but don't run the fixup. That means that only a subset of entries have entryuuid.

In this case, we have say cn=william which only has nsuniqueid. In this case the syncrepl client application still tracks us based on nsuniqueid. If we had cn=thierry who was added later, and he does have an entryuuid, then sync repl tracks him based on entryuuid. So that would be no problem. Syncrepl still knows which entry is which, because it still gets an entryuuid in the ldapmsg header and those ID's remain stable.

The tricky part is when the fixup is run. In that case cn=william's syncrepl uuid would change from the nsuniqueid to the entryuuid. BUT in this case, it would actually be fine. Most applications would then see the "abscence of cn=william referenced by nsuniqueid" as a delete, and then see the "presence of cn=william referenced by entryuuid" as an add. Because these fixups would be entered into the retrocl then the cookie would advance so these changes would have to be sent.

As well, many applications for syncrepl (ie bind in ipa) only keep the data in memory as I understand it.

A possibility is that we make syncrepl and entryuuid a requirement of syncrepl plugin in the future to resolve this because then we don't need to deal with a nsuniqueid/entryuuid split.

sync repl is based on retroCL that logs, for each update, the unique identifier of the entry. It can be nsuniqueid or entryuuid but it can not be both. In your case, I understand that sync repl client expect 'entryuuid' and retroCL can be configured with this attribute. Of course it requires that entryuuid is always present . In your use case, does it exists 389ds entries without entryuuid ?

In sync_repl, I found a couple of cases where there is an assumption that the retroCL key is nsuniqueid (sync_send_entry_from_changelog and sync_deleted_entry_from_changelog) this can be fixed but likely it exists others cases.

Regarding your suggested fix, it could work but requires a similar fix in retroCL to know which key to record.

I don't think it affects retrocl. A client can request a specific subset of entries in syncrepl by using filters. Those filters would be able to reference attributes like nsuniqueid/entryuuid, but they'll always resolve to the nsuniqueid of the entry meaning that the retrocl as it exists and operates today won't need to change.

So even if we say put entryuuid into the ldapmsg headers to conform to syncrepl's rfc, then the client would do an ldap search for entryuuid=X, which we'd resolve back to the nsuniqueid and retrocl would be fine.

Generally, the entryuuid in the ldapmsg is just about giving the client a unique way to track entries. Most clients probably don't care that entryuuid/nsuniqueid don't match what's in the ldapmsg - they only case that the ldapmsg uuid remains consistent and identifying entries correctly.

So I think no changes to retrocl would be needed in this case because we are not changing how we internally track entry identifiers, but we are making it so that syncrepl's tracking of entry identifiers matches what clients expect. Which means it's the above cases that are tricky :)

Deployment where all entries have 'nsunique' or all entries have 'entryuuid' is a simple case. It requires tuning of retroCL to select the appropriate key and a couple of fix in sync_repl so that it uses that key to retrieve the entries.

If retroCL uses 'entryuuid' but some entries a missing 'entryuuid' is a broken scenario. We can do fallback to other key but likely it will end in problem. May be we can add 'entryuuid' preop to force and entryuuid key. A concern is if sync_repl starts tracking an entry with a key and then swith to an other key.

Deployment where all entries have 'nsunique' or all entries have 'entryuuid' is a simple case. It requires tuning of retroCL to select the appropriate key and a couple of fix in sync_repl so that it uses that key to retrieve the entries.

If retroCL uses 'entryuuid' but some entries a missing 'entryuuid' is a broken scenario. We can do fallback to other key but likely it will end in problem. May be we can add 'entryuuid' preop to force and entryuuid key. A concern is if sync_repl starts tracking an entry with a key and then swith to an other key.

But again, that's all internal to our server. Internally we'll always use nsuniqueid so this has no impact on our retrocl behaviour.

This is purely about what syncrepl sends in the ldap protocol messages to the consuming clients so that the external view is consistent. And when the client then does a search for those entries, then our server will always resolve to the right nsuniqueid anyway.

Ahhhhh I found a place where this doesn't hold true. In a sync_send_deleted we send the uuid as the recorded uuid from the retrocl, and we don't have access to the entry anymore. So this would be a problem case then where we would need entryuuid to be in retrocl.

I think I need to think about this some more, because it's hard to see a way to balance all the requirements, because I'm pretty hesitant to change the retrocl code, but I also think it's important we can import entryuuid's from existing directories.

Ahhhhh I found a place where this doesn't hold true. In a sync_send_deleted we send the uuid as the recorded uuid from the retrocl, and we don't have access to the entry anymore. So this would be a problem case then where we would need entryuuid to be in retrocl.

I think I need to think about this some more, because it's hard to see a way to balance all the requirements, because I'm pretty hesitant to change the retrocl code, but I also think it's important we can import entryuuid's from existing directories.

this is also true for nsuniqueid in deletes and therefor retrocl has to be configured to keep it: https://www.port389.org/docs/389ds/design/content-synchronization-plugin.html#access-to-nsuniqueid
you could probably do the same for entryuuid

This is a really good tip @elkris and it helps me understand more about how retrocl works. I think I want to read the code some more before I propose what a possible way to resolve this is. Thank you!

So I've reviewed this again with fresh eyes and I think there are two ways to tackle it.

One is that we remove entryuuid from entries as we send them in syncrepl, so that clients that require the entry uuid at the other end have to synthesise it from the nsuniqueid. But I think this is not a great solution, as it does mean tampering with entries, and the consuming application may want the entryuuid (and for it to be the same as entryuuid in 389!) So I'm probably not in favour of this, but it's an option.

The other option is as mentioned - use the entryuuid if present, or fall back to nsuniqueid. I think this is possible if we:

  • Change sync_create_state_control use entryuuid if available else use nsuniqueid. This function is where entries are added into the stream of changes for add/modify/modrdn. It's called from a variety of places throughout syncrepl
  • Extend struct sync_update to have an entryuuid field. The field is populated in sync_read_entry_from_changelog using the entryuuid we store in the retrocl
  • Change sync_send_deleted_entries to use struct sync_update.entryuuid if not NULL, else use upd_uuid as today.

This configuration relies on the entryuuid to come from different sources (the entry, and the retro CL). This is because sync_create_state_control can be triggered by persistent search, which doesn't interact with the changelog, so we can't rely on the struct sync_update fields to be populated. Of course, this could create inconsistencies if retrocl is not configured to store the entryuuid, but this situation already exists, as syncrepl requires retrocl to store the nsuniqueid of the entry in the targetUniqueId attribute.

Since this posses a possible misconfiguration risk, we could consider using the new on-upgrade framework to fix-up the retrocl config by always ensuring that the values listed exist.

nsslapd-attribute: entryUUID:targetEntryUUID
nsslapd-attribute: nsuniqueid:targetUniqueId

I have already tested, and if an entry does NOT have an entryUUID, then targetEntryUUID isn't writen to the retrocl, and no other issues are encountered.

In general, I think that it could be a positive thing to have these values automatically configured, because that reduces human error when managing the directory, and means features "just work". Of course, there could be unforseen consequences, so I would appreciate your thoughts on this both @tbordaz and @elkris (if you are not too busy enjoying 'PTO' :) )

At the very least I think that extending the sync plugin test suite that I have added is going to be a requirement for this to ensure we understand the various situations. So I need to add some extra tests in there for modrdn and different scopes I think.

Looking forward to your review :)

PS: it's probably worth mentioning that I think this issue is really a edge case. Most applications won't check the entryuuid in the entry, and most people won't use syncrepl (outside ipa). Even then, the scenario this fixes is openldap as a readonly replica from 389 and that's also a rare situation that as yet, and we still could impose limitations and conditions on that feature to avoid complexity.

The unique identifier of the entry needs to registered in retroCL and sync_repl relies on it to send updates (both in the correct order and the appropriate entry).

At the moment we can configure retroCL to log the unique identifier.
Sync_repl may updated to support a new configured option to use the appropriate unique identifier.

Like I said this should work if the unique identifier is present in the entry (nsuniqueid and/or entryuuid). I think it is getting into trouble if the unique identifier is missing . The fallback option looks (for example priority entryuuid if present else fallback to nsuniqueid) looks dangerous. The worse scenario is if we priority identifier is initially missing and then is added (or the opposite).

IMHO It would be good to simplify the use case and make sure there is only one identifier that always exists and that is stable.

So I've thought about this a lot the last few days. In principle I agree with you. We should have one id and it's stable. But reality is cruel, and the deep histories of seperate projects is now colliding here to make an annoying situation.

Realistically I believe there are two major scenarios that must be considered from the client view.

  • Clients that expect an entryuuid attribute in the entry AND entryuuid in the LDAPMSG AND that both entryuuid values match
  • Clients that use entryuuid in the LDAPMSG and synthesise the entryuuid attribute (ignoring the value in the entr).
  • Clients that only use the entryUUID from the LDAPMSG and ignore the entry entryuuid attribute

I believe at the moment there is only one client that fits in the first category and that is 389-ds replicating to openldap. OpenLDAP very much requires entryUUID as an attribute on the entry and that it matches the entryUUID in the LDAPMSG. This is also important for other applications to ensure the entryUUID is stable across replication.

Most other clients would be the latter two. python-ldap syncrepl utils for example, expose the UUID from the LDAPMSG and don't make indications about the presence of the UUID in the entry, it would be up to a consumer to do further work there. I'm not aware of how bind-dyndb-ldap works internally to know if they attempt to validate the entry attribute values and the LDAPMSG provided uuid. However given that today bind-dyndb-ldap works, and there is no entryuuid plugin, I can only assume it uses the uuid from the LDAPMSG only.

This leads me to conclude that a possible solution is:

  • entries sent to clients should have entryuuid removed to prevent confusion between the LDAPMSG uuid (derived from nsuniqueid) and the entryuuid on the entry.
  • if the recieved cookie is openldap syncrepl mode, the entry is only sent if an entryuuid attribute is present, and the entryuuid attribute is used as the LDAPMSG uuid.

This solution I think strikes a balance between preserving today's behaviour, while allowing the openldap syncrepl mode to exist and proceed. What do you think @tbordaz ?

Thanks @firstyear for this description. I agree with the use cases but I tend to disagree on the conclusion :(

When the server needs to retrieve updates (refresh+cookie, or persistent) it uses unique key to identify the updated entries. That key is 'nsuniqueid'. The way sync_repl retrieves the updates is purely internal and the choice of 'nsuniqueid' to do this is valid (it is unique and always exist) and is not dependent on the sync_repl client.

The way sync repl identify returned entries (syncUUID https://tools.ietf.org/html/rfc4533#section-2.3) is also internal. It uses 'nsuniqueid' for that. It could have been 'entryid' or 'entryuuid' but the important point is that it identifies (unique and stable) the entry. The sync_repl client has to rely on it when it receives the entry along with the sync state control.

I did not find in the RFC a statement that syncUUID is an existing value of the sent entry. SO IMHO client should not expect the the syncUUID exists as an attribute value.

In that sense, the client should not care it exists nsuniqueid, entryuuid or something else. And it should not expect syncUUID to be nsuniqueid, entryuuid or something else.

In conclusion, I would suggest that the client adds a 'syncUUID' to the received entries. When using 389ds syncRepl the resulting entry will have 'nsuniqueid' and 'syncUUID' identical. The received entry will contain nsuniqueid and may contain 'entryuuid'

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 issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/4224

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix
- Issue status updated to: Closed (was: Open)

3 years ago

Login to comment on this ticket.

Metadata