[#1051] Replicate a change made under the Relax Rules control like any other

[vharseko]

Fixes #1051.

The Relax Rules control (1.3.6.1.4.1.4203.666.5.12) was implemented by having LocalBackendModifyOperation and LocalBackendAddOperation report themselves as synchronization operations while the control is present. That let them write attributes marked NO-USER-MODIFICATION. Everywhere else, though, the flag means "replayed from another replica":

• On a replicated suffix the change was never published. LDAPReplicationDomain.synchronize() took it for a replay, remotePendingChanges.commit() found nothing and threw, and ERR_OPERATION_NOT_FOUND_IN_PENDING went to the error log. ds-sync-hist was left alone, so nothing could publish the change on a later session either.
• The request was hidden from the access log and skipped the pre-operation plugins and the password policy.
• An add carrying such an attribute did not get that far. AddOperationBasis refuses it while parsing the attributes, based on the real synchronization flag, which the control never set.

The control now relaxes what it is meant to relax, for a client which may use it:

• LocalBackendWorkflowElement.isRelaxRulesRequested() reads the request. These checks consult it:
  • the checks on NO-USER-MODIFICATION and OBSOLETE attributes;
  • the schema check of the resulting entry;
  • AddOperationBasis.
• The rules are relaxed only for the bypass-acl privilege of the identity the request runs as. A control that the control ACI does not let through is dropped, and when it is non-critical the request goes on as an ordinary one (RFC 4511 4.1.11).
  • Modify: the flag is set once the controls are processed, after the disallowed controls are removed and the proxied authorization is applied, as before A Relax Rules change on a replicated suffix is not replicated and logs an internal error #1051.
  • Add: the checks run before the controls are processed. So the flag is decided up front, from the control and the privilege of the authentication identity. After the controls are processed, the privilege check verifies it again: a control still there needs bypass-acl on the identity the request runs as, and a control dropped after the rules were relaxed refuses the request.
• The values the client supplies are kept, the way the replay on another replica keeps them. This covers creatorsName, createTimestamp, modifiersName and modifyTimestamp (LastModPlugin), and pwdChangedTime (the password policy of the add and of the modify). A pre-encoded password is accepted.
• Otherwise the operations are ordinary: they get a CSN, update ds-sync-hist and are published to the replication servers. There the replay is already a synchronization operation and accepts the relaxed attributes without the control.

Tests:

• RelaxRulesReplicationTest runs a relaxed modify and a relaxed add over a replicated suffix, with a broker listening on the replication server. Each change must be published for its entry, carry the relaxed attribute, and leave historical information behind: the attribute's value for the modify, dn:<csn>:add for the add. Neither may be reported as a missing pending change. Both failed before the fix. The modify published nothing, and the add ended with Unwilling to Perform ... NO-USER-MODIFICATION.

• RelaxRulesTestCase covers:

  • the pre-operation plugins still running;
  • the last modified values and a migrated password with its change time being kept, on add and on modify;
  • the schema check and the OBSOLETE check being relaxed;
  • a non-critical control that a client may not use being ignored;
  • a client without bypass-acl, and a bypass-acl client proxying as one, relaxing nothing.

  The cases on dropped controls remove the test configuration's global (targetcontrol="*") ACI for their duration.

• Stay green: LastModPluginTestCase (61), AddOperationTestCase (138), ModifyOperationTestCase (936) and ReplicationRepairControlTest.

The reference appendix (asciidoc and docbook) gains a description of what the control does in OpenDJ.

Related: #1050, #1052 (the replication repair control, which is the tool for a change that must stay on one replica).

[maximthomas]

praise: The fix removes the cause of #1051 instead of working around it in replication.

• The isSynchronizationOperation() overrides in LocalBackendModifyOperation and LocalBackendAddOperation are gone, so a relaxed change gets a CSN, ds-sync-hist and a published update like any other.
• aRelaxedModifyIsPublishedLikeAnyOtherChange pins the published ModifyMsg, its mods, the ds-sync-hist value and the absence of ERR_OPERATION_NOT_FOUND_IN_PENDING. It matches that record by msgID, so the check does not depend on the locale.

---

issue (blocking): relaxRules also counts a Relax Rules control that removeAllDisallowedControls later drops, so a client without bypass-acl that sends it non-critical now has its write refused.

opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java:174, :544, LocalBackendAddOperation.java:132, :419

The flag is final and is read from the raw request controls in the constructors. removeAllDisallowedControls (LocalBackendWorkflowElement.java:264-287, called at modify :617 and add :911) removes a non-critical control that the control ACI denies. The default global ACIs (config.ldif:84/86) do not list 1.3.6.1.4.1.4203.666.5.12. The control-loop arms that used to set the flag after that removal are only comments now. As a result, relaxRules && !hasPrivilege(BYPASS_ACL) returns INSUFFICIENT_ACCESS_RIGHTS for a control the server has already discarded. At the base the request ran as an ordinary one, which is what RFC 4511 4.1.11 asks for. Reading the flag again after the strip is not enough on the add road, because checkHasReadOnlyAttributes (:306) and the schema check (:393) run before processControls (:406). Instead, relax only for a client that may use the control, and refuse only a control that survived the strip:

// constructors (modify shown; add is the same with `add`)
relaxRules = LocalBackendWorkflowElement.isRelaxRulesRequested(modify)
    && modify.getClientConnection().hasPrivilege(Privilege.BYPASS_ACL, modify);

// operationIsAllowed() / processAdd(): after removeAllDisallowedControls has run
if (!getAccessControlHandler().isAllowed(this)
    || (LocalBackendWorkflowElement.isRelaxRulesRequested(this) && !relaxRules))

AddOperationBasis can keep reading the raw controls, because checkHasReadOnlyAttributes then refuses a stripped control's NO-USER-MODIFICATION attribute with CONSTRAINT_VIOLATION. Not run: no test binds as a user without bypass-acl and sends the control (see the privilege-check pin below).

---

question (blocking): Is it intended that LastModPlugin overwrites modifyTimestamp/modifiersName, and createTimestamp/creatorsName on an add, when a client writes them under the control?

opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java:326, opendj-server-legacy/src/main/java/org/opends/server/plugins/LastModPlugin.java:132, :138, :166, :184

A relaxed change now runs the pre-operation plugins. LastModPlugin is enabled by default, and it appends an unconditional REPLACE of modifiersName/modifyTimestamp after the client's mods, and setAttributes creatorsName/createTimestamp on an add. By contrast, EntryUUIDPlugin.java:165 leaves a client's entryUUID alone. So a relaxed REPLACE modifyTimestamp with a past value returns SUCCESS, but the value stored and replicated is "now". At the base the plugins were skipped and the client's value was kept. This also contradicts the new doc sentence that the control "lets a client … add or modify attributes marked NO-USER-MODIFICATION".

• If not intended: this is an issue, and the fix is to have LastModPlugin skip an attribute the relaxed request supplies, the way EntryUUIDPlugin does. For an add, check getOperationalAttributes(); for a modify, check for a client modification of that type. Gate both on LocalBackendWorkflowElement.isRelaxRulesRequested(op).
• If intended: add one clause to both doc files saying that the server still sets these four attributes.

---

issue (non-blocking): The add case does not check the relaxed attribute in the published AddMsg or the entry's ds-sync-hist, although the description says each case checks both.

opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/RelaxRulesReplicationTest.java:174-177

aRelaxedAddIsPublishedLikeAnyOtherChange checks the local pwdChangedTime, that the update is an AddMsg for ADDED_DN, and that no not-in-pending record was written. A road that publishes the add without pwdChangedTime, or without the add's historical value (dn:<csn>:add, EntryHistorical.java:272), stays green. Not run: whether today's AddMsg carries the attribute; the pin below would show it.

// after :175
assertThat(((AddMsg) published).getAttributes())
    .as("the published add does not carry the relaxed attribute")
    .anyMatch(attr -> attr.getAttributeDescription().getAttributeType().hasName(RELAXED_ATTRIBUTE));
assertThat(valuesOf(ADDED_DN, "ds-sync-hist"))
    .as("the relaxed add left no historical information")
    .anyMatch(value -> value.startsWith("dn:") && value.endsWith(":add"));

---

question (non-blocking): Is carrying a hashed password together with its original pwdChangedTime a supported use of the control?

opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java:802

passwordChanged has no relax arm. For a relaxed REPLACE userPassword + REPLACE pwdChangedTime=<old>, setPasswordChangedTime() (:1530) and applyModifications(getModifications()) (:1557) replace the client's value with "now". A pre-encoded password is refused at :996-1003 unless allow-pre-encoded-passwords is set. The description says running the password policy is intended, but it does not mention that the relaxed value is lost. If this use is supported, it is an issue: skip setPasswordChangedTime() when the relaxed request supplies pwdChangedTime. If it is not, a clause in the doc sentence covers it.

---

suggestion (non-blocking): No test sends the control from a client without bypass-acl. On the add road, the privilege check is now the only barrier.

opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java:419, LocalBackendModifyOperation.java:544

Both relax test classes bind as cn=Directory Manager. AddOperationBasis and checkHasReadOnlyAttributes let NO-USER-MODIFICATION attributes through whenever the control is present. So a mutant that drops relaxRules && !hasPrivilege(BYPASS_ACL) survives the suite, and with that mutant any client could write entryUUID or createTimestamp.

Pin: bind as a user without bypass-acl, with a global ACI (targetcontrol="1.3.6.1.4.1.4203.666.5.12")(version 3.0; acl "relax"; allow(read) userdn="ldap:///all";) so the control survives the strip (without it, a critical control ends with UNAVAILABLE_CRITICAL_EXTENSION before the check). A relaxed add carrying pwdChangedTime must end with INSUFFICIENT_ACCESS_RIGHTS, and the entry must not exist. Without the ACI, the same user's modify carrying the control non-critical and no relaxed attribute must return SUCCESS; that pins the blocking issue above.

---

suggestion (non-blocking): Nothing pins the schema-check and OBSOLETE arms of the relaxation.

opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java:1206, :739-744, LocalBackendAddOperation.java:391

Every relax test writes pwdChangedTime. It is operational, not OBSOLETE, and a valid value, and the objectClass check reads user attributes only. So removing && !relaxRules from mustCheckSchema() or from the add's schema check, or narrowing the OBSOLETE arm back to isInternalOrSynchro(m), leaves the tests green.

Pin: a relaxed add, or a relaxed modify, of mail on an objectClass: person entry succeeds with the control and ends with OBJECTCLASS_VIOLATION without it.

---

suggestion (non-blocking): Nothing pins the other effects the PR restores: pre-operation plugins, password policy, access log, and the replay without the control.

opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/RelaxRulesReplicationTest.java:125-155

The mutant if (!isSynchronizationOperation() && !relaxRules) in invokePreModifyPlugins (LocalBackendModifyOperation.java:326) survives, because ds-sync-hist comes from the replication domain's conflict handling, not from a plugin.

Pin: after the relaxed modify, assert that modifiersName is cn=Directory Manager. Settle the LastMod question above first.

---

issue (non-blocking): When the add case fails, the modify case fails too, with a misleading message.

opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/RelaxRulesReplicationTest.java:134

The cases run in name order, so aRelaxedAdd… runs first on the class-scoped broker, and nothing drains the broker between cases. If the add case fails at :171, its AddMsg(ADDED_DN) stays queued. The bare isNotNull() at :134 then accepts it as the modify case's setup add, and :144 reports "the relaxed modify was not published" even though the modify road works.

final LDAPUpdateMsg setupAdd = nextUpdate();
assertThat(setupAdd).as("the add of the entry to modify was not published").isInstanceOf(AddMsg.class);
assertThat((Object) setupAdd.getDN()).isEqualTo(DN.valueOf(MODIFIED_DN));

[vharseko]

Round 2 is pushed as 5beeffb. The branch was rebased onto the current master first. Point by point:

issue (blocking), a control removeAllDisallowedControls drops: accepted. The fix differs from the suggested snippet, because that snippet opens a hole under proxied authorization. In the constructor, hasPrivilege(BYPASS_ACL, add) runs before evaluateProxyAuthControls, so it checks the authentication identity. Take a client with bypass-acl and proxied-auth that acts as an ordinary user and sends a relaxed add. relaxRules is true, and the early checks (AddOperationBasis, checkHasReadOnlyAttributes, the schema check) run relaxed. After the strip, isRelaxRulesRequested(this) && !relaxRules is false, so nothing refuses the request. I ran that mutant: the proxied add of pwdChangedTime returns Success.

What the round does instead:

• modify: processRequestControls runs before every check the control relaxes. So relaxRules is set in the control loop again, after the strip and after the proxied authorization, as it was before A Relax Rules change on a replicated suffix is not replicated and logs an internal error #1051. operationIsAllowed() keeps the privilege check, which runs against the identity the request runs as.
• add: the checks run before the controls are processed, so relaxRules is decided up front: control present and bypass-acl on the authentication identity. At the privilege check, relaxedRulesStillHold() verifies it again once the strip and the proxied authorization have happened:
  • a control still there needs bypass-acl on the identity the request runs as;
  • a control that is gone, after the rules were relaxed on the early checks, refuses the request;
  • a control that is gone and relaxed nothing leaves an ordinary request, as RFC 4511 4.1.11 asks.

question (blocking), LastModPlugin: not intended. The plugin now keeps creatorsName/createTimestamp when a relaxed add supplies them. It also keeps modifiersName/modifyTimestamp when a relaxed modify modifies them. This keeps the client's value the way the replay on another replica keeps it. The plugin still sets anything the client does not supply.

question (non-blocking), pwdChangedTime with a hashed password: supported, and handled on both roads:

• the modify skips setPasswordChangedTime() when the relaxed request modifies pwdChangedTime;
• the add does not overwrite a pwdChangedTime the relaxed request supplies. handlePasswordPolicy() put "now" there as well, which the review did not list;
• a pre-encoded password is accepted under the control. Before A Relax Rules change on a replicated suffix is not replicated and logs an internal error #1051 the control skipped the password policy altogether, so the migration of hashed passwords worked, and without this it would regress.

Both doc files list what is kept.

issue (non-blocking), add-case assertions: added. The case now checks that the published AddMsg carries pwdChangedTime and that the entry has a dn:<csn>:add value in ds-sync-hist.

issue (non-blocking), the broker shared between the cases: fixed differently. nextUpdate() is now nextUpdateFor(dn), which skips the updates of other entries. An update left behind by a failed case can no longer be taken for the one the next case waits for, whatever the order.

suggestion (non-blocking), a client without bypass-acl: added to RelaxRulesTestCase. One point about the pin as written: the test configuration has a global (targetcontrol="*") ACI, so a control is never stripped there. The cases therefore remove that ACI for their duration.

• aClientWithoutBypassAclMayNotRelaxTheRules: with only a Relax Rules control ACI, the user's relaxed add and modify are refused and change nothing. A relaxed add with no relaxed attribute ends with INSUFFICIENT_ACCESS_RIGHTS.
• aNonCriticalControlTheClientMayNotUseIsIgnored: with no control ACI, the same user's non-critical control is dropped, and the add and modify succeed as ordinary ones.
• aProxiedIdentityWithoutBypassAclMayNotRelaxTheRules: the proxied case above, with the control kept and with it dropped.

suggestion (non-blocking), schema check and OBSOLETE: added. The mail on person pin covers the schema check only, and the default schema has no OBSOLETE attribute type. So aRelaxedModifyMayWriteAnObsoleteAttribute adds one to the schema for its duration and writes it on an extensibleObject entry. aRelaxedAddSkipsTheSchemaCheck and aRelaxedModifySkipsTheSchemaCheck cover the schema check.

suggestion (non-blocking), pre-operation plugins: added as aRelaxedModifyStillRunsThePreOperationPlugins (modifiersName after a relaxed modify of description).

Mutants, each red against the new tests:

• the suggested privilege snippet;
• the add without its privilege check;
• the modify flag read from the raw request;
• mustCheckSchema() without !relaxRules;
• the add schema check without !relaxRules;
• the OBSOLETE arm narrowed to isInternalOrSynchro(m);
• invokePreModifyPlugins skipped under the control.

Green: RelaxRulesTestCase (12), RelaxRulesReplicationTest (2), LastModPluginTestCase (61), AddOperationTestCase (138), ModifyOperationTestCase (936), ReplicationRepairControlTest. javadoc under JDK 11 is clean, with a negative control.

[maximthomas]

praise: The round-2 fix closes the privilege hole in the right place on both roads.

• relaxedRulesStillHold() checks the add's up-front decision again against the identity the request runs as. The proxied-auth hole that the round-1 snippet would have opened stays closed.
• The modify sets relaxRules in the control loop, after removeAllDisallowedControls and the proxied authorization (LocalBackendModifyOperation.java:706).
• nextUpdateFor(dn) stops a failed case from leaving an update behind that the next case would take for its own.