Return specific cert verification errors on Android

To implement CertVerifyProc on Android we refer to the Java 
side to query the platform trust managers. Currently the 
information we get from the platform is binary - each 
certificate chain is either identified as trusted or not, in 
which case we assume that this is due to not-trusted root.

This patch provides better granularity distinguishing the 
following cases: expired, not yet valid, incorrect (could not 
be parsed), not trusted root.

This allowed to reenable two net unittests:
- CertVerifyProcTest.ExtraneousMD5RootCert
- CertVerifyProcTest.IntermediateCARequireExplicitPolicy

The following net unittest had to be disabled as it joins 
the club of CertVerifyProc tests failing on bots with 
incorrect time/date settings:
- CertVerifyProcTest.InvalidKeyUsage

BUG=169762

Review URL: https://chromiumcodereview.appspot.com/12212135

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@182280 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 159 insertions, 58 deletions

diff --git a/net/android/cert_verify_result_android.h b/net/android/cert_verify_result_android.h
new file mode 100644
index 0000000..ac18c21
--- /dev/null
+++ b/net/android/cert_verify_result_android.h
@@ -0,0 +1,26 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef NET_ANDROID_CERT_VERIFY_RESULT_ANDROID_H_
+#define NET_ANDROID_CERT_VERIFY_RESULT_ANDROID_H_
+
+#include "base/basictypes.h"
+#include "base/platform_file.h"
+#include "net/base/net_export.h"
+
+namespace net {
+
+namespace android {
+
+enum CertVerifyResultAndroid {
+#define CERT_VERIFY_RESULT_ANDROID(label, value) VERIFY_ ## label = value,
+#include "net/android/cert_verify_result_android_list.h"
+#undef CERT_VERIFY_RESULT_ANDROID
+};
+
+}  // namespace android
+
+}  // namespace net
+
+#endif  // NET_ANDROID_CERT_VERIFY_RESULT_ANDROID_H_
diff --git a/net/android/cert_verify_result_android_list.h b/net/android/cert_verify_result_android_list.h
new file mode 100644
index 0000000..201ea5a
--- /dev/null
+++ b/net/android/cert_verify_result_android_list.h
@@ -0,0 +1,27 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// This file intentionally does not have header guards, it's included
+// inside a macro to generate enum.
+
+// This file contains the list of certificate verification results returned
+// from Java side to the C++ side.
+
+// Certificate is trusted.
+CERT_VERIFY_RESULT_ANDROID(OK, 0)
+
+// Certificate verification could not be conducted.
+CERT_VERIFY_RESULT_ANDROID(FAILED, -1)
+
+// Certificate is not trusted due to non-trusted root of the certificate chain.
+CERT_VERIFY_RESULT_ANDROID(NO_TRUSTED_ROOT, -2)
+
+// Certificate is not trusted because it has expired.
+CERT_VERIFY_RESULT_ANDROID(EXPIRED, -3)
+
+// Certificate is not trusted because it is not valid yet.
+CERT_VERIFY_RESULT_ANDROID(NOT_YET_VALID, -4)
+
+// Certificate is not trusted because it could not be parsed.
+CERT_VERIFY_RESULT_ANDROID(UNABLE_TO_PARSE, -5)
diff --git a/net/android/java/CertVerifyResultAndroid.template b/net/android/java/CertVerifyResultAndroid.template
new file mode 100644
index 0000000..b19e937
--- /dev/null
+++ b/net/android/java/CertVerifyResultAndroid.template
@@ -0,0 +1,10 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.net;
+
+public class CertVerifyResultAndroid {
+#define CERT_VERIFY_RESULT_ANDROID(name, value) public static final int VERIFY_##name = value;
+#include "net/android/cert_verify_result_android_list.h"
+}
diff --git a/net/android/java/src/org/chromium/net/AndroidNetworkLibrary.java b/net/android/java/src/org/chromium/net/AndroidNetworkLibrary.java
index 69a5722..a075685 100644
--- a/net/android/java/src/org/chromium/net/AndroidNetworkLibrary.java
+++ b/net/android/java/src/org/chromium/net/AndroidNetworkLibrary.java
@@ -10,9 +10,10 @@ import android.content.Intent;
 import android.security.KeyChain;
 import android.util.Log;
 
-import org.chromium.net.CertificateMimeType;
 import org.chromium.base.CalledByNative;
 import org.chromium.base.CalledByNativeUnchecked;
+import org.chromium.net.CertVerifyResultAndroid;
+import org.chromium.net.CertificateMimeType;
 
 import java.net.Inet6Address;
 import java.net.InetAddress;
