Add support for LDAPS testing #48315

CatiaCorreia · 2025-11-27T16:15:00Z

Add to EmbeddedLdapProperties:
-boolean ldaps
-String sslBundleName
Create setLdapsListener method to create and set the LDAPS listener for the server. Add test for new embedded LDAP setup.
Issue#48060

Signed-off-by: CatiaCorreia [email protected]

snicoll

Can you please review the proposal. I've added a couple of comments for a start.

snicoll · 2025-11-27T16:53:08Z

...java/org/springframework/boot/ldap/autoconfigure/embedded/EmbeddedLdapAutoConfiguration.java

 		}
 	}

+	@ConditionalOnBean(SslBundles.class)


I don't know what this is but you can't add a condition on a private method. What's that supposed to do?

snicoll · 2025-11-27T16:54:16Z

...org/springframework/boot/ldap/autoconfigure/embedded/EmbeddedLdapAutoConfigurationTests.java

 	}

+	@Test
+	void testLdapsVersion() {


That doesn't test the SSL bundles, nor the binding. It doesn't work atm anyway (see previous comment).

wilkinsona

Thanks very much for the PR, @CatiaCorreia. I've left a few comments for your consideration.

wilkinsona · 2025-11-27T16:53:18Z

...c/main/java/org/springframework/boot/ldap/autoconfigure/embedded/EmbeddedLdapProperties.java

+	/**
+	 * Listener type.
+	 */
+	private boolean ldaps;
+
+	/**
+	 * Embedded LDAPS client SSL bundle name.
+	 */
+	@Nullable private String sslBundleName;


For consistency with other SSL configuration in Spring Boot, the properties should be grouped together in an SSL inner-class. That class should have an enabled property, a bundle property as well as properties for configuring things without using an SSL bundle. org.springframework.boot.amqp.autoconfigure.RabbitProperties.Ssl is a client-side example of that sort of arrangement.

Hi @wilkinsona.
What's the plan of the Spring Boot team regarding such double configuration possibilities?

It is my understanding (e.g.: https://spring.io/blog/2023/06/07/securing-spring-boot-applications-with-ssl#securing-embedded-web-servers), which can easily be wrong, that using SSL bundles is now the preferred way and that the "old" way of specifying single properties should not be used anymore.
If it's indeed the case, does it really make sense to introduce something which is potentially already deprecated?

wilkinsona · 2025-11-27T16:56:32Z

...java/org/springframework/boot/ldap/autoconfigure/embedded/EmbeddedLdapAutoConfiguration.java

 	}

 	@Bean
 	InMemoryDirectoryServer directoryServer(ApplicationContext applicationContext) throws LDAPException {


SslBundles should be injected here using ObjectProvider<SslBundles> sslBundles. You can then use getIfAvailable() to get the bean if it exists or null if it does not.

wilkinsona · 2025-11-27T16:58:23Z

...java/org/springframework/boot/ldap/autoconfigure/embedded/EmbeddedLdapAutoConfiguration.java

 		}
 	}

+	@ConditionalOnBean(SslBundles.class)


This won't have any effect here as conditions only work on classes and @Bean methods. It can be removed if you inject SslBundles as suggested above.

CatiaCorreia · 2025-11-28T09:42:11Z

Thank you for all the feedback. I will start working on the changes right away.

CatiaCorreia · 2025-12-02T21:00:01Z

I'm writing this message to give a status report. I have implemented the changes discussed above and have tried testing them. I believe I have successfully implemented the creation of the server with SSL without SslBundles. However when it comes to the SslBundles I haven't been able to get a successful test even thought I can't find a problem. There seems to be a problem when the client tries to get a connection to the server (LDAPException code 91). I have committed the changes to a separate branch as I was unsure if I should commit them to this one directly. The branch is found here: https://github.com/CatiaCorreia/spring-boot/tree/gh%2348060-Troubleshooting
At this stage I am trying to ascertain the motive for this error.

CatiaCorreia · 2025-12-04T17:58:06Z

As I haven't been able to figure out the problem I would like to know if its possible to receive some kind of advice or suggestion. If it isn't possible I will have to move on and tackle another issue.
Thank you for your time.

cdprete · 2025-12-10T16:20:49Z

...java/org/springframework/boot/ldap/autoconfigure/embedded/EmbeddedLdapAutoConfiguration.java

