Prefer PKIX algorithm name for TrustManagerFactory and KeyManagerFactory

Change-Id: I3da5bdf6739c6aee5ec0174e93cd6c06d6dfeeb3

14 files changed, 63 insertions, 59 deletions

diff --git a/luni/src/main/java/java/security/KeyStore.java b/luni/src/main/java/java/security/KeyStore.java
index 3d856f7..020010e 100644
--- a/luni/src/main/java/java/security/KeyStore.java
+++ b/luni/src/main/java/java/security/KeyStore.java
@@ -55,7 +55,7 @@ public class KeyStore {
     private static final Engine ENGINE = new Engine(SERVICE);
 
     //  Store KeyStore property name
-    private static final String PROPERTYNAME = "keystore.type";
+    private static final String PROPERTY_NAME = "keystore.type";
 
     //  Store default KeyStore type
     private static final String DEFAULT_KEYSTORE_TYPE = "jks";
@@ -204,7 +204,7 @@ public class KeyStore {
      * @return the default type for {@code KeyStore} instances
      */
     public static final String getDefaultType() {
-        String dt = Security.getProperty(PROPERTYNAME);
+        String dt = Security.getProperty(PROPERTY_NAME);
         return (dt == null ? DEFAULT_KEYSTORE_TYPE : dt);
     }
 
diff --git a/luni/src/main/java/java/security/cert/CertPathBuilder.java b/luni/src/main/java/java/security/cert/CertPathBuilder.java
index 42029e5..0817fc0 100644
--- a/luni/src/main/java/java/security/cert/CertPathBuilder.java
+++ b/luni/src/main/java/java/security/cert/CertPathBuilder.java
@@ -37,11 +37,10 @@ public class CertPathBuilder {
     private static final Engine ENGINE = new Engine(SERVICE);
 
     // Store default property name
-    private static final String PROPERTYNAME = "certpathbuilder.type";
+    private static final String PROPERTY_NAME = "certpathbuilder.type";
 
-    // Default value of CertPathBuilder type. It returns if certpathbuild.type
-    // property is not defined in java.security file
-    private static final String DEFAULTPROPERTY = "PKIX";
+    // Default value of CertPathBuilder type.
+    private static final String DEFAULT_PROPERTY = "PKIX";
 
     // Store used provider
     private final Provider provider;
@@ -192,7 +191,7 @@ public class CertPathBuilder {
      *         determined.
      */
     public static final String getDefaultType() {
-        String defaultType = Security.getProperty(PROPERTYNAME);
-        return (defaultType != null ? defaultType : DEFAULTPROPERTY);
+        String defaultType = Security.getProperty(PROPERTY_NAME);
+        return (defaultType != null ? defaultType : DEFAULT_PROPERTY);
     }
 }
diff --git a/luni/src/main/java/java/security/cert/CertPathValidator.java b/luni/src/main/java/java/security/cert/CertPathValidator.java
index ddf78bf..fda3aeb 100644
--- a/luni/src/main/java/java/security/cert/CertPathValidator.java
+++ b/luni/src/main/java/java/security/cert/CertPathValidator.java
@@ -37,11 +37,11 @@ public class CertPathValidator {
     private static final Engine ENGINE = new Engine(SERVICE);
 
     // Store default property name
-    private static final String PROPERTYNAME = "certpathvalidator.type";
+    private static final String PROPERTY_NAME = "certpathvalidator.type";
 
     // Default value of CertPathBuilder type. It returns if certpathbuild.type
     // property is not defined in java.security file
-    private static final String DEFAULTPROPERTY = "PKIX";
+    private static final String DEFAULT_PROPERTY = "PKIX";
 
     // Store used provider
     private final Provider provider;
@@ -199,7 +199,7 @@ public class CertPathValidator {
      *         determined.
      */
     public static final String getDefaultType() {
-        String defaultType = Security.getProperty(PROPERTYNAME);
-        return (defaultType != null ? defaultType : DEFAULTPROPERTY);
+        String defaultType = Security.getProperty(PROPERTY_NAME);
+        return (defaultType != null ? defaultType : DEFAULT_PROPERTY);
     }
 }
diff --git a/luni/src/main/java/java/security/cert/CertStore.java b/luni/src/main/java/java/security/cert/CertStore.java
index 2e28828..72d356f 100644
--- a/luni/src/main/java/java/security/cert/CertStore.java
+++ b/luni/src/main/java/java/security/cert/CertStore.java
@@ -39,11 +39,11 @@ public class CertStore {
     private static final Engine ENGINE = new Engine(SERVICE);
 
     // Store default property name
-    private static final String PROPERTYNAME = "certstore.type";
+    private static final String PROPERTY_NAME = "certstore.type";
 
     // Default value of CertStore type. It returns if certpathbuild.type
     // property is not defined in java.security file
-    private static final String DEFAULTPROPERTY = "LDAP";
+    private static final String DEFAULT_PROPERTY = "LDAP";
 
     // Store used provider
     private final Provider provider;
@@ -266,7 +266,7 @@ public class CertStore {
      *         determined.
      */
     public static final String getDefaultType() {
-        String defaultType = Security.getProperty(PROPERTYNAME);
-        return (defaultType == null ? DEFAULTPROPERTY : defaultType);
+        String defaultType = Security.getProperty(PROPERTY_NAME);
+        return (defaultType == null ? DEFAULT_PROPERTY : defaultType);
     }
 }
diff --git a/luni/src/main/java/java/security/security.properties b/luni/src/main/java/java/security/security.properties
index 361e2ad..b124271 100644
--- a/luni/src/main/java/java/security/security.properties
+++ b/luni/src/main/java/java/security/security.properties
@@ -55,8 +55,8 @@ keystore.type=BKS
 # See specification for
 # javax/net/ssl/KeyManagerFactory.html#getDefaultAlgorithm()
 # javax/net/ssl/TrustManagerFactory.html#getDefaultAlgorithm()
-ssl.KeyManagerFactory.algorithm=X509
-ssl.TrustManagerFactory.algorithm=X509
+ssl.KeyManagerFactory.algorithm=PKIX
+ssl.TrustManagerFactory.algorithm=PKIX
 
 # system.scope is used to specify implementation class of IdentityScope
 system.scope=org.apache.harmony.security.SystemScope
diff --git a/luni/src/main/java/javax/net/ssl/HttpsURLConnection.java b/luni/src/main/java/javax/net/ssl/HttpsURLConnection.java
index 0f22f38..9803f3d 100644
--- a/luni/src/main/java/javax/net/ssl/HttpsURLConnection.java
+++ b/luni/src/main/java/javax/net/ssl/HttpsURLConnection.java
@@ -44,7 +44,8 @@ import java.security.cert.X509Certificate;
  * <p>For example, to trust a set of certificates specified by a {@code KeyStore}:
  * <pre>   {@code
  *   KeyStore keyStore = ...;
- *   TrustManagerFactory tmf = TrustManagerFactory.getInstance("X509");
+ *   String algorithm = TrustManagerFactory.getDefaultAlgorithm();
+ *   TrustManagerFactory tmf = TrustManagerFactory.getInstance(algorithm);
  *   tmf.init(keyStore);
  *
  *   SSLContext context = SSLContext.getInstance("TLS");
@@ -77,7 +78,8 @@ import java.security.cert.X509Certificate;
  * <p>For example, to supply client certificates from a {@code KeyStore}:
  * <pre>   {@code
  *   KeyStore keyStore = ...;
- *   KeyManagerFactory kmf = KeyManagerFactory.getInstance("X509");
+ *   String algorithm = KeyManagerFactory.getDefaultAlgorithm();
+ *   KeyManagerFactory kmf = KeyManagerFactory.getInstance(algorithm);
  *   kmf.init(keyStore);
  *
  *   SSLContext context = SSLContext.getInstance("TLS");
diff --git a/luni/src/main/java/javax/net/ssl/KeyManagerFactory.java b/luni/src/main/java/javax/net/ssl/KeyManagerFactory.java
index 0b3db61..ef085e4 100644
--- a/luni/src/main/java/javax/net/ssl/KeyManagerFactory.java
+++ b/luni/src/main/java/javax/net/ssl/KeyManagerFactory.java
@@ -40,6 +40,9 @@ public class KeyManagerFactory {
     // Store default property name
     private static final String PROPERTY_NAME = "ssl.KeyManagerFactory.algorithm";
 
+    // Default value of KeyManagerFactory type.
+    private static final String DEFAULT_PROPERTY = "PKIX";
+
     /**
      * Returns the default key manager factory algorithm name.
      * <p>
@@ -49,7 +52,8 @@ public class KeyManagerFactory {
      * @return the default algorithm name.
      */
     public static final String getDefaultAlgorithm() {
-        return Security.getProperty(PROPERTY_NAME);
+        String algorithm = Security.getProperty(PROPERTY_NAME);
+        return (algorithm != null ? algorithm : DEFAULT_PROPERTY);
     }
 
     /**
diff --git a/luni/src/main/java/javax/net/ssl/TrustManagerFactory.java b/luni/src/main/java/javax/net/ssl/TrustManagerFactory.java
index be9db06..72023f5 100644
--- a/luni/src/main/java/javax/net/ssl/TrustManagerFactory.java
+++ b/luni/src/main/java/javax/net/ssl/TrustManagerFactory.java
@@ -38,7 +38,10 @@ public class TrustManagerFactory {
     private static final Engine ENGINE = new Engine(SERVICE);
 
     // Store default property name
-    private static final String PROPERTYNAME = "ssl.TrustManagerFactory.algorithm";
+    private static final String PROPERTY_NAME = "ssl.TrustManagerFactory.algorithm";
+
+    // Default value of TrustManagerFactory type.
+    private static final String DEFAULT_PROPERTY = "PKIX";
 
     /**
      * Returns the default algorithm name for the {@code TrustManagerFactory}. The
@@ -48,7 +51,8 @@ public class TrustManagerFactory {
      * @return the default algorithm name.
      */
     public static final String getDefaultAlgorithm() {
-        return Security.getProperty(PROPERTYNAME);
+        String algorithm = Security.getProperty(PROPERTY_NAME);
+        return (algorithm != null ? algorithm : DEFAULT_PROPERTY);
     }
 
     /**
diff --git a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/DefaultSSLContextImpl.java b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/DefaultSSLContextImpl.java
index 5057518..66b9ebe 100644
--- a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/DefaultSSLContextImpl.java
+++ b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/DefaultSSLContextImpl.java
@@ -83,11 +83,7 @@ public final class DefaultSSLContextImpl extends OpenSSLContextImpl {
             }
         }
 
-        String kmfAlg = Security.getProperty("ssl.KeyManagerFactory.algorithm");
-        if (kmfAlg == null) {
-            kmfAlg = "SunX509";
-        }
-
+        String kmfAlg = KeyManagerFactory.getDefaultAlgorithm();
         KeyManagerFactory kmf = KeyManagerFactory.getInstance(kmfAlg);
         kmf.init(ks, pwd);
         KEY_MANAGERS = kmf.getKeyManagers();
@@ -119,11 +115,7 @@ public final class DefaultSSLContextImpl extends OpenSSLContextImpl {
                 is.close();
             }
         }
-        String tmfAlg = Security.getProperty("ssl.TrustManagerFactory.algorithm");
-        if (tmfAlg == null) {
-            tmfAlg = "PKIX";
-        }
-
+        String tmfAlg = TrustManagerFactory.getDefaultAlgorithm();
         TrustManagerFactory tmf = TrustManagerFactory.getInstance(tmfAlg);
         tmf.init(ks);
         TRUST_MANAGERS = tmf.getTrustManagers();
diff --git a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/JSSEProvider.java b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/JSSEProvider.java
index d9b7659..58dad45 100644
--- a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/JSSEProvider.java
+++ b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/JSSEProvider.java
@@ -112,8 +112,12 @@ public final class JSSEProvider extends Provider {
         put("SSLContext.TLS", SSLContextImpl.class.getName());
         put("SSLContext.TLSv1", SSLContextImpl.class.getName());
 
-        put("KeyManagerFactory.X509", KeyManagerFactoryImpl.class.getName());
-        put("TrustManagerFactory.X509", TrustManagerFactoryImpl.class.getName());
+        put("KeyManagerFactory.PKIX", KeyManagerFactoryImpl.class.getName());
+        put("Alg.Alias.KeyManagerFactory.X509", "PKIX");
+
+        put("TrustManagerFactory.PKIX", TrustManagerFactoryImpl.class.getName());
+        put("Alg.Alias.TrustManagerFactory.X509", "PKIX");
+
         put("KeyStore.AndroidCAStore", TrustedCertificateKeyStoreSpi.class.getName());
     }
 }
diff --git a/luni/src/test/java/libcore/javax/net/ssl/TrustManagerFactoryTest.java b/luni/src/test/java/libcore/javax/net/ssl/TrustManagerFactoryTest.java
index 8a3fe25..ad931af 100644
--- a/luni/src/test/java/libcore/javax/net/ssl/TrustManagerFactoryTest.java
+++ b/luni/src/test/java/libcore/javax/net/ssl/TrustManagerFactoryTest.java
@@ -53,17 +53,20 @@ public class TrustManagerFactoryTest extends TestCase {
         return TEST_KEY_STORE;
     }
 
+    private static boolean supportsManagerFactoryParameters(String algorithm) {
+        return (StandardNames.IS_RI && algorithm.equals("PKIX"));
+    }
+
     public void test_TrustManagerFactory_getDefaultAlgorithm() throws Exception {
         String algorithm = TrustManagerFactory.getDefaultAlgorithm();
         assertEquals(StandardNames.TRUST_MANAGER_FACTORY_DEFAULT, algorithm);
         TrustManagerFactory tmf = TrustManagerFactory.getInstance(algorithm);
-        test_TrustManagerFactory(tmf, StandardNames.IS_RI);
+        test_TrustManagerFactory(tmf);
     }
 
     private static class UselessManagerFactoryParameters implements ManagerFactoryParameters {}
 
-    private void test_TrustManagerFactory(TrustManagerFactory tmf,
-                                          boolean supportsManagerFactoryParameters)
+    private void test_TrustManagerFactory(TrustManagerFactory tmf)
             throws Exception {
         assertNotNull(tmf);
         assertNotNull(tmf.getAlgorithm());
@@ -103,7 +106,7 @@ public class TrustManagerFactoryTest extends TestCase {
         X509CertSelector xcs = new X509CertSelector();
         PKIXBuilderParameters pbp = new PKIXBuilderParameters(getTestKeyStore().keyStore, xcs);
         CertPathTrustManagerParameters cptmp = new CertPathTrustManagerParameters(pbp);
-        if (supportsManagerFactoryParameters) {
+        if (supportsManagerFactoryParameters(tmf.getAlgorithm())) {
             tmf.init(cptmp);
             test_TrustManagerFactory_getTrustManagers(tmf);
         } else {
@@ -179,11 +182,10 @@ public class TrustManagerFactoryTest extends TestCase {
                     continue;
                 }
                 String algorithm = service.getAlgorithm();
-                boolean supportsManagerFactoryParameters = algorithm.equals("PKIX");
                 {
                     TrustManagerFactory tmf = TrustManagerFactory.getInstance(algorithm);
                     assertEquals(algorithm, tmf.getAlgorithm());
-                    test_TrustManagerFactory(tmf, supportsManagerFactoryParameters);
+                    test_TrustManagerFactory(tmf);
                 }
 
                 {
@@ -191,7 +193,7 @@ public class TrustManagerFactoryTest extends TestCase {
                                                                           provider);
                     assertEquals(algorithm, tmf.getAlgorithm());
                     assertEquals(provider, tmf.getProvider());
-                    test_TrustManagerFactory(tmf, supportsManagerFactoryParameters);
+                    test_TrustManagerFactory(tmf);
                 }
 
                 {
@@ -199,7 +201,7 @@ public class TrustManagerFactoryTest extends TestCase {
                                                                           provider.getName());
                     assertEquals(algorithm, tmf.getAlgorithm());
                     assertEquals(provider, tmf.getProvider());
-                    test_TrustManagerFactory(tmf, supportsManagerFactoryParameters);
+                    test_TrustManagerFactory(tmf);
                 }
             }
         }
diff --git a/luni/src/test/java/tests/targets/security/cert/CertPathBuilderTestPKIX.java b/luni/src/test/java/tests/targets/security/cert/CertPathBuilderTestPKIX.java
index fc67261..dc49de0 100644
--- a/luni/src/test/java/tests/targets/security/cert/CertPathBuilderTestPKIX.java
+++ b/luni/src/test/java/tests/targets/security/cert/CertPathBuilderTestPKIX.java
@@ -44,8 +44,7 @@ public class CertPathBuilderTestPKIX extends CertPathBuilderTest {
 
         keyStore.load(null, null);
 
-        CertificateFactory certificateFactory = CertificateFactory.getInstance(
-                "X509");
+        CertificateFactory certificateFactory = CertificateFactory.getInstance("X509");
 
         X509Certificate selfSignedcertificate =
                 (X509Certificate) certificateFactory.generateCertificate(
diff --git a/luni/src/test/java/tests/targets/security/cert/CertPathValidatorTestPKIX.java b/luni/src/test/java/tests/targets/security/cert/CertPathValidatorTestPKIX.java
index af4037f..62b5f4a 100644
--- a/luni/src/test/java/tests/targets/security/cert/CertPathValidatorTestPKIX.java
+++ b/luni/src/test/java/tests/targets/security/cert/CertPathValidatorTestPKIX.java
@@ -56,8 +56,7 @@ public class CertPathValidatorTestPKIX extends CertPathValidatorTest {
         KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
         keyStore.load(null, null);
 
-        CertificateFactory certificateFactory = CertificateFactory.getInstance(
-                "X509");
+        CertificateFactory certificateFactory = CertificateFactory.getInstance("X509");
 
         X509Certificate selfSignedcertificate =
                 (X509Certificate) certificateFactory.generateCertificate(
diff --git a/support/src/test/java/libcore/java/security/StandardNames.java b/support/src/test/java/libcore/java/security/StandardNames.java
index d5ceedf..56a14cb 100644
--- a/support/src/test/java/libcore/java/security/StandardNames.java
+++ b/support/src/test/java/libcore/java/security/StandardNames.java
@@ -69,8 +69,8 @@ public final class StandardNames extends Assert {
     public static final String JSSE_PROVIDER_NAME = (IS_RI) ? "SunJSSE" : "AndroidOpenSSL";
     public static final String SECURITY_PROVIDER_NAME = (IS_RI) ? "SUN" : "BC";
 
-    public static final String KEY_MANAGER_FACTORY_DEFAULT = (IS_RI) ? "SunX509" : "X509";
-    public static final String TRUST_MANAGER_FACTORY_DEFAULT = (IS_RI) ? "PKIX" : "X509";
+    public static final String KEY_MANAGER_FACTORY_DEFAULT = (IS_RI) ? "SunX509" : "PKIX";
+    public static final String TRUST_MANAGER_FACTORY_DEFAULT = "PKIX";
 
     public static final String KEY_STORE_ALGORITHM = (IS_RI) ? "JKS" : "BKS";
 
@@ -178,7 +178,7 @@ public final class StandardNames extends Assert {
         provide("KeyGenerator", "HmacSHA512");
         provide("KeyGenerator", "RC2");
         provide("KeyInfoFactory", "DOM");
-        provide("KeyManagerFactory", "SunX509");
+        provide("KeyManagerFactory", "PKIX");
         provide("KeyPairGenerator", "DSA");
         provide("KeyPairGenerator", "DiffieHellman");
         provide("KeyPairGenerator", "RSA");
@@ -263,7 +263,6 @@ public final class StandardNames extends Assert {
             provide("KeyGenerator", "SunTlsMasterSecret");
             provide("KeyGenerator", "SunTlsPrf");
             provide("KeyGenerator", "SunTlsRsaPremasterSecret");
-            provide("KeyManagerFactory", "NewSunX509");
             provide("KeyStore", "CaseExactJKS");
             provide("Mac", "HmacPBESHA1");
             provide("Mac", "SslMacMD5");
@@ -307,6 +306,14 @@ public final class StandardNames extends Assert {
             unprovide("SSLContext", "TLSv1.2");
         }
 
+        // Fixups for the RI
+        if (IS_RI) {
+            // different names: Standard Names says PKIX, JSSE Reference Guide says SunX509 or NewSunX509
+            unprovide("KeyManagerFactory", "PKIX");
+            provide("KeyManagerFactory", "SunX509");
+            provide("KeyManagerFactory", "NewSunX509");
+        }
+
         // Fixups for dalvik
         if (!IS_RI) {
 
@@ -341,10 +348,6 @@ public final class StandardNames extends Assert {
             provide("Cipher", "PBEWithSHAAnd3-KEYTripleDES-CBC");
             provide("SecretKeyFactory", "PBEWithSHAAnd3-KEYTripleDES-CBC");
 
-            // different names: dropped Sun
-            unprovide("KeyManagerFactory", "SunX509");
-            provide("KeyManagerFactory", "X509");
-
             // different names: BouncyCastle actually uses the Standard name of SHA-1 vs SHA
             unprovide("MessageDigest", "SHA");
             provide("MessageDigest", "SHA-1");
@@ -366,10 +369,6 @@ public final class StandardNames extends Assert {
             provide("Cipher", "RSA/ECB/NOPADDING");
             provide("Cipher", "RSA/ECB/PKCS1PADDING");
 
-            // different names: JSSE Reference Guide says PKIX aka X509
-            unprovide("TrustManagerFactory", "PKIX");
-            provide("TrustManagerFactory", "X509");
-
             // different names: ARCFOUR vs ARC4
             unprovide("Cipher", "ARCFOUR");
             provide("Cipher", "ARC4");