KEYCLOAK-14055 Add SASL EXTERNAL authentication for LDAP federation #7365

tsaarni · 2020-08-24T07:12:17Z

This change adds support for SASL EXTERNAL method, using x509 certificate as client credentials for LDAP server administrator. It works with both: LDAPS and LDAP + StartTLS

What is included:

keystore SPI: introduced new SPI for configuring keystores (ref: comment)
default keystore SPI implementation: keystore that can load credentials from keystore files and PEM files, and hot-reload the files when they change (during rotation / renewal of the certificates)
implemented new socket factory LDAPSSLSocketFactory that is used by LDAP federation code only and not shared with other functionality
tests: added support for SASL EXTERNAL authentication to LDAPEmbeddedServer
tests: added support for LDAP admin to authenticate with X509 cert towards LDAPEmbeddedServer
tests: added unit test for keystore

What is not inlcuded:

Documentation, which needs to be rewritten (was ~~Documentation is in another PR: KEYCLOAK-14055: Add SASL EXTERNAL authentication for LDAP federation keycloak-documentation#1373~~ now obsolete)
UI changes

KeyStore SPI

The reason to introduce new SPI is to allow administrator to configure client certificate and private key for LDAP (ref: comment).

KeyStore SPI simply returns KeyStores - either as plain JDK KeyStore object or KeyStore.Builder which encapsulates also the password required to access key entries. The SPI can serve several different KeyStores. The caller gives keystore identifier as argument when requesting a keystore (string such as ldap-client-keystore) and the SPI implementation must figure out which concrete keystore to instantiate and return for that particular identifier / use case. See keycloak/keycloak-community#345 for more information.

Since KeyStore can include also CA certificates, in future it would be possible to use KeyStore SPI also for trust anchors. The difference betweek using KeyStore for credentials vs trust anchors is just wether it is passed to KeyManager or TrustManager by the code using it. Therefore, there is possibility of moving from Truststore SPI to KeyStore SPI. We could have many truststores: LDAP code could request KeyStore SPI to instantiate ldap-client-truststore, SMTP code could request for smtp-client-truststore and the instantiated keystores could have different CA certificates in them. It would be possible not to "leak" trust anchors: SMTP use case should not trust CAs that issued LDAP server certificate, LDAP use case should not trust CAs that issued SMTP server certificate.

However, this PR does not change Truststore SPI or remove its use in LDAP federation code. Currently administrator still can confiugre only one set of CA certificates that will be shared between the use cases.

Default KeyStore SPI implementation

The default KeyStore SPI implementation can load credentials from PKCS#12 and JKS keystore files, and also directly from PEM files. It can also reload the credentials from disk when they changed, without requiring server to be restarted.

The implementation of the KeyStore handling code is a copy from my own project https://github.com/tsaarni/reloading-keystore which I'm willing to contribute under Keycloak (=Red Hat) copyright if you want, or implement something completely new.

LDAP SSL Socket Factory

Current Truststore SPI provides also socket factory. The LDAP code is now changed to use its own socket factory which is initialized with the LDAP credentials. It does not use the Truststore SPI socket factory anymore. The motivation here is to make it clear which credentials are used for what purpose and not allow them to "leak" via shared socket factory. Configured LDAP credentials should not be used for SMTP for example, which would have happened if Truststore SPI socket factory would be used.

However, this PR does not remove the socket factory in Truststore SPI. which is still used by DefaultEmailSenderProvider. TrustStore SPI is also used for trusted certificates for LDAP federation. In my opinion, it would be good to separate these as well in the future. Both LDAP code and the email sender provider could fetch their trust anchors from KeyStore SPI and to have its own respective socket factories initialized with their own CA certificates and optional client credentials. The additional benefit would be that trust anchors could be provided as PEM in addition to keystore files. Adding PEM support for more use cases seems aligned with the fact that Quarkus can use credentials in PEM format for HTTPS.

Fixes #11725

Note: the code in this PR and the description has been modified several times since the creation of this PR (note the early creation date)

mposolda

@tsaarni Thanks a lot for the PR! Nice work!

I've replied to your email to keycloak-dev with the proposal of introduce Keystore SPI. So we can eventually continue discuss there.

Besides that, the PR may need also documentation and maybe some very minor changes, but maybe we can wait with that until there is more feedback for the keycloak-dev email?

tsaarni · 2020-10-06T14:07:21Z

Link to keycloak-dev https://groups.google.com/g/keycloak-dev/c/QwKjr2I8Eyg/m/qMAAZ0-1BAAJ

mposolda · 2020-10-12T12:04:20Z

@tsaarni Thanks for sending "ping" mail to the keycloak-dev . I was wondering about doing this, but you were faster. We will see if there is some more feedback...

mposolda · 2021-10-01T06:42:26Z

@tsaarni Sorry for the delay with this PR :/ Do we want still to proceed with this PR or is it out of date from your point of view?

tsaarni · 2021-10-01T08:07:25Z

Hi @mposolda! Yes the feature is still valid from my point of view.

SASL EXTERNAL can be used with LDAP today, by using small tricks https://github.com/tsaarni/cloud-playground/tree/master/kubernetes/keycloak, but it would be still interesting to me to have it officially.

Previously this was blocked due to architectural issues. But I wonder, are you asking now because you are cleaning up old PRs or do you think it would be now possible to get guidance to develop this feature?

mposolda · 2021-10-14T17:51:49Z

@tsaarni I think it will be nice to have it in since you already did the work.

There is no other response on keycloak-dev, so my vote would be to do it in a way I originally proposed. So just introducing "Keystore SPI" and use it in JSSETruststoreConfigurator instead of directly using system properties in this configurator.

From my point of view, it is fine if we do separate Keystore SPI (no attempt to use the same certificates used by "https"). We can adjust later if needed. I hope most of the testsuite work, which you did for this PR, will work fine with this approach and will be possible to re-use? WDYT?

mposolda · 2021-11-16T14:10:21Z

@tsaarni Hi, Any update for this PR?

tsaarni · 2021-11-16T14:13:08Z

@mposolda Sorry, I have not had time yet to start with a proposal for the new Keystore SPI.

trixpan · 2022-07-27T11:46:06Z

@mposolda this is a terrific PR. We have the same need and our implementation ended up being intercepting LDAPS calls with a Java agent.

Would be great to support EXTERNAL straight out of the box.

tsaarni · 2022-08-09T09:37:59Z

Hi @mposolda!

I wonder if I'm right with my observation that maintainers might have too much work and it is difficult for you to arrange time for reviews and guiding the contributors? It's not just problem with this PR. I understand the flood of PRs might be overwhelming.

I hate to be "pushy" here but is there anything that could be done to make this still happen?

This PR has now became critical for us because of migration to Quarkus (our workaround does not work anymore). I really would like to work on TLS improvements for Keycloak:

I have better proposal for KeyStore SPI based on JSSE KeyStore SPI - rewriting parts of what I have done so far during the two years that this PR has existed. I will push it here within couple of days, I will also drop the old UI as it needs to be redone in keycloak-admin-ui.
Since truststore basically is just a KeyStore used by Trust Manager, I'd be also interested to present an idea how to unify the two SPIs (KeyStore and TrustStore) as the next step, as well as adding support for PEM files as trust anchors.
Unrelated to this PR, I'd like to add certificate rotation for the HTTPS certifcate Reloading rotated certificate with Keycloak on Quarkus #10654 which we lost at Quarkus migration.

I think these could be valuable for the project if there is any possibility of getting them merged.

tsaarni · 2022-08-11T15:16:08Z

I've updated this PR, including its description and marking some old comments as outdated. Some unit tests failed, I'll investigate.

sbellerwork · 2024-10-07T12:00:11Z

Looks like an interesting feature, is it still in the works?

tsaarni · 2024-10-09T06:34:56Z

Looks like an interesting feature, is it still in the works?

Not right now, unless we'd come up with an approach that works for Keycloak. I'm still very much interested in working with this feature. The PR is blocked until we come up with an acceptable approach for how a new set of credentials could be configured.

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2025-11-26T18:12:28Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.oid4vc.issuance.signing.LDCredentialSignerTest#testLdpSignedCredentialWithoutAdditionalClaims

Keycloak CI - Base IT (6)

org.keycloak.testsuite.runonserver.RunOnServerException: org.keycloak.protocol.oid4vc.issuance.signing.CredentialSignerException: Was not able to create a JsonLD Document from the serialized string.
	at org.keycloak.testsuite.client.KeycloakTestingClient$Server.run(KeycloakTestingClient.java:207)
	at org.keycloak.testsuite.oid4vc.issuance.signing.LDCredentialSignerTest.testLdpSignedCredentialWithoutAdditionalClaims(LDCredentialSignerTest.java:160)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
...

Report flaky test

* Added KeyStore SPI * LDAP SASL EXTERNAL with KeyStore SPI This change also refactors initialization of LDAP context into explicit separate steps: 1. Initialize context with LDAP connection properties only 2. Optionally send StartTLS extended request 3. Send bind Previously both connection and authentication properties were set together which triggers implicit authentication in the JDK LDAP and it happened differently depending if StartTLS was used or not. This change moves bind into explicit context.reconnect() call, which also makes is possible in future to send LDAP control messages with bind. That is not possible with implicit bind. Signed-off-by: Tero Saarni <[email protected]>

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2025-11-27T08:04:35Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.oid4vc.issuance.signing.LDCredentialSignerTest#testLdpSignedCredentialWithOutIssuanceDate

Keycloak CI - Base IT (6)

org.keycloak.testsuite.runonserver.RunOnServerException: org.keycloak.protocol.oid4vc.issuance.signing.CredentialSignerException: Was not able to create a JsonLD Document from the serialized string.
	at org.keycloak.testsuite.client.KeycloakTestingClient$Server.run(KeycloakTestingClient.java:207)
	at org.keycloak.testsuite.oid4vc.issuance.signing.LDCredentialSignerTest.testLdpSignedCredentialWithOutIssuanceDate(LDCredentialSignerTest.java:113)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
...

Report flaky test

This comment was marked as outdated.

Sign in to view

mposolda requested changes Sep 8, 2020

View reviewed changes

mposolda added area/user-federation status/needs-discussion PR needs discussion on developer mailing list labels Sep 8, 2020

mposolda self-assigned this Sep 8, 2020

mposolda mentioned this pull request Oct 15, 2021

KEYCLOAK-17354 respond to tls CertificateRequest #7928

Closed

stianst added area/ldap and removed area/user-federation labels Nov 3, 2021

tsaarni marked this pull request as draft January 10, 2022 10:26

This comment was marked as outdated.

Sign in to view

tsaarni force-pushed the ldap-starttls-sasl-external branch 3 times, most recently from 7eb7aa6 to bb59d83 Compare January 20, 2022 15:46

tsaarni mentioned this pull request Jan 22, 2022

KEYCLOAK-14055: Add SASL EXTERNAL authentication for LDAP federation keycloak/keycloak-documentation#1373

Closed

tsaarni marked this pull request as ready for review January 24, 2022 19:14

This comment was marked as outdated.

Sign in to view

stianst added the team/core label Jul 26, 2023

stianst added team/core-shared and removed team/core labels Feb 13, 2024

tsaarni mentioned this pull request Mar 27, 2025

Support EC in PEM/DER utils. #38483

Merged

stianst removed impact/medium labels Sep 30, 2025

tsaarni force-pushed the ldap-starttls-sasl-external branch from b1650dd to 7627e8d Compare November 26, 2025 16:49

tsaarni requested review from a team as code owners November 26, 2025 16:49

keycloak-github-bot bot added team/cloud-native team/core-clients team/core-iam labels Nov 26, 2025

tsaarni force-pushed the ldap-starttls-sasl-external branch from 7627e8d to b1b8a7e Compare November 26, 2025 16:58

keycloak-github-bot bot added the flaky-test label Nov 26, 2025

keycloak-github-bot bot reviewed Nov 26, 2025

View reviewed changes

tsaarni force-pushed the ldap-starttls-sasl-external branch from b1b8a7e to 24514c2 Compare November 26, 2025 20:34

tsaarni mentioned this pull request Nov 26, 2025

MTLS support for LDAP integration #29654

Open

tsaarni force-pushed the ldap-starttls-sasl-external branch 2 times, most recently from c4a9de3 to 0fb582d Compare November 27, 2025 06:37

tsaarni force-pushed the ldap-starttls-sasl-external branch from 0fb582d to 60d5e02 Compare November 27, 2025 06:47

keycloak-github-bot bot reviewed Nov 27, 2025

View reviewed changes

KEYCLOAK-14055 Add SASL EXTERNAL authentication for LDAP federation #7365

Are you sure you want to change the base?

KEYCLOAK-14055 Add SASL EXTERNAL authentication for LDAP federation #7365

Uh oh!

Conversation

tsaarni commented Aug 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

mposolda left a comment

Choose a reason for hiding this comment

Uh oh!

tsaarni commented Oct 6, 2020

Uh oh!

mposolda commented Oct 12, 2020

Uh oh!

mposolda commented Oct 1, 2021

Uh oh!

tsaarni commented Oct 1, 2021

Uh oh!

mposolda commented Oct 14, 2021

Uh oh!

mposolda commented Nov 16, 2021

Uh oh!

tsaarni commented Nov 16, 2021

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

trixpan commented Jul 27, 2022

Uh oh!

tsaarni commented Aug 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tsaarni commented Aug 11, 2022

Uh oh!

sbellerwork commented Oct 7, 2024

Uh oh!

tsaarni commented Oct 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keycloak-github-bot bot left a comment

Choose a reason for hiding this comment

Uh oh!

keycloak-github-bot bot commented Nov 26, 2025

Unreported flaky test detected

org.keycloak.testsuite.oid4vc.issuance.signing.LDCredentialSignerTest#testLdpSignedCredentialWithoutAdditionalClaims

Uh oh!

keycloak-github-bot bot left a comment

Choose a reason for hiding this comment

Uh oh!

keycloak-github-bot bot commented Nov 27, 2025

Unreported flaky test detected

org.keycloak.testsuite.oid4vc.issuance.signing.LDCredentialSignerTest#testLdpSignedCredentialWithOutIssuanceDate

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

tsaarni commented Aug 24, 2020 •

edited

Loading

tsaarni commented Aug 9, 2022 •

edited

Loading

tsaarni commented Oct 9, 2024 •

edited

Loading