@@ -66,7 +67,7 @@ class AndroidNetworkLibrary {
       * PKCS#12 keychain) through the system's CertInstaller activity.
       *
       * @param context: current application context.
-      * @param file_type: cryptographic file type. E.g. CertificateMimeType.X509_USER_CERT
+      * @param cert_type: cryptographic file type. E.g. CertificateMimeType.X509_USER_CERT
       * @param data: certificate/keychain data bytes.
       * @return true on success, false on failure.
       *
@@ -196,15 +197,17 @@ class AndroidNetworkLibrary {
      *
      * @param certChain The ASN.1 DER encoded bytes for certificates.
      * @param authType The key exchange algorithm name (e.g. RSA)
-     * @return true if the server is trusted
-     * @throws CertificateException,KeyStoreException,NoSuchAlgorithmException
-     *             on error initializing the TrustManager or reading the
-     *             certChain
+     * @return Android certificate verification result code.
      */
-    @CalledByNativeUnchecked
-    public static boolean verifyServerCertificates(byte[][] certChain, String authType)
-            throws CertificateException, KeyStoreException, NoSuchAlgorithmException {
-        return X509Util.verifyServerCertificates(certChain, authType);
+    @CalledByNative
+    public static int verifyServerCertificates(byte[][] certChain, String authType) {
+        try {
+            return X509Util.verifyServerCertificates(certChain, authType);
+        } catch (KeyStoreException e) {
+            return CertVerifyResultAndroid.VERIFY_FAILED;
+        } catch (NoSuchAlgorithmException e) {
+            return CertVerifyResultAndroid.VERIFY_FAILED;
+        }
     }
 
     /**
@@ -226,4 +229,4 @@ class AndroidNetworkLibrary {
             CertificateException, KeyStoreException {
         X509Util.clearTestRootCertificates();
     }
-}
\ No newline at end of file
+}
diff --git a/net/android/java/src/org/chromium/net/X509Util.java b/net/android/java/src/org/chromium/net/X509Util.java
index cbd4f4cf..6bbf171 100644
--- a/net/android/java/src/org/chromium/net/X509Util.java
+++ b/net/android/java/src/org/chromium/net/X509Util.java
@@ -6,12 +6,16 @@ package org.chromium.net;
 
 import android.util.Log;
 
+import org.chromium.net.CertVerifyResultAndroid;
+
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.security.KeyStore;
 import java.security.KeyStoreException;
 import java.security.NoSuchAlgorithmException;
 import java.security.cert.CertificateException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
 
@@ -107,7 +111,7 @@ public class X509Util {
             KeyStoreException, NoSuchAlgorithmException {
         ensureInitialized();
         X509Certificate rootCert = createCertificateFromBytes(rootCertBytes);
-        synchronized(sLock) {
+        synchronized (sLock) {
             sTestKeyStore.setCertificateEntry(
                     "root_cert_" + Integer.toString(sTestKeyStore.size()), rootCert);
             reloadTestTrustManager();
@@ -117,45 +121,64 @@ public class X509Util {
     public static void clearTestRootCertificates() throws NoSuchAlgorithmException,
             CertificateException, KeyStoreException {
         ensureInitialized();
-        synchronized(sLock) {
+        synchronized (sLock) {
             try {
                 sTestKeyStore.load(null);
                 reloadTestTrustManager();
-            } catch(IOException e) {}  // No IO operation is attempted.
+            } catch (IOException e) {}  // No IO operation is attempted.
         }
     }
 
-    public static boolean verifyServerCertificates(byte[][] certChain, String authType)
-            throws CertificateException, KeyStoreException, NoSuchAlgorithmException {
+    public static int verifyServerCertificates(byte[][] certChain, String authType)
+            throws KeyStoreException, NoSuchAlgorithmException {
         if (certChain == null || certChain.length == 0 || certChain[0] == null) {
             throw new IllegalArgumentException("Expected non-null and non-empty certificate " +
                     "chain passed as |certChain|. |certChain|=" + certChain);
         }
 
-        ensureInitialized();
+        try {
+            ensureInitialized();
+        } catch (CertificateException e) {
+            return CertVerifyResultAndroid.VERIFY_FAILED;
+        }
+
         X509Certificate[] serverCertificates = new X509Certificate[certChain.length];
-        for (int i = 0; i < certChain.length; ++i) {
-            serverCertificates[i] = createCertificateFromBytes(certChain[i]);
+        try {
+            for (int i = 0; i < certChain.length; ++i) {
+                serverCertificates[i] = createCertificateFromBytes(certChain[i]);
+            }
+        } catch (CertificateException e) {
+            return CertVerifyResultAndroid.VERIFY_UNABLE_TO_PARSE;
+        }
+
+        // Expired and not yet valid certificates would be rejected by the trust managers, but the
+        // trust managers report all certificate errors using the general CertificateException. In
+        // order to get more granular error information, cert validity time range is being checked
+        // separately.
+        try {
+            serverCertificates[0].checkValidity();
+        } catch (CertificateExpiredException e) {
+            return CertVerifyResultAndroid.VERIFY_EXPIRED;
+        } catch (CertificateNotYetValidException e) {
+            return CertVerifyResultAndroid.VERIFY_NOT_YET_VALID;
         }
 
         synchronized (sLock) {
             try {
                 sDefaultTrustManager.checkServerTrusted(serverCertificates, authType);
-                return true;
+                return CertVerifyResultAndroid.VERIFY_OK;
             } catch (CertificateException eDefaultManager) {
                 try {
                     sTestTrustManager.checkServerTrusted(serverCertificates, authType);
-                    return true;
+                    return CertVerifyResultAndroid.VERIFY_OK;
                 } catch (CertificateException eTestManager) {
-                    /*
-                     *  Neither of the trust managers confirms the validity of the certificate
-                     *  chain, we emit the error message returned by the system trust manager.
-                     */
-                    Log.i(TAG, "failed to validate the certificate chain, error: " +
-                               eDefaultManager.getMessage());
+                    // Neither of the trust managers confirms the validity of the certificate chain,
+                    // log the error message returned by the system trust manager.
+                    Log.i(TAG, "Failed to validate the certificate chain, error: " +
+                              eDefaultManager.getMessage());
+                    return CertVerifyResultAndroid.VERIFY_NO_TRUSTED_ROOT;
                 }
             }
         }
