From c092c76ae86313ae7b16777590a847c19dfb26c2 Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Fri, 16 Jun 2023 16:22:58 +0200 Subject: [PATCH] Remove ldapsOnly (Java) In `LDAPConstants.java`, the function to set the Truststore SPI system property was removed, as this is now handled by the `shouldUseTruststoreSpi` method in `LdapUtil`. Closes: #9313 --- .../release_notes/topics/22_0_0.adoc | 9 +++++++ .../topics/user-federation/ldap.adoc | 2 +- .../org/keycloak/storage/ldap/LDAPConfig.java | 8 +++++-- .../ldap/LDAPStorageProviderFactory.java | 2 +- .../idm/store/ldap/LDAPContextManager.java | 8 +++---- .../idm/store/ldap/LDAPOperationManager.java | 3 +-- .../storage/ldap/idm/store/ldap/LDAPUtil.java | 16 +++++++++++++ model/map-ldap/pom.xml | 4 ++++ .../ldap/store/LdapMapContextManager.java | 8 +++---- .../map/storage/ldap/store/LdapMapUtil.java | 17 +++++++++++++ .../org/keycloak/models/LDAPConstants.java | 24 ++++--------------- .../org/keycloak/testsuite/util/LDAPRule.java | 2 +- 12 files changed, 67 insertions(+), 36 deletions(-) diff --git a/docs/documentation/release_notes/topics/22_0_0.adoc b/docs/documentation/release_notes/topics/22_0_0.adoc index a694e5424d5..5266cb41801 100644 --- a/docs/documentation/release_notes/topics/22_0_0.adoc +++ b/docs/documentation/release_notes/topics/22_0_0.adoc @@ -126,3 +126,12 @@ q=: : ... ``` Where `` and `` represent the attribute name and value, respectively. + += LDAPS-only Truststore option removed + +LDAP option to use truststore SPI `Only for ldaps` has been removed. This parameter is used to +select truststore for TLS-secured LDAP connection: either internal Keycloak truststore is +picked (`Always`), or the global JVM one (`Never`). + +Deployments where `Only for ldaps` was used will automatically behave as if `Always` option was +selected for TLS-secured LDAP connections. \ No newline at end of file diff --git a/docs/documentation/server_admin/topics/user-federation/ldap.adoc b/docs/documentation/server_admin/topics/user-federation/ldap.adoc index e3e08551d6a..4df2909017e 100644 --- a/docs/documentation/server_admin/topics/user-federation/ldap.adoc +++ b/docs/documentation/server_admin/topics/user-federation/ldap.adoc @@ -78,7 +78,7 @@ When you configure a secure connection URL to your LDAP store (for example,`ldap Configure the global truststore for {project_name} with the Truststore SPI. For more information about configuring the global truststore, see the https://www.keycloak.org/server/keycloak-truststore[Configuring a Truststore] {section}. If you do not configure the Truststore SPI, the truststore falls back to the default mechanism provided by Java, which can be the file supplied by the `javax.net.ssl.trustStore` system property or the cacerts file from the JDK if the system property is unset. -The `Use Truststore SPI` configuration property, in the LDAP federation provider configuration, controls the truststore SPI. By default, {project_name} sets the property to `Only for ldaps`, which is adequate for most deployments. {project_name} uses the Truststore SPI if the connection URL to LDAP starts with `ldaps` only. +The `Use Truststore SPI` configuration property, in the LDAP federation provider configuration, controls the truststore SPI. By default, {project_name} sets the property to `Always`, which is adequate for most deployments. {project_name} uses the Truststore SPI if the connection URL to LDAP starts with `ldaps` only. ==== Synchronizing LDAP users to {project_name} diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPConfig.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPConfig.java index 1044f269da7..acbb4213c48 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPConfig.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPConfig.java @@ -25,7 +25,6 @@ import javax.naming.directory.SearchControls; import java.util.Collection; import java.util.HashSet; -import java.util.List; import java.util.Properties; import java.util.Set; @@ -66,7 +65,12 @@ public class LDAPConfig { } public String getUseTruststoreSpi() { - return config.getFirst(LDAPConstants.USE_TRUSTSTORE_SPI); + String value = config.getFirst(LDAPConstants.USE_TRUSTSTORE_SPI); + if (LDAPConstants.USE_TRUSTSTORE_LDAPS_ONLY.equals(value)) { + value = LDAPConstants.USE_TRUSTSTORE_ALWAYS; + config.putSingle(LDAPConstants.USE_TRUSTSTORE_SPI, value); + } + return value; } public String getUsersDn() { diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java index fbb37836614..c8d88c60f91 100755 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java @@ -164,7 +164,7 @@ public class LDAPStorageProviderFactory implements UserStorageProviderFactoryorg.keycloak keycloak-model-map + + org.keycloak + keycloak-server-spi-private + commons-lang commons-lang diff --git a/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/store/LdapMapContextManager.java b/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/store/LdapMapContextManager.java index 535eed62330..a8af3e55bdc 100644 --- a/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/store/LdapMapContextManager.java +++ b/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/store/LdapMapContextManager.java @@ -96,8 +96,7 @@ public final class LdapMapContextManager implements AutoCloseable { ldapContext = new InitialLdapContext(connProp, null); if (ldapMapConfig.isStartTls()) { SSLSocketFactory sslSocketFactory = null; - String useTruststoreSpi = ldapMapConfig.getUseTruststoreSpi(); - if (useTruststoreSpi != null && useTruststoreSpi.equals(LDAPConstants.USE_TRUSTSTORE_ALWAYS)) { + if (LdapMapUtil.shouldUseTruststoreSpi(ldapMapConfig)) { TruststoreProvider provider = session.getProvider(TruststoreProvider.class); sslSocketFactory = provider.getSSLSocketFactory(); } @@ -204,9 +203,8 @@ public final class LdapMapContextManager implements AutoCloseable { // when using Start TLS, use default socket factory for LDAP client but pass the TrustStore SSL socket factory later // when calling StartTlsResponse.negotiate(trustStoreSSLSocketFactory) - if (!ldapMapConfig.isStartTls()) { - String useTruststoreSpi = ldapMapConfig.getUseTruststoreSpi(); - LDAPConstants.setTruststoreSpiIfNeeded(useTruststoreSpi, url, env); + if (LdapMapUtil.shouldUseTruststoreSpi(ldapMapConfig)) { + env.put("java.naming.ldap.factory.socket", "org.keycloak.truststore.SSLSocketFactory"); } String connectionPooling = ldapMapConfig.getConnectionPooling(); diff --git a/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/store/LdapMapUtil.java b/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/store/LdapMapUtil.java index c2aa933e1f2..af3ca55af4c 100644 --- a/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/store/LdapMapUtil.java +++ b/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/store/LdapMapUtil.java @@ -18,7 +18,9 @@ package org.keycloak.models.map.storage.ldap.store; import org.jboss.logging.Logger; +import org.keycloak.models.LDAPConstants; import org.keycloak.models.ModelException; +import org.keycloak.models.map.storage.ldap.config.LdapMapConfig; import java.text.SimpleDateFormat; import java.util.Date; @@ -253,4 +255,19 @@ public class LdapMapUtil { } } + public static boolean shouldUseTruststoreSpi(LdapMapConfig ldapConfig) { + boolean useSSL = ldapConfig.getConnectionUrl().toLowerCase().contains("ldaps://"); + boolean defaultUseTruststore = useSSL || ldapConfig.isStartTls(); + + String useTruststoreSpi = ldapConfig.getUseTruststoreSpi(); + if (useTruststoreSpi == null) { + return defaultUseTruststore; + } + + if (LDAPConstants.USE_TRUSTSTORE_NEVER.equals(useTruststoreSpi)) { + return false; + } + + return defaultUseTruststore; + } } diff --git a/server-spi-private/src/main/java/org/keycloak/models/LDAPConstants.java b/server-spi-private/src/main/java/org/keycloak/models/LDAPConstants.java index 6ce21ac134a..228e84687ce 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/LDAPConstants.java +++ b/server-spi-private/src/main/java/org/keycloak/models/LDAPConstants.java @@ -59,6 +59,11 @@ public class LDAPConstants { public static final String USE_TRUSTSTORE_SPI = "useTruststoreSpi"; public static final String USE_TRUSTSTORE_ALWAYS = "always"; public static final String USE_TRUSTSTORE_NEVER = "never"; + + /** + * @deprecated Use {@link #USE_TRUSTSTORE_ALWAYS} instead. + */ + @Deprecated public static final String USE_TRUSTSTORE_LDAPS_ONLY = "ldapsOnly"; public static final String SEARCH_SCOPE = "searchScope"; @@ -157,25 +162,6 @@ public class LDAPConstants { return ENTRY_UUID; } - - - public static void setTruststoreSpiIfNeeded(String useTruststoreSpi, String url, Map env) { - boolean shouldSetTruststore; - if (useTruststoreSpi != null && useTruststoreSpi.equals(LDAPConstants.USE_TRUSTSTORE_ALWAYS)) { - shouldSetTruststore = true; - } else if (useTruststoreSpi != null && useTruststoreSpi.equals(LDAPConstants.USE_TRUSTSTORE_NEVER)) { - shouldSetTruststore = false; - } else { - shouldSetTruststore = toLdapUrls(url).stream() - .anyMatch(urlPart -> urlPart.toLowerCase().startsWith("ldaps")); - } - - if (shouldSetTruststore) { - env.put("java.naming.ldap.factory.socket", "org.keycloak.truststore.SSLSocketFactory"); - } - } - - /** * @see com.sun.jndi.ldap.LdapURL#fromList(String) (Not using it directly to avoid usage of internal Java classes) * diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/LDAPRule.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/LDAPRule.java index 7c7fb47f9c2..ce8f346af2f 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/LDAPRule.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/LDAPRule.java @@ -249,7 +249,7 @@ public class LDAPRule extends ExternalResource { // Default to startTLS disabled config.put(LDAPConstants.START_TLS, "false"); // By default use truststore from TruststoreSPI only for LDAP over SSL connections - config.put(LDAPConstants.USE_TRUSTSTORE_SPI, LDAPConstants.USE_TRUSTSTORE_LDAPS_ONLY); + config.put(LDAPConstants.USE_TRUSTSTORE_SPI, LDAPConstants.USE_TRUSTSTORE_ALWAYS); } switch (defaultProperties.getProperty(LDAPEmbeddedServer.PROPERTY_SET_CONFIDENTIALITY_REQUIRED)) { case "true":