From 3784ae049b649d99de03f85fe3abba3a7c97e913 Mon Sep 17 00:00:00 2001 From: Benedict Date: Fri, 26 Jun 2026 14:58:18 +0530 Subject: [PATCH 1/3] Validate SSL keystore alias has key entry and certificate chain SslBundleKey.assertContainsAlias() now verifies not only that the alias exists in the keystore but also that it is a genuine key entry with a non-empty certificate chain. This catches passwordless PKCS12 keystores where the JDK silently drops certificate entries while keeping the key entry, causing cryptic SSL_ERROR_NO_CYPHER_OVERLAP failures downstream. Closes gh-25112 Signed-off-by: Benedict --- .../boot/ssl/SslBundleKey.java | 11 ++++- .../boot/ssl/SslBundleKeyTests.java | 45 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/core/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundleKey.java b/core/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundleKey.java index c423709d7cd3..b7c1c6285a8d 100644 --- a/core/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundleKey.java +++ b/core/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundleKey.java @@ -29,6 +29,7 @@ * A reference to a single key obtained via {@link SslBundle}. * * @author Phillip Webb + * @author Benedict * @since 3.1.0 */ public interface SslBundleKey { @@ -52,7 +53,10 @@ public interface SslBundleKey { @Nullable String getAlias(); /** - * Assert that the alias is contained in the given keystore. + * Assert that the alias is contained in the given keystore and that it is a valid + * key entry with a certificate chain. Some JDK keystore implementations (notably + * passwordless PKCS12) may load a key entry but silently drop its certificate + * entries, causing cryptic handshake failures downstream. * @param keyStore the keystore to check */ default void assertContainsAlias(@Nullable KeyStore keyStore) { @@ -61,6 +65,11 @@ default void assertContainsAlias(@Nullable KeyStore keyStore) { try { Assert.state(keyStore.containsAlias(alias), () -> String.format("Keystore does not contain alias '%s'", alias)); + Assert.state(keyStore.isKeyEntry(alias), + () -> String.format("Keystore alias '%s' is not a key entry", alias)); + Assert.state(keyStore.getCertificateChain(alias) != null + && keyStore.getCertificateChain(alias).length > 0, + () -> String.format("Keystore alias '%s' does not have a certificate chain", alias)); } catch (KeyStoreException ex) { throw new IllegalStateException( diff --git a/core/spring-boot/src/test/java/org/springframework/boot/ssl/SslBundleKeyTests.java b/core/spring-boot/src/test/java/org/springframework/boot/ssl/SslBundleKeyTests.java index eb9605ce7ed5..fda941f6c3ac 100644 --- a/core/spring-boot/src/test/java/org/springframework/boot/ssl/SslBundleKeyTests.java +++ b/core/spring-boot/src/test/java/org/springframework/boot/ssl/SslBundleKeyTests.java @@ -18,11 +18,13 @@ import java.security.KeyStore; import java.security.KeyStoreException; +import java.security.cert.Certificate; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -30,6 +32,7 @@ * Tests for {@link SslBundleKey}. * * @author Phillip Webb + * @author Benedict */ class SslBundleKeyTests { @@ -72,4 +75,46 @@ void getKeyManagerFactoryWhenHasAliasNotDeterminedInStoreThrowsException() throw .withMessage("Could not determine if keystore contains alias 'alias'"); } + @Test + void assertContainsAliasWhenAliasIsNotKeyEntryThrowsException() throws Exception { + KeyStore keyStore = mock(KeyStore.class); + given(keyStore.containsAlias("alias")).willReturn(true); + given(keyStore.isKeyEntry("alias")).willReturn(false); + SslBundleKey key = SslBundleKey.of("secret", "alias"); + assertThatIllegalStateException().isThrownBy(() -> key.assertContainsAlias(keyStore)) + .withMessage("Keystore alias 'alias' is not a key entry"); + } + + @Test + void assertContainsAliasWhenAliasHasNoCertificateChainThrowsException() throws Exception { + KeyStore keyStore = mock(KeyStore.class); + given(keyStore.containsAlias("alias")).willReturn(true); + given(keyStore.isKeyEntry("alias")).willReturn(true); + given(keyStore.getCertificateChain("alias")).willReturn(null); + SslBundleKey key = SslBundleKey.of("secret", "alias"); + assertThatIllegalStateException().isThrownBy(() -> key.assertContainsAlias(keyStore)) + .withMessage("Keystore alias 'alias' does not have a certificate chain"); + } + + @Test + void assertContainsAliasWhenAliasHasEmptyCertificateChainThrowsException() throws Exception { + KeyStore keyStore = mock(KeyStore.class); + given(keyStore.containsAlias("alias")).willReturn(true); + given(keyStore.isKeyEntry("alias")).willReturn(true); + given(keyStore.getCertificateChain("alias")).willReturn(new Certificate[0]); + SslBundleKey key = SslBundleKey.of("secret", "alias"); + assertThatIllegalStateException().isThrownBy(() -> key.assertContainsAlias(keyStore)) + .withMessage("Keystore alias 'alias' does not have a certificate chain"); + } + + @Test + void assertContainsAliasWhenAliasIsValidKeyEntryDoesNotThrow() throws Exception { + KeyStore keyStore = mock(KeyStore.class); + given(keyStore.containsAlias("alias")).willReturn(true); + given(keyStore.isKeyEntry("alias")).willReturn(true); + given(keyStore.getCertificateChain("alias")).willReturn(new Certificate[] { mock(Certificate.class) }); + SslBundleKey key = SslBundleKey.of("secret", "alias"); + assertThatNoException().isThrownBy(() -> key.assertContainsAlias(keyStore)); + } + } From 8dcd3cf0be2692375d3c8ab9183cffad1f805c01 Mon Sep 17 00:00:00 2001 From: Benedict Date: Fri, 26 Jun 2026 15:13:30 +0530 Subject: [PATCH 2/3] Cache getCertificateChain result and use more precise error message Store the certificate chain in a local variable to avoid calling getCertificateChain() twice. Also update the KeyStoreException catch message to reflect that validation now covers multiple operations beyond just alias containment. Signed-off-by: Benedict --- .../java/org/springframework/boot/ssl/SslBundleKey.java | 6 +++--- .../boot/ssl/DefaultSslManagerBundleTests.java | 2 +- .../org/springframework/boot/ssl/SslBundleKeyTests.java | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundleKey.java b/core/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundleKey.java index b7c1c6285a8d..d5d1d2a4aaff 100644 --- a/core/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundleKey.java +++ b/core/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundleKey.java @@ -67,13 +67,13 @@ default void assertContainsAlias(@Nullable KeyStore keyStore) { () -> String.format("Keystore does not contain alias '%s'", alias)); Assert.state(keyStore.isKeyEntry(alias), () -> String.format("Keystore alias '%s' is not a key entry", alias)); - Assert.state(keyStore.getCertificateChain(alias) != null - && keyStore.getCertificateChain(alias).length > 0, + var chain = keyStore.getCertificateChain(alias); + Assert.state(chain != null && chain.length > 0, () -> String.format("Keystore alias '%s' does not have a certificate chain", alias)); } catch (KeyStoreException ex) { throw new IllegalStateException( - String.format("Could not determine if keystore contains alias '%s'", alias), ex); + String.format("Could not validate keystore alias '%s'", alias), ex); } } } diff --git a/core/spring-boot/src/test/java/org/springframework/boot/ssl/DefaultSslManagerBundleTests.java b/core/spring-boot/src/test/java/org/springframework/boot/ssl/DefaultSslManagerBundleTests.java index 4d2ada89d717..a02dfe3e638c 100644 --- a/core/spring-boot/src/test/java/org/springframework/boot/ssl/DefaultSslManagerBundleTests.java +++ b/core/spring-boot/src/test/java/org/springframework/boot/ssl/DefaultSslManagerBundleTests.java @@ -102,7 +102,7 @@ void getKeyManagerFactoryWhenHasAliasNotDeterminedInStoreThrowsException() throw DefaultSslManagerBundle bundle = new TestDefaultSslManagerBundle(storeBundle, SslBundleKey.of("secret", "alias")); assertThatIllegalStateException().isThrownBy(bundle::getKeyManagerFactory) - .withMessage("Could not determine if keystore contains alias 'alias'"); + .withMessage("Could not validate keystore alias 'alias'"); } @Test diff --git a/core/spring-boot/src/test/java/org/springframework/boot/ssl/SslBundleKeyTests.java b/core/spring-boot/src/test/java/org/springframework/boot/ssl/SslBundleKeyTests.java index fda941f6c3ac..2c9e9a8f12c8 100644 --- a/core/spring-boot/src/test/java/org/springframework/boot/ssl/SslBundleKeyTests.java +++ b/core/spring-boot/src/test/java/org/springframework/boot/ssl/SslBundleKeyTests.java @@ -72,7 +72,7 @@ void getKeyManagerFactoryWhenHasAliasNotDeterminedInStoreThrowsException() throw given(keyStore.containsAlias("alias")).willThrow(KeyStoreException.class); SslBundleKey key = SslBundleKey.of("secret", "alias"); assertThatIllegalStateException().isThrownBy(() -> key.assertContainsAlias(keyStore)) - .withMessage("Could not determine if keystore contains alias 'alias'"); + .withMessage("Could not validate keystore alias 'alias'"); } @Test From fcf36d203810cf96ca5419bbc137df3b428bf096 Mon Sep 17 00:00:00 2001 From: Benedict Date: Wed, 9 Sep 2026 23:53:01 +0530 Subject: [PATCH 3/3] Address review: split validation errors, preserve back-compat message Keep the original 'Could not determine if keystore contains alias' message for containsAlias failures and use 'Could not validate keystore alias' only for the new isKeyEntry/getCertificateChain checks. Use explicit Certificate[] instead of var. Add real-keystore trusted-cert-entry test and validation-exception test. Revert DefaultSslManagerBundleTests expectation to the original message. Signed-off-by: Benedict --- .../boot/ssl/SslBundleKey.java | 14 +++++++---- .../ssl/DefaultSslManagerBundleTests.java | 2 +- .../boot/ssl/SslBundleKeyTests.java | 23 +++++++++++++++++-- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/core/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundleKey.java b/core/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundleKey.java index d5d1d2a4aaff..eb812452f4bb 100644 --- a/core/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundleKey.java +++ b/core/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundleKey.java @@ -18,6 +18,7 @@ import java.security.KeyStore; import java.security.KeyStoreException; +import java.security.cert.Certificate; import org.jspecify.annotations.Nullable; @@ -29,7 +30,6 @@ * A reference to a single key obtained via {@link SslBundle}. * * @author Phillip Webb - * @author Benedict * @since 3.1.0 */ public interface SslBundleKey { @@ -65,15 +65,21 @@ default void assertContainsAlias(@Nullable KeyStore keyStore) { try { Assert.state(keyStore.containsAlias(alias), () -> String.format("Keystore does not contain alias '%s'", alias)); + } + catch (KeyStoreException ex) { + throw new IllegalStateException( + String.format("Could not determine if keystore contains alias '%s'", alias), ex); + } + try { Assert.state(keyStore.isKeyEntry(alias), () -> String.format("Keystore alias '%s' is not a key entry", alias)); - var chain = keyStore.getCertificateChain(alias); + Certificate[] chain = keyStore.getCertificateChain(alias); Assert.state(chain != null && chain.length > 0, () -> String.format("Keystore alias '%s' does not have a certificate chain", alias)); } catch (KeyStoreException ex) { - throw new IllegalStateException( - String.format("Could not validate keystore alias '%s'", alias), ex); + throw new IllegalStateException(String.format("Could not validate keystore alias '%s'", alias), + ex); } } } diff --git a/core/spring-boot/src/test/java/org/springframework/boot/ssl/DefaultSslManagerBundleTests.java b/core/spring-boot/src/test/java/org/springframework/boot/ssl/DefaultSslManagerBundleTests.java index a02dfe3e638c..4d2ada89d717 100644 --- a/core/spring-boot/src/test/java/org/springframework/boot/ssl/DefaultSslManagerBundleTests.java +++ b/core/spring-boot/src/test/java/org/springframework/boot/ssl/DefaultSslManagerBundleTests.java @@ -102,7 +102,7 @@ void getKeyManagerFactoryWhenHasAliasNotDeterminedInStoreThrowsException() throw DefaultSslManagerBundle bundle = new TestDefaultSslManagerBundle(storeBundle, SslBundleKey.of("secret", "alias")); assertThatIllegalStateException().isThrownBy(bundle::getKeyManagerFactory) - .withMessage("Could not validate keystore alias 'alias'"); + .withMessage("Could not determine if keystore contains alias 'alias'"); } @Test diff --git a/core/spring-boot/src/test/java/org/springframework/boot/ssl/SslBundleKeyTests.java b/core/spring-boot/src/test/java/org/springframework/boot/ssl/SslBundleKeyTests.java index 2c9e9a8f12c8..021ed2e17ce0 100644 --- a/core/spring-boot/src/test/java/org/springframework/boot/ssl/SslBundleKeyTests.java +++ b/core/spring-boot/src/test/java/org/springframework/boot/ssl/SslBundleKeyTests.java @@ -32,7 +32,6 @@ * Tests for {@link SslBundleKey}. * * @author Phillip Webb - * @author Benedict */ class SslBundleKeyTests { @@ -72,7 +71,7 @@ void getKeyManagerFactoryWhenHasAliasNotDeterminedInStoreThrowsException() throw given(keyStore.containsAlias("alias")).willThrow(KeyStoreException.class); SslBundleKey key = SslBundleKey.of("secret", "alias"); assertThatIllegalStateException().isThrownBy(() -> key.assertContainsAlias(keyStore)) - .withMessage("Could not validate keystore alias 'alias'"); + .withMessage("Could not determine if keystore contains alias 'alias'"); } @Test @@ -85,6 +84,16 @@ void assertContainsAliasWhenAliasIsNotKeyEntryThrowsException() throws Exception .withMessage("Keystore alias 'alias' is not a key entry"); } + @Test + void assertContainsAliasWhenAliasIsTrustedCertificateEntryThrowsException() throws Exception { + KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); + keyStore.load(null); + keyStore.setCertificateEntry("alias", mock(Certificate.class)); + SslBundleKey key = SslBundleKey.of("secret", "alias"); + assertThatIllegalStateException().isThrownBy(() -> key.assertContainsAlias(keyStore)) + .withMessage("Keystore alias 'alias' is not a key entry"); + } + @Test void assertContainsAliasWhenAliasHasNoCertificateChainThrowsException() throws Exception { KeyStore keyStore = mock(KeyStore.class); @@ -117,4 +126,14 @@ void assertContainsAliasWhenAliasIsValidKeyEntryDoesNotThrow() throws Exception assertThatNoException().isThrownBy(() -> key.assertContainsAlias(keyStore)); } + @Test + void assertContainsAliasWhenKeyEntryValidationFailsThrowsValidationException() throws Exception { + KeyStore keyStore = mock(KeyStore.class); + given(keyStore.containsAlias("alias")).willReturn(true); + given(keyStore.isKeyEntry("alias")).willThrow(KeyStoreException.class); + SslBundleKey key = SslBundleKey.of("secret", "alias"); + assertThatIllegalStateException().isThrownBy(() -> key.assertContainsAlias(keyStore)) + .withMessage("Could not validate keystore alias 'alias'"); + } + }