Fix AliasX509ExtendedKeyManager to process both server and client aliases properly

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,23 @@ 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).orElse(null);
 		}
 
 		@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).orElse(null);
 		}
 
 		@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).orElse(null);
 		}
 
 		@Override
 		public String chooseServerAlias(String keyType, Principal[] issuers, Socket socket) {
-			return this.delegate.chooseServerAlias(keyType, issuers, socket);
+			return findFirstMatchingAlias(keyType, issuers, this::getServerAliases).orElse(null);
 		}
 
 		@Override
@@ -144,6 +149,50 @@ public String[] getServerAliases(String keyType, Principal[] issuers) {
 			return this.delegate.getServerAliases(keyType, issuers);
 		}
 
+		/**
+		 * 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<String> 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<String> 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());
+		}
+
+		/**
+		 * Typed-BiFunction for better readability.
+		 */
+		private interface KeyAliasFinder extends BiFunction<String, Principal[], String[]> {
+
+		}
+
 	}
 
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/AliasKeyManagerFactoryTests.java

@@ -23,32 +23,298 @@
 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;
 
 /**
- * Tests for {@link AliasKeyManagerFactory}.
+ * Tests for {@link org.springframework.boot.ssl.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 chooseEngineServerAliasReturnsNullWhenTestAliasIsUnknown() 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 = 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 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 chooseServerAliasReturnsNullWhenTestAliasIsUnknown() 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 = 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 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 chooseEngineClientAliasReturnsNullWhenTestAliasIsUnknown() 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 = 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 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 chooseClientAliasReturnsNullWhenTestAliasIsUnknown() 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 = 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);
+	}
+
+	@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<String[]> arrayContains(String expected) {
+		return (array) -> array != null && Arrays.asList(array).contains(expected);
 	}
 
 }