Skip to content

A key store and PIN renewed together fail to load in getKeyManagers() / getTrustManagers() until a handshake reloads the PIN #1105

Description

@vharseko

Problem

#1101 (for #1095) makes FileBasedKeyManagerProvider and FileBasedTrustManagerProvider load a store file again when it changes. The reload on the handshake path reads the PIN again from the configuration: currentKeyManager() (FileBasedKeyManagerProvider.java:279-284 at a1e78af) and its trust counterpart (FileBasedTrustManagerProvider.java:258-263). A store renewed together with a new password is therefore picked up there.

getKeyManagers() (:237-246) and getTrustManagers() (:214-226) do not read the PIN again. They go through loadKeyManagers() → getKeystore() (trustStore.load(inputStream, trustStorePIN) on the trust side, :289), which open the file with the PIN held in memory since the provider was initialized, since its configuration last changed, or since the last reload on a handshake.

The problem shows up when the store and its PIN file have both been replaced and no handshake has come through since. A caller of getKeyManagers() / getTrustManagers() then opens the new store with the old password, and the load fails with ERR_FILE_KEYMANAGER_CANNOT_LOAD (or the trust store equivalent). The usual caller at run time is a connection handler rebuilding its SSL context: a dsconfig set-connection-handler-prop on the LDAPS, LDAP (StartTLS) or HTTP handler is rejected, and it keeps failing until a handshake reloads the PIN or the server restarts.

It matters where the password is rotated along with the certificate. An example is the Docker image with a secret volume (#1087 / #1100), where the watcher copies keystore and keystore.pin into config/ while the server runs. In that image the window is at most one health-check interval (30 s), because the probe's LDAPS handshake reloads the PIN. Without such a probe it stays open until the first client connects over TLS.

Found in the review of #1100 (the discussion is on #1100 and #1101). This is based on reading the code at a1e78af; I have not reproduced it.

Proposed fix

In getKeyManagers() / getTrustManagers(), read the PIN again from currentConfig before loading the store, the same way currentKeyManager() / currentTrustManager() do. Then the stamp the handed-out manager records and the PIN it was loaded with belong to the same files. A test in FileBasedKeyManagerProviderTestCase and FileBasedTrustManagerProviderTestCase would replace the store and its PIN file, call getKeyManagers() / getTrustManagers() with no handshake in between, and expect the new store.

Depends on #1101.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugjavaChanges to Java sourcessecuritySecurity fixes / CodeQL code-scanning alerts

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions