[26.4] Only allow LDAP URL references when following referrals (#285)

* Only allow LDAP URL references when following referrals

Closes #280

Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>

* Updating docs

Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>

* Adjusting CI for slowness

Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>

* Update docs/documentation/release_notes/topics/26_4_6.adoc

Co-authored-by: Pedro Igor <pigor.craveiro@gmail.com>
Signed-off-by: Stian Thorgersen <stian@redhat.com>

---------

Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
Signed-off-by: Stian Thorgersen <stian@redhat.com>
Co-authored-by: Alexander Schwartz <alexander.schwartz@ibm.com>
Co-authored-by: Stian Thorgersen <stianst@gmail.com>

.github/workflows/ci.yml

@@ -5,6 +5,7 @@ on:
     branches-ignore:
       - main
       - dependabot/**
+      - issue*
   pull_request:
   workflow_dispatch:
 
@@ -573,7 +574,7 @@ jobs:
     name: Store IT
     needs: build
     runs-on: ubuntu-latest
-    timeout-minutes: 75
+    timeout-minutes: 90
     strategy:
       matrix:
         db: [postgres, mysql, oracle, mssql, mariadb, tidb]
@@ -871,7 +872,7 @@ jobs:
     runs-on: ubuntu-latest
     needs:
       - build
-    timeout-minutes: 45
+    timeout-minutes: 60
     steps:
       - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
 

.github/workflows/documentation.yml

@@ -6,6 +6,7 @@ on:
       - main
       - dependabot/**
       - quarkus-next
+      - issue*
   pull_request:
   workflow_dispatch:
 

.github/workflows/js-ci.yml

@@ -6,6 +6,7 @@ on:
       - main
       - dependabot/**
       - quarkus-next
+      - issue*
   pull_request:
   workflow_dispatch:
 

.github/workflows/operator-ci.yml

@@ -5,6 +5,7 @@ on:
     branches-ignore:
       - main
       - dependabot/**
+      - issue*
   pull_request:
   workflow_dispatch:
 

docs/documentation/release_notes/index.adoc

@@ -13,6 +13,9 @@ include::topics/templates/document-attributes.adoc[]
 :release_header_latest_link: {releasenotes_link_latest}
 include::topics/templates/release-header.adoc[]
 
+== {project_name_full} 26.4.6
+include::topics/26_4_6.adoc[leveloffset=2]
+
 == {project_name_full} 26.4.0
 include::topics/26_4_0.adoc[leveloffset=2]
 

docs/documentation/release_notes/topics/26_4_6.adoc

@@ -0,0 +1,9 @@
+// Release notes should contain only headline-worthy new features,
+// assuming that people who migrate will read the upgrading guide anyway.
+
+This release adds filtering of LDAP referrals by default.
+This change enhances security and aligns with best practices for LDAP configurations.
+
+If you can not upgrade to this release yet, we recommend disabling LDAP referrals in all LDAP providers in all of your realms.
+
+For detailed upgrade instructions, https://www.keycloak.org/docs/latest/upgrading/index.html[review the upgrading guide].

docs/documentation/upgrading/topics/changes/changes-26_4_6.adoc

@@ -0,0 +1,21 @@
+// ------------------------ Breaking changes ------------------------  //
+== Notable changes
+
+Notable changes may include internal behavior changes that prevent common misconfigurations, bugs that are fixed, or changes to simplify running {project_name}.
+
+=== LDAP referrals filtered to allow only LDAP referrals
+
+LDAP referrals now by default are only allowed to include LDAP URLs.
+This change enhances security and aligns with best practices for LDAP configurations.
+
+This also prevents other JDNI references from being used in case you have written custom extensions.
+To restore the original behavior, set the option `spi-storage--ldap--secure-referral` to `false`.
+When doing this, we recommend to disable LDAP referrals in all LDAP providers.
+
+== Deprecated features
+
+The following sections provide details on deprecated features.
+
+=== Disabling filtering of LDAP referrals
+
+The option `spi-storage--ldap--secure-referral` to disable filtering referrals is deprecated. It will be removed in a future release and filtering will then be enforced.

docs/documentation/upgrading/topics/changes/changes.adoc

@@ -1,6 +1,10 @@
 [[migration-changes]]
 == Migration Changes
 
+=== Migrating to 26.4.6
+
+include::changes-26_4_6.adoc[leveloffset=2]
+
 === Migrating to 26.4.3
 
 include::changes-26_4_3.adoc[leveloffset=2]

federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java

@@ -73,6 +73,9 @@ import java.util.Optional;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
+import javax.naming.NamingException;
+import javax.naming.spi.NamingManager;
+
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
@@ -84,6 +87,8 @@ public class LDAPStorageProviderFactory implements UserStorageProviderFactory<LD
     private static final Logger logger = Logger.getLogger(LDAPStorageProviderFactory.class);
     public static final String PROVIDER_NAME = LDAPConstants.LDAP_PROVIDER;
     private static final String LDAP_CONNECTION_POOL_PROTOCOL = "com.sun.jndi.ldap.connect.pool.protocol";
+    private static final String SECURE_REFERRAL = "secureReferral";
+    private static final boolean SECURE_REFERRAL_DEFAULT = true;
 
     private LDAPIdentityStoreRegistry ldapStoreRegistry;
 
@@ -301,13 +306,36 @@ public class LDAPStorageProviderFactory implements UserStorageProviderFactory<LD
 
     @Override
     public void init(Config.Scope config) {
+        if (config.getBoolean(SECURE_REFERRAL, SECURE_REFERRAL_DEFAULT)) {
+            setObjectFactoryBuilder();
+        } else {
+            logger.warnf("Insecure LDAP referrals are enabled. The option 'secure-referral' is deprecated and it will be removed in future releases.");
+        }
+
         // set connection pooling for plain and tls protocols by default
         if (System.getProperty(LDAP_CONNECTION_POOL_PROTOCOL) == null) {
             System.setProperty(LDAP_CONNECTION_POOL_PROTOCOL, "plain ssl");
         }
+
         this.ldapStoreRegistry = new LDAPIdentityStoreRegistry();
     }
 
+    @Override
+    public List<ProviderConfigProperty> getConfigMetadata() {
+
+        ProviderConfigurationBuilder builder = ProviderConfigurationBuilder.create();
+
+        builder.property()
+                .name(SECURE_REFERRAL)
+                .type("boolean")
+                .helpText("Allow only secure LDAP referrals (deprecated)")
+                .defaultValue(SECURE_REFERRAL_DEFAULT)
+                .add();
+
+        return builder.build();
+
+    }
+
     @Override
     public void close() {
         this.ldapStoreRegistry = null;
@@ -727,4 +755,15 @@ public class LDAPStorageProviderFactory implements UserStorageProviderFactory<LD
         return new KerberosUsernamePasswordAuthenticator(kerberosConfig);
     }
 
+    private void setObjectFactoryBuilder() {
+        try {
+            NamingManager.setObjectFactoryBuilder(new ObjectFactoryBuilder());
+        } catch (NamingException | IllegalStateException e) {
+            if (e instanceof IllegalStateException && ObjectFactoryBuilder.isSet()) {
+                return;
+            }
+
+            throw new RuntimeException("Failed to set the server JNDI ObjectFactoryBuilder", e);
+        }
+    }
  }

federation/ldap/src/main/java/org/keycloak/storage/ldap/ObjectFactoryBuilder.java

@@ -0,0 +1,124 @@
+package org.keycloak.storage.ldap;
+
+import java.util.Hashtable;
+import java.util.List;
+import javax.naming.CommunicationException;
+import javax.naming.Context;
+import javax.naming.Name;
+import javax.naming.NamingException;
+import javax.naming.RefAddr;
+import javax.naming.Reference;
+import javax.naming.ldap.LdapContext;
+import javax.naming.spi.NamingManager;
+import javax.naming.spi.ObjectFactory;
+
+import org.jboss.logging.Logger;
+import org.keycloak.storage.ldap.idm.store.ldap.SessionBoundInitialLdapContext;
+import org.keycloak.utils.KeycloakSessionUtil;
+
+/**
+ * <p>A {@link javax.naming.spi.ObjectFactoryBuilder} implementation to filter out referral references if they do not
+ * point to an LDAP URL.
+ *
+ * <p>When the LDAP provider encounters a referral, it tries to create an {@link ObjectFactory} from this builder.
+ * If the referral reference contains an LDAP URL, a {@link DirContextObjectFactory} is created to handle the referral.
+ * Otherwise, a {@link CommunicationException} is thrown to indicate that the referral cannot be processed.
+ */
+final class ObjectFactoryBuilder implements javax.naming.spi.ObjectFactoryBuilder, ObjectFactory {
+
+    private static final Logger logger = Logger.getLogger(ObjectFactoryBuilder.class);
+    private static final String IS_KC_OBJECT_FACTORY_BUILDER = "kc.jndi.object.factory.builder";
+
+    static boolean isSet() {
+        Hashtable<Object, Object> env = new Hashtable<>();
+
+        env.put(ObjectFactoryBuilder.IS_KC_OBJECT_FACTORY_BUILDER, Boolean.TRUE);
+
+        try {
+            Object instance = NamingManager.getObjectInstance(null, null, null, env);
+
+            if (instance != null && instance.getClass().getName().equals(ObjectFactoryBuilder.class.getName())) {
+                return true;
+            }
+        } catch (Exception e) {
+            throw new RuntimeException("Failed to determine if ObjectFactoryBuilder is set", e);
+        }
+
+        return false;
+    }
+
+    @Override
+    public ObjectFactory createObjectFactory(Object obj, Hashtable<?, ?> environment) throws NamingException {
+        if (logger.isTraceEnabled()) {
+            logger.tracef("Creating ObjectFactory for object: %s", obj);
+        }
+
+        if (obj instanceof Reference ref) {
+            String factoryClassName = ref.getFactoryClassName();
+
+            if (factoryClassName != null) {
+                logger.warnf("Referral refence contains an object factory %s but it will be ignored", factoryClassName);
+            }
+
+            String ldapUrl = getLdapUrl(ref);
+
+            if (ldapUrl != null) {
+                return new DirContextObjectFactory(ldapUrl);
+            }
+        } else {
+            logger.debugf("Unsupported reference object of type %s: ", obj);
+            return this;
+        }
+
+        throw new CommunicationException("Referral reference does not contain an LDAP URL: " + obj);
+    }
+
+    @Override
+    public Object getObjectInstance(Object obj, Name name, Context nameCtx, Hashtable<?, ?> env) {
+        if (env != null && env.containsKey(IS_KC_OBJECT_FACTORY_BUILDER)) {
+            return this;
+        }
+        return obj;
+    }
+
+    private String getLdapUrl(Reference ref) {
+        for (int i = 0; i < ref.size(); i++) {
+            RefAddr addr = ref.get(i);
+            String addrType = addr.getType();
+
+            if ("URL".equalsIgnoreCase(addrType)) {
+                Object content = addr.getContent();
+
+                if (content == null) {
+                    return null;
+                }
+
+                String rawUrl = content.toString();
+
+                for (String url : List.of(rawUrl.split(" "))) {
+                    if (!url.toLowerCase().startsWith("ldap")) {
+                        logger.warnf("Unsupported scheme from reference URL %s. Ignoring reference.", url);
+                        return null;
+                    }
+                }
+
+                return rawUrl;
+            } else {
+                logger.warnf("Ignoring address of type '%s' from referral reference", addrType);
+            }
+        }
+
+        return null;
+    }
+
+    private record DirContextObjectFactory(String ldapUrl) implements ObjectFactory {
+
+        @Override
+        public Object getObjectInstance(Object obj, Name name, Context nameCtx, Hashtable<?, ?> env) throws Exception {
+            @SuppressWarnings("unchecked")
+            Hashtable<Object, Object> newEnv = (Hashtable<Object, Object>) env.clone();
+            newEnv.put(LdapContext.PROVIDER_URL, ldapUrl);
+            return new SessionBoundInitialLdapContext(KeycloakSessionUtil.getKeycloakSession(), newEnv, null);
+        }
+    }
+}

quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ImportDistTest.java

@@ -85,7 +85,7 @@ public class ImportDistTest {
         ExecutorService ex = Executors.newFixedThreadPool(1);
         Future<CLIResult> result = ex.submit(() -> dist.run("import", "--dir=" + dir.getAbsolutePath()));
         try {
-            cliResult = result.get(20, TimeUnit.SECONDS);
+            cliResult = result.get(40, TimeUnit.SECONDS);
             cliResult.assertMessage("Realm 'master' imported");
             cliResult.assertMessage("Import finished successfully");
             cliResult.assertMessage("master-users-0.json");