Merge pull request #51290 from scottfrederick

Closes gh-51290

* pr/51290:
  Polish "Retain SSL host config customizations when reloading SSL bundles"
  Retain SSL host config customizations when reloading SSL bundles

integration-test/spring-boot-sni-integration-tests/spring-boot-sni-reactive-app/build.gradle

@@ -46,6 +46,7 @@ configurations {
 
 dependencies {
 	compileOnly("org.springframework:spring-webflux")
+	compileOnly("org.springframework.boot:spring-boot-starter-tomcat")
 
 	implementation(platform(org.springframework.boot.gradle.plugin.SpringBootPlugin.BOM_COORDINATES))
 	implementation("org.springframework.boot:spring-boot-starter")

integration-test/spring-boot-sni-integration-tests/spring-boot-sni-reactive-app/src/main/java/org/springframework/boot/sni/server/SniServerApplication.java

@@ -16,16 +16,6 @@
 
 package org.springframework.boot.sni.server;
 
-import java.io.File;
-import java.io.IOException;
-import java.io.UncheckedIOException;
-import java.net.JarURLConnection;
-import java.net.URL;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.util.Arrays;
-
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.SpringBootApplication;
 

integration-test/spring-boot-sni-integration-tests/spring-boot-sni-reactive-app/src/main/java/org/springframework/boot/sni/server/TomcatSslHostConfigConfiguration.java

@@ -0,0 +1,115 @@
+/*
+ * Copyright 2012-present the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.sni.server;
+
+import java.util.function.Consumer;
+
+import org.apache.catalina.connector.Connector;
+import org.apache.coyote.http11.AbstractHttp11Protocol;
+import org.apache.tomcat.util.net.SSLHostConfig;
+
+import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
+import org.springframework.boot.ssl.SslBundleRegistry;
+import org.springframework.boot.ssl.SslBundles;
+import org.springframework.boot.tomcat.TomcatConnectorCustomizer;
+import org.springframework.boot.tomcat.TomcatWebServer;
+import org.springframework.boot.tomcat.TomcatWebServerFactory;
+import org.springframework.boot.web.server.WebServer;
+import org.springframework.boot.web.server.WebServerFactoryCustomizer;
+import org.springframework.boot.web.server.context.WebServerInitializedEvent;
+import org.springframework.context.ApplicationListener;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+/**
+ * Applies a customization to each of Tomcat's {@code SSLHostConfig} instances and reports
+ * their state once the server is running and again after an SSL bundle has been reloaded.
+ */
+@Configuration(proxyBeanMethods = false)
+@ConditionalOnClass(TomcatWebServerFactory.class)
+class TomcatSslHostConfigConfiguration {
+
+	private static final int CUSTOMIZED_SESSION_TIMEOUT = 12345;
+
+	@Bean
+	WebServerFactoryCustomizer<TomcatWebServerFactory> sslHostConfigCustomizer() {
+		return (factory) -> factory.addConnectorCustomizers(new SslHostConfigCustomizer());
+	}
+
+	@Bean
+	ApplicationListener<WebServerInitializedEvent> sslHostConfigReporter(SslBundles sslBundles) {
+		return new SslHostConfigReporter(sslBundles);
+	}
+
+	private static void forEachSslHostConfig(Connector connector, Consumer<SSLHostConfig> action) {
+		if (connector.getProtocolHandler() instanceof AbstractHttp11Protocol<?> protocol) {
+			for (SSLHostConfig sslHostConfig : protocol.findSslHostConfigs()) {
+				action.accept(sslHostConfig);
+			}
+		}
+	}
+
+	static final class SslHostConfigCustomizer implements TomcatConnectorCustomizer {
+
+		@Override
+		public void customize(Connector connector) {
+			forEachSslHostConfig(connector,
+					(sslHostConfig) -> sslHostConfig.setSessionTimeout(CUSTOMIZED_SESSION_TIMEOUT));
+		}
+
+	}
+
+	static final class SslHostConfigReporter implements ApplicationListener<WebServerInitializedEvent> {
+
+		private final SslBundles sslBundles;
+
+		SslHostConfigReporter(SslBundles sslBundles) {
+			this.sslBundles = sslBundles;
+		}
+
+		@Override
+		public void onApplicationEvent(WebServerInitializedEvent event) {
+			WebServer webServer = event.getWebServer();
+			if (!(webServer instanceof TomcatWebServer tomcatWebServer)) {
+				return;
+			}
+			Connector connector = tomcatWebServer.getTomcat().getConnector();
+			report(connector, "start");
+			reloadBundles();
+			report(connector, "reload");
+		}
+
+		private void reloadBundles() {
+			if (!(this.sslBundles instanceof SslBundleRegistry registry)) {
+				return;
+			}
+			for (String name : new String[] { "default", "alt" }) {
+				registry.updateBundle(name, this.sslBundles.getBundle(name));
+			}
+		}
+
+		private void report(Connector connector, String phase) {
+			forEachSslHostConfig(connector,
+					(sslHostConfig) -> System.out.println(">>>>> on " + phase + ", host="
+							+ sslHostConfig.getHostName() + ", port=" + connector.getPort()
+							+ ", sessionTimeout=" + sslHostConfig.getSessionTimeout()
+							+ ", certificates.size=" + sslHostConfig.getCertificates().size()));
+		}
+
+	}
+
+}

integration-test/spring-boot-sni-integration-tests/spring-boot-sni-servlet-app/build.gradle

@@ -43,6 +43,7 @@ configurations {
 
 dependencies {
 	compileOnly("jakarta.servlet:jakarta.servlet-api:6.0.0")
+	compileOnly("org.springframework.boot:spring-boot-starter-tomcat")
 
 	implementation(platform(org.springframework.boot.gradle.plugin.SpringBootPlugin.BOM_COORDINATES))
 	implementation("org.springframework.boot:spring-boot-starter-webmvc") {

integration-test/spring-boot-sni-integration-tests/spring-boot-sni-servlet-app/src/main/java/org/springframework/boot/sni/server/TomcatSslHostConfigConfiguration.java

@@ -0,0 +1,113 @@
+/*
+ * Copyright 2012-present the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.sni.server;
+
+import java.util.function.Consumer;
+
+import org.apache.catalina.connector.Connector;
+import org.apache.coyote.http11.AbstractHttp11Protocol;
+import org.apache.tomcat.util.net.SSLHostConfig;
+
+import org.springframework.boot.ssl.SslBundleRegistry;
+import org.springframework.boot.ssl.SslBundles;
+import org.springframework.boot.tomcat.TomcatConnectorCustomizer;
+import org.springframework.boot.tomcat.TomcatWebServer;
+import org.springframework.boot.tomcat.TomcatWebServerFactory;
+import org.springframework.boot.web.server.WebServer;
+import org.springframework.boot.web.server.WebServerFactoryCustomizer;
+import org.springframework.boot.web.server.context.WebServerInitializedEvent;
+import org.springframework.context.ApplicationListener;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+/**
+ * Applies a customization to each of Tomcat's {@code SSLHostConfig} instances and reports
+ * their state once the server is running and again after an SSL bundle has been reloaded.
+ */
+@Configuration(proxyBeanMethods = false)
+class TomcatSslHostConfigConfiguration {
+
+	private static final int CUSTOMIZED_SESSION_TIMEOUT = 12345;
+
+	@Bean
+	WebServerFactoryCustomizer<TomcatWebServerFactory> sslHostConfigCustomizer() {
+		return (factory) -> factory.addConnectorCustomizers(new SslHostConfigCustomizer());
+	}
+
+	@Bean
+	ApplicationListener<WebServerInitializedEvent> sslHostConfigReporter(SslBundles sslBundles) {
+		return new SslHostConfigReporter(sslBundles);
+	}
+
+	private static void forEachSslHostConfig(Connector connector, Consumer<SSLHostConfig> action) {
+		if (connector.getProtocolHandler() instanceof AbstractHttp11Protocol<?> protocol) {
+			for (SSLHostConfig sslHostConfig : protocol.findSslHostConfigs()) {
+				action.accept(sslHostConfig);
+			}
+		}
+	}
+
+	static final class SslHostConfigCustomizer implements TomcatConnectorCustomizer {
+
+		@Override
+		public void customize(Connector connector) {
+			forEachSslHostConfig(connector,
+					(sslHostConfig) -> sslHostConfig.setSessionTimeout(CUSTOMIZED_SESSION_TIMEOUT));
+		}
+
+	}
+
+	static final class SslHostConfigReporter implements ApplicationListener<WebServerInitializedEvent> {
+
+		private final SslBundles sslBundles;
+
+		SslHostConfigReporter(SslBundles sslBundles) {
+			this.sslBundles = sslBundles;
+		}
+
+		@Override
+		public void onApplicationEvent(WebServerInitializedEvent event) {
+			WebServer webServer = event.getWebServer();
+			if (!(webServer instanceof TomcatWebServer tomcatWebServer)) {
+				return;
+			}
+			Connector connector = tomcatWebServer.getTomcat().getConnector();
+			report(connector, "start");
+			reloadBundles();
+			report(connector, "reload");
+		}
+
+		private void reloadBundles() {
+			if (!(this.sslBundles instanceof SslBundleRegistry registry)) {
+				return;
+			}
+			for (String name : new String[] { "default", "alt" }) {
+				registry.updateBundle(name, this.sslBundles.getBundle(name));
+			}
+		}
+
+		private void report(Connector connector, String phase) {
+			forEachSslHostConfig(connector,
+					(sslHostConfig) -> System.out.println(">>>>> on " + phase + ", host="
+							+ sslHostConfig.getHostName() + ", port=" + connector.getPort()
+							+ ", sessionTimeout=" + sslHostConfig.getSessionTimeout()
+							+ ", certificates.size=" + sslHostConfig.getCertificates().size()));
+		}
+
+	}
+
+}

integration-test/spring-boot-sni-integration-tests/src/intTest/java/org/springframework/boot/sni/SniIntegrationTests.java

@@ -48,6 +48,10 @@ class SniIntegrationTests {
 
 	public static final String ALT_SERVER_NAME = "hello-alt.example.com";
 
+	private static final String DEFAULT_SERVER_NAME = "_default_";
+
+	private static final int CUSTOMIZED_SESSION_TIMEOUT = 12345;
+
 	private static final Integer SERVER_PORT = 8443;
 
 	private static final Network SHARED_NETWORK = Network.newNetwork();
@@ -66,6 +70,9 @@ class SniIntegrationTests {
 			}
 			String serverLogs = serverContainer.getLogs();
 			assertThat(serverLogs).contains(SERVER_START_MESSAGES.get(server));
+			if ("tomcat".equals(server)) {
+				assertSslHostConfigCustomizationsRetained(serverContainer);
+			}
 			try (ApplicationContainer clientContainer = new ClientApplicationContainer()) {
 				clientContainer.start();
 				Awaitility.await().atMost(Duration.ofSeconds(60)).until(() -> !clientContainer.isRunning());
@@ -78,6 +85,24 @@ class SniIntegrationTests {
 		}
 	}
 
+	private void assertSslHostConfigCustomizationsRetained(ApplicationContainer serverContainer) {
+		for (String phase : new String[] { "start", "reload" }) {
+			for (String serverName : new String[] { PRIMARY_SERVER_NAME, ALT_SERVER_NAME, DEFAULT_SERVER_NAME }) {
+				String expected = ">>>>> on " + phase + ", host=" + serverName + ", port=" + SERVER_PORT
+						+ ", sessionTimeout=" + CUSTOMIZED_SESSION_TIMEOUT + ", certificates.size=1";
+				try {
+					Awaitility.await()
+						.atMost(Duration.ofSeconds(60))
+						.until(() -> serverContainer.getLogs().contains(expected));
+				}
+				catch (ConditionTimeoutException ex) {
+					assertThat(serverContainer.getLogs()).contains(expected);
+					throw ex;
+				}
+			}
+		}
+	}
+
 	private void assertServerCalledWithName(String clientLogs, String serverName) {
 		assertThat(clientLogs).contains("Calling server at 'https://" + serverName + ":8443/'")
 			.contains("Hello from https://" + serverName + ":8443/");

module/spring-boot-tomcat/src/main/java/org/springframework/boot/tomcat/SslConnectorCustomizer.java

@@ -17,8 +17,10 @@
 package org.springframework.boot.tomcat;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.catalina.connector.Connector;
 import org.apache.commons.logging.Log;
@@ -64,9 +66,15 @@ public class SslConnectorCustomizer {
 
 	public void update(@Nullable String serverName, SslBundle updatedSslBundle) {
 		AbstractHttp11Protocol<?> protocol = (AbstractHttp11Protocol<?>) this.connector.getProtocolHandler();
-		String host = (serverName != null) ? serverName : protocol.getDefaultSSLHostConfigName();
-		this.logger.debug("SSL Bundle for host " + host + " has been updated, reloading SSL configuration");
-		addSslHostConfig(protocol, host, updatedSslBundle);
+		String hostName = (serverName != null) ? serverName : protocol.getDefaultSSLHostConfigName();
+		this.logger.debug("SSL Bundle for host " + hostName + " has been updated, reloading SSL configuration");
+		SSLHostConfig sslHostConfig = findSslHostConfig(protocol, hostName);
+		if (sslHostConfig == null) {
+			addSslHostConfig(protocol, hostName, updatedSslBundle);
+			return;
+		}
+		applySslBundle(protocol, sslHostConfig, updatedSslBundle);
+		protocol.addSslHostConfig(sslHostConfig, true);
 	}
 
 	public void customize(SslBundle sslBundle, Map<String, SslBundle> serverNameSslBundles) {
@@ -93,20 +101,27 @@ public class SslConnectorCustomizer {
 		serverNameSslBundles.forEach((serverName, bundle) -> addSslHostConfig(protocol, serverName, bundle));
 	}
 
-	private void addSslHostConfig(AbstractHttp11Protocol<?> protocol, String serverName, SslBundle sslBundle) {
+	private void addSslHostConfig(AbstractHttp11Protocol<?> protocol, String hostName, SslBundle sslBundle) {
 		SSLHostConfig sslHostConfig = new SSLHostConfig();
-		sslHostConfig.setHostName(serverName);
+		sslHostConfig.setHostName(hostName);
 		configureSslClientAuth(sslHostConfig);
 		applySslBundle(protocol, sslHostConfig, sslBundle);
 		protocol.addSslHostConfig(sslHostConfig, true);
 	}
 
+	private @Nullable SSLHostConfig findSslHostConfig(AbstractHttp11Protocol<?> protocol, String hostName) {
+		return Arrays.stream(protocol.findSslHostConfigs())
+			.filter((candidate) -> hostName.equalsIgnoreCase(candidate.getHostName()))
+			.findFirst()
+			.orElse(null);
+	}
+
 	private void applySslBundle(AbstractHttp11Protocol<?> protocol, SSLHostConfig sslHostConfig, SslBundle sslBundle) {
 		SslBundleKey key = sslBundle.getKey();
 		SslStoreBundle stores = sslBundle.getStores();
 		SslOptions options = sslBundle.getOptions();
 		sslHostConfig.setSslProtocol(sslBundle.getProtocol());
-		SSLHostConfigCertificate certificate = new SSLHostConfigCertificate(sslHostConfig, Type.UNDEFINED);
+		SSLHostConfigCertificate certificate = getCertificate(sslHostConfig);
 		String keystorePassword = (stores.getKeyStorePassword() != null) ? stores.getKeyStorePassword() : "";
 		certificate.setCertificateKeystorePassword(keystorePassword);
 		if (key.getPassword() != null) {
@@ -115,12 +130,21 @@ public class SslConnectorCustomizer {
 		if (key.getAlias() != null) {
 			certificate.setCertificateKeyAlias(key.getAlias());
 		}
-		sslHostConfig.addCertificate(certificate);
 		configureCiphers(options, sslHostConfig);
 		configureSslStores(sslHostConfig, certificate, stores);
 		configureEnabledProtocols(sslHostConfig, options);
 	}
 
+	private SSLHostConfigCertificate getCertificate(SSLHostConfig sslHostConfig) {
+		Set<SSLHostConfigCertificate> certificates = sslHostConfig.getCertificates();
+		if (certificates.size() == 1) {
+			return certificates.iterator().next();
+		}
+		SSLHostConfigCertificate certificate = new SSLHostConfigCertificate(sslHostConfig, Type.UNDEFINED);
+		sslHostConfig.addCertificate(certificate);
+		return certificate;
+	}
+
 	private void configureCiphers(SslOptions options, SSLHostConfig sslHostConfig) {
 		CipherConfiguration cipherConfiguration = CipherConfiguration.from(options);
 		if (cipherConfiguration != null) {

module/spring-boot-tomcat/src/test/java/org/springframework/boot/tomcat/SslConnectorCustomizerTests.java

@@ -22,6 +22,7 @@ import org.apache.catalina.connector.Connector;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.coyote.http11.AbstractHttp11Protocol;
 import org.apache.tomcat.util.net.SSLHostConfig;
 import org.apache.tomcat.util.net.openssl.ciphers.Cipher;
 import org.junit.jupiter.api.AfterEach;
@@ -157,6 +158,47 @@ class SslConnectorCustomizerTests {
 		assertThat(sslHostConfig.getEnabledProtocols()).containsExactly("TLSv1.2");
 	}
 
+	@Test
+	@WithPackageResources("test.jks")
+	void updateRetainsCustomizationsAppliedToSslHostConfig() {
+		Ssl ssl = new Ssl();
+		ssl.setKeyPassword("password");
+		ssl.setKeyStore("classpath:test.jks");
+		Connector connector = this.tomcat.getConnector();
+		AbstractHttp11Protocol<?> protocol = (AbstractHttp11Protocol<?>) connector.getProtocolHandler();
+		SslConnectorCustomizer customizer = new SslConnectorCustomizer(this.logger, connector, ssl.getClientAuth());
+		customizer.customize(WebServerSslBundle.get(ssl), Collections.emptyMap());
+		SSLHostConfig sslHostConfig = protocol.findSslHostConfigs()[0];
+		sslHostConfig.setTruststoreProvider(MockPkcs11SecurityProvider.NAME);
+		customizer.update(null, WebServerSslBundle.get(ssl));
+		assertThat(protocol.findSslHostConfigs()).hasSize(1);
+		SSLHostConfig updated = protocol.findSslHostConfigs()[0];
+		assertThat(updated.getTruststoreProvider()).isEqualTo(MockPkcs11SecurityProvider.NAME);
+		assertThat(updated.getCertificates()).hasSize(1);
+	}
+
+	@Test
+	@WithPackageResources("test.jks")
+	void updateAppliesUpdatedBundleToExistingSslHostConfig() throws Exception {
+		Ssl ssl = new Ssl();
+		ssl.setKeyPassword("password");
+		ssl.setKeyStore("classpath:test.jks");
+		ssl.setEnabledProtocols(new String[] { "TLSv1.2" });
+		Connector connector = this.tomcat.getConnector();
+		AbstractHttp11Protocol<?> protocol = (AbstractHttp11Protocol<?>) connector.getProtocolHandler();
+		SslConnectorCustomizer customizer = new SslConnectorCustomizer(this.logger, connector, ssl.getClientAuth());
+		customizer.customize(WebServerSslBundle.get(ssl), Collections.emptyMap());
+		this.tomcat.start();
+		assertThat(protocol.findSslHostConfigs()[0].getEnabledProtocols()).containsExactly("TLSv1.2");
+		Ssl updatedSsl = new Ssl();
+		updatedSsl.setKeyPassword("password");
+		updatedSsl.setKeyStore("classpath:test.jks");
+		updatedSsl.setEnabledProtocols(new String[] { "TLSv1.3" });
+		customizer.update(null, WebServerSslBundle.get(updatedSsl));
+		SSLHostConfig sslHostConfig = protocol.findSslHostConfigs()[0];
+		assertThat(sslHostConfig.getEnabledProtocols()).containsExactly("TLSv1.3");
+	}
+
 	@Test
 	void customizeWhenSslIsEnabledWithNoKeyStoreAndNotPkcs11ThrowsException() {
 		assertThatIllegalStateException().isThrownBy(() -> {

module/spring-boot-tomcat/src/test/java/org/springframework/boot/tomcat/servlet/TomcatServletWebServerFactoryTests.java

@@ -72,6 +72,7 @@ import org.apache.hc.core5.ssl.SSLContextBuilder;
 import org.apache.jasper.servlet.JspServlet;
 import org.apache.tomcat.JarScanFilter;
 import org.apache.tomcat.JarScanType;
+import org.apache.tomcat.util.net.SSLHostConfig;
 import org.apache.tomcat.util.scan.StandardJarScanFilter;
 import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
 import org.awaitility.Awaitility;
@@ -697,6 +698,33 @@ class TomcatServletWebServerFactoryTests extends AbstractServletWebServerFactory
 		assertThat(verifier.getLastPrincipal()).isEqualTo("CN=2");
 	}
 
+	@Test
+	@WithPackageResources({ "1.crt", "1.key", "2.crt", "2.key" })
+	void shouldRetainSslHostConfigCustomizationsWhenReloadingSslBundles() throws Exception {
+		TomcatServletWebServerFactory factory = getFactory();
+		addTestTxtFile(factory);
+		DefaultSslBundleRegistry bundles = new DefaultSslBundleRegistry("test",
+				createPemSslBundle("classpath:1.crt", "classpath:1.key"));
+		factory.setSslBundles(bundles);
+		factory.setSsl(Ssl.forBundle("test"));
+		factory.addConnectorCustomizers((connector) -> {
+			if (connector.getProtocolHandler() instanceof AbstractHttp11Protocol<?> protocol) {
+				for (SSLHostConfig sslHostConfig : protocol.findSslHostConfigs()) {
+					sslHostConfig.setSessionTimeout(12345);
+				}
+			}
+		});
+		this.webServer = factory.getWebServer();
+		this.webServer.start();
+		bundles.updateBundle("test", createPemSslBundle("classpath:2.crt", "classpath:2.key"));
+		Connector connector = ((TomcatWebServer) this.webServer).getTomcat().getConnector();
+		AbstractHttp11Protocol<?> protocol = (AbstractHttp11Protocol<?>) connector.getProtocolHandler();
+		assertThat(protocol.findSslHostConfigs()).hasSize(1);
+		SSLHostConfig sslHostConfig = protocol.findSslHostConfigs()[0];
+		assertThat(sslHostConfig.getSessionTimeout()).isEqualTo(12345);
+		assertThat(sslHostConfig.getCertificates()).hasSize(1);
+	}
+
 	@Test
 	@WithPackageResources("test.jks")
 	void sslWithHttp11Nio2Protocol() throws Exception {