Skip to content

Commit 754c070

Browse files
pedroigorahus1stianst
authored
[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 <[email protected]> * Updating docs Signed-off-by: Alexander Schwartz <[email protected]> * Adjusting CI for slowness Signed-off-by: Alexander Schwartz <[email protected]> * Update docs/documentation/release_notes/topics/26_4_6.adoc Co-authored-by: Pedro Igor <[email protected]> Signed-off-by: Stian Thorgersen <[email protected]> --------- Signed-off-by: Pedro Igor <[email protected]> Signed-off-by: Alexander Schwartz <[email protected]> Signed-off-by: Stian Thorgersen <[email protected]> Co-authored-by: Alexander Schwartz <[email protected]> Co-authored-by: Stian Thorgersen <[email protected]>
1 parent dcbb5c7 commit 754c070

File tree

11 files changed

+207
-3
lines changed

11 files changed

+207
-3
lines changed

.github/workflows/ci.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ on:
55
branches-ignore:
66
- main
77
- dependabot/**
8+
- issue*
89
pull_request:
910
workflow_dispatch:
1011

@@ -573,7 +574,7 @@ jobs:
573574
name: Store IT
574575
needs: build
575576
runs-on: ubuntu-latest
576-
timeout-minutes: 75
577+
timeout-minutes: 90
577578
strategy:
578579
matrix:
579580
db: [postgres, mysql, oracle, mssql, mariadb, tidb]
@@ -871,7 +872,7 @@ jobs:
871872
runs-on: ubuntu-latest
872873
needs:
873874
- build
874-
timeout-minutes: 45
875+
timeout-minutes: 60
875876
steps:
876877
- uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
877878

.github/workflows/documentation.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ on:
66
- main
77
- dependabot/**
88
- quarkus-next
9+
- issue*
910
pull_request:
1011
workflow_dispatch:
1112

.github/workflows/js-ci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ on:
66
- main
77
- dependabot/**
88
- quarkus-next
9+
- issue*
910
pull_request:
1011
workflow_dispatch:
1112

.github/workflows/operator-ci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ on:
55
branches-ignore:
66
- main
77
- dependabot/**
8+
- issue*
89
pull_request:
910
workflow_dispatch:
1011

docs/documentation/release_notes/index.adoc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ include::topics/templates/document-attributes.adoc[]
1313
:release_header_latest_link: {releasenotes_link_latest}
1414
include::topics/templates/release-header.adoc[]
1515

16+
== {project_name_full} 26.4.6
17+
include::topics/26_4_6.adoc[leveloffset=2]
18+
1619
== {project_name_full} 26.4.0
1720
include::topics/26_4_0.adoc[leveloffset=2]
1821

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// Release notes should contain only headline-worthy new features,
2+
// assuming that people who migrate will read the upgrading guide anyway.
3+
4+
This release adds filtering of LDAP referrals by default.
5+
This change enhances security and aligns with best practices for LDAP configurations.
6+
7+
If you can not upgrade to this release yet, we recommend disabling LDAP referrals in all LDAP providers in all of your realms.
8+
9+
For detailed upgrade instructions, https://www.keycloak.org/docs/latest/upgrading/index.html[review the upgrading guide].
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// ------------------------ Breaking changes ------------------------ //
2+
== Notable changes
3+
4+
Notable changes may include internal behavior changes that prevent common misconfigurations, bugs that are fixed, or changes to simplify running {project_name}.
5+
6+
=== LDAP referrals filtered to allow only LDAP referrals
7+
8+
LDAP referrals now by default are only allowed to include LDAP URLs.
9+
This change enhances security and aligns with best practices for LDAP configurations.
10+
11+
This also prevents other JDNI references from being used in case you have written custom extensions.
12+
To restore the original behavior, set the option `spi-storage--ldap--secure-referral` to `false`.
13+
When doing this, we recommend to disable LDAP referrals in all LDAP providers.
14+
15+
== Deprecated features
16+
17+
The following sections provide details on deprecated features.
18+
19+
=== Disabling filtering of LDAP referrals
20+
21+
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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
[[migration-changes]]
22
== Migration Changes
33

4+
=== Migrating to 26.4.6
5+
6+
include::changes-26_4_6.adoc[leveloffset=2]
7+
48
=== Migrating to 26.4.3
59

610
include::changes-26_4_3.adoc[leveloffset=2]

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@
7373
import java.util.function.Function;
7474
import java.util.stream.Collectors;
7575

76+
import javax.naming.NamingException;
77+
import javax.naming.spi.NamingManager;
78+
7679
/**
7780
* @author <a href="mailto:[email protected]">Marek Posolda</a>
7881
* @author <a href="mailto:[email protected]">Bill Burke</a>
@@ -84,6 +87,8 @@ public class LDAPStorageProviderFactory implements UserStorageProviderFactory<LD
8487
private static final Logger logger = Logger.getLogger(LDAPStorageProviderFactory.class);
8588
public static final String PROVIDER_NAME = LDAPConstants.LDAP_PROVIDER;
8689
private static final String LDAP_CONNECTION_POOL_PROTOCOL = "com.sun.jndi.ldap.connect.pool.protocol";
90+
private static final String SECURE_REFERRAL = "secureReferral";
91+
private static final boolean SECURE_REFERRAL_DEFAULT = true;
8792

8893
private LDAPIdentityStoreRegistry ldapStoreRegistry;
8994

@@ -301,13 +306,36 @@ public void validateConfiguration(KeycloakSession session, RealmModel realm, Com
301306

302307
@Override
303308
public void init(Config.Scope config) {
309+
if (config.getBoolean(SECURE_REFERRAL, SECURE_REFERRAL_DEFAULT)) {
310+
setObjectFactoryBuilder();
311+
} else {
312+
logger.warnf("Insecure LDAP referrals are enabled. The option 'secure-referral' is deprecated and it will be removed in future releases.");
313+
}
314+
304315
// set connection pooling for plain and tls protocols by default
305316
if (System.getProperty(LDAP_CONNECTION_POOL_PROTOCOL) == null) {
306317
System.setProperty(LDAP_CONNECTION_POOL_PROTOCOL, "plain ssl");
307318
}
319+
308320
this.ldapStoreRegistry = new LDAPIdentityStoreRegistry();
309321
}
310322

323+
@Override
324+
public List<ProviderConfigProperty> getConfigMetadata() {
325+
326+
ProviderConfigurationBuilder builder = ProviderConfigurationBuilder.create();
327+
328+
builder.property()
329+
.name(SECURE_REFERRAL)
330+
.type("boolean")
331+
.helpText("Allow only secure LDAP referrals (deprecated)")
332+
.defaultValue(SECURE_REFERRAL_DEFAULT)
333+
.add();
334+
335+
return builder.build();
336+
337+
}
338+
311339
@Override
312340
public void close() {
313341
this.ldapStoreRegistry = null;
@@ -727,4 +755,15 @@ protected KerberosUsernamePasswordAuthenticator createKerberosUsernamePasswordAu
727755
return new KerberosUsernamePasswordAuthenticator(kerberosConfig);
728756
}
729757

758+
private void setObjectFactoryBuilder() {
759+
try {
760+
NamingManager.setObjectFactoryBuilder(new ObjectFactoryBuilder());
761+
} catch (NamingException | IllegalStateException e) {
762+
if (e instanceof IllegalStateException && ObjectFactoryBuilder.isSet()) {
763+
return;
764+
}
765+
766+
throw new RuntimeException("Failed to set the server JNDI ObjectFactoryBuilder", e);
767+
}
768+
}
730769
}
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
package org.keycloak.storage.ldap;
2+
3+
import java.util.Hashtable;
4+
import java.util.List;
5+
import javax.naming.CommunicationException;
6+
import javax.naming.Context;
7+
import javax.naming.Name;
8+
import javax.naming.NamingException;
9+
import javax.naming.RefAddr;
10+
import javax.naming.Reference;
11+
import javax.naming.ldap.LdapContext;
12+
import javax.naming.spi.NamingManager;
13+
import javax.naming.spi.ObjectFactory;
14+
15+
import org.jboss.logging.Logger;
16+
import org.keycloak.storage.ldap.idm.store.ldap.SessionBoundInitialLdapContext;
17+
import org.keycloak.utils.KeycloakSessionUtil;
18+
19+
/**
20+
* <p>A {@link javax.naming.spi.ObjectFactoryBuilder} implementation to filter out referral references if they do not
21+
* point to an LDAP URL.
22+
*
23+
* <p>When the LDAP provider encounters a referral, it tries to create an {@link ObjectFactory} from this builder.
24+
* If the referral reference contains an LDAP URL, a {@link DirContextObjectFactory} is created to handle the referral.
25+
* Otherwise, a {@link CommunicationException} is thrown to indicate that the referral cannot be processed.
26+
*/
27+
final class ObjectFactoryBuilder implements javax.naming.spi.ObjectFactoryBuilder, ObjectFactory {
28+
29+
private static final Logger logger = Logger.getLogger(ObjectFactoryBuilder.class);
30+
private static final String IS_KC_OBJECT_FACTORY_BUILDER = "kc.jndi.object.factory.builder";
31+
32+
static boolean isSet() {
33+
Hashtable<Object, Object> env = new Hashtable<>();
34+
35+
env.put(ObjectFactoryBuilder.IS_KC_OBJECT_FACTORY_BUILDER, Boolean.TRUE);
36+
37+
try {
38+
Object instance = NamingManager.getObjectInstance(null, null, null, env);
39+
40+
if (instance != null && instance.getClass().getName().equals(ObjectFactoryBuilder.class.getName())) {
41+
return true;
42+
}
43+
} catch (Exception e) {
44+
throw new RuntimeException("Failed to determine if ObjectFactoryBuilder is set", e);
45+
}
46+
47+
return false;
48+
}
49+
50+
@Override
51+
public ObjectFactory createObjectFactory(Object obj, Hashtable<?, ?> environment) throws NamingException {
52+
if (logger.isTraceEnabled()) {
53+
logger.tracef("Creating ObjectFactory for object: %s", obj);
54+
}
55+
56+
if (obj instanceof Reference ref) {
57+
String factoryClassName = ref.getFactoryClassName();
58+
59+
if (factoryClassName != null) {
60+
logger.warnf("Referral refence contains an object factory %s but it will be ignored", factoryClassName);
61+
}
62+
63+
String ldapUrl = getLdapUrl(ref);
64+
65+
if (ldapUrl != null) {
66+
return new DirContextObjectFactory(ldapUrl);
67+
}
68+
} else {
69+
logger.debugf("Unsupported reference object of type %s: ", obj);
70+
return this;
71+
}
72+
73+
throw new CommunicationException("Referral reference does not contain an LDAP URL: " + obj);
74+
}
75+
76+
@Override
77+
public Object getObjectInstance(Object obj, Name name, Context nameCtx, Hashtable<?, ?> env) {
78+
if (env != null && env.containsKey(IS_KC_OBJECT_FACTORY_BUILDER)) {
79+
return this;
80+
}
81+
return obj;
82+
}
83+
84+
private String getLdapUrl(Reference ref) {
85+
for (int i = 0; i < ref.size(); i++) {
86+
RefAddr addr = ref.get(i);
87+
String addrType = addr.getType();
88+
89+
if ("URL".equalsIgnoreCase(addrType)) {
90+
Object content = addr.getContent();
91+
92+
if (content == null) {
93+
return null;
94+
}
95+
96+
String rawUrl = content.toString();
97+
98+
for (String url : List.of(rawUrl.split(" "))) {
99+
if (!url.toLowerCase().startsWith("ldap")) {
100+
logger.warnf("Unsupported scheme from reference URL %s. Ignoring reference.", url);
101+
return null;
102+
}
103+
}
104+
105+
return rawUrl;
106+
} else {
107+
logger.warnf("Ignoring address of type '%s' from referral reference", addrType);
108+
}
109+
}
110+
111+
return null;
112+
}
113+
114+
private record DirContextObjectFactory(String ldapUrl) implements ObjectFactory {
115+
116+
@Override
117+
public Object getObjectInstance(Object obj, Name name, Context nameCtx, Hashtable<?, ?> env) throws Exception {
118+
@SuppressWarnings("unchecked")
119+
Hashtable<Object, Object> newEnv = (Hashtable<Object, Object>) env.clone();
120+
newEnv.put(LdapContext.PROVIDER_URL, ldapUrl);
121+
return new SessionBoundInitialLdapContext(KeycloakSessionUtil.getKeycloakSession(), newEnv, null);
122+
}
123+
}
124+
}

0 commit comments

Comments
 (0)