Switch to VaultStringSecret to avoid encoding issues when special characters (such as §) are present in the ldap bind credential

Closes #29808

Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>
This commit is contained in:
Stefan Guilhen 2024-05-23 15:47:00 -03:00 committed by Pedro Igor
parent 568a5cb678
commit 7f232f1510
2 changed files with 14 additions and 21 deletions

View file

@ -5,7 +5,7 @@ import org.keycloak.models.KeycloakSession;
import org.keycloak.models.LDAPConstants; import org.keycloak.models.LDAPConstants;
import org.keycloak.storage.ldap.LDAPConfig; import org.keycloak.storage.ldap.LDAPConfig;
import org.keycloak.truststore.TruststoreProvider; import org.keycloak.truststore.TruststoreProvider;
import org.keycloak.vault.VaultCharSecret; import org.keycloak.vault.VaultStringSecret;
import javax.naming.AuthenticationException; import javax.naming.AuthenticationException;
import javax.naming.Context; import javax.naming.Context;
@ -17,7 +17,6 @@ import javax.naming.ldap.StartTlsResponse;
import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.SSLSocketFactory;
import java.io.IOException; import java.io.IOException;
import java.nio.CharBuffer;
import java.util.HashMap; import java.util.HashMap;
import java.util.Hashtable; import java.util.Hashtable;
import java.util.Map; import java.util.Map;
@ -37,14 +36,9 @@ public final class LDAPContextManager implements AutoCloseable {
private final LDAPConfig ldapConfig; private final LDAPConfig ldapConfig;
private StartTlsResponse tlsResponse; private StartTlsResponse tlsResponse;
private VaultCharSecret vaultCharSecret = new VaultCharSecret() { private VaultStringSecret vaultStringSecret = new VaultStringSecret() {
@Override @Override
public Optional<CharBuffer> get() { public Optional<String> get() {
return Optional.empty();
}
@Override
public Optional<char[]> getAsArray() {
return Optional.empty(); return Optional.empty();
} }
@ -69,11 +63,11 @@ public final class LDAPContextManager implements AutoCloseable {
Hashtable<Object, Object> connProp = getConnectionProperties(ldapConfig); Hashtable<Object, Object> connProp = getConnectionProperties(ldapConfig);
if (!LDAPConstants.AUTH_TYPE_NONE.equals(ldapConfig.getAuthType())) { if (!LDAPConstants.AUTH_TYPE_NONE.equals(ldapConfig.getAuthType())) {
vaultCharSecret = getVaultSecret(); vaultStringSecret = getVaultSecret();
if (vaultCharSecret != null && !ldapConfig.isStartTls() && ldapConfig.getBindCredential() != null) { if (vaultStringSecret != null && !ldapConfig.isStartTls() && ldapConfig.getBindCredential() != null) {
connProp.put(SECURITY_CREDENTIALS, vaultCharSecret.getAsArray() connProp.put(SECURITY_CREDENTIALS, vaultStringSecret.get()
.orElse(ldapConfig.getBindCredential().toCharArray())); .orElse(ldapConfig.getBindCredential()).toCharArray());
} }
} }
@ -86,8 +80,7 @@ public final class LDAPContextManager implements AutoCloseable {
} }
tlsResponse = startTLS(ldapContext, ldapConfig.getAuthType(), ldapConfig.getBindDN(), tlsResponse = startTLS(ldapContext, ldapConfig.getAuthType(), ldapConfig.getBindDN(),
vaultCharSecret.getAsArray().orElse(ldapConfig.getBindCredential() != null? ldapConfig.getBindCredential().toCharArray() : null), vaultStringSecret.get().orElse(ldapConfig.getBindCredential()), sslSocketFactory);
sslSocketFactory);
// Exception should be already thrown by LDAPContextManager.startTLS if "startTLS" could not be established, but rather do some additional check // Exception should be already thrown by LDAPContextManager.startTLS if "startTLS" could not be established, but rather do some additional check
if (tlsResponse == null) { if (tlsResponse == null) {
@ -102,13 +95,13 @@ public final class LDAPContextManager implements AutoCloseable {
return ldapContext; return ldapContext;
} }
private VaultCharSecret getVaultSecret() { private VaultStringSecret getVaultSecret() {
return LDAPConstants.AUTH_TYPE_NONE.equals(ldapConfig.getAuthType()) return LDAPConstants.AUTH_TYPE_NONE.equals(ldapConfig.getAuthType())
? null ? 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; StartTlsResponse tls = null;
try { try {
@ -119,7 +112,7 @@ public final class LDAPContextManager implements AutoCloseable {
if (!LDAPConstants.AUTH_TYPE_NONE.equals(authType)) { if (!LDAPConstants.AUTH_TYPE_NONE.equals(authType)) {
ldapContext.addToEnvironment(Context.SECURITY_PRINCIPAL, bindDN); 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) { } catch (Exception e) {
logger.error("Could not negotiate TLS", e); logger.error("Could not negotiate TLS", e);
@ -245,7 +238,7 @@ public final class LDAPContextManager implements AutoCloseable {
@Override @Override
public void close() { public void close() {
if (vaultCharSecret != null) vaultCharSecret.close(); if (vaultStringSecret != null) vaultStringSecret.close();
if (tlsResponse != null) { if (tlsResponse != null) {
try { try {
tlsResponse.close(); tlsResponse.close();

View file

@ -507,7 +507,7 @@ public class LDAPOperationManager {
sslSocketFactory = provider.getSSLSocketFactory(); 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 // Exception should be already thrown by LDAPContextManager.startTLS if "startTLS" could not be established, but rather do some additional check
if (tlsResponse == null) { if (tlsResponse == null) {