Skip to content

[#992] Apply the confidentiality of a backend index to the running backend - #1000

Open
vharseko wants to merge 4 commits into
OpenIdentityPlatform:masterfrom
vharseko:feature/992-index-confidentiality-applied
Open

vharseko wants to merge 4 commits into
OpenIdentityPlatform:masterfrom
vharseko:feature/992-index-confidentiality-applied

Conversation

@vharseko

@vharseko vharseko commented Sep 9, 2026

Copy link
Copy Markdown
Member

Fixes #992

On master after #997 (b0970cc1a8), which restructured the same method and which this change was stacked on until it merged. Four commits: this change in #997's shape; the fix of a defect of its own found while reviewing #997 (below); the give-up of the write which reopens the tree put back to what the index answered before it (below); and the trust that reopen binds put back with it (below).

DefaultIndex.setConfidential only compared the setting with the one the index was opened with. Nothing moved the CryptoSuite the indexes of the attribute share to the new value, and nothing bound their codecs again, so confidentiality-enabled reached the running backend in neither direction, and the comparison never converged: every later change of that index - index-entry-limit included - untrusted it again and asked for a rebuild which could not help.

The declared admin action of the property is that the index has to be rebuilt (BackendIndexConfiguration.xml:230), not that the component has to be restarted, so this makes the implementation answer for what is documented.

What the change does

AttributeIndex.applyConfigurationChange asks each index it keeps whether its codec was bound under the setting now asked for - DefaultIndex holds the confidentiality it was opened under, setConfidential leaves the Index interface - and for each index whose answer is no:

  • puts the new setting in force on the shared CryptoSuite, through EntryContainer.setIndexConfidentiality, which keeps the cipher of the backend - whenever the suite does not carry the setting asked for. Done before the write which opens the indexes this change adds, so the key hashed index that enabling confidentiality of an equality index creates binds its codec to the new setting and encrypts its records, rather than only hashing its keys. Applied outside the writes, which the storage may replay: it changes a live object and is idempotent;
  • reports the rebuild before the first write, with the entry limit's, as [#991] Decide and report an index configuration change outside the write which is replayed #997 does: the configuration entry holds the new setting whichever way the writes go, so a write the storage gives up on leaves the instruction in the result next to the failure;
  • gives up the tree of the index: one write untrusts and deletes it, a second opens it again, which is what binds its codec to the setting now in force. Both in a branch of their own, which takes and releases the same exclusive access the removal of an index takes; a change which gives up no tree - of the entry limit alone - takes none, and shares the first of the two writes through writeUntrust().

Asked of the index rather than decided from the configuration this attribute index holds, because that configuration is replaced after the first write: after a write the storage gave up on it already read as applied while the index was as it had been - codec bound to the old setting, tree in that encoding, TRUSTED still stored - and the same change asked for again found nothing to do, so the next open of the container read that tree with the codec of the other setting. Asking the index makes the re-apply a change again.

The same gap opens on the second write, the one which reopens the tree: what it binds the index to answering is memory too, and outlives that write's own rollback. A give-up of it is put back to what the index answered before the write ran, so the change asked for again still finds it to give up rather than one already agreeing with a setting that write never durably reached; the first write, asked for again, finds the tree its own earlier give-up already deleted, and does not try to delete it a second time. What the reopen binds includes the trust: an index opened over an empty entry container is trusted on the spot, in memory before the flag is written, and a trust left behind by a give-up is what an operation which then fails writes down for every index in its buffer - TRUSTED stored for a tree the give-up took with it, while the index skipped the entries added meanwhile. It is put back with the codec.

The trees are given up rather than left to the rebuild because their records are in the encoding of the setting being given up, and the codec of the new one does not read them back as what they are: an encrypted record read as clear text decodes to an empty set of entry IDs rather than failing, which is a search answered with no entries at all. An empty untrusted index answers "undefined" instead, and so is not used until the rebuild has run. They are deleted rather than emptied record by record, which for an index of any size would be a transaction of its own making, and in two writes rather than one because the storage engines delete and create the tree of an index as operations of their own - as removing and adding an index does. That order also decides how an interrupted change ends: the write which deletes is the one which untrusts, so what is left is an index the next open of the container creates empty and keeps degraded, never a trusted one holding nothing.

DefaultIndex.codec becomes volatile, since it is now bound again on a live instance other threads are holding, and DefaultIndex.encrypted records what it was bound to; isEncrypted() answers from it, which is also the per-index answer backendstat show-index-status prints. Both - and the trust, which afterOpen binds the same way - are put back to what they were before the reopen if that write's commit does not.

Tests

IndexConfidentialityChangeTestCase, with a subclass per storage engine (PDBIndexConfidentialityChangeTest, JEIndexConfidentialityChangeTest) because deleting and creating the tree of an index is the engine's own operation. Five behaviours each, all failing before the change:

what it pins before
enabling empties the index it asks to rebuild, which then answers "undefined" the tree kept its record
what the index writes next is encrypted the record was written in clear text
the key hashed index the change creates encrypts its records too its records were written in clear text
disabling stops the encryption the record was still encrypted
a later unrelated change leaves the index trusted it was untrusted again, with the rebuild message repeated

The disabling case also checks the tree given up is empty before the index is trusted again, not only what it writes next: a mutant which deletes the tree given up only where confidentiality is being enabled read the disabled record's encrypted bytes back as an empty set with either codec, and passed.

And four cases in ReplayedConfigChangeTest, on a presence index, whose tree a confidentiality change keeps:

what it pins red with
aConfidentialityChangeUntrustsTheIndexWhenTheTransactionIsReplayed: a conflict on the write which gives the tree up is replayed, reported once, and the index writes encrypted after the reopen; a re-apply asks nothing and opens no third write -
aConfidentialityChangeIsReportedWhenTheWriteWhichGivesUpTheTreeGivesUp: a give-up on that write is reported with the failure, leaves TRUSTED stored, and the same change asked for again untrusts, reports once and encrypts the decision taken from the configuration ("still a change"); the report moved behind the writes ("on the road which failed")
aConfidentialityChangeIsReportedWhenTheWriteWhichReopensTheTreeGivesUp: a give-up on the write which reopens the tree - the fourth, once the third has committed - leaves the index still to give up and open again when the same change is asked for, not one already agreeing with the setting that write never durably reached; and the codec that reopen bound is put back the binding stuck: the re-apply found nothing changed and answered SUCCESS with no instruction, and the tree the third write had deleted was never reopened; the codec half of the revert dropped
aGivenUpReopenOverAnEmptyBackendDoesNotLeaveTheIndexTrusted: the same give-up over an empty backend, where the reopen trusts the index it opens, leaves it untrusted; an add which fails in the window writes no TRUSTED down for it; and the re-apply trusts the index it opens again the trust the reopen bound left behind: isTrusted() true over no tree, and the failed add stored TRUSTED for it

Run green: the ten tests of IndexConfidentialityChangeTestCase and the four of ReplayedConfigChangeTest together with EncryptedPDBTestCase, EncryptedJETestCase, PDBTestCase, JETestCase, DefaultIndexTest, EntryIDSetTest, StateTest, OnDiskMergeImporterTest, VLVControlTestCase and ServerSideSortControlTestCase - 270 tests, no failures.

Not in this change

An online rebuild-index of an index whose confidentiality was never changed is unaffected. The offline rebuild already applied the setting correctly, since it opens a container of its own; it still does.

@vharseko
vharseko requested a review from maximthomas September 9, 2026 14:30
@vharseko vharseko added bug security Security fixes / CodeQL code-scanning alerts tests Test suites: fixing, enabling, un-disabling concurrency Thread-safety / race-condition bugs labels Sep 9, 2026
@vharseko
vharseko requested review from maximthomas and removed request for maximthomas September 10, 2026 06:51
@vharseko vharseko added the index Attribute/VLV index subsystem: build, trust, rebuild, confidentiality label Sep 12, 2026
@vharseko
vharseko force-pushed the feature/992-index-confidentiality-applied branch from 9c99def to 21ec570 Compare September 17, 2026 16:51
@vharseko

Copy link
Copy Markdown
Member Author

Round 3 pushed as 21ec570: the change rebased onto #997, and a defect of its own fixed on top.

Stacked on #997. #997 (approved) restructures AttributeIndex.applyConfigurationChange — what the kept indexes are asked for is planned and reported before the first write, and the untrust write opens only when there is something to untrust — and this change restructures the same method, so it is rebased onto #997's head 97c8773023 rather than onto master, with the two commits of the previous head squashed into one (bcf69836fd, same tree as 9c99defefd) and resolved into #997's shape: the confidentiality joins planIndexUpdates, the untrust write becomes writeUntrust(), which also gives the trees up, and the reopen write follows it under the same exclusive access as before. Merge after #997; the stack drops out on its own once #997 is in.

The defect (fixed in 21ec570383). Whether the confidentiality had anything to apply was decided from the configuration this attribute index holds — config.isConfidentialityEnabled() != newConfiguration.isConfidentialityEnabled() — and config is replaced after the first write. On a write the storage gives up (the retry bound spent, or a failure it does not replay) the configuration already read as applied while the index was as it had been: codec bound to the old setting, tree in that encoding, TRUSTED still stored. Asked for again, the change found nothing changed and answered SUCCESS with no instruction, and the next open of the container read that tree with the codec of the other setting — the state this change gives the tree up to avoid. The report of NOTE_CONFIG_INDEX_CONFIDENTIALITY_REQUIRES_REBUILD also sat behind the writes, so the give-up road delivered a stack trace only — the shape of #997's round-1 blocking issue.

Now each DefaultIndex holds the confidentiality its codec was bound to (encrypted, set in afterOpen; isEncrypted() answers from it, which is also the per-index answer backendstat wants), and planIndexUpdates asks the index: isEncrypted() != newConfig.isConfidentialityEnabled() is what puts an index among the trees to give up. The suite the indexes share is put in force by the same comparison against what it carries, since that is idempotent. The instruction is reported before the first write, with the entry limit's.

Pins, in ReplayedConfigChangeTest on a presence index — the type whose tree a confidentiality change keeps; the fixture gains the cipher of the backend and a confidential index configuration:

  • aConfidentialityChangeUntrustsTheIndexWhenTheTransactionIsReplayed: a conflict on the write which gives the tree up is replayed (attempts() == 2), reported once, TRUSTED gone, the index encrypted after the reopen; a re-apply asks nothing of the index and opens no third write.
  • aConfidentialityChangeIsReportedWhenTheWriteWhichGivesUpTheTreeGivesUp: a give-up on that write answers the server error code with the failure named next to the rebuild, leaves TRUSTED stored, and the same change asked for again is a change: untrusts, reports once, and the index writes encrypted after it. Red with the decision taken from the configuration (bcf69836fd) at "still a change", and red at "reported, on the road which failed" with the report moved back behind the writes.

Both cases hold an entry in the backend: an index of an empty backend is trusted when it is opened, whatever its tree holds, so on an empty one the reopen trusts the index again — which is right, and which the cases are not about.

Runs: ReplayedConfigChangeTest 22/22; with PDBIndexConfidentialityChangeTest, JEIndexConfidentialityChangeTest, EncryptedPDBTestCase, EncryptedJETestCase, PDBTestCase, JETestCase, DefaultIndexTest, EntryIDSetTest, StateTest, OnDiskMergeImporterTest, VLVControlTestCase, ServerSideSortControlTestCase — 13 classes, 268 tests, no failures. The reactor run lost the JE confidentiality class to an admin-port collision with a neighbouring test JVM on this machine (startServer, Address already in use); run alone afterwards it is 5/5.

@maximthomas maximthomas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: The change is decided where it can be re-answered, and the reasons are written next to the code.

  • Asking each index what it was opened under (DefaultIndex.encrypted, planIndexUpdates at AttributeIndex.java:1139) instead of the configuration the attribute index holds — a replayed or given-up write is answered again by the index, and setConfidential leaving Index closes the road where it could not converge.
  • The suite switched outside the writes and compared with the suite, not the configuration (EntryContainer.setIndexConfidentiality): idempotent, and in force before the added indexes bind their codecs.
  • The tree given up rather than left to the rebuild, with the exact reason: a V3 record read by the V2 codec decodes to a defined empty set (EntryIDSet.java:490-494, :530-532 — confirmed while reviewing), a search answered with nothing.
  • Two writes for delete and reopen, and the ordering argument at AttributeIndex.java:1025-1029 for which of them untrusts.
  • One test body per behaviour with a subclass per engine, and the two ReplayedConfigChangeTest cases counting writes() and attempts() rather than trusting the result code.

issue (blocking): On PDB a give-up of the reopen write leaves a live index whose tree is gone, and the same change asked for again is a silent no-op.

opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java:1038, :1139
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DefaultIndex.java:111-115

Write 3 commits: TRUSTED removed, the presence tree deleted. Write 4 opens it again, and afterOpen assigns encrypted = cryptoSuite.isEncrypted() (now true) and the V3 codec in memory before the commit. When that commit gives up (PDBStorage.java:696-728), Persistit rolls the tree creation back (measured in #998) while the binding stays: encrypted=true, trusted=false, no tree. The re-apply then finds isEncrypted() == newConfig.isConfidentialityEnabled() at :1139, gives nothing up, and answers SUCCESS with no message — the premise at :938-942 ("the index is as it was") does not hold on this road. Until the rebuild or a restart every cn add/modify and every (cn=*) search throws TreeNotFoundException through DefaultIndex.get (:286; the presence query has no trusted gate, IndexQueryFactoryImpl.java:358-365). JE recreates the tree empty and is not affected. Not run: no case arms write 4.

// DefaultIndex
private volatile EntryIDSetCodec plainCodec;   // V1 or V2, from the COMPACTED flag

@Override
final void afterOpen(WriteableTransaction txn, boolean createOnDemand)
{
  final EnumSet<IndexFlag> flags = state.getIndexFlags(txn, getName());
  plainCodec = flags.contains(COMPACTED) ? CODEC_V2 : CODEC_V1;
  bindCodec(cryptoSuite.isEncrypted());
  trusted = flags.contains(TRUSTED);
  ...
}

/**
 * Binds the codec to the setting given. What afterOpen binds is memory, and outlives a write the
 * storage rolled back — which on PDB takes the tree opened in it with it.
 */
final void bindCodec(boolean encrypted)
{
  this.encrypted = encrypted;
  codec = encrypted ? new EntryIDSet.EntryIDSetCodecV3(plainCodec, cryptoSuite) : plainCodec;
}
// AttributeIndex, around the reopen write (:1029-1039)
try
{
  entryContainer.getRootContainer().getStorage().write(reopen);
}
catch (Exception e)
{
  // The binding the reopen made is memory: put back the setting the index answered before, so that
  // the change asked for again gives the tree up (absent on PDB, empty on JE) and opens it again.
  for (MatchingRuleIndex givenUp : treesToGiveUp)
  {
    givenUp.bindCodec(!newConfiguration.isConfidentialityEnabled());
  }
  throw e;
}

Pin, in ReplayedConfigChangeTest, next to the write-3 give-up case — red at the head on PDB at the addEntry, and red in again on both engines (no rebuild message):

backend.storage.failWithoutReplayOnWrite(4);
final ConfigChangeResult ccr = index.applyConfigurationChange(confidentialPresenceIndexCfg());
assertThat(ccr.getResultCode()).isEqualTo(serverErrorResultCode());
assertThat(persistedFlags(rootContainer, ec, cnIndex.getName())).as("write 3 committed").doesNotContain(TRUSTED);
assertThat(cnIndex.isEncrypted()).as("the setting the tree is still to be opened under").isFalse();

final ConfigChangeResult again = index.applyConfigurationChange(confidentialPresenceIndexCfg());
assertThat(again.getResultCode()).isEqualTo(ResultCode.SUCCESS);
assertThat(ordinalsOf(again)).containsOnly(NOTE_CONFIG_INDEX_CONFIDENTIALITY_REQUIRES_REBUILD.ordinal());
assertThat(cnIndex.isEncrypted()).isTrue();
addEntry(backend, "user.1");   // TreeNotFoundException at the head on PDB

suggestion (non-blocking): The delete of the given-up tree is pinned on the enable side only; a delete-only-when-enabling mutant is green.

opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/IndexConfidentialityChangeTestCase.java:215-236

disablingConfidentialityStopsEncryptingWhatTheIndexWritesNext reads only user.1 after the change. Under if (newConfiguration.isConfidentialityEnabled()) givenUp.delete(txn); in writeUntrust the kept V3 record [0x00, …] is read by the rebound V2 codec as size 0 — a defined empty set — so user.1 is written clear as {id1} and both asserts at :230-231 pass. That is the silent decode the comment at AttributeIndex.java:1085-1090 motivates the delete with, in the one direction it is not pinned (on the enable side the V3 codec falls through to its delegate, so a kept tree reads correctly with or without the delete).

// after attributeIndex.applyConfigurationChange(indexCfg(false, ENTRY_LIMIT)), before trust():
assertThat(recordCount(backend, presenceIndex(attributeIndex))).as("the encrypted tree, given up").isEqualTo(0);

Red under the mutant (recordCount == 1), as :144 already is on the enable side.


suggestion (non-blocking): Both new ReplayedConfigChangeTest cases arm write 3; which of the two writes untrusts is not pinned.

opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ReplayedConfigChangeTest.java:888, :940

The replay case pins writes() == before + 4 (the count) and the give-up case pins TRUSTED present after a write-3 give-up — true both at the head (removal rolled back) and with setTrusted(txn, false) moved into the reopen write (never removed). The ordering AttributeIndex.java:1025-1029 argues for has no case. The write-4 case above pins it: doesNotContain(TRUSTED) after a write-4 give-up is red under that move. While there, the write-3 give-up case could assert treesOf(ec) still holds cnIndex.getName() — whether PDB rolls a deleteTree back is measured for creation only (#998); if it does not, the state between that give-up and the re-apply is a trusted index with no tree.

@vharseko
vharseko force-pushed the feature/992-index-confidentiality-applied branch from 21ec570 to 7a841f9 Compare September 18, 2026 18:58
@vharseko

Copy link
Copy Markdown
Member Author

Rebased onto #997's round 4 (728dd78077). Mechanical: git rebase --onto replayed both commits clean, no conflicts — bcf69836fdfd2baed085, 21ec5703837a841f9055. The diff-stat against the new base matches the old one exactly (same 8 files, same 152 lines in ReplayedConfigChangeTest.java), so nothing of this change's own content moved.

Verified after the rebase: ReplayedConfigChangeTest 22/22, JEIndexConfidentialityChangeTest 5/5, PDBIndexConfidentialityChangeTest 5/5 (run alone — together with the JE test in the same fork it failed startServer on the fixed admin port 65534, a known JVM-level collision between two server tests in one run, not a regression).

Head now 7a841f9055.

@vharseko

Copy link
Copy Markdown
Member Author

Round 4 pushed as 3dd6ca3ca6: the give-up of the reopen put back, and a second defect it exposed fixed alongside it.

The blocking issue. Confirmed exactly as described: DefaultIndex.afterOpen binds encrypted and codec before the write which carries it commits, and nothing put that binding back when a give-up of the reopen (the fourth write, once the third has committed) left the tree it opened rolled back while the binding stayed. A re-apply then found isEncrypted() == newConfig.isConfidentialityEnabled(), answered SUCCESS with no instruction, and the tree the third write had deleted was never reopened.

Fixed by snapshotting what each index in treesToGiveUp answers before the reopen write, and putting it back if that write's commit does not (AttributeIndex.IndexBinding, DefaultIndex.revertFailedReopen). A re-apply now still finds the index to give up and open again.

Putting it back exposed a second failure on the same road, not in the report: the write which untrusts and deletes (the third) runs again on that re-apply — writeUntrust is asked for from planIndexUpdates's fresh answer every time, not just once — and its givenUp.delete(txn) found nothing to delete a second time, since the first attempt's had already committed. writeUntrust now checks txn.treeExists(...) first.

Pinned in ReplayedConfigChangeTest#aConfidentialityChangeIsReportedWhenTheWriteWhichReopensTheTreeGivesUp, arming the fourth write rather than the third the two existing cases arm. Red at isEncrypted() still true after the give-up with only the revert; green with both fixes.

The delete-only-when-enabling mutant. Confirmed, and fixed the way suggested: disablingConfidentialityStopsEncryptingWhatTheIndexWritesNext now asserts recordCount(...) == 0 on the tree given up before trust() is called, next to what it already checked about the record written after. The mutant is red on that assertion now.

Which write untrusts. The new case above pins it as a side effect: persistedFlags(...) no longer contains TRUSTED after the third write commits and the fourth is armed and given up, which is exactly the assertion that would go red if the untrust moved to the fourth write. I left the treesOf(ec) question - whether PDB rolls a deleteTree back on a give-up of the write which issues it, same as it does a tree creation - open: nothing in this round measures it, and the existing write-3 give-up case's contains(TRUSTED) assertion doesn't depend on the answer either way (that write's setTrusted(false) and delete() are asked of the same transaction, so both roll back or neither does). Happy to open a probe for it if you'd rather have it pinned directly rather than argued.

Runs: the three ReplayedConfigChangeTest confidentiality cases plus the ten of IndexConfidentialityChangeTestCase (both engines), together with EncryptedPDBTestCase, EncryptedJETestCase, PDBTestCase, JETestCase, DefaultIndexTest, EntryIDSetTest, StateTest, OnDiskMergeImporterTest, VLVControlTestCase, ServerSideSortControlTestCase - 269 tests, no failures.

…ex to the running backend

setConfidential() only compared the setting with the one the index was opened
with, so a change of confidentiality-enabled reached neither the CryptoSuite the
indexes of the attribute share nor the codecs bound to it, and the comparison
never converged: every later change of that index untrusted it again and asked
for a rebuild which could not help.

The change now puts the new setting in force on that suite before the indexes it
adds bind their codecs, so the key hashed index enabling confidentiality creates
encrypts its records as well, and gives up the trees of the indexes it keeps: one
write untrusts and deletes them, a second opens them again, which is what binds
their codecs to the setting now in force. Their records are in the encoding being
given up, which the codec of the new one reads back as an empty set of entry IDs
rather than failing, so leaving them would answer searches with no entries at all
until the rebuild had run.

Fixes OpenIdentityPlatform#992
…her its confidentiality changes

Whether a change of confidentiality-enabled had anything to apply was decided by comparing the
new configuration with the one this attribute index holds, and that one is replaced after the
first of the writes. A write the storage gave up on - the bound of its retry loop spent, or a
failure it does not replay - left the configuration reading as applied while the index was as it
had been: its codec bound to the setting it was opened under, its tree in that encoding, TRUSTED
still stored. Asked for again, the change found nothing changed, answered SUCCESS with no
instruction, and the next open of the container read that tree with the codec of the other
setting, which is the very state the change gives the tree up to avoid.

Each kept index now holds the confidentiality its codec was bound to, and the plan asks the
index: an index whose codec was bound under the other setting has its tree given up and is
opened again, whichever configuration this attribute index holds. The suite the indexes share is
put in force by the same comparison against what it carries, since that is idempotent. The
instruction goes before the first write with the entry limit's, on the road OpenIdentityPlatform#991 closed for the
limit, so a give-up leaves it in the result next to the failure.

Pinned on the presence index, whose tree a change of confidentiality keeps: a conflict on the
write which gives the tree up is replayed and reported once, and the index writes encrypted after
it; a give-up on that write is reported, leaves TRUSTED stored, and is still a change when asked
for again - red with the decision taken from the configuration, and red at "reported" with the
report moved back behind the writes.
…he index answered before

What DefaultIndex.afterOpen binds - the codec, and the confidentiality it
answers under - is memory, and outlives a write the storage gave up on: the
reopen AttributeIndex.applyConfigurationChange asks for runs that binding
before the write which carries it commits, and nothing put it back when that
commit did not. A change asked for again then found the index already
agreeing with the new setting, answered SUCCESS with no instruction, and
never reopened the tree the write before it had deleted - the index left
degraded, untrusted, with no tree, until a rebuild or a restart nobody was
told to run.

Each index whose tree is about to be given up now has what it answered
before snapshotted, and put back if the write which reopens it does not
commit, so a re-apply still finds it to give up and open again rather than
one which already agrees with a setting that write never durably reached.

Putting it back exposed a second failure on the same road: the write which
untrusts and deletes runs again on that re-apply, and its delete found
nothing to delete a second time, since the first attempt's had already
committed. Guarded by whether the tree is still there.

Pinned in ReplayedConfigChangeTest on the write which reopens the tree
(the fourth, once the third has committed) - the road neither of the two
existing give-up cases exercises, both arming the third. And in
IndexConfidentialityChangeTestCase, disablingConfidentialityStopsEncrypting-
WhatTheIndexWritesNext now checks the tree given up is empty before the
index is trusted again, not only what it writes next - a delete-only-on-
enable mutant read back the disabled record as empty regardless.
@vharseko
vharseko force-pushed the feature/992-index-confidentiality-applied branch from 3dd6ca3 to 76d5517 Compare September 19, 2026 08:50
@vharseko

Copy link
Copy Markdown
Member Author

Rebased onto master now that #997 is in (squash-merged as b0970cc1a8): the stack is gone, and the branch no longer conflicts.

Mechanical. The four [#991] commits this branch carried were what master got, byte for byte — the diff between b0970cc1a8 and #997's head over the three files that squash touched is empty — so git rebase --onto origin/master 728dd78077 dropped them and replayed this change's own three commits clean, with no conflicts: fd2baed085b07074b068, 7a841f9055b6d640cca0, 3dd6ca3ca676d551791f. git range-diff answers = for all three, so nothing of this change's content moved, and the main and test sources compile against the new base.

Head now 76d551791f. No round in this push: round 4 above still stands as the answer to the blocking issue.

@maximthomas maximthomas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: The give-up of the reopen is now put back, and the road round 1 raised is closed.

  • IndexBinding is captured after writeUntrust (AttributeIndex.java:1028-1032), so a re-apply after a given-up write 4 finds isEncrypted() still differing and gives the tree up again — no silent no-op.
  • aConfidentialityChangeIsReportedWhenTheWriteWhichReopensTheTreeGivesUp (ReplayedConfigChangeTest.java:977-1017) pins both halves asked for: write 3 untrusts and commits, write 4 is armed and given up, again reports the rebuild.
  • The disable road now asserts the presence tree is empty (IndexConfidentialityChangeTestCase.java:225-229).
  • The rebase onto b0970cc1a8 is byte-for-byte mechanical; the stack is gone.

issue (blocking): revertFailedReopen puts back encrypted and codec, but not the trusted which afterOpen bound in the same write.

opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DefaultIndex.java:361-365, :117-122, :322-324; opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java:1140-1156

afterOpen binds three fields. On an empty container (createOnDemand && !trusted && entryContainer.isEmpty(txn)) it calls setTrusted(txn, true), which assigns the in-memory trusted before the state write. When the reopen's commit gives up, the flag and (on PDB) the tree are rolled back, revert() restores encrypted/codec, and memory stays trusted=true over no tree — the "trusted one holding nothing" the comment at AttributeIndex.java:1036-1041 says the ordering avoids. Empty backend + confidentiality toggled is the ordinary road (the setting goes on before the import). In the window reads and writes on PDB behave as untrusted (StorageRuntimeException → undefined), so alone it self-heals on re-apply or restart; but any add/modify/delete which fails in the window with this index in its IndexBuffer runs writeTrustStateindex.setTrusted(txn, index.isTrusted()) (IndexBuffer.java:216) and commits TRUSTED for a tree which does not exist. Entries added meanwhile were skipped by update() (get() → undefined → return at :207); after a restart the tree is created empty and trusted, and an equality/presence search answers "nothing" for them, definitively.

// DefaultIndex
final void revertFailedReopen(boolean encrypted, EntryIDSetCodec codec, boolean trusted)
{
  this.encrypted = encrypted;
  this.codec = codec;
  this.trusted = trusted;
}

// AttributeIndex.IndexBinding — captured after writeUntrust, so this is false
private final boolean trusted;

IndexBinding(MatchingRuleIndex index)
{
  this.index = index;
  this.encrypted = index.isEncrypted();
  this.codec = index.codec();
  this.trusted = index.isTrusted();
}

void revert()
{
  index.revertFailedReopen(encrypted, codec, trusted);
}

Pin: the write-4 case once more without addBaseEntry — red today, isTrusted() answers true:

backend.storage.failWithoutReplayOnWrite(4);
final ConfigChangeResult ccr = index.applyConfigurationChange(confidentialPresenceIndexCfg());
assertThat(ccr.getResultCode()).isEqualTo(serverErrorResultCode());
assertThat(cnIndex.isTrusted())
    .as("the reopen which trusted the index over an empty backend gave up").isFalse();
assertThat(persistedFlags(rootContainer, ec, cnIndex.getName())).doesNotContain(TRUSTED);

suggestion (non-blocking): The codec half of revertFailedReopen has no pin: a mutant dropping this.codec = codec survives the suite.

opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ReplayedConfigChangeTest.java:1002

Between the give-up and the re-apply the index is untrusted and its tree is gone, so nothing decodes through the stale codec, and afterOpen rebinds it on the re-apply — no behavioural road turns red. The only pin is white-box, one line on the invariant the javadoc at DefaultIndex.java:353-360 states:

final EntryIDSetCodec before = cnIndex.codec();
// ... failWithoutReplayOnWrite(4), applyConfigurationChange ...
assertThat(cnIndex.codec()).as("the codec the given-up reopen bound is put back").isSameAs(before);

Or leave it: with trusted added to the revert, the pin above covers the one half of it with an observable.

… with the codec

DefaultIndex.afterOpen binds three fields, not two: over an empty entry
container it trusts the index it opens, in memory before the flag is
written, the same way it binds the codec. A give-up of the reopen put the
codec and the confidentiality back, but not the trust - an index answering
trusted over no tree. That answer is what an operation which then fails
writes down for every index in its buffer (IndexBuffer.writeTrustState):
TRUSTED stored for a tree the give-up took with it, while the index skipped
the entries added meanwhile, which a search after a restart then never
finds.

IndexBinding holds the trust too, taken after the write which untrusts so it
is the one that write committed, and revertFailedReopen puts the three back
together.

Pinned in ReplayedConfigChangeTest over an empty backend: the give-up leaves
the index untrusted, an add the container refuses in the window writes no
TRUSTED down for it, and the reopen asked for again trusts the index it
opens. Red at the head on both the answer and the flag the failed add
stored. The codec half of the revert gets its one-line pin alongside, in the
write-4 case - nothing decodes through the codec before the re-apply binds
it afresh, so the invariant is what there is to pin.
@vharseko

vharseko commented Sep 19, 2026

Copy link
Copy Markdown
Member Author

Round 5 pushed as a1d4410d0b: the trust the reopen binds put back with the codec.

The blocking issue. Confirmed as described. afterOpen binds three fields, and over an empty entry container the third - setTrusted(txn, true) - is assigned in memory before the flag is written, so a give-up of the reopen left trusted=true over no tree while revertFailedReopen put back only encrypted and codec. IndexBinding now holds the trust too - taken after writeUntrust, so it is the false that write committed - and revertFailedReopen puts the three back together.

Pinned in ReplayedConfigChangeTest#aGivenUpReopenOverAnEmptyBackendDoesNotLeaveTheIndexTrusted: the write-4 case without addBaseEntry, as suggested, plus the road the leftover trust takes to disk. After the give-up, an add the container refuses (no parent, NO_SUCH_OBJECT) has the cn index in its buffer all the same and runs writeTrustState; persistedFlags must still not contain TRUSTED afterwards. Red at the head on isTrusted(), and - with that assertion removed - on the persisted flags alone: the failed add stored [TRUSTED, COMPACTED] for the tree the give-up had taken with it. The case then re-applies and checks the index is trusted again, with TRUSTED stored, since the backend is still empty: the revert must not cost the reopen which commits its trust.

The codec pin. Added as the one line suggested, in the write-4 case: codec() captured before the arm, isSameAs after the give-up. Red with this.codec = codec dropped from the revert; this.trusted = trusted dropped is red at isTrusted() in the new case, and this.encrypted = encrypted dropped at the existing isEncrypted() assertion.

Runs: ReplayedConfigChangeTest 24/24; with PDBIndexConfidentialityChangeTest, JEIndexConfidentialityChangeTest, EncryptedPDBTestCase, EncryptedJETestCase, PDBTestCase, JETestCase, DefaultIndexTest, EntryIDSetTest, StateTest, OnDiskMergeImporterTest, VLVControlTestCase, ServerSideSortControlTestCase - 270 tests, no failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug concurrency Thread-safety / race-condition bugs index Attribute/VLV index subsystem: build, trust, rebuild, confidentiality security Security fixes / CodeQL code-scanning alerts tests Test suites: fixing, enabling, un-disabling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Changing confidentiality-enabled on a backend index applies nothing until the backend is restarted, and untrusts the index again on every later change

3 participants