Cipher.init incorrectly implements RFC 3280 key usage validation

Issue: http://code.google.com/p/android/issues/detail?id=12955
Bug: 3381582
Change-Id: Ida63c1356634c8e287ce5b0234418a656dffedf0

7 files changed, 155 insertions, 15 deletions

diff --git a/luni/src/main/java/javax/crypto/Cipher.java b/luni/src/main/java/javax/crypto/Cipher.java
index 7d5f9ed..aaf6b35 100644
--- a/luni/src/main/java/javax/crypto/Cipher.java
+++ b/luni/src/main/java/javax/crypto/Cipher.java
@@ -790,30 +790,35 @@ public class Cipher {
             boolean critical = false;
             if (ce != null && !ce.isEmpty()) {
                 for (String oid : ce) {
-                    if (oid.equals("2.5.29.15")) { //KeyUsage OID = 2.5.29.15
+                    if (oid.equals("2.5.29.15")) { // KeyUsage OID = 2.5.29.15
                         critical = true;
                         break;
                     }
                 }
                 if (critical) {
-                    boolean[] keyUsage = ((X509Certificate) certificate)
-                            .getKeyUsage();
-                    // As specified in RFC 3280 -
-                    // Internet X.509 Public Key Infrastructure
-                    // Certificate and Certificate Revocation List (CRL) Profile.
-                    // (http://www.ietf.org/rfc/rfc3280.txt)
+                    boolean[] keyUsage = ((X509Certificate) certificate).getKeyUsage();
+                    // As specified in RFC 3280:
+                    //   Internet X.509 Public Key Infrastructure
+                    //   Certificate and Certificate Revocation List (CRL) Profile.
+                    // Section 4.2.1.3  Key Usage
+                    // http://www.ietf.org/rfc/rfc3280.txt
                     //
                     // KeyUsage ::= BIT STRING {digitalSignature (0),
-                    //                          ...
-                    //                          encipherOnly (7),
-                    //                          decipherOnly (8) }
+                    //                          nonRepudiation   (1),
+                    //                          keyEncipherment  (2),
+                    //                          dataEncipherment (3),
+                    //                          keyAgreement     (4),
+                    //                          keyCertSign      (5),
+                    //                          cRLSign          (6),
+                    //                          encipherOnly     (7),
+                    //                          decipherOnly     (8) }
                     if (keyUsage != null) {
-                        if (opmode == ENCRYPT_MODE && (!keyUsage[7])) {
+                        if (opmode == ENCRYPT_MODE && !keyUsage[3]) {
                             throw new InvalidKeyException("The public key in the certificate "
                                                           + "cannot be used for ENCRYPT_MODE");
-                        } else if (opmode == DECRYPT_MODE && (!keyUsage[8])) {
+                        } else if (opmode == WRAP_MODE && !keyUsage[2]) {
                             throw new InvalidKeyException("The public key in the certificate "
-                                                          + "cannot be used for DECRYPT_MODE");
+                                                          + "cannot be used for WRAP_MODE");
                         }
                     }
                 }
diff --git a/luni/src/test/java/libcore/javax/crypto/CipherTest.java b/luni/src/test/java/libcore/javax/crypto/CipherTest.java
new file mode 100644
index 0000000..a4dd0e6
--- /dev/null
+++ b/luni/src/test/java/libcore/javax/crypto/CipherTest.java
@@ -0,0 +1,115 @@
+/*
+ * 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 libcore.javax.crypto;
+
+import java.security.InvalidKeyException;
+import java.security.cert.Certificate;
+import javax.crypto.Cipher;
+import junit.framework.TestCase;
+import libcore.java.security.TestKeyStore;
+import org.bouncycastle.asn1.x509.KeyUsage;
+
+public final class CipherTest extends TestCase {
+
+    public void testCipherInitWithCertificate () throws Exception {
+        // no key usage specified, everything is fine
+        assertCipherInitWithKeyUsage(0,                         true,  true, true,  true);
+
+        // common case is that encrypt/wrap is prohibited when special usage is specified
+        assertCipherInitWithKeyUsage(KeyUsage.digitalSignature, false, true, false, true);
+        assertCipherInitWithKeyUsage(KeyUsage.nonRepudiation,   false, true, false, true);
+        assertCipherInitWithKeyUsage(KeyUsage.keyAgreement,     false, true, false, true);
+        assertCipherInitWithKeyUsage(KeyUsage.keyCertSign,      false, true, false, true);
+        assertCipherInitWithKeyUsage(KeyUsage.cRLSign,          false, true, false, true);
+
+        // Note they encipherOnly/decipherOnly don't have to do with
+        // ENCRYPT_MODE or DECRYPT_MODE, but restrict usage relative
+        // to keyAgreement. There is not a *_MODE option that
+        // corresponds to this in Cipher, the RI does not enforce
+        // anything in Cipher.
+        // http://code.google.com/p/android/issues/detail?id=12955
+        assertCipherInitWithKeyUsage(KeyUsage.encipherOnly,     false, true, false, true);
+        assertCipherInitWithKeyUsage(KeyUsage.decipherOnly,     false, true, false, true);
+        assertCipherInitWithKeyUsage(KeyUsage.keyAgreement | KeyUsage.encipherOnly,
+                                                                false, true, false, true);
+        assertCipherInitWithKeyUsage(KeyUsage.keyAgreement | KeyUsage.decipherOnly,
+                                                                false, true, false, true);
+
+        // except when wraping a key is specifically allowed or
+        assertCipherInitWithKeyUsage(KeyUsage.keyEncipherment,  false, true, true,  true);
+        // except when wraping data encryption is specifically allowed
+        assertCipherInitWithKeyUsage(KeyUsage.dataEncipherment, true,  true, false, true);
+    }
+
+    private void assertCipherInitWithKeyUsage (int keyUsage,
+                                               boolean allowEncrypt,
+                                               boolean allowDecrypt,
+                                               boolean allowWrap,
+                                               boolean allowUnwrap) throws Exception {
+        Certificate certificate = certificateWithKeyUsage(keyUsage);
+        assertCipherInitWithKeyUsage(certificate, allowEncrypt, Cipher.ENCRYPT_MODE);
+        assertCipherInitWithKeyUsage(certificate, allowDecrypt, Cipher.DECRYPT_MODE);
+        assertCipherInitWithKeyUsage(certificate, allowWrap,    Cipher.WRAP_MODE);
+        assertCipherInitWithKeyUsage(certificate, allowUnwrap,  Cipher.UNWRAP_MODE);
+    }
+
+    private void assertCipherInitWithKeyUsage(Certificate certificate,
+                                              boolean allowMode,
+                                              int mode) throws Exception {
+        Cipher cipher = Cipher.getInstance("RSA");
+        if (allowMode) {
+            cipher.init(mode, certificate);
+        } else {
+            try {
+                cipher.init(mode, certificate);
+                String modeString;
+                switch (mode) {
+                    case Cipher.ENCRYPT_MODE:
+                        modeString = "ENCRYPT_MODE";
+                        break;
+                    case Cipher.DECRYPT_MODE:
+                        modeString = "DECRYPT_MODE";
+                        break;
+                    case Cipher.WRAP_MODE:
+                        modeString = "WRAP_MODE";
+                        break;
+                    case Cipher.UNWRAP_MODE:
+                        modeString = "UNWRAP_MODE";
+                        break;
+                    default:
+                        throw new AssertionError("Unknown Cipher.*_MODE " + mode);
+                }
+                fail("Should have had InvalidKeyException for " + modeString
+                     + " for " + certificate);
+            } catch (InvalidKeyException expected) {
+            }
+        }
+    }
+
+    private Certificate certificateWithKeyUsage(int keyUsage) {
+        // note the rare usage of non-zero keyUsage
+        return TestKeyStore.create(new String[] { "RSA" },
+                                   null,
+                                   null,
+                                   "rsa-dsa-ec",
+                                   TestKeyStore.localhost(),
+                                   keyUsage,
+                                   true,
+                                   null,
+                                   null).getPrivateKey("RSA", "RSA").getCertificate();
+    }
+}
diff --git a/luni/src/test/java/libcore/javax/net/ssl/KeyManagerFactoryTest.java b/luni/src/test/java/libcore/javax/net/ssl/KeyManagerFactoryTest.java
index cd436ee..2ff9567 100644
--- a/luni/src/test/java/libcore/javax/net/ssl/KeyManagerFactoryTest.java
+++ b/luni/src/test/java/libcore/javax/net/ssl/KeyManagerFactoryTest.java
@@ -46,6 +46,7 @@ public class KeyManagerFactoryTest extends TestCase {
                                   null,
                                   "rsa-dsa-ec",
                                   TestKeyStore.localhost(),
+                                  0,
                                   true,
                                   null,
                                   null);
diff --git a/luni/src/test/java/libcore/javax/net/ssl/SSLEngineTest.java b/luni/src/test/java/libcore/javax/net/ssl/SSLEngineTest.java
index 4a16fe8..d7e7c67 100644
--- a/luni/src/test/java/libcore/javax/net/ssl/SSLEngineTest.java
+++ b/luni/src/test/java/libcore/javax/net/ssl/SSLEngineTest.java
@@ -61,7 +61,8 @@ public class SSLEngineTest extends TestCase {
         TestSSLContext c = TestSSLContext.create();
         SSLEngine e = c.clientContext.createSSLEngine();
         String[] cipherSuites = e.getSupportedCipherSuites();
-        StandardNames.assertSupportedCipherSuites(StandardNames.CIPHER_SUITES_SSLENGINE, cipherSuites);
+        StandardNames.assertSupportedCipherSuites(StandardNames.CIPHER_SUITES_SSLENGINE,
+                                                  cipherSuites);
         assertNotSame(cipherSuites, e.getSupportedCipherSuites());
         c.close();
     }
@@ -74,6 +75,7 @@ public class SSLEngineTest extends TestCase {
                                       null,
                                       "rsa-dsa-ec",
                                       TestKeyStore.localhost(),
+                                      0,
                                       true,
                                       null,
                                       null);
diff --git a/luni/src/test/java/libcore/javax/net/ssl/SSLSocketTest.java b/luni/src/test/java/libcore/javax/net/ssl/SSLSocketTest.java
index a307e5b..3d0861f 100644
--- a/luni/src/test/java/libcore/javax/net/ssl/SSLSocketTest.java
+++ b/luni/src/test/java/libcore/javax/net/ssl/SSLSocketTest.java
@@ -61,6 +61,7 @@ public class SSLSocketTest extends TestCase {
                                       null,
                                       "rsa-dsa-ec",
                                       TestKeyStore.localhost(),
+                                      0,
                                       true,
                                       null,
                                       null);
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 0dc2139..e92bf67 100644
--- a/luni/src/test/java/libcore/javax/net/ssl/TrustManagerFactoryTest.java
+++ b/luni/src/test/java/libcore/javax/net/ssl/TrustManagerFactoryTest.java
@@ -47,6 +47,7 @@ public class TrustManagerFactoryTest extends TestCase {
                                   null,
                                   "rsa-dsa-ec",
                                   TestKeyStore.localhost(),
+                                  0,
                                   true,
                                   null,
                                   null);
diff --git a/support/src/test/java/libcore/java/security/TestKeyStore.java b/support/src/test/java/libcore/java/security/TestKeyStore.java
index 23faee7..d50706b 100644
--- a/support/src/test/java/libcore/java/security/TestKeyStore.java
+++ b/support/src/test/java/libcore/java/security/TestKeyStore.java
@@ -30,7 +30,6 @@ import java.security.KeyStore.TrustedCertificateEntry;
 import java.security.KeyStore;
 import java.security.Principal;
 import java.security.PrivateKey;
-import java.security.Provider;
 import java.security.PublicKey;
 import java.security.SecureRandom;
 import java.security.Security;
@@ -51,6 +50,7 @@ import junit.framework.Assert;
 import libcore.javax.net.ssl.TestKeyManager;
 import libcore.javax.net.ssl.TestTrustManager;
 import org.bouncycastle.asn1.x509.BasicConstraints;
+import org.bouncycastle.asn1.x509.KeyUsage;
 import org.bouncycastle.asn1.x509.X509Extensions;
 import org.bouncycastle.asn1.x509.X509Name;
 import org.bouncycastle.jce.X509Principal;
@@ -125,6 +125,7 @@ public final class TestKeyStore extends Assert {
                      null,
                      "RootCA",
                      x509Principal("Test Root Certificate Authority"),
+                     0,
                      true,
                      null,
                      null);
@@ -134,6 +135,7 @@ public final class TestKeyStore extends Assert {
                      null,
                      "IntermediateCA",
                      x509Principal("Test Intermediate Certificate Authority"),
+                     0,
                      true,
                      ROOT_CA.getPrivateKey("RSA", "RSA"),
                      ROOT_CA.getRootCertificate("RSA"));
@@ -143,6 +145,7 @@ public final class TestKeyStore extends Assert {
                      null,
                      "server",
                      localhost(),
+                     0,
                      false,
                      INTERMEDIATE_CA.getPrivateKey("RSA", "RSA"),
                      INTERMEDIATE_CA.getRootCertificate("RSA"));
@@ -154,6 +157,7 @@ public final class TestKeyStore extends Assert {
                      null,
                      "client",
                      x509Principal("test@user"),
+                     0,
                      false,
                      INTERMEDIATE_CA.getPrivateKey("RSA", "RSA"),
                      INTERMEDIATE_CA.getRootCertificate("RSA"));
@@ -164,6 +168,7 @@ public final class TestKeyStore extends Assert {
                      null,
                      "RootCA2",
                      x509Principal("Test Root Certificate Authority 2"),
+                     0,
                      true,
                      null,
                      null);
@@ -210,6 +215,7 @@ public final class TestKeyStore extends Assert {
      * @param keyAlgorithms The requested key types to generate and include
      * @param keyStorePassword Password used to protect the private key
      * @param aliasPrefix A unique prefix to identify the key aliases
+     * @param keyUsage {@link KeyUsage} bit mask for 2.5.29.15 extension
      * @param ca true If the keys being created are for a CA
      * @param signer If non-null, a private key entry to be used for signing, otherwise self-sign
      * @param signer If non-null, a root CA to include in the final store
@@ -219,6 +225,7 @@ public final class TestKeyStore extends Assert {
                                       char[] keyPassword,
                                       String aliasPrefix,
                                       X509Principal subject,
+                                      int keyUsage,
                                       boolean ca,
                                       PrivateKeyEntry signer,
                                       Certificate rootCa) {
@@ -242,6 +249,7 @@ public final class TestKeyStore extends Assert {
                                keyAlgorithm,
                                publicAlias, privateAlias,
                                subject,
+                               keyUsage,
                                ca,
                                privateKey(keyStore, keyPassword, "RSA", "RSA"));
                     continue;
@@ -250,6 +258,7 @@ public final class TestKeyStore extends Assert {
                            keyAlgorithm,
                            publicAlias, privateAlias,
                            subject,
+                           keyUsage,
                            ca,
                            signer);
             }
@@ -332,6 +341,7 @@ public final class TestKeyStore extends Assert {
                                       String publicAlias,
                                       String privateAlias,
                                       X509Principal subject,
+                                      int keyUsage,
                                       boolean ca,
                                       PrivateKeyEntry signer) throws Exception {
         PrivateKey caKey;
@@ -409,6 +419,11 @@ public final class TestKeyStore extends Assert {
             x509cg.setPublicKey(publicKey);
             x509cg.setSignatureAlgorithm(signatureAlgorithm);
             x509cg.setSerialNumber(serial);
+            if (keyUsage != 0) {
+                x509cg.addExtension(X509Extensions.KeyUsage,
+                                    true,
+                                    new KeyUsage(keyUsage));
+            }
             if (ca) {
                 x509cg.addExtension(X509Extensions.BasicConstraints,
                                     true,