Remove IndexedPKIXParameters

Change-Id: Idaaa1952d1b6148c51b3da5d1771105e8bde8a03

8 files changed, 219 insertions, 174 deletions

diff --git a/luni/src/main/java/java/security/cert/PKIXParameters.java b/luni/src/main/java/java/security/cert/PKIXParameters.java
index 450bdd1..8e24dfd 100644
--- a/luni/src/main/java/java/security/cert/PKIXParameters.java
+++ b/luni/src/main/java/java/security/cert/PKIXParameters.java
@@ -68,11 +68,6 @@ public class PKIXParameters implements CertPathParameters {
     private boolean policyQualifiersRejected = true;
 
     /**
-     * @hide For use by IndexedPKIXParameters which lazily discovers TrustAnchors
-     */
-    protected PKIXParameters() {}
-
-    /**
      * Creates a new {@code PKIXParameters} instance with the specified set of
      * <i>trusted</i> certificate authorities.
      *
@@ -581,16 +576,14 @@ public class PKIXParameters implements CertPathParameters {
         return sb.toString();
     }
 
-    //
-    // Private stuff
-    //
-
-    //
-    // Checks that 'trustAnchors' contains
-    // only TrustAnchor instances.
-    // Throws InvalidAlgorithmParameterException if trustAnchors set is empty.
-    //
-    private void checkTrustAnchors(Set<TrustAnchor> trustAnchors) throws InvalidAlgorithmParameterException {
+    /**
+     * Checks that {@code trustAnchors} contains only {@code
+     * TrustAnchor} instances.
+     *
+     * @throws InvalidAlgorithmParameterException if trustAnchors set is empty.
+     */
+    private void checkTrustAnchors(Set<TrustAnchor> trustAnchors)
+            throws InvalidAlgorithmParameterException {
         if (trustAnchors.isEmpty()) {
             throw new InvalidAlgorithmParameterException("trustAnchors.isEmpty()");
         }
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 79861a4..d9b7659 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
@@ -114,6 +114,6 @@ public final class JSSEProvider extends Provider {
 
         put("KeyManagerFactory.X509", KeyManagerFactoryImpl.class.getName());
         put("TrustManagerFactory.X509", TrustManagerFactoryImpl.class.getName());
-        put("KeyStore.AndroidCAStore", RootKeyStoreSpi.class.getName());
+        put("KeyStore.AndroidCAStore", TrustedCertificateKeyStoreSpi.class.getName());
     }
 }
diff --git a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/SSLParametersImpl.java b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/SSLParametersImpl.java
index 7578550..a17492f 100644
--- a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/SSLParametersImpl.java
+++ b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/SSLParametersImpl.java
@@ -17,7 +17,6 @@
 
 package org.apache.harmony.xnet.provider.jsse;
 
-import org.apache.harmony.xnet.provider.jsse.IndexedPKIXParameters;
 import org.apache.harmony.xnet.provider.jsse.TrustManagerImpl;
 import java.security.KeyManagementException;
 import java.security.KeyStore;
diff --git a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/TrustManagerImpl.java b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/TrustManagerImpl.java
index 99a154d..653dca4 100644
--- a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/TrustManagerImpl.java
+++ b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/TrustManagerImpl.java
@@ -60,7 +60,11 @@ public final class TrustManagerImpl implements X509TrustManager {
 
     private final CertPathValidator validator;
 
-    private final IndexedPKIXParameters params;
+    /**
+     * An index of TrustAnchor instances that we've seen. Unlike the
+     * TrustedCertificateStore, this may contain intermediate CAs.
+     */
+    private final TrustedCertificateIndex trustedCertificateIndex;
 
     /**
      * This is lazily initialized in the AndroidCAStore case since it
@@ -68,7 +72,7 @@ public final class TrustManagerImpl implements X509TrustManager {
      * non-AndroidCAStore, we initialize this as part of the
      * constructor.
      */
-    private volatile X509Certificate[] acceptedIssuers;
+    private final X509Certificate[] acceptedIssuers;
 
     private final Exception err;
     private final CertificateFactory factory;
@@ -83,7 +87,7 @@ public final class TrustManagerImpl implements X509TrustManager {
         CertificateFactory factoryLocal = null;
         KeyStore rootKeyStoreLocal = null;
         TrustedCertificateStore trustedCertificateStoreLocal = null;
-        IndexedPKIXParameters paramsLocal = null;
+        TrustedCertificateIndex trustedCertificateIndexLocal = null;
         X509Certificate[] acceptedIssuersLocal = null;
         Exception errLocal = null;
         try {
@@ -95,15 +99,15 @@ public final class TrustManagerImpl implements X509TrustManager {
                 rootKeyStoreLocal = keyStore;
                 trustedCertificateStoreLocal = new TrustedCertificateStore();
                 acceptedIssuersLocal = null;
-                paramsLocal = new IndexedPKIXParameters();
+                trustedCertificateIndexLocal = new TrustedCertificateIndex();
             } else {
                 rootKeyStoreLocal = null;
                 trustedCertificateStoreLocal = null;
                 acceptedIssuersLocal = acceptedIssuers(keyStore);
-                paramsLocal = new IndexedPKIXParameters(trustAnchors(acceptedIssuersLocal));
+                trustedCertificateIndexLocal
+                        = new TrustedCertificateIndex(trustAnchors(acceptedIssuersLocal));
             }
 
-            paramsLocal.setRevocationEnabled(false);
         } catch (Exception e) {
             errLocal = e;
         }
@@ -111,31 +115,34 @@ public final class TrustManagerImpl implements X509TrustManager {
         this.trustedCertificateStore = trustedCertificateStoreLocal;
         this.validator = validatorLocal;
         this.factory = factoryLocal;
-        this.params = paramsLocal;
+        this.trustedCertificateIndex = trustedCertificateIndexLocal;
         this.acceptedIssuers = acceptedIssuersLocal;
         this.err = errLocal;
     }
 
-    private static X509Certificate[] acceptedIssuers(KeyStore ks)
-            throws KeyStoreException {
-        // Note that unlike the PKIXParameters code to create a Set of
-        // TrustAnchors from a KeyStore, this version takes from both
-        // TrustedCertificateEntry and PrivateKeyEntry, not just
-        // TrustedCertificateEntry, which is why TrustManagerImpl
-        // cannot just use an PKIXParameters(KeyStore)
-        // constructor.
+    private static X509Certificate[] acceptedIssuers(KeyStore ks) {
+        try {
+            // Note that unlike the PKIXParameters code to create a Set of
+            // TrustAnchors from a KeyStore, this version takes from both
+            // TrustedCertificateEntry and PrivateKeyEntry, not just
+            // TrustedCertificateEntry, which is why TrustManagerImpl
+            // cannot just use an PKIXParameters(KeyStore)
+            // constructor.
 
-        // TODO remove duplicates if same cert is found in both a
-        // PrivateKeyEntry and TrustedCertificateEntry
-        List<X509Certificate> trusted = new ArrayList<X509Certificate>();
-        for (Enumeration<String> en = ks.aliases(); en.hasMoreElements();) {
-            final String alias = en.nextElement();
-            final X509Certificate cert = (X509Certificate) ks.getCertificate(alias);
-            if (cert != null) {
-                trusted.add(cert);
+            // TODO remove duplicates if same cert is found in both a
+            // PrivateKeyEntry and TrustedCertificateEntry
+            List<X509Certificate> trusted = new ArrayList<X509Certificate>();
+            for (Enumeration<String> en = ks.aliases(); en.hasMoreElements();) {
+                final String alias = en.nextElement();
+                final X509Certificate cert = (X509Certificate) ks.getCertificate(alias);
+                if (cert != null) {
+                    trusted.add(cert);
+                }
             }
+            return trusted.toArray(new X509Certificate[trusted.size()]);
+        } catch (KeyStoreException e) {
+            return new X509Certificate[0];
         }
-        return trusted.toArray(new X509Certificate[trusted.size()]);
     }
 
     private static Set<TrustAnchor> trustAnchors(X509Certificate[] certs) {
@@ -165,43 +172,31 @@ public final class TrustManagerImpl implements X509TrustManager {
             throw new CertificateException(err);
         }
 
-        X509Certificate[] newChain = cleanupCertChain(chain);
+        Set<TrustAnchor> trustAnchors = new HashSet<TrustAnchor>();
+        X509Certificate[] newChain = cleanupCertChainAndFindTrustAnchors(chain, trustAnchors);
         if (newChain.length == 0) {
             // chain was entirely trusted, skip the validator
             return;
         }
 
         CertPath certPath = factory.generateCertPath(Arrays.asList(newChain));
-        if (!Arrays.equals(chain[0].getEncoded(),
-                           certPath.getCertificates().get(0).getEncoded())) {
-            // Sanity check failed (shouldn't ever happen, but we
-            // are using pretty remote code)
-            throw new CertificateException("Certificate chain error");
-        }
-
-        if (trustedCertificateStore != null) {
-            // check if we need to add a missing TrustAnchor value to
-            // the IndexedPKIXParameters from the KeyStore.
-            boolean found = optionallyAddTrustAnchorToIndex(newChain[newChain.length-1]);
-            // If we can't find the TrustAnchor, we throw
-            // CertPathValidatorException to avoid an
-            // ExtendedPKIXParameters constructor error in validate().
-            if (!found) {
-                throw new CertificateException(new CertPathValidatorException(
-                        "Trust anchor for certification path not found.", null, certPath, -1));
-            }
+        if (trustAnchors.isEmpty()) {
+            throw new CertificateException(new CertPathValidatorException(
+                    "Trust anchor for certification path not found.", null, certPath, -1));
         }
 
         try {
+            PKIXParameters params = new PKIXParameters(trustAnchors);
+            params.setRevocationEnabled(false);
             validator.validate(certPath, params);
             // Add intermediate CAs to the index to tolerate sites
             // that assume that the browser will have cached these.
             // The server certificate is skipped by skipping the
             // zeroth element of new chain and note that the root CA
-            // will have been removed in cleanupCertChain.
-            // http://b/3404902
+            // will have been removed in
+            // cleanupCertChainAndFindTrustAnchors.  http://b/3404902
             for (int i = 1; i < newChain.length; i++) {
-                index(newChain[i]);
+                trustedCertificateIndex.index(newChain[i]);
             }
         } catch (InvalidAlgorithmParameterException e) {
             throw new CertificateException(e);
@@ -220,7 +215,8 @@ public final class TrustManagerImpl implements X509TrustManager {
      * md2WithRSAEncryption. This also handles removing old certs
      * after bridge CA certs.
      */
-    private X509Certificate[] cleanupCertChain(X509Certificate[] chain) {
+    private X509Certificate[] cleanupCertChainAndFindTrustAnchors(X509Certificate[] chain,
+                                                                  Set<TrustAnchor> trustAnchors) {
         X509Certificate[] original = chain;
 
         // 1. Clean the received certificates chain.
@@ -230,7 +226,9 @@ public final class TrustManagerImpl implements X509TrustManager {
         for (currIndex = 0; currIndex < chain.length; currIndex++) {
             // If the current cert is a TrustAnchor, we can ignore the rest of the chain.
             // This avoids including "bridge" CA certs that added for legacy compatability.
-            if (isTrustAnchor(chain[currIndex])) {
+            TrustAnchor trustAnchor = findTrustAnchorBySubjectAndPublicKey(chain[currIndex]);
+            if (trustAnchor != null) {
+                trustAnchors.add(trustAnchor);
                 currIndex--;
                 break;
             }
@@ -267,51 +265,33 @@ public final class TrustManagerImpl implements X509TrustManager {
 
         // 2. If the chain is now shorter, copy to an appropriately sized array.
         int chainLength = currIndex + 1;
-        if (chainLength == chain.length) {
-            return chain;
-        }
-        return Arrays.copyOf(chain, chainLength);
-    }
+        X509Certificate[] newChain = ((chainLength == chain.length)
+                                      ? chain
+                                      : Arrays.copyOf(chain, chainLength));
 
-    private boolean optionallyAddTrustAnchorToIndex(X509Certificate lastCert) {
-        TrustAnchor trustAnchor;
-        try {
-            // returns null if no match based on issuer
-            trustAnchor = params.findTrustAnchor(lastCert);
-        } catch (CertPathValidatorException e) {
-            // set to null if there seemed to be a match but
-            // failed verification, we might have another CA to
-            // discover with the same subject as one already known
-            // to the IndexedPKIXParameters.
-            trustAnchor = null;
-        }
-        if (trustAnchor != null) {
-            return true;
-        }
-        // we have a KeyStore and the issuer of the last cert in
-        // the chain seems to be missing from the
-        // IndexedPKIXParameters, check the KeyStore for a hit
-        X509Certificate issuer = trustedCertificateStore.findIssuer(lastCert);
-        if (issuer != null) {
-            index(issuer);
-            return true;
+        // 3. If no TrustAnchor was found in cleanup, look for one now
+        if (trustAnchors.isEmpty()) {
+            TrustAnchor trustAnchor = findTrustAnchorByIssuerAndSignature(newChain[chainLength-1]);
+            if (trustAnchor != null) {
+                trustAnchors.add(trustAnchor);
+            }
         }
-        return false;
+        return newChain;
     }
 
     /**
-     * Check the IndexedPKIXParameters for the cert to see if it is
+     * Check the trustedCertificateIndex for the cert to see if it is
      * already trusted and failing that check the KeyStore if it is
      * available.
      */
-    private boolean isTrustAnchor(X509Certificate cert) {
-        boolean isTrustAnchor = params.isTrustAnchor(cert);
-        if (isTrustAnchor) {
-            return true;
+    private TrustAnchor findTrustAnchorBySubjectAndPublicKey(X509Certificate cert) {
+        TrustAnchor trustAnchor = trustedCertificateIndex.findBySubjectAndPublicKey(cert);
+        if (trustAnchor != null) {
+            return trustAnchor;
         }
         if (trustedCertificateStore == null) {
             // not trusted and no TrustedCertificateStore to check
-            return false;
+            return null;
         }
         // probe KeyStore for a cert. AndroidCAStore stores its
         // contents hashed by cert subject on the filesystem to make
@@ -319,29 +299,30 @@ public final class TrustManagerImpl implements X509TrustManager {
         if (trustedCertificateStore.isTrustAnchor(cert)) {
             // add new TrustAnchor to params index to avoid
             // checking filesystem next time around.
-            index(cert);
-            return true;
+            return trustedCertificateIndex.index(cert);
         }
-        return false;
+        return null;
     }
 
-    /**
-     * Add a new TrustAnchor to the IndexedPKIXParameters
-     */
-    private void index(X509Certificate cert) {
-        params.index(new TrustAnchor(cert, null));
+    private TrustAnchor findTrustAnchorByIssuerAndSignature(X509Certificate lastCert) {
+        TrustAnchor trustAnchor = trustedCertificateIndex.findByIssuerAndSignature(lastCert);
+        if (trustAnchor != null) {
+            return trustAnchor;
+        }
+        if (trustedCertificateStore == null) {
+            return null;
+        }
+        // we have a KeyStore and the issuer of the last cert in
+        // the chain seems to be missing from the
+        // TrustedCertificateIndex, check the KeyStore for a hit
+        X509Certificate issuer = trustedCertificateStore.findIssuer(lastCert);
+        if (issuer != null) {
+            return trustedCertificateIndex.index(issuer);
+        }
+        return null;
     }
 
     @Override public X509Certificate[] getAcceptedIssuers() {
-        X509Certificate[] result = acceptedIssuers;
-        if (result == null) {
-            // single-check idiom
-            try {
-                acceptedIssuers = result = acceptedIssuers(rootKeyStore);
-            } catch (KeyStoreException e) {
-                acceptedIssuers = result = new X509Certificate[0];
-            }
-        }
-        return acceptedIssuers.clone();
+        return (acceptedIssuers != null) ? acceptedIssuers.clone() : acceptedIssuers(rootKeyStore);
     }
 }
diff --git a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/IndexedPKIXParameters.java b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/TrustedCertificateIndex.java
index e8acf18..9138b19 100644
--- a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/IndexedPKIXParameters.java
+++ b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/TrustedCertificateIndex.java
@@ -16,56 +16,43 @@
 
 package org.apache.harmony.xnet.provider.jsse;
 
-import java.security.InvalidAlgorithmParameterException;
 import java.security.PublicKey;
 import java.security.cert.CertPathValidatorException;
-import java.security.cert.PKIXParameters;
 import java.security.cert.TrustAnchor;
 import java.security.cert.X509Certificate;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import javax.security.auth.x500.X500Principal;
 
 /**
- * Indexes trust anchors so they can be found in O(1) time instead of O(N).
+ * Indexes {@code TrustAnchor} instances so they can be found in O(1)
+ * time instead of O(N).
  */
-public final class IndexedPKIXParameters extends PKIXParameters {
+public final class TrustedCertificateIndex {
 
     private final Map<X500Principal, List<TrustAnchor>> subjectToTrustAnchors
             = new HashMap<X500Principal, List<TrustAnchor>>();
 
-    public IndexedPKIXParameters() {}
+    public TrustedCertificateIndex() {}
 
-    public IndexedPKIXParameters(Set<TrustAnchor> anchors)
-            throws InvalidAlgorithmParameterException {
-        super(anchors);
-        index();
+    public TrustedCertificateIndex(Set<TrustAnchor> anchors) {
+        index(anchors);
     }
 
-    @Override public Set<TrustAnchor> getTrustAnchors() {
-        Set<TrustAnchor> result = new HashSet<TrustAnchor>();
-        synchronized (subjectToTrustAnchors) {
-            for (List<TrustAnchor> trustAnchors : subjectToTrustAnchors.values()) {
-                result.addAll(trustAnchors);
-            }
+    private void index(Set<TrustAnchor> anchors) {
+        for (TrustAnchor anchor : anchors) {
+            index(anchor);
         }
-        return Collections.unmodifiableSet(result);
     }
 
-    @Override public void setTrustAnchors(Set<TrustAnchor> trustAnchors) {
-        throw new UnsupportedOperationException();
-    }
-
-    private void index() {
-        for (TrustAnchor anchor : super.getTrustAnchors()) {
-            index(anchor);
-        }
+    public TrustAnchor index(X509Certificate cert) {
+        TrustAnchor anchor = new TrustAnchor(cert, null);
+        index(anchor);
+        return anchor;
     }
 
     public void index(TrustAnchor anchor) {
@@ -87,10 +74,8 @@ public final class IndexedPKIXParameters extends PKIXParameters {
         }
     }
 
-    public TrustAnchor findTrustAnchor(X509Certificate cert)
-            throws CertPathValidatorException {
+    public TrustAnchor findByIssuerAndSignature(X509Certificate cert) {
         X500Principal issuer = cert.getIssuerX500Principal();
-        Exception verificationException = null;
         synchronized (subjectToTrustAnchors) {
             List<TrustAnchor> anchors = subjectToTrustAnchors.get(issuer);
             if (anchors == null) {
@@ -108,34 +93,26 @@ public final class IndexedPKIXParameters extends PKIXParameters {
                     }
                     cert.verify(publicKey);
                     return anchor;
-                } catch (Exception e) {
-                    verificationException = e;
+                } catch (Exception ignored) {
                 }
             }
         }
-
-        // Throw last verification exception.
-        if (verificationException != null) {
-            throw new CertPathValidatorException("TrustAnchor found but"
-                    + " certificate verification failed.",
-                    verificationException);
-        }
-
         return null;
     }
 
-    public boolean isTrustAnchor(X509Certificate cert) {
+    public TrustAnchor findBySubjectAndPublicKey(X509Certificate cert) {
         X500Principal subject = cert.getSubjectX500Principal();
         synchronized (subjectToTrustAnchors) {
             List<TrustAnchor> anchors = subjectToTrustAnchors.get(subject);
             if (anchors == null) {
-                return false;
+                return null;
             }
-            return isTrustAnchor(cert, anchors);
+            return findBySubjectAndPublicKey(cert, anchors);
         }
     }
 
-    private static boolean isTrustAnchor(X509Certificate cert, Collection<TrustAnchor> anchors) {
+    private static TrustAnchor findBySubjectAndPublicKey(X509Certificate cert,
+                                                         Collection<TrustAnchor> anchors) {
         PublicKey certPublicKey = cert.getPublicKey();
         for (TrustAnchor anchor : anchors) {
             PublicKey caPublicKey;
@@ -147,12 +124,12 @@ public final class IndexedPKIXParameters extends PKIXParameters {
                     caPublicKey = anchor.getCAPublicKey();
                 }
                 if (caPublicKey.equals(certPublicKey)) {
-                    return true;
+                    return anchor;
                 }
             } catch (Exception e) {
                 // can happen with unsupported public key types
             }
         }
-        return false;
+        return null;
     }
 }
diff --git a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/RootKeyStoreSpi.java b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/TrustedCertificateKeyStoreSpi.java
index 9ae4139..62a6647 100644
--- a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/RootKeyStoreSpi.java
+++ b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/TrustedCertificateKeyStoreSpi.java
@@ -28,7 +28,7 @@ import java.util.Enumeration;
 /**
  * A KeyStoreSpi wrapper for the TrustedCertificateStore.
  */
-public final class RootKeyStoreSpi extends KeyStoreSpi {
+public final class TrustedCertificateKeyStoreSpi extends KeyStoreSpi {
 
     private final TrustedCertificateStore store = new TrustedCertificateStore();
 
diff --git a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/TrustedCertificateStore.java b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/TrustedCertificateStore.java
index f440eec..aed2e42 100644
--- a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/TrustedCertificateStore.java
+++ b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/TrustedCertificateStore.java
@@ -41,9 +41,10 @@ import libcore.io.IoUtils;
  * A source for trusted root certificate authority (CA) certificates
  * supporting an immutable system CA directory along with mutable
  * directories allowing the user addition of custom CAs and user
- * removal of system CAs. This store supports the RootKeyStoreSpi
- * wrapper to allow a traditional KeyStore interface for use with
- * {@link javax.net.ssl.TrustManagerFactory.init}.
+ * removal of system CAs. This store supports the {@code
+ * TrustedCertificateKeyStoreSpi} wrapper to allow a traditional
+ * KeyStore interface for use with {@link
+ * javax.net.ssl.TrustManagerFactory.init}.
  *
  * <p>The CAs are accessed via {@code KeyStore} style aliases. Aliases
  * are made up of a prefix identifying the source ("system:" vs
@@ -55,11 +56,12 @@ import libcore.io.IoUtils;
  * getCertificateAlias} can be implemented efficiently without
  * scanning the entire store.
  *
- * <p>In addition to supporting the {@code RootKeyStoreSpi}
- * implementation, {@code TrustedCertificateStore} also provides the
- * additional public methods {@link #isTrustAnchor} and {@link
- * #findIssuer} to allow efficient lookup operations for CAs again
- * based on the file naming convention.
+ * <p>In addition to supporting the {@code
+ * TrustedCertificateKeyStoreSpi} implementation, {@code
+ * TrustedCertificateStore} also provides the additional public
+ * methods {@link #isTrustAnchor} and {@link #findIssuer} to allow
+ * efficient lookup operations for CAs again based on the file naming
+ * convention.
  *
  * <p>The KeyChainService users the {@link installCertificate} and
  * {@link #deleteCertificateEntry} to install user CAs as well as
@@ -433,9 +435,10 @@ public final class TrustedCertificateStore {
 
     /**
      * This could be considered the implementation of {@code
-     * RootKeyStoreSpi.engineDeleteEntry} but we consider
-     * RootKeyStoreSpi to be read only. Instead, this is used by the
-     * {@code KeyChainService} to delete CA certificates.
+     * TrustedCertificateKeyStoreSpi.engineDeleteEntry} but we
+     * consider {@code TrustedCertificateKeyStoreSpi} to be read
+     * only. Instead, this is used by the {@code KeyChainService} to
+     * delete CA certificates.
      */
     public void deleteCertificateEntry(String alias) throws IOException, CertificateException {
         if (alias == null) {
diff --git a/luni/src/test/java/org/apache/harmony/xnet/provider/jsse/TrustManagerImplTest.java b/luni/src/test/java/org/apache/harmony/xnet/provider/jsse/TrustManagerImplTest.java
new file mode 100644
index 0000000..26ebc85
--- /dev/null
+++ b/luni/src/test/java/org/apache/harmony/xnet/provider/jsse/TrustManagerImplTest.java
@@ -0,0 +1,92 @@
+/*
+ * Copyright (C) 2011 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.harmony.xnet.provider.jsse;
+
+import java.security.KeyStore;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import javax.net.ssl.X509TrustManager;
+import junit.framework.TestCase;
+import libcore.java.security.TestKeyStore;
+
+public class TrustManagerImplTest extends TestCase {
+
+    /**
+     * Ensure that our non-standard behavior of learning to trust new
+     * intermediate CAs does not regress. http://b/3404902
+     */
+    public void testLearnIntermediate() throws Exception {
+
+        // chain3 should be server/intermediate/root
+        KeyStore.PrivateKeyEntry pke = TestKeyStore.getServer().getPrivateKey("RSA", "RSA");
+        X509Certificate[] chain3 = (X509Certificate[])pke.getCertificateChain();
+        X509Certificate root = chain3[2];
+        X509Certificate intermediate = chain3[1];
+        X509Certificate server = chain3[0];
+        X509Certificate[] chain2 =  new X509Certificate[] { server, intermediate };
+        X509Certificate[] chain1 =  new X509Certificate[] { server };
+
+        // Normal behavior
+        assertValid(chain3,   trustManager(root));
+        assertValid(chain2,   trustManager(root));
+        assertInvalid(chain1, trustManager(root));
+        assertValid(chain3,   trustManager(intermediate));
+        assertValid(chain2,   trustManager(intermediate));
+        assertValid(chain1,   trustManager(intermediate));
+        assertValid(chain3,   trustManager(server));
+        assertValid(chain2,   trustManager(server));
+        assertValid(chain1,   trustManager(server));
+
+        // non-standard behavior
+        X509TrustManager tm = trustManager(root);
+        // fail on short chain with only root trusted
+        assertInvalid(chain1, tm);
+        // succeed on longer chain, learn intermediate
+        assertValid(chain2, tm);
+        // now we can validate the short chain
+        assertValid(chain1, tm);
+    }
+
+    private X509TrustManager trustManager(X509Certificate ca) throws Exception {
+        KeyStore keyStore = TestKeyStore.createKeyStore();
+        keyStore.setCertificateEntry("alias", ca);
+
+        String algorithm = TrustManagerFactory.getDefaultAlgorithm();
+        TrustManagerFactory tmf = TrustManagerFactory.getInstance(algorithm);
+        tmf.init(keyStore);
+        return (X509TrustManager) tmf.getTrustManagers()[0];
+    }
+
+    private void assertValid(X509Certificate[] chain, X509TrustManager tm) throws Exception {
+        tm.checkClientTrusted(chain, "RSA");
+        tm.checkServerTrusted(chain, "RSA");
+    }
+    private void assertInvalid(X509Certificate[] chain, X509TrustManager tm) {
+        try {
+            tm.checkClientTrusted(chain, "RSA");
+            fail();
+        } catch (CertificateException expected) {
+        }
+        try {
+            tm.checkServerTrusted(chain, "RSA");
+            fail();
+        } catch (CertificateException expected) {
+        }
+    }
+}