+					this.embeddedProperties.getPort(), serverSocketFactory, clientSocketFactory));
+		}
+		else {
+			throw new LDAPException(ResultCode.PARAM_ERROR, "SslBundleName property not specified");


I would argue here that this is not really the right exception type. Here you didn't use nor setup LDAP at all yet, so it's misleading to say that there is an error with LDAP itself.
It's purely a configuration issue.

@wilkinsona isn't there maybe some Spring(Boot) config-specific exception which may be used here?

Thanks for trying to help, @cdprete, but things have moved on with https://github.com/CatiaCorreia/spring-boot/tree/gh%2348060-Troubleshooting containing the latest code.

Thanks for pointing it out @wilkinsona.

wilkinsona · 2025-12-15T11:53:27Z

Thanks for the latest updates, @CatiaCorreia.

As I haven't been able to figure out the problem I would like to know if its possible to receive some kind of advice or suggestion.

The SSL bundle test that was failing was trying to use a different protocol when creating the SSL context (TLS rather than TLSv1.2). testServerWithSslBundle passes when I add this line to it:

propertyValues.add("spring.ssl.bundle.jks.test.protocol=TLSv1.2");

Would you like to force-push your gh#48060 branch with the changes in gh#48060-Troubleshooting and this additional line? That should get us into a position where we can merge this once we've created our 4.0.x branch and main is producing 4.1.0 snapshots.

CatiaCorreia · 2025-12-15T12:02:22Z

Thank you very much for the help @wilkinsona. I'll make the changes as soon as I can and do the force push as requested.

Add to EmbeddedLdapProperties: -boolean ldaps -String sslBundleName Create setLdapsListener method to create and set the LDAPS listener for the server. Add test for new embedded LDAP setup. Issue#48060 Signed-off-by: CatiaCorreia <[email protected]>

…rposes. Signed-off-by: CatiaCorreia <[email protected]>

Setup autoconfiguration of LDAP listener with ssl. Add tests using ssl. Issue#48060 Signed-off-by: CatiaCorreia [email protected] Signed-off-by: CatiaCorreia <[email protected]>

cdprete · 2025-12-15T13:03:12Z

...c/main/java/org/springframework/boot/ldap/autoconfigure/embedded/EmbeddedLdapProperties.java

+		private static final String DEFAULT_PROTOCOL;
+
+		static {
+			String protocol = "TLSv1.1";


Security-wise, TLS v3 should be the default and not v1 which is not really supported anymore.

The default will be TLSv1.2 as long as the JDK supports it (see the try block below). AFAIK, there's no TLS v3. There is, however, TLSv1.3.

Yes, with v3 I meant 1.3.

The default will be TLSv1.2 as long as the JDK supports it

Which I would argue, security-wise, why not v1.3 if available?
Personally, I think that the default should be the most secure supported option and the user must take an explicit risk by explicitly configure the application to something lower if he/she really wants to.

Also, although it would be weird, getProtocols() can actually return null which would lead to a NPE when the loop gets initiated. Dunno if you want to handle/check such weird case to begin with.

I chose to use the TLSv1.2 by default to be consistent with what is present in other modules of spring boot. @wilkinsona should I keep the consistency or would it be better to use the TLSv1.3? Regarding the possibility of getProtocols() returning null I will make the changes to test for it.

I chose to use the TLSv1.2 by default to be consistent with what is present in other modules of spring boot.

I see.
Fine for me then.

...java/org/springframework/boot/ldap/autoconfigure/embedded/EmbeddedLdapAutoConfiguration.java

…rposes.

...c/main/java/org/springframework/boot/ldap/autoconfigure/embedded/EmbeddedLdapProperties.java

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 27, 2025

snicoll requested changes Nov 27, 2025

View reviewed changes

snicoll added the status: waiting-for-feedback We need additional information before we can continue label Nov 27, 2025

wilkinsona requested changes Nov 27, 2025

View reviewed changes

spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 28, 2025

wilkinsona mentioned this pull request Nov 30, 2025

Add support for LDAPS testing #48060

Closed

wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Nov 30, 2025

spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 2, 2025

snicoll self-requested a review December 5, 2025 20:34

cdprete reviewed Dec 10, 2025

View reviewed changes

wilkinsona self-assigned this Dec 15, 2025

wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Dec 15, 2025

spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 15, 2025

CatiaCorreia added 3 commits December 15, 2025 12:46

Add support for LDAPS testing

8d5bde9

Add to EmbeddedLdapProperties: -boolean ldaps -String sslBundleName Create setLdapsListener method to create and set the LDAPS listener for the server. Add test for new embedded LDAP setup. Issue#48060 Signed-off-by: CatiaCorreia <[email protected]>

Documenting changes and existing problem in code for demonstration pu…

fd7c3e2

…rposes. Signed-off-by: CatiaCorreia <[email protected]>

Add ssl nested class in EmbeddedLdapProperties.

e844851

Setup autoconfiguration of LDAP listener with ssl. Add tests using ssl. Issue#48060 Signed-off-by: CatiaCorreia [email protected] Signed-off-by: CatiaCorreia <[email protected]>

CatiaCorreia force-pushed the gh#48060 branch from a7de815 to e844851 Compare December 15, 2025 12:46

cdprete reviewed Dec 15, 2025

View reviewed changes

...java/org/springframework/boot/ldap/autoconfigure/embedded/EmbeddedLdapAutoConfiguration.java Show resolved Hide resolved

cdprete referenced this pull request in CatiaCorreia/spring-boot Dec 15, 2025

Documenting changes and existing problem in code for demonstration pu…

2272141

…rposes.

cdprete reviewed Dec 15, 2025

View reviewed changes

...c/main/java/org/springframework/boot/ldap/autoconfigure/embedded/EmbeddedLdapProperties.java Show resolved Hide resolved

wilkinsona added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Dec 15, 2025

wilkinsona added this to the 4.1.x milestone Dec 15, 2025

Add support for LDAPS testing #48315

Are you sure you want to change the base?

Add support for LDAPS testing #48315

Conversation

CatiaCorreia commented Nov 27, 2025

Uh oh!

snicoll left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilkinsona left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilkinsona Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CatiaCorreia commented Nov 28, 2025

Uh oh!

CatiaCorreia commented Dec 2, 2025

Uh oh!

CatiaCorreia commented Dec 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cdprete Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilkinsona commented Dec 15, 2025

Uh oh!

CatiaCorreia commented Dec 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cdprete Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cdprete Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wilkinsona Nov 27, 2025 •

edited

Loading

cdprete Dec 15, 2025 •

edited

Loading

cdprete Dec 15, 2025 •

edited

Loading

cdprete Dec 16, 2025 •

edited

Loading