Respect the KMF algorithm property

Author: dagnir

.changes/next-release/bugfix-AWSSDKforJavav2-a18734d.json

@@ -0,0 +1,5 @@
+{
+    "type": "bugfix",
+    "category": "AWS SDK for Java v2",
+    "description": "This change makes the `FileStoreTlsKeyManagersProvider` and `SystemPropertyTlsKeyManagersProvider` respect the `ssl.KeyManagerFactory.algorithm` when instantiating the `KeyManagerFactory` rather than always using the hardcoded value of `SunX509`."
+}

http-client-spi/src/main/java/software/amazon/awssdk/http/FileStoreTlsKeyManagersProvider.java

@@ -17,6 +17,7 @@
 
 import java.nio.file.Path;
 import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
 import software.amazon.awssdk.annotations.SdkPublicApi;
 import software.amazon.awssdk.internal.http.AbstractFileStoreTlsKeyManagersProvider;
 import software.amazon.awssdk.utils.Logger;
@@ -25,6 +26,9 @@
 /**
  * Implementation of {@link FileStoreTlsKeyManagersProvider} that loads a the
  * key store from a file.
+ * <p>
+ * This uses {@link KeyManagerFactory#getDefaultAlgorithm()} to determine the
+ * {@code KeyManagerFactory} algorithm to use.
  */
 @SdkPublicApi
 public final class FileStoreTlsKeyManagersProvider extends AbstractFileStoreTlsKeyManagersProvider {

http-client-spi/src/main/java/software/amazon/awssdk/http/SystemPropertyTlsKeyManagersProvider.java

@@ -24,6 +24,7 @@
 import java.security.KeyStore;
 import java.util.Optional;
 import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
 import software.amazon.awssdk.annotations.SdkPublicApi;
 import software.amazon.awssdk.internal.http.AbstractFileStoreTlsKeyManagersProvider;
 import software.amazon.awssdk.utils.Logger;
@@ -37,6 +38,9 @@
  * {@code javax.net.ssl.keyStorePassword}, and
  * {@code javax.net.ssl.keyStoreType} properties defined by the
  * <a href="https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/JSSERefGuide.html">JSSE</a>.
+ * <p>
+ * This uses {@link KeyManagerFactory#getDefaultAlgorithm()} to determine the
+ * {@code KeyManagerFactory} algorithm to use.
  */
 @SdkPublicApi
 public final class SystemPropertyTlsKeyManagersProvider extends AbstractFileStoreTlsKeyManagersProvider {

http-client-spi/src/main/java/software/amazon/awssdk/internal/http/AbstractFileStoreTlsKeyManagersProvider.java

@@ -32,14 +32,17 @@
 /**
  * Abstract {@link TlsKeyManagersProvider} that loads the key store from a
  * a given file path.
+ * <p>
+ * This uses {@link KeyManagerFactory#getDefaultAlgorithm()} to determine the
+ * {@code KeyManagerFactory} algorithm to use.
  */
 @SdkInternalApi
 public abstract class AbstractFileStoreTlsKeyManagersProvider implements TlsKeyManagersProvider {
 
     protected final KeyManager[] createKeyManagers(Path storePath, String storeType, char[] password)
             throws CertificateException, NoSuchAlgorithmException, KeyStoreException, IOException, UnrecoverableKeyException {
         KeyStore ks = createKeyStore(storePath, storeType, password);
-        KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509");
+        KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
         kmf.init(ks, password);
         return kmf.getKeyManagers();
     }

http-client-spi/src/test/java/software/amazon/awssdk/http/FileStoreTlsKeyManagersProviderTest.java

@@ -18,6 +18,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import java.io.IOException;
 import java.nio.file.Paths;
+import java.security.Security;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -77,4 +78,26 @@ public void passwordIncorrect_returnsNull() {
         FileStoreTlsKeyManagersProvider provider = FileStoreTlsKeyManagersProvider.create(clientKeyStore, CLIENT_STORE_TYPE, "not correct password");
         assertThat(provider.keyManagers()).isNull();
     }
+
+    @Test
+    public void customKmfAlgorithmSetInProperty_usesAlgorithm() {
+        FileStoreTlsKeyManagersProvider beforePropSetProvider = FileStoreTlsKeyManagersProvider.create(clientKeyStore,
+                CLIENT_STORE_TYPE, STORE_PASSWORD);
+
+        assertThat(beforePropSetProvider.keyManagers()).isNotNull();
+
+        String property = "ssl.KeyManagerFactory.algorithm";
+        String previousValue = Security.getProperty(property);
+        Security.setProperty(property, "some-bogus-value");
+        try {
+            FileStoreTlsKeyManagersProvider afterPropSetProvider = FileStoreTlsKeyManagersProvider.create(
+            clientKeyStore, CLIENT_STORE_TYPE, STORE_PASSWORD);
+            // This would otherwise be non-null if using the right algorithm,
+            // i.e. not setting the algorithm property will cause the assertion
+            // to fail
+            assertThat(afterPropSetProvider.keyManagers()).isNull();
+        } finally {
+            Security.setProperty(property, previousValue);
+        }
+    }
 }

http-client-spi/src/test/java/software/amazon/awssdk/http/SystemPropertyTlsKeyManagersProviderTest.java

@@ -21,6 +21,7 @@
 import static software.amazon.awssdk.utils.JavaSystemSetting.SSL_KEY_STORE_TYPE;
 import java.io.IOException;
 import java.nio.file.Paths;
+import java.security.Security;
 import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
@@ -86,4 +87,26 @@ public void passwordIncorrect_returnsNull() {
 
         assertThat(PROVIDER.keyManagers()).isNull();
     }
+
+    @Test
+    public void customKmfAlgorithmSetInProperty_usesAlgorithm() {
+        System.setProperty(SSL_KEY_STORE.property(), clientKeyStore.toAbsolutePath().toString());
+        System.setProperty(SSL_KEY_STORE_TYPE.property(), CLIENT_STORE_TYPE);
+        System.setProperty(SSL_KEY_STORE_PASSWORD.property(), STORE_PASSWORD);
+
+        assertThat(PROVIDER.keyManagers()).isNotNull();
+
+        String property = "ssl.KeyManagerFactory.algorithm";
+        String previousValue = Security.getProperty(property);
+        Security.setProperty(property, "some-bogus-value");
+
+        try {
+            // This would otherwise be non-null if using the right algorithm,
+            // i.e. not setting the algorithm property will cause the assertion
+            // to fail
+            assertThat(PROVIDER.keyManagers()).isNull();
+        } finally {
+            Security.setProperty(property, previousValue);
+        }
+    }
 }