From be65d052c28de282c5adf6137d257bcf34792eae Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 25 Sep 2026 17:26:58 +0300 Subject: [PATCH 1/2] [#1109] Keep a connection handler listening when a change to it is rejected The check of a proposed configuration built its SSL context through the same code as the start of the handler, and that code disables the handler when its key store holds no usable key. A key store that cannot be loaded takes that branch, so the change was rejected and the running handler stopped listening, until a later change was accepted or the server restarted. The administration connector, an LDAPConnectionHandler2, and the HTTP connection handler behaved the same way; with the administration connector down, dsconfig could not reach the server any more. createSSLContext in LDAPConnectionHandler2, LDAPConnectionHandler and HTTPConnectionHandler now takes whether the handler is going to use the context. Only the start of the handler and an applied change disable it. The check still rejects the change with the reason it gave before. Fixes #1109 --- .../reactive/LDAPConnectionHandler2.java | 29 +- .../protocols/http/HTTPConnectionHandler.java | 43 ++- .../protocols/ldap/LDAPConnectionHandler.java | 30 +- ...ejectedSSLConfigurationChangeTestCase.java | 310 ++++++++++++++++++ 4 files changed, 385 insertions(+), 27 deletions(-) create mode 100644 opendj-server-legacy/src/test/java/org/opends/server/protocols/RejectedSSLConfigurationChangeTestCase.java diff --git a/opendj-server-legacy/src/main/java/org/forgerock/opendj/reactive/LDAPConnectionHandler2.java b/opendj-server-legacy/src/main/java/org/forgerock/opendj/reactive/LDAPConnectionHandler2.java index 482b3188be..5f40bd1215 100644 --- a/opendj-server-legacy/src/main/java/org/forgerock/opendj/reactive/LDAPConnectionHandler2.java +++ b/opendj-server-legacy/src/main/java/org/forgerock/opendj/reactive/LDAPConnectionHandler2.java @@ -316,7 +316,7 @@ public ConfigChangeResult applyConfigurationChange(LDAPConnectionHandlerCfg conf private void configureSSL(LDAPConnectionHandlerCfg config) throws DirectoryException { protocol = config.isUseSSL() ? "LDAPS" : "LDAP"; if (config.isUseSSL() || config.isAllowStartTLS()) { - sslContext = createSSLContext(config); + sslContext = createSSLContext(config, true); sslEngine = createSSLEngine(config, sslContext); } else { sslContext = null; @@ -581,7 +581,7 @@ public boolean isConfigurationAcceptable(ConnectionHandlerCfg configuration, // Check that the SSL configuration is valid. && (config.isUseSSL() || config.isAllowStartTLS())) { try { - createSSLEngine(config, createSSLContext(config)); + createSSLEngine(config, createSSLContext(config, false)); } catch (DirectoryException e) { logger.traceException(e); @@ -937,26 +937,39 @@ private SSLEngine createSSLEngine(LDAPConnectionHandlerCfg config, SSLContext ss } } - private void disableAndWarnIfUseSSL(LDAPConnectionHandlerCfg config) { - if (config.isUseSSL()) { + private void disableAndWarnIfUseSSL(LDAPConnectionHandlerCfg config, boolean forUse) { + if (forUse && config.isUseSSL()) { logger.warn(INFO_DISABLE_CONNECTION, friendlyName); enabled = false; } } - private SSLContext createSSLContext(LDAPConnectionHandlerCfg config) throws DirectoryException { + /** + * Creates the SSL context for the provided configuration. + * + * @param config + * the configuration to create the SSL context for + * @param forUse + * {@code true} when the handler is going to use the SSL context, at its start or when a change is + * applied, so that an SSL handler without a usable key is disabled; {@code false} when the SSL context + * only checks a proposed configuration, which must leave the running handler as it is + * @return the SSL context + * @throws DirectoryException + * if the SSL context cannot be created + */ + private SSLContext createSSLContext(LDAPConnectionHandlerCfg config, boolean forUse) throws DirectoryException { try { DN keyMgrDN = config.getKeyManagerProviderDN(); final ServerContext serverContext = DirectoryServer.getInstance().getServerContext(); KeyManagerProvider keyManagerProvider = serverContext.getKeyManagerProvider(keyMgrDN); if (keyManagerProvider == null) { logger.error(ERR_NULL_KEY_PROVIDER_MANAGER, keyMgrDN, friendlyName); - disableAndWarnIfUseSSL(config); + disableAndWarnIfUseSSL(config, forUse); keyManagerProvider = new NullKeyManagerProvider(); // The SSL connection is unusable without a key manager provider } else if (!keyManagerProvider.containsAtLeastOneKey()) { logger.error(ERR_INVALID_KEYSTORE, friendlyName); - disableAndWarnIfUseSSL(config); + disableAndWarnIfUseSSL(config, forUse); } final SortedSet aliases = new TreeSet<>(config.getSSLCertNickname()); @@ -973,7 +986,7 @@ private SSLContext createSSLContext(LDAPConnectionHandlerCfg config) throws Dire } if (aliases.isEmpty()) { - disableAndWarnIfUseSSL(config); + disableAndWarnIfUseSSL(config, forUse); } keyManagers = SelectableCertificateKeyManager.wrap(keyManagerProvider.getKeyManagers(), aliases, friendlyName); diff --git a/opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java b/opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java index 7f9cce50e5..49c8eb8d9b 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java @@ -283,7 +283,7 @@ private void configureSSL(HTTPConnectionHandlerCfg config) protocol = config.isUseSSL() ? "HTTPS" : "HTTP"; if (config.isUseSSL()) { - sslEngineConfigurator = createSSLEngineConfigurator(config); + sslEngineConfigurator = createSSLEngineConfigurator(config, true); } else { @@ -497,7 +497,7 @@ public boolean isConfigurationAcceptable( { try { - createSSLEngineConfigurator(config); + createSSLEngineConfigurator(config, false); } catch (DirectoryException e) { @@ -817,7 +817,22 @@ public void toString(StringBuilder buffer) buffer.append(handlerName); } - private SSLEngineConfigurator createSSLEngineConfigurator(HTTPConnectionHandlerCfg config) throws DirectoryException + /** + * Creates the SSL engine configurator for the provided configuration. + * + * @param config + * the configuration to create the SSL engine configurator for + * @param forUse + * {@code true} when the handler is going to use the configurator, at its start or when + * a change is applied, so that a handler without a usable key is disabled; + * {@code false} when the configurator only checks a proposed configuration, which must + * leave the running handler as it is + * @return the SSL engine configurator, or {@code null} if the configuration does not use SSL + * @throws DirectoryException + * if the SSL context cannot be created + */ + private SSLEngineConfigurator createSSLEngineConfigurator(HTTPConnectionHandlerCfg config, boolean forUse) + throws DirectoryException { if (!config.isUseSSL()) { @@ -826,7 +841,7 @@ private SSLEngineConfigurator createSSLEngineConfigurator(HTTPConnectionHandlerC try { - SSLContext sslContext = createSSLContext(config); + SSLContext sslContext = createSSLContext(config, forUse); SSLEngineConfigurator configurator = new SSLEngineConfigurator(sslContext); configurator.setClientMode(false); @@ -874,7 +889,16 @@ private SSLEngineConfigurator createSSLEngineConfigurator(HTTPConnectionHandlerC } } - private SSLContext createSSLContext(HTTPConnectionHandlerCfg config) throws Exception + private void disableAndWarn(boolean forUse) + { + if (forUse) + { + logger.warn(INFO_DISABLE_CONNECTION, friendlyName); + enabled = false; + } + } + + private SSLContext createSSLContext(HTTPConnectionHandlerCfg config, boolean forUse) throws Exception { if (!config.isUseSSL()) { @@ -886,15 +910,13 @@ private SSLContext createSSLContext(HTTPConnectionHandlerCfg config) throws Exce if (keyManagerProvider == null) { logger.error(ERR_NULL_KEY_PROVIDER_MANAGER, keyMgrDN, friendlyName); - logger.warn(INFO_DISABLE_CONNECTION, friendlyName); keyManagerProvider = new NullKeyManagerProvider(); - enabled = false; + disableAndWarn(forUse); } else if (!keyManagerProvider.containsAtLeastOneKey()) { logger.error(ERR_INVALID_KEYSTORE, friendlyName); - logger.warn(INFO_DISABLE_CONNECTION, friendlyName); - enabled = false; + disableAndWarn(forUse); } final SortedSet aliases = new TreeSet<>(config.getSSLCertNickname()); @@ -916,8 +938,7 @@ else if (!keyManagerProvider.containsAtLeastOneKey()) } if (aliases.isEmpty()) { - logger.warn(INFO_DISABLE_CONNECTION, friendlyName); - enabled = false; + disableAndWarn(forUse); } keyManagers = SelectableCertificateKeyManager.wrap(keyManagerProvider.getKeyManagers(), aliases, friendlyName); } diff --git a/opendj-server-legacy/src/main/java/org/opends/server/protocols/ldap/LDAPConnectionHandler.java b/opendj-server-legacy/src/main/java/org/opends/server/protocols/ldap/LDAPConnectionHandler.java index f404bd961c..e80cadc5a5 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/protocols/ldap/LDAPConnectionHandler.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/protocols/ldap/LDAPConnectionHandler.java @@ -331,7 +331,7 @@ private void configureSSL(LDAPConnectionHandlerCfg config) protocol = config.isUseSSL() ? "LDAPS" : "LDAP"; if (config.isUseSSL() || config.isAllowStartTLS()) { - sslContext = createSSLContext(config); + sslContext = createSSLContext(config, true); sslEngine = createSSLEngine(config, sslContext); } else @@ -717,7 +717,7 @@ public boolean isConfigurationAcceptable(ConnectionHandlerCfg configuration, { try { - createSSLEngine(config, createSSLContext(config)); + createSSLEngine(config, createSSLContext(config, false)); } catch (DirectoryException e) { @@ -1291,16 +1291,30 @@ private SSLEngine createSSLEngine(LDAPConnectionHandlerCfg config, } } - private void disableAndWarnIfUseSSL(LDAPConnectionHandlerCfg config) + private void disableAndWarnIfUseSSL(LDAPConnectionHandlerCfg config, boolean forUse) { - if (config.isUseSSL()) + if (forUse && config.isUseSSL()) { logger.warn(INFO_DISABLE_CONNECTION, friendlyName); enabled = false; } } - private SSLContext createSSLContext(LDAPConnectionHandlerCfg config) + /** + * Creates the SSL context for the provided configuration. + * + * @param config + * the configuration to create the SSL context for + * @param forUse + * {@code true} when the handler is going to use the SSL context, at its start or when + * a change is applied, so that an SSL handler without a usable key is disabled; + * {@code false} when the SSL context only checks a proposed configuration, which must + * leave the running handler as it is + * @return the SSL context + * @throws DirectoryException + * if the SSL context cannot be created + */ + private SSLContext createSSLContext(LDAPConnectionHandlerCfg config, boolean forUse) throws DirectoryException { try @@ -1311,14 +1325,14 @@ private SSLContext createSSLContext(LDAPConnectionHandlerCfg config) if (keyManagerProvider == null) { logger.error(ERR_NULL_KEY_PROVIDER_MANAGER, keyMgrDN, friendlyName); - disableAndWarnIfUseSSL(config); + disableAndWarnIfUseSSL(config, forUse); keyManagerProvider = new NullKeyManagerProvider(); // The SSL connection is unusable without a key manager provider } else if (! keyManagerProvider.containsAtLeastOneKey()) { logger.error(ERR_INVALID_KEYSTORE, friendlyName); - disableAndWarnIfUseSSL(config); + disableAndWarnIfUseSSL(config, forUse); } final SortedSet aliases = new TreeSet<>(config.getSSLCertNickname()); @@ -1341,7 +1355,7 @@ else if (! keyManagerProvider.containsAtLeastOneKey()) if (aliases.isEmpty()) { - disableAndWarnIfUseSSL(config); + disableAndWarnIfUseSSL(config, forUse); } keyManagers = SelectableCertificateKeyManager.wrap(keyManagerProvider.getKeyManagers(), aliases, friendlyName); } diff --git a/opendj-server-legacy/src/test/java/org/opends/server/protocols/RejectedSSLConfigurationChangeTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/protocols/RejectedSSLConfigurationChangeTestCase.java new file mode 100644 index 0000000000..5d48e4cac8 --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/server/protocols/RejectedSSLConfigurationChangeTestCase.java @@ -0,0 +1,310 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.opends.server.protocols; + +import static org.opends.server.protocols.internal.InternalClientConnection.getRootConnection; +import static org.opends.server.util.StaticUtils.*; +import static org.testng.Assert.*; + +import java.io.File; +import java.io.IOException; +import java.net.ConnectException; +import java.net.Socket; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.TimeUnit; + +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.TrustManager; + +import org.forgerock.i18n.LocalizableMessage; +import org.forgerock.opendj.config.Configuration; +import org.forgerock.opendj.config.server.ConfigChangeResult; +import org.forgerock.opendj.config.server.ConfigurationChangeListener; +import org.forgerock.opendj.ldap.DN; +import org.forgerock.opendj.ldap.ResultCode; +import org.forgerock.opendj.reactive.LDAPConnectionHandler2; +import org.forgerock.opendj.server.config.meta.HTTPConnectionHandlerCfgDefn; +import org.forgerock.opendj.server.config.meta.LDAPConnectionHandlerCfgDefn; +import org.forgerock.opendj.server.config.server.ConnectionHandlerCfg; +import org.forgerock.opendj.server.config.server.HTTPConnectionHandlerCfg; +import org.forgerock.opendj.server.config.server.LDAPConnectionHandlerCfg; +import org.opends.admin.ads.util.BlindTrustManager; +import org.opends.server.DirectoryServerTestCase; +import org.opends.server.TestCaseUtils; +import org.opends.server.api.ConnectionHandler; +import org.opends.server.api.ServerShutdownListener; +import org.opends.server.core.DeleteOperation; +import org.opends.server.core.DirectoryServer; +import org.opends.server.core.ServerContext; +import org.opends.server.extensions.InitializationUtils; +import org.opends.server.protocols.http.HTTPConnectionHandler; +import org.opends.server.protocols.ldap.LDAPConnectionHandler; +import org.opends.server.types.Entry; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +/** + * A change to an SSL connection handler that is rejected because its key store cannot be loaded + * must leave the running handler as it was: listening, with the SSL context it had. + */ +@SuppressWarnings("javadoc") +@Test(groups = { "precommit" }, sequential = true) +public class RejectedSSLConfigurationChangeTestCase extends DirectoryServerTestCase +{ + private static final LocalizableMessage STOP_REASON = LocalizableMessage.raw("Don't need a reason."); + private static final DN KEY_MANAGER_DN = DN.valueOf("cn=Rejected Change Keys,cn=Key Manager Providers,cn=config"); + + /** How long the handler must keep serving TLS after the rejected change: its thread checks every second. */ + private static final long KEEPS_SERVING_MS = TimeUnit.SECONDS.toMillis(3); + + private enum Kind + { + LDAP2, LDAP_LEGACY, HTTP + } + + private static final class NoChanges implements ConfigurationChangeListener + { + @Override + public boolean isConfigurationChangeAcceptable(C configuration, List unacceptableReasons) + { + return true; + } + + @Override + public ConfigChangeResult applyConfigurationChange(C configuration) + { + return new ConfigChangeResult(); + } + } + + private File keyStore; + + @BeforeClass + public void setUp() throws Exception + { + TestCaseUtils.startServer(); + keyStore = File.createTempFile("rejected-change", ".keystore"); + keyStore.deleteOnExit(); + restoreKeyStore(); + TestCaseUtils.addEntry( + "dn: " + KEY_MANAGER_DN, + "objectClass: top", + "objectClass: ds-cfg-key-manager-provider", + "objectClass: ds-cfg-file-based-key-manager-provider", + "cn: Rejected Change Keys", + "ds-cfg-java-class: org.opends.server.extensions.FileBasedKeyManagerProvider", + "ds-cfg-enabled: true", + "ds-cfg-key-store-type: JKS", + "ds-cfg-key-store-file: " + keyStore.getAbsolutePath(), + "ds-cfg-key-store-pin: password"); + } + + @AfterClass + public void tearDown() throws Exception + { + // A handler registering for changes to its configuration also registers a reference to its key + // manager provider, which keeps the provider from being deleted and outlives the handler. + // Registering for changes to the same entry without a key manager provider drops the reference. + final LDAPConnectionHandlerCfg ldap = + (LDAPConnectionHandlerCfg) configuration(Kind.LDAP2, 1, null, "5 megabytes", false); + final NoChanges ldapListener = new NoChanges<>(); + ldap.addLDAPChangeListener(ldapListener); + ldap.removeLDAPChangeListener(ldapListener); + final HTTPConnectionHandlerCfg http = + (HTTPConnectionHandlerCfg) configuration(Kind.HTTP, 1, null, "5 megabytes", false); + final NoChanges httpListener = new NoChanges<>(); + http.addHTTPChangeListener(httpListener); + http.removeHTTPChangeListener(httpListener); + + final DeleteOperation delete = getRootConnection().processDelete(KEY_MANAGER_DN); + assertEquals(delete.getResultCode(), ResultCode.SUCCESS, String.valueOf(delete.getErrorMessage())); + Files.deleteIfExists(keyStore.toPath()); + } + + @BeforeMethod + public void restoreKeyStore() throws IOException + { + Files.copy(getFileForPath("config/server.keystore").toPath(), keyStore.toPath(), + StandardCopyOption.REPLACE_EXISTING); + } + + @DataProvider + public Object[][] handlers() + { + return new Object[][] { + { Kind.LDAP2, null }, { Kind.LDAP2, "server-cert" }, + { Kind.LDAP_LEGACY, null }, { Kind.LDAP_LEGACY, "server-cert" }, + { Kind.HTTP, null }, { Kind.HTTP, "server-cert" }, + }; + } + + @Test(dataProvider = "handlers") + public void rejectedChangeKeepsTheHandlerListening(Kind kind, String certNickname) throws Exception + { + final int port = TestCaseUtils.findFreePort(); + final ConnectionHandler handler = start(kind, configuration(kind, port, certNickname, "5 megabytes", true)); + try + { + assertServesTLS(port); + + Files.write(keyStore.toPath(), new byte[] { 1, 2, 3, 4 }); + final List reasons = new ArrayList<>(); + assertFalse(handler.isConfigurationAcceptable(configuration(kind, port, certNickname, "6 megabytes", true), reasons), + "a change needing a key store that cannot be loaded was accepted"); + assertFalse(reasons.isEmpty(), "the change was rejected without a reason"); + + final long deadline = System.currentTimeMillis() + KEEPS_SERVING_MS; + do + { + assertServesTLS(port); + Thread.sleep(250); + } + while (System.currentTimeMillis() < deadline); + } + finally + { + ((ServerShutdownListener) handler).processServerShutdown(STOP_REASON); + handler.finalizeConnectionHandler(STOP_REASON); + handler.join(10000); + assertFalse(handler.isAlive(), "the connection handler thread is still running"); + } + } + + @DataProvider + public Object[][] kinds() + { + return new Object[][] { { Kind.LDAP2 }, { Kind.LDAP_LEGACY }, { Kind.HTTP } }; + } + + /** The start of a handler still disables it when its key store holds no key it can present. */ + @Test(dataProvider = "kinds") + public void handlerWithoutItsCertificateDoesNotListen(Kind kind) throws Exception + { + final int port = TestCaseUtils.findFreePort(); + final ConnectionHandler handler = start(kind, configuration(kind, port, "no-such-cert", "5 megabytes", true)); + try (Socket socket = new Socket("127.0.0.1", port)) + { + fail("the handler listens on port " + port + " without the certificate it is configured to present"); + } + catch (ConnectException expected) + { + // disabled at its start + } + finally + { + ((ServerShutdownListener) handler).processServerShutdown(STOP_REASON); + handler.finalizeConnectionHandler(STOP_REASON); + handler.join(10000); + assertFalse(handler.isAlive(), "the connection handler thread is still running"); + } + } + + private static ConnectionHandlerCfg configuration(Kind kind, int port, String certNickname, String maxRequestSize, + boolean useSSL) throws Exception + { + final List lines = new ArrayList<>(); + lines.add("dn: cn=Rejected Change Handler,cn=Connection Handlers,cn=config"); + lines.add("objectClass: top"); + lines.add("objectClass: ds-cfg-connection-handler"); + lines.add("cn: Rejected Change Handler"); + lines.add("ds-cfg-enabled: true"); + lines.add("ds-cfg-listen-address: 127.0.0.1"); + lines.add("ds-cfg-listen-port: " + port); + lines.add("ds-cfg-accept-backlog: 128"); + lines.add("ds-cfg-keep-stats: false"); + lines.add("ds-cfg-use-tcp-keep-alive: true"); + lines.add("ds-cfg-use-tcp-no-delay: true"); + lines.add("ds-cfg-allow-tcp-reuse-address: true"); + lines.add("ds-cfg-max-request-size: " + maxRequestSize); + lines.add("ds-cfg-use-ssl: " + useSSL); + lines.add("ds-cfg-ssl-client-auth-policy: disabled"); + if (useSSL) + { + lines.add("ds-cfg-key-manager-provider: " + KEY_MANAGER_DN); + } + if (certNickname != null) + { + lines.add("ds-cfg-ssl-cert-nickname: " + certNickname); + } + if (kind == Kind.HTTP) + { + lines.add("objectClass: ds-cfg-http-connection-handler"); + lines.add("ds-cfg-java-class: " + HTTPConnectionHandler.class.getName()); + lines.add("ds-cfg-buffer-size: 4096 bytes"); + lines.add("ds-cfg-max-blocked-write-time-limit: 2 minutes"); + final Entry entry = TestCaseUtils.makeEntry(lines.toArray(new String[0])); + return InitializationUtils.getConfiguration(HTTPConnectionHandlerCfgDefn.getInstance(), entry); + } + lines.add("objectClass: ds-cfg-ldap-connection-handler"); + lines.add("ds-cfg-java-class: " + + (kind == Kind.LDAP2 ? LDAPConnectionHandler2.class : LDAPConnectionHandler.class).getName()); + lines.add("ds-cfg-allow-ldap-v2: false"); + lines.add("ds-cfg-send-rejection-notice: true"); + lines.add("ds-cfg-num-request-handlers: 2"); + lines.add("ds-cfg-allow-start-tls: false"); + final Entry entry = TestCaseUtils.makeEntry(lines.toArray(new String[0])); + return InitializationUtils.getConfiguration(LDAPConnectionHandlerCfgDefn.getInstance(), entry); + } + + private static ConnectionHandler start(Kind kind, ConnectionHandlerCfg config) throws Exception + { + final ServerContext serverContext = DirectoryServer.getInstance().getServerContext(); + final ConnectionHandler handler; + switch (kind) + { + case LDAP2: + final LDAPConnectionHandler2 ldap2 = new LDAPConnectionHandler2(); + ldap2.initializeConnectionHandler(serverContext, (LDAPConnectionHandlerCfg) config); + handler = ldap2; + break; + case LDAP_LEGACY: + final LDAPConnectionHandler legacy = new LDAPConnectionHandler(); + legacy.initializeConnectionHandler(serverContext, (LDAPConnectionHandlerCfg) config); + handler = legacy; + break; + default: + final HTTPConnectionHandler http = new HTTPConnectionHandler(); + http.initializeConnectionHandler(serverContext, (HTTPConnectionHandlerCfg) config); + handler = http; + break; + } + handler.start(); + return handler; + } + + private static void assertServesTLS(int port) throws Exception + { + final SSLContext client = SSLContext.getInstance("TLS"); + client.init(null, new TrustManager[] { new BlindTrustManager() }, null); + try (SSLSocket socket = (SSLSocket) client.getSocketFactory().createSocket("127.0.0.1", port)) + { + socket.setSoTimeout(10000); + socket.startHandshake(); + assertTrue(socket.getSession().getPeerCertificates().length > 0); + } + catch (IOException e) + { + fail("the handler does not serve TLS on port " + port + ": " + e, e); + } + } +} From b4b0cadea76f7269d3729c3633c5ab2f93890101 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Sun, 27 Sep 2026 11:48:29 +0300 Subject: [PATCH 2/2] [#1109] Pin the applied change and the check without a key manager provider The forUse javadoc of HTTPConnectionHandler no longer says that an applied change disables the handler: applyConfigurationChange sets enabled from the configuration after configureSSL, so only the start does. The previous commit says the same of all three handlers; that holds for the two LDAP handlers only. The HTTP behaviour predates this change and is #1111. RejectedSSLConfigurationChangeTestCase gains: - appliedChangeWithoutItsCertificateStopsListening: an applied change that takes the certificate of LDAPConnectionHandler2 away is accepted by the check and stops the listener; - checkWithoutKeyManagerProviderKeepsTheHandlerListening, for the three handlers: a check that finds no key manager provider registered under the configured DN leaves the running handler serving TLS; - rejectedChangeKeepsTheHandlerListening checks that the change is rejected with ERR_CONNHANDLER_SSL_CANNOT_INITIALIZE, not with any reason. --- .../protocols/http/HTTPConnectionHandler.java | 8 +- ...ejectedSSLConfigurationChangeTestCase.java | 124 +++++++++++++++--- 2 files changed, 112 insertions(+), 20 deletions(-) diff --git a/opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java b/opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java index 49c8eb8d9b..2b118d577f 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java @@ -823,10 +823,10 @@ public void toString(StringBuilder buffer) * @param config * the configuration to create the SSL engine configurator for * @param forUse - * {@code true} when the handler is going to use the configurator, at its start or when - * a change is applied, so that a handler without a usable key is disabled; - * {@code false} when the configurator only checks a proposed configuration, which must - * leave the running handler as it is + * {@code true} when the handler is going to use the configurator: at its start a handler + * without a usable key is disabled ({@link #applyConfigurationChange} sets {@code enabled} + * from the configuration afterwards); {@code false} when the configurator only checks a + * proposed configuration, which must leave the running handler as it is * @return the SSL engine configurator, or {@code null} if the configuration does not use SSL * @throws DirectoryException * if the SSL context cannot be created diff --git a/opendj-server-legacy/src/test/java/org/opends/server/protocols/RejectedSSLConfigurationChangeTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/protocols/RejectedSSLConfigurationChangeTestCase.java index 5d48e4cac8..22ce5c3561 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/protocols/RejectedSSLConfigurationChangeTestCase.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/protocols/RejectedSSLConfigurationChangeTestCase.java @@ -15,6 +15,7 @@ */ package org.opends.server.protocols; +import static org.opends.messages.ProtocolMessages.ERR_CONNHANDLER_SSL_CANNOT_INITIALIZE; import static org.opends.server.protocols.internal.InternalClientConnection.getRootConnection; import static org.opends.server.util.StaticUtils.*; import static org.testng.Assert.*; @@ -49,6 +50,7 @@ import org.opends.server.DirectoryServerTestCase; import org.opends.server.TestCaseUtils; import org.opends.server.api.ConnectionHandler; +import org.opends.server.api.KeyManagerProvider; import org.opends.server.api.ServerShutdownListener; import org.opends.server.core.DeleteOperation; import org.opends.server.core.DirectoryServer; @@ -65,7 +67,8 @@ /** * A change to an SSL connection handler that is rejected because its key store cannot be loaded - * must leave the running handler as it was: listening, with the SSL context it had. + * must leave the running handler as it was: listening, with the SSL context it had. The start of a handler, and a + * change applied to an LDAP handler, still disable it when it has no key it can present. */ @SuppressWarnings("javadoc") @Test(groups = { "precommit" }, sequential = true) @@ -76,6 +79,8 @@ public class RejectedSSLConfigurationChangeTestCase extends DirectoryServerTestC /** How long the handler must keep serving TLS after the rejected change: its thread checks every second. */ private static final long KEEPS_SERVING_MS = TimeUnit.SECONDS.toMillis(3); + /** How long a handler that has been disabled may take to stop listening: its thread checks every second. */ + private static final long STOPS_LISTENING_MS = TimeUnit.SECONDS.toMillis(10); private enum Kind { @@ -172,21 +177,13 @@ public void rejectedChangeKeepsTheHandlerListening(Kind kind, String certNicknam assertFalse(handler.isConfigurationAcceptable(configuration(kind, port, certNickname, "6 megabytes", true), reasons), "a change needing a key store that cannot be loaded was accepted"); assertFalse(reasons.isEmpty(), "the change was rejected without a reason"); + assertEquals(reasons.get(0).ordinal(), ERR_CONNHANDLER_SSL_CANNOT_INITIALIZE.ordinal(), String.valueOf(reasons)); - final long deadline = System.currentTimeMillis() + KEEPS_SERVING_MS; - do - { - assertServesTLS(port); - Thread.sleep(250); - } - while (System.currentTimeMillis() < deadline); + assertKeepsServingTLS(port); } finally { - ((ServerShutdownListener) handler).processServerShutdown(STOP_REASON); - handler.finalizeConnectionHandler(STOP_REASON); - handler.join(10000); - assertFalse(handler.isAlive(), "the connection handler thread is still running"); + stop(handler); } } @@ -196,6 +193,42 @@ public Object[][] kinds() return new Object[][] { { Kind.LDAP2 }, { Kind.LDAP_LEGACY }, { Kind.HTTP } }; } + /** + * A check that finds no key manager provider registered under the configured DN leaves the running handler as it + * is. The configuration entry of an enabled provider whose key store could not be loaded at the start of the + * server exists, but the provider is not registered. + */ + @Test(dataProvider = "kinds") + public void checkWithoutKeyManagerProviderKeepsTheHandlerListening(Kind kind) throws Exception + { + final int port = TestCaseUtils.findFreePort(); + final ConnectionHandler handler = start(kind, configuration(kind, port, null, "5 megabytes", true)); + try + { + assertServesTLS(port); + + final KeyManagerProvider provider = DirectoryServer.getKeyManagerProvider(KEY_MANAGER_DN); + assertNotNull(provider, "the key manager provider of the test is not registered"); + DirectoryServer.deregisterKeyManagerProvider(KEY_MANAGER_DN); + try + { + // Whether the change is accepted does not matter here: the running handler must not change either way. + handler.isConfigurationAcceptable(configuration(kind, port, null, "6 megabytes", true), + new ArrayList()); + } + finally + { + DirectoryServer.registerKeyManagerProvider(KEY_MANAGER_DN, provider); + } + + assertKeepsServingTLS(port); + } + finally + { + stop(handler); + } + } + /** The start of a handler still disables it when its key store holds no key it can present. */ @Test(dataProvider = "kinds") public void handlerWithoutItsCertificateDoesNotListen(Kind kind) throws Exception @@ -212,10 +245,50 @@ public void handlerWithoutItsCertificateDoesNotListen(Kind kind) throws Exceptio } finally { - ((ServerShutdownListener) handler).processServerShutdown(STOP_REASON); - handler.finalizeConnectionHandler(STOP_REASON); - handler.join(10000); - assertFalse(handler.isAlive(), "the connection handler thread is still running"); + stop(handler); + } + } + + /** + * An applied change that leaves an LDAP handler without the certificate it is configured to present still disables + * it. The check accepts such a change, so dsconfig reaches this road. Only {@link LDAPConnectionHandler2} is + * covered: the legacy handler keeps accepting TCP connections while it is disabled, so a refused connect cannot + * tell, and the HTTP handler sets {@code enabled} from the configuration after the SSL context is built. + */ + @SuppressWarnings("unchecked") + @Test + public void appliedChangeWithoutItsCertificateStopsListening() throws Exception + { + final int port = TestCaseUtils.findFreePort(); + final ConnectionHandler handler = start(Kind.LDAP2, configuration(Kind.LDAP2, port, null, "5 megabytes", true)); + try + { + assertServesTLS(port); + + final LDAPConnectionHandlerCfg change = + (LDAPConnectionHandlerCfg) configuration(Kind.LDAP2, port, "no-such-cert", "5 megabytes", true); + final List reasons = new ArrayList<>(); + assertTrue(handler.isConfigurationAcceptable(change, reasons), String.valueOf(reasons)); + ((ConfigurationChangeListener) handler).applyConfigurationChange(change); + + final long deadline = System.currentTimeMillis() + STOPS_LISTENING_MS; + while (true) + { + try (Socket socket = new Socket("127.0.0.1", port)) + { + assertTrue(System.currentTimeMillis() < deadline, + "the handler still listens on port " + port + " after a change took its certificate away"); + } + catch (ConnectException expected) + { + break; + } + Thread.sleep(250); + } + } + finally + { + stop(handler); } } @@ -292,6 +365,25 @@ private static ConnectionHandler start(Kind kind, ConnectionHandlerCfg config return handler; } + private static void stop(ConnectionHandler handler) throws InterruptedException + { + ((ServerShutdownListener) handler).processServerShutdown(STOP_REASON); + handler.finalizeConnectionHandler(STOP_REASON); + handler.join(10000); + assertFalse(handler.isAlive(), "the connection handler thread is still running"); + } + + private static void assertKeepsServingTLS(int port) throws Exception + { + final long deadline = System.currentTimeMillis() + KEEPS_SERVING_MS; + do + { + assertServesTLS(port); + Thread.sleep(250); + } + while (System.currentTimeMillis() < deadline); + } + private static void assertServesTLS(int port) throws Exception { final SSLContext client = SSLContext.getInstance("TLS");