From 7f232f15107790f260532b49581814348fd4d9fc Mon Sep 17 00:00:00 2001 From: Stefan Guilhen Date: Thu, 23 May 2024 15:47:00 -0300 Subject: [PATCH] =?UTF-8?q?Switch=20to=20VaultStringSecret=20to=20avoid=20?= =?UTF-8?q?encoding=20issues=20when=20special=20characters=20(such=20as=20?= =?UTF-8?q?=C2=A7)=20are=20present=20in=20the=20ldap=20bind=20credential?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #29808 Signed-off-by: Stefan Guilhen --- .../idm/store/ldap/LDAPContextManager.java | 33 ++++++++----------- .../idm/store/ldap/LDAPOperationManager.java | 2 +- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPContextManager.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPContextManager.java index ad4c67e5a7..6e22057e61 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPContextManager.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPContextManager.java @@ -5,7 +5,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.LDAPConstants; import org.keycloak.storage.ldap.LDAPConfig; import org.keycloak.truststore.TruststoreProvider; -import org.keycloak.vault.VaultCharSecret; +import org.keycloak.vault.VaultStringSecret; import javax.naming.AuthenticationException; import javax.naming.Context; @@ -17,7 +17,6 @@ import javax.naming.ldap.StartTlsResponse; import javax.net.ssl.SSLSocketFactory; import java.io.IOException; -import java.nio.CharBuffer; import java.util.HashMap; import java.util.Hashtable; import java.util.Map; @@ -37,14 +36,9 @@ public final class LDAPContextManager implements AutoCloseable { private final LDAPConfig ldapConfig; private StartTlsResponse tlsResponse; - private VaultCharSecret vaultCharSecret = new VaultCharSecret() { + private VaultStringSecret vaultStringSecret = new VaultStringSecret() { @Override - public Optional get() { - return Optional.empty(); - } - - @Override - public Optional getAsArray() { + public Optional get() { return Optional.empty(); } @@ -69,11 +63,11 @@ public final class LDAPContextManager implements AutoCloseable { Hashtable connProp = getConnectionProperties(ldapConfig); if (!LDAPConstants.AUTH_TYPE_NONE.equals(ldapConfig.getAuthType())) { - vaultCharSecret = getVaultSecret(); + vaultStringSecret = getVaultSecret(); - if (vaultCharSecret != null && !ldapConfig.isStartTls() && ldapConfig.getBindCredential() != null) { - connProp.put(SECURITY_CREDENTIALS, vaultCharSecret.getAsArray() - .orElse(ldapConfig.getBindCredential().toCharArray())); + if (vaultStringSecret != null && !ldapConfig.isStartTls() && ldapConfig.getBindCredential() != null) { + connProp.put(SECURITY_CREDENTIALS, vaultStringSecret.get() + .orElse(ldapConfig.getBindCredential()).toCharArray()); } } @@ -86,8 +80,7 @@ public final class LDAPContextManager implements AutoCloseable { } tlsResponse = startTLS(ldapContext, ldapConfig.getAuthType(), ldapConfig.getBindDN(), - vaultCharSecret.getAsArray().orElse(ldapConfig.getBindCredential() != null? ldapConfig.getBindCredential().toCharArray() : null), - sslSocketFactory); + vaultStringSecret.get().orElse(ldapConfig.getBindCredential()), sslSocketFactory); // Exception should be already thrown by LDAPContextManager.startTLS if "startTLS" could not be established, but rather do some additional check if (tlsResponse == null) { @@ -102,13 +95,13 @@ public final class LDAPContextManager implements AutoCloseable { return ldapContext; } - private VaultCharSecret getVaultSecret() { + private VaultStringSecret getVaultSecret() { return LDAPConstants.AUTH_TYPE_NONE.equals(ldapConfig.getAuthType()) ? null - : session.vault().getCharSecret(ldapConfig.getBindCredential()); + : session.vault().getStringSecret(ldapConfig.getBindCredential()); } - public static StartTlsResponse startTLS(LdapContext ldapContext, String authType, String bindDN, char[] bindCredential, SSLSocketFactory sslSocketFactory) throws NamingException { + public static StartTlsResponse startTLS(LdapContext ldapContext, String authType, String bindDN, String bindCredential, SSLSocketFactory sslSocketFactory) throws NamingException { StartTlsResponse tls = null; try { @@ -119,7 +112,7 @@ public final class LDAPContextManager implements AutoCloseable { if (!LDAPConstants.AUTH_TYPE_NONE.equals(authType)) { ldapContext.addToEnvironment(Context.SECURITY_PRINCIPAL, bindDN); - ldapContext.addToEnvironment(Context.SECURITY_CREDENTIALS, bindCredential); + ldapContext.addToEnvironment(Context.SECURITY_CREDENTIALS, bindCredential != null ? bindCredential.toCharArray() : null); } } catch (Exception e) { logger.error("Could not negotiate TLS", e); @@ -245,7 +238,7 @@ public final class LDAPContextManager implements AutoCloseable { @Override public void close() { - if (vaultCharSecret != null) vaultCharSecret.close(); + if (vaultStringSecret != null) vaultStringSecret.close(); if (tlsResponse != null) { try { tlsResponse.close(); diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java index 8f61aaad41..89fda2d183 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java @@ -507,7 +507,7 @@ public class LDAPOperationManager { sslSocketFactory = provider.getSSLSocketFactory(); } - tlsResponse = LDAPContextManager.startTLS(authCtx, "simple", dn.toString(), password.toCharArray(), sslSocketFactory); + tlsResponse = LDAPContextManager.startTLS(authCtx, "simple", dn.toString(), password, sslSocketFactory); // Exception should be already thrown by LDAPContextManager.startTLS if "startTLS" could not be established, but rather do some additional check if (tlsResponse == null) {