Merge "javax.crypto.Cipher: try less specific Cipher/Mode/Padding combinations before throwing InvalidKeyException" into mnc-dev

3 files changed, 80 insertions, 24 deletions

diff --git a/luni/src/main/java/javax/crypto/Cipher.java b/luni/src/main/java/javax/crypto/Cipher.java
index 9d6bd22..db40117 100644
--- a/luni/src/main/java/javax/crypto/Cipher.java
+++ b/luni/src/main/java/javax/crypto/Cipher.java
@@ -298,14 +298,7 @@ public class Cipher {
 
         String[] transformParts = checkTransformation(transformation);
 
-        boolean providerSupportsAlgorithm;
-        try {
-            providerSupportsAlgorithm =
-                    tryCombinations(null /* key */, provider, transformParts) != null;
-        } catch (InvalidKeyException e) {
-            throw new IllegalStateException("InvalidKeyException thrown when key == null", e);
-        }
-        if (!providerSupportsAlgorithm) {
+        if (tryCombinations(null /* key */, provider, transformParts) == null) {
             if (provider == null) {
                 throw new NoSuchAlgorithmException("No provider found for " + transformation);
             } else {
@@ -349,6 +342,9 @@ public class Cipher {
     /**
      * Makes sure a CipherSpi that matches this type is selected.
      *
+     * If {@code key != null} then it assumes that a suitable provider exists for this instance
+     * (used by {@link #init}.
+     *
      * @throws InvalidKeyException if the specified key cannot be used to
      *             initialize this cipher.
      */
@@ -358,14 +354,26 @@ public class Cipher {
         }
 
         synchronized (initLock) {
+            // This is not only a matter of performance. Many methods like update, doFinal, etc.
+            // call {@code #getSpi()} (ie, {@code #getSpi(null /* key */)}) and without this
+            // shortcut they would override an spi that was chosen using the key.
             if (spiImpl != null && key == null) {
                 return spiImpl;
             }
 
             final Engine.SpiAndProvider sap = tryCombinations(
                     key, specifiedProvider, transformParts);
+
             if (sap == null) {
-                throw new ProviderException("No provider for " + transformation);
+                if (key == null) {
+                    throw new ProviderException("No provider for " + transformation);
+                }
+                // Since the key is not null, a suitable provider exists,
+                // and it is an InvalidKeyException.
+                throw new InvalidKeyException(
+                        "No provider offers " + transformation + " for " + key.getAlgorithm()
+                                + " key of class " + key.getClass().getName()
+                                + " and export format " + key.getFormat());
             }
 
             spiImpl = (CipherSpi) sap.spi;
@@ -411,12 +419,9 @@ public class Cipher {
      *   [cipher]//[padding]
      *   [cipher]
      * </pre>
-     *
-     * @throws InvalidKeyException if the specified key cannot be used to
-     *             initialize any provider.
      */
     private static Engine.SpiAndProvider tryCombinations(Key key, Provider provider,
-            String[] transformParts) throws InvalidKeyException {
+            String[] transformParts) {
         Engine.SpiAndProvider sap = null;
 
         if (transformParts[1] != null && transformParts[2] != null) {
@@ -446,12 +451,8 @@ public class Cipher {
         return tryTransform(key, provider, transformParts[0], transformParts, NeedToSet.BOTH);
     }
 
-    /**
-     * @throws InvalidKeyException if the specified key cannot be used to
-     *             initialize this cipher.
-     */
     private static Engine.SpiAndProvider tryTransform(Key key, Provider provider, String transform,
-            String[] transformParts, NeedToSet type) throws InvalidKeyException {
+            String[] transformParts, NeedToSet type) {
         if (provider != null) {
             Provider.Service service = provider.getService(SERVICE, transform);
             if (service == null) {
@@ -463,19 +464,14 @@ public class Cipher {
         if (services == null || services.isEmpty()) {
             return null;
         }
-        boolean keySupported = false;
         for (Provider.Service service : services) {
             if (key == null || service.supportsParameter(key)) {
-                keySupported = true;
                 Engine.SpiAndProvider sap = tryTransformWithProvider(transformParts, type, service);
                 if (sap != null) {
                     return sap;
                 }
             }
         }
-        if (!keySupported) {
-            throw new InvalidKeyException("No provider supports the provided key");
-        }
         return null;
     }
 
diff --git a/luni/src/test/java/libcore/javax/crypto/CipherTest.java b/luni/src/test/java/libcore/javax/crypto/CipherTest.java
index dcd1478..3c7ff0e 100644
--- a/luni/src/test/java/libcore/javax/crypto/CipherTest.java
+++ b/luni/src/test/java/libcore/javax/crypto/CipherTest.java
@@ -27,6 +27,7 @@ import java.security.InvalidAlgorithmParameterException;
 import java.security.InvalidKeyException;
 import java.security.Key;
 import java.security.KeyFactory;
+import java.security.KeyPairGenerator;
 import java.security.NoSuchAlgorithmException;
 import java.security.PrivateKey;
 import java.security.Provider;
@@ -3339,4 +3340,63 @@ public final class CipherTest extends TestCase {
             assertEquals("", new String(buffer, 0, bytesProduced, StandardCharsets.US_ASCII));
         }
     }
+
+    /**
+     * If a provider rejects a key for "Cipher/Mode/Padding"", there might be another that
+     * accepts the key for "Cipher". Don't throw InvalidKeyException when trying the first one.
+     * http://b/22208820
+     */
+    public void testCipher_init_tryAllCombinationsBeforeThrowingInvalidKey()
+            throws Exception {
+        Provider mockProvider = new MockProvider("MockProvider") {
+            public void setup() {
+                put("Cipher.FOO/FOO/FOO", MockCipherSpi.AllKeyTypes.class.getName());
+                put("Cipher.FOO/FOO/FOO SupportedKeyClasses", "none");
+            }
+        };
+
+        Provider mockProvider2 = new MockProvider("MockProvider2") {
+            public void setup() {
+                put("Cipher.FOO", MockCipherSpi.AllKeyTypes.class.getName());
+            }
+        };
+
+        Security.addProvider(mockProvider);
+
+        try {
+            try {
+                // The provider installed doesn't accept the key.
+                Cipher c = Cipher.getInstance("FOO/FOO/FOO");
+                c.init(Cipher.DECRYPT_MODE, new MockKey());
+                fail("Expected InvalidKeyException");
+            } catch (InvalidKeyException expected) {
+            }
+
+            Security.addProvider(mockProvider2);
+
+            try {
+                // The new provider accepts "FOO" with this key. Use it despite the other provider
+                // accepts "FOO/FOO/FOO" but doesn't accept the key.
+                Cipher c = Cipher.getInstance("FOO/FOO/FOO");
+                c.init(Cipher.DECRYPT_MODE, new MockKey());
+                assertEquals("MockProvider2", c.getProvider().getName());
+            } finally {
+                Security.removeProvider(mockProvider2.getName());
+            }
+        } finally {
+            Security.removeProvider(mockProvider.getName());
+        }
+    }
+
+    /**
+     * Check that RSA with OAEPPadding is supported.
+     * http://b/22208820
+     */
+    public void test_RSA_OAEPPadding() throws Exception {
+        KeyPairGenerator keyGen = KeyPairGenerator.getInstance("RSA");
+        keyGen.initialize(1024, SecureRandom.getInstance("SHA1PRNG"));
+        Cipher cipher = Cipher.getInstance("RSA/NONE/OAEPPadding");
+        cipher.init(Cipher.ENCRYPT_MODE, keyGen.generateKeyPair().getPublic());
+        cipher.doFinal(new byte[] {1,2,3,4});
+    }
 }
diff --git a/luni/src/test/java/libcore/javax/crypto/MockKey.java b/luni/src/test/java/libcore/javax/crypto/MockKey.java
index 248e2de..1c758f3 100644
--- a/luni/src/test/java/libcore/javax/crypto/MockKey.java
+++ b/luni/src/test/java/libcore/javax/crypto/MockKey.java
@@ -25,7 +25,7 @@ import java.security.Key;
 public class MockKey implements Key {
     @Override
     public String getAlgorithm() {
-        throw new UnsupportedOperationException("not implemented");
+        return "MOCK";
     }
 
     @Override