-        return false;
     }
-}
\ No newline at end of file
+}
diff --git a/net/android/network_library.cc b/net/android/network_library.cc
index c1c4d5d..2407100 100644
--- a/net/android/network_library.cc
+++ b/net/android/network_library.cc
@@ -23,8 +23,9 @@ using base::android::ToJavaByteArray;
 namespace net {
 namespace android {
 
-VerifyResult VerifyX509CertChain(const std::vector<std::string>& cert_chain,
-                                 const std::string& auth_type) {
+CertVerifyResultAndroid VerifyX509CertChain(
+    const std::vector<std::string>& cert_chain,
+    const std::string& auth_type) {
   JNIEnv* env = AttachCurrentThread();
 
   ScopedJavaLocalRef<jobjectArray> chain_byte_array =
@@ -35,12 +36,10 @@ VerifyResult VerifyX509CertChain(const std::vector<std::string>& cert_chain,
       ConvertUTF8ToJavaString(env, auth_type);
   DCHECK(!auth_string.is_null());
 
-  jboolean trusted = Java_AndroidNetworkLibrary_verifyServerCertificates(
+  jint result = Java_AndroidNetworkLibrary_verifyServerCertificates(
       env, chain_byte_array.obj(), auth_string.obj());
-  if (ClearException(env))
-    return VERIFY_INVOCATION_ERROR;
 
-  return trusted ? VERIFY_OK : VERIFY_NO_TRUSTED_ROOT;
+  return static_cast<CertVerifyResultAndroid>(result);
 }
 
 void AddTestRootCertificate(const uint8* cert, size_t len) {
diff --git a/net/android/network_library.h b/net/android/network_library.h
index 9db3e0e..451834e 100644
--- a/net/android/network_library.h
+++ b/net/android/network_library.h
@@ -11,28 +11,19 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "net/android/cert_verify_result_android.h"
 #include "net/base/mime_util.h"
 #include "net/base/net_export.h"
 
 namespace net {
 namespace android {
 
-enum VerifyResult {
-  // Certificate verification was successful.
-  VERIFY_OK,
-  // Certificate verification was failed. There is no detail error information
-  // given by Android API.
-  VERIFY_NO_TRUSTED_ROOT,
-  // Error occurs when invoke JNI methods.
-  VERIFY_INVOCATION_ERROR,
-};
-
 // |cert_chain| is DER encoded chain of certificates, with the server's own
 // certificate listed first.
 // |auth_type| is as per the Java X509Certificate.checkServerTrusted method.
-
-VerifyResult VerifyX509CertChain(const std::vector<std::string>& cert_chain,
-                                 const std::string& auth_type);
+CertVerifyResultAndroid VerifyX509CertChain(
+    const std::vector<std::string>& cert_chain,
+    const std::string& auth_type);
 
 // Adds a certificate as a root trust certificate to the trust manager.
 // |cert| is DER encoded certificate, |len| is its length in bytes.
diff --git a/net/base/cert_verify_proc_android.cc b/net/base/cert_verify_proc_android.cc
index a7b17e6..cbf9e29 100644
--- a/net/base/cert_verify_proc_android.cc
+++ b/net/base/cert_verify_proc_android.cc
@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "base/logging.h"
+#include "net/android/cert_verify_result_android.h"
 #include "net/android/network_library.h"
 #include "net/base/cert_status_flags.h"
 #include "net/base/cert_verify_result.h"
@@ -23,23 +24,29 @@ namespace {
 bool VerifyFromAndroidTrustManager(const std::vector<std::string>& cert_bytes,
                                    CertVerifyResult* verify_result) {
   // TODO(joth): Fetch the authentication type from SSL rather than hardcode.
-  bool verified = true;
-  android::VerifyResult result =
+  android::CertVerifyResultAndroid android_result =
       android::VerifyX509CertChain(cert_bytes, "RSA");
-  switch (result) {
+  switch (android_result) {
+    case android::VERIFY_FAILED:
+      return false;
     case android::VERIFY_OK:
       break;
     case android::VERIFY_NO_TRUSTED_ROOT:
       verify_result->cert_status |= CERT_STATUS_AUTHORITY_INVALID;
       break;
-    case android::VERIFY_INVOCATION_ERROR:
-      verified = false;
+    case android::VERIFY_EXPIRED:
+    case android::VERIFY_NOT_YET_VALID:
+      verify_result->cert_status |= CERT_STATUS_DATE_INVALID;
+      break;
+    case android::VERIFY_UNABLE_TO_PARSE:
+      verify_result->cert_status |= CERT_STATUS_INVALID;
       break;
     default:
+      NOTREACHED();
       verify_result->cert_status |= CERT_STATUS_INVALID;
       break;
   }
-  return verified;
+  return true;
 }
 
 bool GetChainDEREncodedBytes(X509Certificate* cert,
diff --git a/net/base/cert_verify_proc_unittest.cc b/net/base/cert_verify_proc_unittest.cc
index 9dae0b2..e440563 100644
--- a/net/base/cert_verify_proc_unittest.cc
+++ b/net/base/cert_verify_proc_unittest.cc
@@ -131,7 +131,7 @@ TEST_F(CertVerifyProcTest, PaypalNullCertParsing) {
   CertVerifyResult verify_result;
   int error = Verify(paypal_null_cert, "www.paypal.com", flags, NULL,
                      &verify_result);
-#if defined(USE_NSS) || defined(OS_IOS)
+#if defined(USE_NSS) || defined(OS_IOS) || defined(OS_ANDROID)
   EXPECT_EQ(ERR_CERT_COMMON_NAME_INVALID, error);
 #else
   // TOOD(bulach): investigate why macosx and win aren't returning
diff --git a/net/net.gyp b/net/net.gyp
index 9fad87b..6e15215 100644
--- a/net/net.gyp
+++ b/net/net.gyp
@@ -56,12 +56,14 @@
         'net_resources',
       ],
       'sources': [
+        'android/cert_verify_result_android.h',
+        'android/cert_verify_result_android_list.h',
+        'android/gurl_utils.cc',
+        'android/gurl_utils.h',
         'android/keystore.cc',
         'android/keystore.h',
         'android/keystore_openssl.cc',
         'android/keystore_openssl.h',
-        'android/gurl_utils.cc',
-        'android/gurl_utils.h',
         'android/net_jni_registrar.cc',
         'android/net_jni_registrar.h',
         'android/network_change_notifier_android.cc',
@@ -2383,8 +2385,9 @@
           },
           'dependencies': [
             '../base/base.gyp:base',
-            'net_errors_java',
+            'cert_verify_result_android_java',
             'certificate_mime_types_java',
+            'net_errors_java',
             'private_key_types_java',
           ],
           'includes': [ '../build/java.gypi' ],
@@ -2437,6 +2440,18 @@
           'includes': [ '../build/android/java_cpp_template.gypi' ],
         },
         {
+          'target_name': 'cert_verify_result_android_java',
+          'type': 'none',
+          'sources': [
+            'android/java/CertVerifyResultAndroid.template',
+          ],
+          'variables': {
+            'package_name': 'org.chromium.net',
+            'template_deps': ['android/cert_verify_result_android_list.h'],
+          },
+          'includes': [ '../build/android/java_cpp_template.gypi' ],
+        },
+        {
           'target_name': 'private_key_types_java',
           'type': 'none',
           'sources': [