mirror of https://github.com/OpenIdentityPlatform/OpenDJ.git

Valery Kharseko
yesterday f93487455cc1b5d7062ea6577d99dea25bfd81b3
refs
author Valery Kharseko <vharseko@3a-systems.ru>
Wednesday, October 14, 2026 09:27 +0200
committer GitHub <noreply@github.com>
Wednesday, October 14, 2026 09:27 +0200
commitf93487455cc1b5d7062ea6577d99dea25bfd81b3
tree 3a68e6bf0661f5abde98c80fd613dcd61725aa01 tree | zip | gz
parent 571a0a1649b7e8e1f0ad439fe2208ce0af25be2a view | diff
[#1188] Serialize and replace atomically the writes of the ads-truststore, and report a certificate it cannot take or drop (#1189)

Fixes #1188 in part: the trust store side. The instance key stored in
`cn=admin data` without its certificate is not explained by this change;
see "Not fixed here" below.

## The defect

Every change to the ads-truststore reads the file, changes that copy and
writes the whole file back over the old one, truncating it first, and
nothing kept two of them apart. Replication replays the instance keys of
`cn=admin data` on parallel threads (16 at least), each of which copies
its key into the trust store in `CryptoManagerSync`, and an SSL context
is built from the file for every replication connection. So:

- two keys replayed together lost one of them (the second write is made
from a copy read before the first one);
- a reader could find the file half written: `Keystore was tampered
with`, `signed overrun`, `EOFException`;
- a directory server which lost the certificate of a replication server
that way failed every handshake with it (`bad_certificate`).

A failed copy into the trust store was logged at debug level only, so
the server said nothing about why it did not trust the other one.

## The change

- `TrustStoreBackend` holds one lock over every access to the file and
to its certificate manager: reads, adds, deletes, the instance
certificate it generates and the change of its configuration. The key
managers and keys it hands out are opened under that lock as well, with
the PIN of the file they were read from rather than that of a
configuration changed meanwhile.
- On a POSIX file system, `Platform` writes every key store to a file of
its own next to it and moves that over the old one at once
(`ATOMIC_MOVE`): a reader outside the lock never finds the file half
written, and a server stopped part way through a write keeps the old
file whole instead of an empty one it cannot start with. The new file
takes the owner, the group and then the mode of the old one (not its
ACLs), so that it is never open to a group the old one was not; a key
store path which is a link goes on pointing where it did. The key store
is written in place, as before, where replacing the file would change
what it is or cannot be done: a new key store, one this process may not
write to (refused, as before), one with other hard links, one whose
owner or group the new file cannot take, one whose links, owner or group
cannot be told, one in a directory this process cannot write to, and one
which cannot be replaced (a file mounted on its own into a container).
On a file system without POSIX attributes (Windows) the key store is
always written in place, as before: neither the owner, the ACLs nor the
hard links of the old file would reach a new one there.
- `CryptoManagerSync` reports a certificate it cannot copy into the
trust store, and one it cannot drop from it (a compromised key the
server would go on trusting), at error level:
`INFO_TRUSTSTORESYNC_ADD_FAILED_655` and
`INFO_TRUSTSTORESYNC_DELETE_FAILED_656` become `ERR_`, same ordinals.
Nothing is reported where nothing failed: a certificate the trust store
holds already (dsreplication seeds the trust store of the servers it
enables) and one it holds no longer (it leaves when its key is marked
compromised).
- `TrustStoreBackend` keeps the result code of those two refusals, an
alias in use (entry already exists) and an alias not in use (no such
entry), instead of turning them into a server error, which
`CryptoManagerSync` would have reported.

## Not fixed here

#1188 also found an instance key stored in `cn=admin data` on the
directory server without `ds-cfg-public-key-certificate`, which no trust
store can take. I ran the scenario of the issue (the #534 topology of
#1184, a replication server and a directory server set up again on empty
volumes while the first directory server is down) three times on an
image built from this branch, with temporary logging of every `AddMsg`
of an instance key: built, decoded on the replication and the directory
servers, replayed, and what each add left in `cn=admin data`. It did not
reproduce: every message carried the certificate (745 bytes), the
encoder threw nothing, all four servers held every key with its
certificate in `cn=admin data` and in the trust store, before and after
a restart of the directory server. The cause of the entry without a
certificate is still open.

One thing the runs did show: the directory server that was down fails
three handshakes with the new replication server, one per domain, in the
second it starts, and none after. That is before it has learned the new
certificate through the other replication server; I have not verified
that further.

## Tests

- `CryptoManagerTestCase`:
- sixteen instance keys added together all reach the trust store, while
a reader builds trust managers from the file the whole time — once with
the file replaced at once, once with it written in place, where only the
lock keeps the reader whole;
- sixteen instance keys added while sixteen others are deleted: every
added key is in the trust store, no deleted one;
- an instance key the trust store cannot take (refused by the schema
check of its copy) is in the error log, and one it holds already is not;
- a compromised key it cannot drop is in the error log, and the delete
of a key whose certificate has left already is not;
- the trust store refuses an alias in use and one not in use with their
own result codes.
- `CertificateManagerTestCase`:
- a reader of a key store file never finds it half written while it is
rewritten (POSIX only);
- the rewrite keeps the mode of the file, its group and the link to it,
and still replaces the file;
- a key store with another hard link, one whose group the new file
cannot take (ignored or refused), one in a directory this process cannot
write to, and one which cannot be replaced are rewritten in place;
- a key store file this process may not write to is refused.

RED on master for the concurrency, error-log and result-code tests, and
for the half-written file. Each of the changes reverted on its own turns
its test red (22 mutants over the four rounds). `CryptoManagerTestCase`,
`CertificateManagerTestCase`, `AdsTrustStoreProvisionerTest` and
`AdsTrustStoreInstallTestCase`: 138 tests, all green. Without a test:
the PIN taken under the lock and the configuration change under it (the
race with a change of the configuration cannot be made to happen
reliably in one), the check of the owner the new file took (a process
which is not root cannot make a file whose owner it cannot take), the
order of the group and the mode (the window between them cannot be seen
from a test), the in-place road where the links of the file cannot be
told, and the in-place road on Windows (no Windows run).
13 files modified
1 files added
1307 ■■■■■ changed files
opendj-server-legacy/src/main/java/org/opends/server/backends/TrustStoreBackend.java 156 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/crypto/CryptoManagerSync.java 22 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/util/Platform.java 188 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/messages/org/opends/messages/core.properties 4 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/messages/org/opends/messages/core_de.properties 5 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/messages/org/opends/messages/core_es.properties 5 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/messages/org/opends/messages/core_fr.properties 5 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/messages/org/opends/messages/core_ja.properties 5 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/messages/org/opends/messages/core_ko.properties 5 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/messages/org/opends/messages/core_zh_CN.properties 5 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/messages/org/opends/messages/core_zh_TW.properties 5 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/crypto/CryptoManagerTestCase.java 426 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/util/CertificateManagerTestCase.java 398 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/util/KeyStoreReplaceFailure.java 78 ●●●●● diff | view | raw | blame | history