From 1a9d1786b9c0bcc66013d03e8cdeb2123bbcc25f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Gobanc=C3=A9?= <19359548+ecnabogs@users.noreply.github.com> Date: Thu, 6 Mar 2025 18:00:02 +0100 Subject: [PATCH] Fix AliasX509ExtendedKeyManager to process both server and client aliases properly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Gobancé <19359548+ecnabogs@users.noreply.github.com> --- .../boot/ssl/AliasKeyManagerFactory.java | 67 ++++- .../boot/ssl/AliasKeyManagerFactoryTests.java | 284 +++++++++++++++++- 2 files changed, 335 insertions(+), 16 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/AliasKeyManagerFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/AliasKeyManagerFactory.java index 41f431b6dbfe..a1419088bd21 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/AliasKeyManagerFactory.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/AliasKeyManagerFactory.java @@ -26,6 +26,10 @@ import java.security.UnrecoverableKeyException; import java.security.cert.X509Certificate; import java.util.Arrays; +import java.util.Objects; +import java.util.Optional; +import java.util.function.BiFunction; +import java.util.stream.Stream; import javax.net.ssl.KeyManager; import javax.net.ssl.KeyManagerFactory; @@ -42,6 +46,7 @@ * {@link KeyManagerFactory#getKeyManagers()} is final. * * @author Scott Frederick + * @author Stéphane Gobancé */ final class AliasKeyManagerFactory extends KeyManagerFactory { @@ -105,23 +110,27 @@ private AliasX509ExtendedKeyManager(X509ExtendedKeyManager keyManager, String al } @Override - public String chooseEngineClientAlias(String[] strings, Principal[] principals, SSLEngine sslEngine) { - return this.delegate.chooseEngineClientAlias(strings, principals, sslEngine); + public String chooseEngineClientAlias(String[] keyTypes, Principal[] issuers, SSLEngine sslEngine) { + return findFirstMatchingAlias(keyTypes, issuers, this::getClientAliases) + .orElseGet(() -> this.delegate.chooseEngineClientAlias(keyTypes, issuers, sslEngine)); } @Override - public String chooseEngineServerAlias(String s, Principal[] principals, SSLEngine sslEngine) { - return this.alias; + public String chooseEngineServerAlias(String keyType, Principal[] issuers, SSLEngine sslEngine) { + return findFirstMatchingAlias(keyType, issuers, this::getServerAliases) + .orElseGet(() -> this.delegate.chooseEngineServerAlias(keyType, issuers, sslEngine)); } @Override - public String chooseClientAlias(String[] keyType, Principal[] issuers, Socket socket) { - return this.delegate.chooseClientAlias(keyType, issuers, socket); + public String chooseClientAlias(String[] keyTypes, Principal[] issuers, Socket socket) { + return findFirstMatchingAlias(keyTypes, issuers, this::getClientAliases) + .orElseGet(() -> this.delegate.chooseClientAlias(keyTypes, issuers, socket)); } @Override public String chooseServerAlias(String keyType, Principal[] issuers, Socket socket) { - return this.delegate.chooseServerAlias(keyType, issuers, socket); + return findFirstMatchingAlias(keyType, issuers, this::getServerAliases) + .orElseGet(() -> this.delegate.chooseServerAlias(keyType, issuers, socket)); } @Override @@ -144,6 +153,50 @@ public String[] getServerAliases(String keyType, Principal[] issuers) { return this.delegate.getServerAliases(keyType, issuers); } + /** + * Typed-BiFunction for better readability. + */ + private interface KeyAliasFinder extends BiFunction { + + } + + /** + * Gets this key manager's alias if it matches the given key algorithm and has + * been issued by any of the specified issuers (might be {@code null}, meaning + * issuer does not matter) otherwise returns an {@link Optional#empty() empty + * result }. + * @param keyType the required key algorithm. + * @param issuers the list of acceptable CA issuer subject names or {@code null} + * if it does not matter which issuers are used. + * @param finder the function to find the underlying available key aliases. + * @return this key manager's alias if appropriate or an empty result otherwise. + */ + private Optional findFirstMatchingAlias(String keyType, Principal[] issuers, KeyAliasFinder finder) { + return findFirstMatchingAlias(new String[] { keyType }, issuers, finder); + } + + /** + * Gets this key manager's alias if it matches any of the given key algorithms and + * has been issued by any of the specified issuers (might be {@code null}, meaning + * issuer does not matter) otherwise returns an {@link Optional#empty() empty + * result }. + * @param keyTypes the required key algorithms. + * @param issuers the list of acceptable CA issuer subject names or {@code null} + * if it does not matter which issuers are used. + * @param finder the function to find the underlying available key aliases. + * @return this key manager's alias if appropriate or an empty result otherwise. + */ + private Optional findFirstMatchingAlias(String[] keyTypes, Principal[] issuers, KeyAliasFinder finder) { + return Optional.ofNullable(keyTypes) + .flatMap(types -> Stream.of(types) + .filter(Objects::nonNull) + .map(type -> finder.apply(type, issuers)) + .filter(Objects::nonNull) + .flatMap(Stream::of) + .filter(this.alias::equals) + .findFirst()); + } + } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/AliasKeyManagerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/AliasKeyManagerFactoryTests.java index f858ae32454f..07b748072ed3 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/AliasKeyManagerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/AliasKeyManagerFactoryTests.java @@ -23,8 +23,12 @@ import javax.net.ssl.X509ExtendedKeyManager; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentMatcher; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -32,23 +36,285 @@ * Tests for {@link AliasKeyManagerFactory}. * * @author Phillip Webb + * @author Stéphane Gobancé */ class AliasKeyManagerFactoryTests { @Test - void chooseEngineServerAliasReturnsAlias() throws Exception { - KeyManagerFactory delegate = mock(KeyManagerFactory.class); - given(delegate.getKeyManagers()).willReturn(new KeyManager[] { mock(X509ExtendedKeyManager.class) }); - AliasKeyManagerFactory factory = new AliasKeyManagerFactory(delegate, "test-alias", - KeyManagerFactory.getDefaultAlgorithm()); + void chooseEngineServerAliasReturnsTestAliasMatchingSupportedDelegateAliasAndAlgorithm() throws Exception { + + final String delegateSupportedAlgorithm = "supported-alg"; + final String[] delegateSupportedAlias = new String[] { "alias0", "alias1", "alias2" }; + final String delegateChosenAlias = delegateSupportedAlias[0]; + final String testAlias = delegateSupportedAlias[1]; + final String testAlgorithm = delegateSupportedAlgorithm; + final String expectedAlias = testAlias; + + KeyManagerFactory delegate = mockServerKeyManagerFactory(delegateSupportedAlgorithm, delegateSupportedAlias, + delegateChosenAlias); + AliasKeyManagerFactory factory = createAliasKeyManagerFactory(testAlias, delegate); + X509ExtendedKeyManager x509KeyManager = getX509ExtendedKeyManager(factory); + + String chosenAlias = x509KeyManager.chooseEngineServerAlias(testAlgorithm, null, null); + assertThat(chosenAlias).isEqualTo(expectedAlias); + } + + @Test + void chooseEngineServerAliasReturnsDelegateAliasWhenTestAliasIsUnknown() throws Exception { + + final String delegateSupportedAlgorithm = "supported-alg"; + final String[] delegateSupportedAlias = new String[] { "alias0", "alias1", "alias2" }; + final String delegateChosenAlias = delegateSupportedAlias[0]; + final String testAlias = "unknown-alias"; + final String testAlgorithm = delegateSupportedAlgorithm; + final String expectedAlias = delegateChosenAlias; + + KeyManagerFactory delegate = mockServerKeyManagerFactory(delegateSupportedAlgorithm, delegateSupportedAlias, + delegateChosenAlias); + AliasKeyManagerFactory factory = createAliasKeyManagerFactory(testAlias, delegate); + X509ExtendedKeyManager x509KeyManager = getX509ExtendedKeyManager(factory); + + String chosenAlias = x509KeyManager.chooseEngineServerAlias(testAlgorithm, null, null); + assertThat(chosenAlias).isEqualTo(expectedAlias); + } + + @Test + void chooseEngineServerAliasReturnsNullWhenAlgorithmDoesNotMatch() throws Exception { + + final String delegateSupportedAlgorithm = "supported-alg"; + final String[] delegateSupportedAlias = new String[] { "alias0", "alias1", "alias2" }; + final String delegateChosenAlias = delegateSupportedAlias[1]; + final String testAlias = delegateSupportedAlias[0]; + final String testAlgorithm = "other-alg"; + final String expectedAlias = null; + + KeyManagerFactory delegate = mockServerKeyManagerFactory(delegateSupportedAlgorithm, delegateSupportedAlias, + delegateChosenAlias); + AliasKeyManagerFactory factory = createAliasKeyManagerFactory(testAlias, delegate); + X509ExtendedKeyManager x509KeyManager = getX509ExtendedKeyManager(factory); + + String chosenAlias = x509KeyManager.chooseEngineServerAlias(testAlgorithm, null, null); + assertThat(chosenAlias).isEqualTo(expectedAlias); + } + + @Test + void chooseServerAliasReturnsTestAliasMatchingSupportedDelegateAliasAndAlgorithm() throws Exception { + + final String delegateSupportedAlgorithm = "supported-alg"; + final String[] delegateSupportedAlias = new String[] { "alias0", "alias1", "alias2" }; + final String delegateChosenAlias = delegateSupportedAlias[0]; + final String testAlias = delegateSupportedAlias[1]; + final String testAlgorithm = delegateSupportedAlgorithm; + final String expectedAlias = testAlias; + + KeyManagerFactory delegate = mockServerKeyManagerFactory(delegateSupportedAlgorithm, delegateSupportedAlias, + delegateChosenAlias); + AliasKeyManagerFactory factory = createAliasKeyManagerFactory(testAlias, delegate); + X509ExtendedKeyManager x509KeyManager = getX509ExtendedKeyManager(factory); + + String chosenAlias = x509KeyManager.chooseServerAlias(testAlgorithm, null, null); + assertThat(chosenAlias).isEqualTo(expectedAlias); + } + + @Test + void chooseServerAliasReturnsDelegateAliasWhenTestAliasIsUnknown() throws Exception { + + final String delegateSupportedAlgorithm = "supported-alg"; + final String[] delegateSupportedAlias = new String[] { "alias0", "alias1", "alias2" }; + final String delegateChosenAlias = delegateSupportedAlias[0]; + final String testAlias = "unknown-alias"; + final String testAlgorithm = delegateSupportedAlgorithm; + final String expectedAlias = delegateChosenAlias; + + KeyManagerFactory delegate = mockServerKeyManagerFactory(delegateSupportedAlgorithm, delegateSupportedAlias, + delegateChosenAlias); + AliasKeyManagerFactory factory = createAliasKeyManagerFactory(testAlias, delegate); + X509ExtendedKeyManager x509KeyManager = getX509ExtendedKeyManager(factory); + + String chosenAlias = x509KeyManager.chooseServerAlias(testAlgorithm, null, null); + assertThat(chosenAlias).isEqualTo(expectedAlias); + } + + @Test + void chooseServerAliasReturnsNullWhenAlgorithmDoesNotMatch() throws Exception { + + final String delegateSupportedAlgorithm = "supported-alg"; + final String[] delegateSupportedAlias = new String[] { "alias0", "alias1", "alias2" }; + final String delegateChosenAlias = delegateSupportedAlias[1]; + final String testAlias = delegateSupportedAlias[0]; + final String testAlgorithm = "other-alg"; + final String expectedAlias = null; + + KeyManagerFactory delegate = mockServerKeyManagerFactory(delegateSupportedAlgorithm, delegateSupportedAlias, + delegateChosenAlias); + AliasKeyManagerFactory factory = createAliasKeyManagerFactory(testAlias, delegate); + X509ExtendedKeyManager x509KeyManager = getX509ExtendedKeyManager(factory); + + String chosenAlias = x509KeyManager.chooseServerAlias(testAlgorithm, null, null); + assertThat(chosenAlias).isEqualTo(expectedAlias); + } + + @Test + void chooseEngineClientAliasReturnsTestAliasMatchingSupportedDelegateAliasAndAlgorithm() throws Exception { + + final String delegateSupportedAlgorithm = "supported-alg"; + final String[] delegateSupportedAlias = new String[] { "alias0", "alias1", "alias2" }; + final String delegateChosenAlias = delegateSupportedAlias[0]; + final String testAlias = delegateSupportedAlias[1]; + final String[] testAlgorithms = new String[] { delegateSupportedAlgorithm }; + final String expectedAlias = testAlias; + + KeyManagerFactory delegate = mockClientKeyManagerFactory(delegateSupportedAlgorithm, delegateSupportedAlias, + delegateChosenAlias); + AliasKeyManagerFactory factory = createAliasKeyManagerFactory(testAlias, delegate); + X509ExtendedKeyManager x509KeyManager = getX509ExtendedKeyManager(factory); + + String chosenAlias = x509KeyManager.chooseEngineClientAlias(testAlgorithms, null, null); + assertThat(chosenAlias).isEqualTo(expectedAlias); + } + + @Test + void chooseEngineClientAliasReturnsDelegateAliasWhenTestAliasIsUnknown() throws Exception { + + final String delegateSupportedAlgorithm = "supported-alg"; + final String[] delegateSupportedAlias = new String[] { "alias0", "alias1", "alias2" }; + final String delegateChosenAlias = delegateSupportedAlias[0]; + final String testAlias = "unknown-alias"; + final String[] testAlgorithms = new String[] { delegateSupportedAlgorithm }; + final String expectedAlias = delegateChosenAlias; + + KeyManagerFactory delegate = mockClientKeyManagerFactory(delegateSupportedAlgorithm, delegateSupportedAlias, + delegateChosenAlias); + AliasKeyManagerFactory factory = createAliasKeyManagerFactory(testAlias, delegate); + X509ExtendedKeyManager x509KeyManager = getX509ExtendedKeyManager(factory); + + String chosenAlias = x509KeyManager.chooseEngineClientAlias(testAlgorithms, null, null); + assertThat(chosenAlias).isEqualTo(expectedAlias); + } + + @Test + void chooseEngineClientAliasReturnsNullWhenAlgorithmDoesNotMatch() throws Exception { + + final String delegateSupportedAlgorithm = "supported-alg"; + final String[] delegateSupportedAlias = new String[] { "alias0", "alias1", "alias2" }; + final String delegateChosenAlias = delegateSupportedAlias[1]; + final String testAlias = delegateSupportedAlias[0]; + final String[] testAlgorithms = new String[] { "other-alg" }; + final String expectedAlias = null; + + KeyManagerFactory delegate = mockClientKeyManagerFactory(delegateSupportedAlgorithm, delegateSupportedAlias, + delegateChosenAlias); + AliasKeyManagerFactory factory = createAliasKeyManagerFactory(testAlias, delegate); + X509ExtendedKeyManager x509KeyManager = getX509ExtendedKeyManager(factory); + + String chosenAlias = x509KeyManager.chooseEngineClientAlias(testAlgorithms, null, null); + assertThat(chosenAlias).isEqualTo(expectedAlias); + } + + @Test + void chooseClientAliasReturnsTestAliasMatchingSupportedDelegateAliasAndAlgorithm() throws Exception { + + final String delegateSupportedAlgorithm = "supported-alg"; + final String[] delegateSupportedAlias = new String[] { "alias0", "alias1", "alias2" }; + final String delegateChosenAlias = delegateSupportedAlias[0]; + final String testAlias = delegateSupportedAlias[1]; + final String[] testAlgorithms = new String[] { delegateSupportedAlgorithm }; + final String expectedAlias = testAlias; + + KeyManagerFactory delegate = mockClientKeyManagerFactory(delegateSupportedAlgorithm, delegateSupportedAlias, + delegateChosenAlias); + AliasKeyManagerFactory factory = createAliasKeyManagerFactory(testAlias, delegate); + X509ExtendedKeyManager x509KeyManager = getX509ExtendedKeyManager(factory); + + String chosenAlias = x509KeyManager.chooseClientAlias(testAlgorithms, null, null); + assertThat(chosenAlias).isEqualTo(expectedAlias); + } + + @Test + void chooseClientAliasReturnsDelegateAliasWhenTestAliasIsUnknown() throws Exception { + + final String delegateSupportedAlgorithm = "supported-alg"; + final String[] delegateSupportedAlias = new String[] { "alias0", "alias1", "alias2" }; + final String delegateChosenAlias = delegateSupportedAlias[0]; + final String testAlias = "unknown-alias"; + final String[] testAlgorithms = new String[] { delegateSupportedAlgorithm }; + final String expectedAlias = delegateChosenAlias; + + KeyManagerFactory delegate = mockClientKeyManagerFactory(delegateSupportedAlgorithm, delegateSupportedAlias, + delegateChosenAlias); + AliasKeyManagerFactory factory = createAliasKeyManagerFactory(testAlias, delegate); + X509ExtendedKeyManager x509KeyManager = getX509ExtendedKeyManager(factory); + + String chosenAlias = x509KeyManager.chooseClientAlias(testAlgorithms, null, null); + assertThat(chosenAlias).isEqualTo(expectedAlias); + } + + @Test + void chooseClientAliasReturnsNullWhenAlgorithmDoesNotMatch() throws Exception { + + final String delegateSupportedAlgorithm = "supported-alg"; + final String[] delegateSupportedAlias = new String[] { "alias0", "alias1", "alias2" }; + final String delegateChosenAlias = delegateSupportedAlias[1]; + final String testAlias = delegateSupportedAlias[0]; + final String[] testAlgorithms = new String[] { "other-alg" }; + final String expectedAlias = null; + + KeyManagerFactory delegate = mockClientKeyManagerFactory(delegateSupportedAlgorithm, delegateSupportedAlias, + delegateChosenAlias); + AliasKeyManagerFactory factory = createAliasKeyManagerFactory(testAlias, delegate); + X509ExtendedKeyManager x509KeyManager = getX509ExtendedKeyManager(factory); + + String chosenAlias = x509KeyManager.chooseClientAlias(testAlgorithms, null, null); + assertThat(chosenAlias).isEqualTo(expectedAlias); + } + + private static AliasKeyManagerFactory createAliasKeyManagerFactory(String alias, KeyManagerFactory delegate) + throws Exception { + AliasKeyManagerFactory factory = new AliasKeyManagerFactory(delegate, alias, delegate.getAlgorithm()); factory.init(null, null); - KeyManager[] keyManagers = factory.getKeyManagers(); - X509ExtendedKeyManager x509KeyManager = (X509ExtendedKeyManager) Arrays.stream(keyManagers) + return factory; + } + + private static X509ExtendedKeyManager getX509ExtendedKeyManager(KeyManagerFactory factory) { + return Arrays.stream(factory.getKeyManagers()) .filter(X509ExtendedKeyManager.class::isInstance) + .map(X509ExtendedKeyManager.class::cast) .findAny() .get(); - String chosenAlias = x509KeyManager.chooseEngineServerAlias(null, null, null); - assertThat(chosenAlias).isEqualTo("test-alias"); + } + + private static KeyManagerFactory mockServerKeyManagerFactory(String algorithm, String[] serverAliases, + String serverChosenAlias) { + + return mockKeyManagerFactory(algorithm, serverAliases, serverChosenAlias, null, null); + } + + private static KeyManagerFactory mockClientKeyManagerFactory(String algorithm, String[] clientAliases, + String clientChosenAlias) { + + return mockKeyManagerFactory(algorithm, null, null, clientAliases, clientChosenAlias); + } + + private static KeyManagerFactory mockKeyManagerFactory(String algorithm, String[] serverAliases, + String serverChosenAlias, String[] clientAliases, String clientChosenAlias) { + + KeyManagerFactory delegate = mock(KeyManagerFactory.class); + X509ExtendedKeyManager x509KeyManagerMock = mock(X509ExtendedKeyManager.class); + given(delegate.getAlgorithm()).willReturn(algorithm); + given(delegate.getKeyManagers()).willReturn(new KeyManager[] { x509KeyManagerMock }); + given(x509KeyManagerMock.getServerAliases(eq(algorithm), any())).willReturn(serverAliases); + given(x509KeyManagerMock.chooseServerAlias(eq(algorithm), any(), any())).willReturn(serverChosenAlias); + given(x509KeyManagerMock.chooseEngineServerAlias(eq(algorithm), any(), any())).willReturn(serverChosenAlias); + given(x509KeyManagerMock.getClientAliases(eq(algorithm), any())).willReturn(clientAliases); + given(x509KeyManagerMock.chooseClientAlias(argThat(arrayContains(algorithm)), any(), any())) + .willReturn(clientChosenAlias); + given(x509KeyManagerMock.chooseEngineClientAlias(argThat(arrayContains(algorithm)), any(), any())) + .willReturn(clientChosenAlias); + return delegate; + } + + private static ArgumentMatcher arrayContains(String expected) { + return array -> array != null && Arrays.asList(array).contains(expected); } }