diff options
author | davidben@chromium.org <davidben@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-01-17 22:52:17 +0000 |
---|---|---|
committer | davidben@chromium.org <davidben@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-01-17 22:52:17 +0000 |
commit | 23073f97dbd9aceb329f9994ee312650efe797af (patch) | |
tree | 42d076e2277c730d208a6cfd8924a2ff7fc2e266 /net | |
parent | 56aca707d0ad5ace8c5513dae5e31f964ad8f8be (diff) | |
download | chromium_src-23073f97dbd9aceb329f9994ee312650efe797af.zip chromium_src-23073f97dbd9aceb329f9994ee312650efe797af.tar.gz chromium_src-23073f97dbd9aceb329f9994ee312650efe797af.tar.bz2 |
Export verified_cert and public_key_hashes on Android.
On API level 17 and up, X509TrustManager can export the verified chain. Use it
to populate some of the fields in CertVerifyResult. Also correctly populate
is_issued_by_known_root and enable intranet host checking.
Add a test to make sure non-standard roots get flagged as such. If the APIs
are not available, is_issued_by_known_root is always false.
BUG=116838,147945
TEST=CertVerifyProcTest.PublicKeyHashes
CertVerifyProcTest.VerifyReturnChainBasic
CertVerifyProcTest.VerifyReturnChainFiltersUnrelatedCerts
CertVerifyProcTest.VerifyReturnChainProperlyOrdered
CertVerifyProcTest.IntranetHostsRejected
CertVerifyProcTest.IsIssuedByKnownRootIgnoresTestRoots
CertVerifyProcTest.ExtraneousMD5RootCert
CertVerifyProcTest.NameConstraintsFailure
Review URL: https://codereview.chromium.org/108653013
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@245649 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r-- | net/android/cert_verify_result_android.cc | 40 | ||||
-rw-r--r-- | net/android/cert_verify_result_android.h | 22 | ||||
-rw-r--r-- | net/android/cert_verify_status_android_list.h (renamed from net/android/cert_verify_result_android_list.h) | 16 | ||||
-rw-r--r-- | net/android/java/CertVerifyResultAndroid.template | 10 | ||||
-rw-r--r-- | net/android/java/CertVerifyStatusAndroid.template | 10 | ||||
-rw-r--r-- | net/android/java/src/org/chromium/net/AndroidCertVerifyResult.java | 73 | ||||
-rw-r--r-- | net/android/java/src/org/chromium/net/AndroidNetworkLibrary.java | 16 | ||||
-rw-r--r-- | net/android/java/src/org/chromium/net/X509Util.java | 172 | ||||
-rw-r--r-- | net/android/net_jni_registrar.cc | 1 | ||||
-rw-r--r-- | net/android/network_library.cc | 25 | ||||
-rw-r--r-- | net/android/network_library.h | 9 | ||||
-rw-r--r-- | net/cert/cert_verify_proc.cc | 5 | ||||
-rw-r--r-- | net/cert/cert_verify_proc_android.cc | 51 | ||||
-rw-r--r-- | net/cert/cert_verify_proc_unittest.cc | 111 | ||||
-rw-r--r-- | net/net.gyp | 12 |
15 files changed, 474 insertions, 99 deletions
diff --git a/net/android/cert_verify_result_android.cc b/net/android/cert_verify_result_android.cc new file mode 100644 index 0000000..151c4ef --- /dev/null +++ b/net/android/cert_verify_result_android.cc @@ -0,0 +1,40 @@ +// Copyright 2014 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. + +#include "net/android/cert_verify_result_android.h" + +#include "base/android/jni_android.h" +#include "base/android/jni_array.h" +#include "jni/AndroidCertVerifyResult_jni.h" + +using base::android::AttachCurrentThread; +using base::android::JavaArrayOfByteArrayToStringVector; + +namespace net { +namespace android { + +void ExtractCertVerifyResult(jobject result, + CertVerifyStatusAndroid* status, + bool* is_issued_by_known_root, + std::vector<std::string>* verified_chain) { + JNIEnv* env = AttachCurrentThread(); + + *status = static_cast<CertVerifyStatusAndroid>( + Java_AndroidCertVerifyResult_getStatus(env, result)); + + *is_issued_by_known_root = + Java_AndroidCertVerifyResult_isIssuedByKnownRoot(env, result); + + ScopedJavaLocalRef<jobjectArray> chain_byte_array = + Java_AndroidCertVerifyResult_getCertificateChainEncoded(env, result); + JavaArrayOfByteArrayToStringVector( + env, chain_byte_array.obj(), verified_chain); +} + +bool RegisterCertVerifyResult(JNIEnv* env) { + return RegisterNativesImpl(env); +} + +} // namespace android +} // namespace net diff --git a/net/android/cert_verify_result_android.h b/net/android/cert_verify_result_android.h index 692a15a..7027c3d 100644 --- a/net/android/cert_verify_result_android.h +++ b/net/android/cert_verify_result_android.h @@ -5,18 +5,32 @@ #ifndef NET_ANDROID_CERT_VERIFY_RESULT_ANDROID_H_ #define NET_ANDROID_CERT_VERIFY_RESULT_ANDROID_H_ +#include <jni.h> + +#include <string> +#include <vector> + #include "base/basictypes.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 +enum CertVerifyStatusAndroid { +#define CERT_VERIFY_STATUS_ANDROID(label, value) VERIFY_ ## label = value, +#include "net/android/cert_verify_status_android_list.h" +#undef CERT_VERIFY_STATUS_ANDROID }; +// Extract parameters out of an AndroidCertVerifyResult object. +void ExtractCertVerifyResult(jobject result, + CertVerifyStatusAndroid* status, + bool* is_issued_by_known_root, + std::vector<std::string>* verified_chain); + +// Register JNI methods. +bool RegisterCertVerifyResult(JNIEnv* env); + } // namespace android } // namespace net diff --git a/net/android/cert_verify_result_android_list.h b/net/android/cert_verify_status_android_list.h index a6d882f..be7cec7 100644 --- a/net/android/cert_verify_result_android_list.h +++ b/net/android/cert_verify_status_android_list.h @@ -1,4 +1,4 @@ -// Copyright (c) 2013 The Chromium Authors. All rights reserved. +// Copyright 2014 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. @@ -9,23 +9,23 @@ // from Java side to the C++ side. // Certificate is trusted. -CERT_VERIFY_RESULT_ANDROID(OK, 0) +CERT_VERIFY_STATUS_ANDROID(OK, 0) // Certificate verification could not be conducted. -CERT_VERIFY_RESULT_ANDROID(FAILED, -1) +CERT_VERIFY_STATUS_ANDROID(FAILED, -1) // Certificate is not trusted due to non-trusted root of the certificate chain. -CERT_VERIFY_RESULT_ANDROID(NO_TRUSTED_ROOT, -2) +CERT_VERIFY_STATUS_ANDROID(NO_TRUSTED_ROOT, -2) // Certificate is not trusted because it has expired. -CERT_VERIFY_RESULT_ANDROID(EXPIRED, -3) +CERT_VERIFY_STATUS_ANDROID(EXPIRED, -3) // Certificate is not trusted because it is not valid yet. -CERT_VERIFY_RESULT_ANDROID(NOT_YET_VALID, -4) +CERT_VERIFY_STATUS_ANDROID(NOT_YET_VALID, -4) // Certificate is not trusted because it could not be parsed. -CERT_VERIFY_RESULT_ANDROID(UNABLE_TO_PARSE, -5) +CERT_VERIFY_STATUS_ANDROID(UNABLE_TO_PARSE, -5) // Certificate is not trusted because it has an extendedKeyUsage field, but // its value is not correct for a web server. -CERT_VERIFY_RESULT_ANDROID(INCORRECT_KEY_USAGE, -6) +CERT_VERIFY_STATUS_ANDROID(INCORRECT_KEY_USAGE, -6) diff --git a/net/android/java/CertVerifyResultAndroid.template b/net/android/java/CertVerifyResultAndroid.template deleted file mode 100644 index b19e937..0000000 --- a/net/android/java/CertVerifyResultAndroid.template +++ /dev/null @@ -1,10 +0,0 @@ -// 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/CertVerifyStatusAndroid.template b/net/android/java/CertVerifyStatusAndroid.template new file mode 100644 index 0000000..e539783 --- /dev/null +++ b/net/android/java/CertVerifyStatusAndroid.template @@ -0,0 +1,10 @@ +// Copyright 2014 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 CertVerifyStatusAndroid { +#define CERT_VERIFY_STATUS_ANDROID(name, value) public static final int VERIFY_##name = value; +#include "net/android/cert_verify_status_android_list.h" +} diff --git a/net/android/java/src/org/chromium/net/AndroidCertVerifyResult.java b/net/android/java/src/org/chromium/net/AndroidCertVerifyResult.java new file mode 100644 index 0000000..fd6fca1 --- /dev/null +++ b/net/android/java/src/org/chromium/net/AndroidCertVerifyResult.java @@ -0,0 +1,73 @@ +// Copyright 2014 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; + +import org.chromium.base.CalledByNative; +import org.chromium.base.JNINamespace; + +import java.security.cert.CertificateEncodingException; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +/** + * The result of a certification verification. + */ +@JNINamespace("net::android") +public class AndroidCertVerifyResult { + + /** + * The verification status. One of the values in CertVerifyStatusAndroid. + */ + private final int mStatus; + + /** + * True if the root CA in the chain is in the system store. + */ + private final boolean mIsIssuedByKnownRoot; + + /** + * The properly ordered certificate chain used for verification. + */ + private final List<X509Certificate> mCertificateChain; + + public AndroidCertVerifyResult(int status, + boolean isIssuedByKnownRoot, + List<X509Certificate> certificateChain) { + mStatus = status; + mIsIssuedByKnownRoot = isIssuedByKnownRoot; + mCertificateChain = new ArrayList<X509Certificate>(certificateChain); + } + + public AndroidCertVerifyResult(int status) { + mStatus = status; + mIsIssuedByKnownRoot = false; + mCertificateChain = Collections.<X509Certificate>emptyList(); + } + + @CalledByNative + public int getStatus() { + return mStatus; + } + + @CalledByNative + public boolean isIssuedByKnownRoot() { + return mIsIssuedByKnownRoot; + } + + @CalledByNative + public byte[][] getCertificateChainEncoded() { + byte[][] verifiedChainArray = new byte[mCertificateChain.size()][]; + try { + for (int i = 0; i < mCertificateChain.size(); i++) { + verifiedChainArray[i] = mCertificateChain.get(i).getEncoded(); + } + } catch (CertificateEncodingException e) { + return new byte[0][]; + } + return verifiedChainArray; + } +} diff --git a/net/android/java/src/org/chromium/net/AndroidNetworkLibrary.java b/net/android/java/src/org/chromium/net/AndroidNetworkLibrary.java index 83775d89..e4caee5 100644 --- a/net/android/java/src/org/chromium/net/AndroidNetworkLibrary.java +++ b/net/android/java/src/org/chromium/net/AndroidNetworkLibrary.java @@ -198,20 +198,24 @@ class AndroidNetworkLibrary { } /** - * Validate the server's certificate chain is trusted. + * Validate the server's certificate chain is trusted. Note that the caller + * must still verify the name matches that of the leaf certificate. * * @param certChain The ASN.1 DER encoded bytes for certificates. - * @param authType The key exchange algorithm name (e.g. RSA) + * @param authType The key exchange algorithm name (e.g. RSA). + * @param host The hostname of the server. * @return Android certificate verification result code. */ @CalledByNative - public static int verifyServerCertificates(byte[][] certChain, String authType) { + public static AndroidCertVerifyResult verifyServerCertificates(byte[][] certChain, + String authType, + String host) { try { - return X509Util.verifyServerCertificates(certChain, authType); + return X509Util.verifyServerCertificates(certChain, authType, host); } catch (KeyStoreException e) { - return CertVerifyResultAndroid.VERIFY_FAILED; + return new AndroidCertVerifyResult(CertVerifyStatusAndroid.VERIFY_FAILED); } catch (NoSuchAlgorithmException e) { - return CertVerifyResultAndroid.VERIFY_FAILED; + return new AndroidCertVerifyResult(CertVerifyStatusAndroid.VERIFY_FAILED); } } diff --git a/net/android/java/src/org/chromium/net/X509Util.java b/net/android/java/src/org/chromium/net/X509Util.java index 76adb89..788f226 100644 --- a/net/android/java/src/org/chromium/net/X509Util.java +++ b/net/android/java/src/org/chromium/net/X509Util.java @@ -8,8 +8,11 @@ import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; +import android.net.http.X509TrustManagerExtensions; +import android.os.Build; import android.security.KeyChain; import android.util.Log; +import android.util.Pair; import org.chromium.base.JNINamespace; @@ -18,23 +21,34 @@ import java.io.IOException; import java.security.KeyStore; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; +import java.security.PublicKey; +import java.security.cert.Certificate; import java.security.cert.CertificateException; import java.security.cert.CertificateExpiredException; import java.security.cert.CertificateFactory; import java.security.cert.CertificateNotYetValidException; import java.security.cert.X509Certificate; +import java.util.Arrays; +import java.util.Collections; +import java.util.Enumeration; +import java.util.HashSet; import java.util.List; +import java.util.Set; import javax.net.ssl.TrustManager; import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509TrustManager; +import javax.security.auth.x500.X500Principal; +/** + * Utility functions for verifying X.509 certificates. + */ @JNINamespace("net") public class X509Util { private static final String TAG = "X509Util"; - public static final class TrustStorageListener extends BroadcastReceiver { + private static final class TrustStorageListener extends BroadcastReceiver { @Override public void onReceive(Context context, Intent intent) { if (intent.getAction().equals(KeyChain.ACTION_STORAGE_CHANGED)) { try { @@ -53,6 +67,49 @@ public class X509Util { } } + /** + * Interface that wraps one of X509TrustManager or + * X509TrustManagerExtensions to support platforms before the latter was + * added. + */ + private static interface X509TrustManagerImplementation { + public List<X509Certificate> checkServerTrusted(X509Certificate[] chain, + String authType, + String host) throws CertificateException; + } + + private static final class X509TrustManagerIceCreamSandwich implements + X509TrustManagerImplementation { + private final X509TrustManager mTrustManager; + + public X509TrustManagerIceCreamSandwich(X509TrustManager trustManager) { + mTrustManager = trustManager; + } + + @Override + public List<X509Certificate> checkServerTrusted(X509Certificate[] chain, + String authType, + String host) throws CertificateException { + mTrustManager.checkServerTrusted(chain, authType); + return Collections.<X509Certificate>emptyList(); + } + } + + private static final class X509TrustManagerJellyBean implements X509TrustManagerImplementation { + private final X509TrustManagerExtensions mTrustManagerExtensions; + + public X509TrustManagerJellyBean(X509TrustManager trustManager) { + mTrustManagerExtensions = new X509TrustManagerExtensions(trustManager); + } + + @Override + public List<X509Certificate> checkServerTrusted(X509Certificate[] chain, + String authType, + String host) throws CertificateException { + return mTrustManagerExtensions.checkServerTrusted(chain, authType, host); + } + } + private static CertificateFactory sCertificateFactory; private static final String OID_TLS_SERVER_AUTH = "1.3.6.1.5.5.7.3.1"; @@ -66,7 +123,7 @@ public class X509Util { /** * Trust manager backed up by the read-only system certificate store. */ - private static X509TrustManager sDefaultTrustManager; + private static X509TrustManagerImplementation sDefaultTrustManager; /** * BroadcastReceiver that listens to change in the system keystore to invalidate certificate @@ -78,10 +135,23 @@ public class X509Util { * Trust manager backed up by a custom certificate store. We need such manager to plant test * root CA to the trust store in testing. */ - private static X509TrustManager sTestTrustManager; + private static X509TrustManagerImplementation sTestTrustManager; private static KeyStore sTestKeyStore; /** + * Hash set of the subject and public key of system roots. This is used to + * determine whether a chain ends at a well-known root or not. + * + * Querying the system KeyStore for the root directly doesn't work as the + * root of the verified chain may be the server's version of a root rather + * than the system one. For instance, the server may send a certificate + * signed by another CA, while the system store contains a self-signed root + * with the same subject and SPKI. The chain will terminate at that root + * but X509TrustManagerExtensions will return the server's version. + */ + private static Set<Pair<X500Principal, PublicKey>> sSystemTrustRoots; + + /** * Lock object used to synchronize all calls that modify or depend on the trust managers. */ private static final Object sLock = new Object(); @@ -105,6 +175,9 @@ public class X509Util { if (sDefaultTrustManager == null) { sDefaultTrustManager = X509Util.createTrustManager(null); } + if (sSystemTrustRoots == null) { + sSystemTrustRoots = buildSystemTrustRootSet(); + } if (sTestKeyStore == null) { sTestKeyStore = KeyStore.getInstance(KeyStore.getDefaultType()); try { @@ -125,20 +198,56 @@ public class X509Util { } } + private static Set<Pair<X500Principal, PublicKey>> buildSystemTrustRootSet() throws + CertificateException, KeyStoreException, NoSuchAlgorithmException { + // Load the Android CA store. + KeyStore systemKeyStore = KeyStore.getInstance("AndroidCAStore"); + try { + systemKeyStore.load(null); + } catch (IOException e) { + // No IO operation is attempted. + } + + // System trust roots have prefix of "system:". + Set<Pair<X500Principal, PublicKey>> roots = new HashSet<Pair<X500Principal, PublicKey>>(); + Enumeration<String> aliases = systemKeyStore.aliases(); + while (aliases.hasMoreElements()) { + String alias = aliases.nextElement(); + if (!alias.startsWith("system:")) + continue; + Certificate cert = systemKeyStore.getCertificate(alias); + if (cert != null && cert instanceof X509Certificate) { + X509Certificate x509Cert = (X509Certificate)cert; + roots.add(new Pair<X500Principal, PublicKey>(x509Cert.getSubjectX500Principal(), + x509Cert.getPublicKey())); + } + } + return roots; + } + /** - * Creates a X509TrustManager backed up by the given key store. When null is passed as a key - * store, system default trust store is used. + * Creates a X509TrustManagerImplementation backed up by the given key + * store. When null is passed as a key store, system default trust store is + * used. * @throws KeyStoreException, NoSuchAlgorithmException on error initializing the TrustManager. */ - private static X509TrustManager createTrustManager(KeyStore keyStore) throws KeyStoreException, - NoSuchAlgorithmException { + private static X509TrustManagerImplementation createTrustManager(KeyStore keyStore) throws + KeyStoreException, NoSuchAlgorithmException { String algorithm = TrustManagerFactory.getDefaultAlgorithm(); TrustManagerFactory tmf = TrustManagerFactory.getInstance(algorithm); tmf.init(keyStore); for (TrustManager tm : tmf.getTrustManagers()) { if (tm instanceof X509TrustManager) { - return (X509TrustManager) tm; + try { + if (Build.VERSION.SDK_INT >= 17) { + return new X509TrustManagerJellyBean((X509TrustManager) tm); + } else { + return new X509TrustManagerIceCreamSandwich((X509TrustManager) tm); + } + } catch (IllegalArgumentException e) { + Log.e(TAG, "Error creating trust manager: " + e); + } } } return null; @@ -158,6 +267,7 @@ public class X509Util { private static void reloadDefaultTrustManager() throws KeyStoreException, NoSuchAlgorithmException, CertificateException { sDefaultTrustManager = null; + sSystemTrustRoots = null; nativeNotifyKeyChainChanged(); ensureInitialized(); } @@ -231,17 +341,20 @@ public class X509Util { return false; } - public static int verifyServerCertificates(byte[][] certChain, String authType) + public static AndroidCertVerifyResult verifyServerCertificates(byte[][] certChain, + String authType, + String host) 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); + "chain passed as |certChain|. |certChain|=" + Arrays.deepToString(certChain)); } + try { ensureInitialized(); } catch (CertificateException e) { - return CertVerifyResultAndroid.VERIFY_FAILED; + return new AndroidCertVerifyResult(CertVerifyStatusAndroid.VERIFY_FAILED); } X509Certificate[] serverCertificates = new X509Certificate[certChain.length]; @@ -250,7 +363,7 @@ public class X509Util { serverCertificates[i] = createCertificateFromBytes(certChain[i]); } } catch (CertificateException e) { - return CertVerifyResultAndroid.VERIFY_UNABLE_TO_PARSE; + return new AndroidCertVerifyResult(CertVerifyStatusAndroid.VERIFY_UNABLE_TO_PARSE); } // Expired and not yet valid certificates would be rejected by the trust managers, but the @@ -259,32 +372,47 @@ public class X509Util { // separately. try { serverCertificates[0].checkValidity(); - if (!verifyKeyUsage(serverCertificates[0])) - return CertVerifyResultAndroid.VERIFY_INCORRECT_KEY_USAGE; + if (!verifyKeyUsage(serverCertificates[0])) { + return new AndroidCertVerifyResult( + CertVerifyStatusAndroid.VERIFY_INCORRECT_KEY_USAGE); + } } catch (CertificateExpiredException e) { - return CertVerifyResultAndroid.VERIFY_EXPIRED; + return new AndroidCertVerifyResult(CertVerifyStatusAndroid.VERIFY_EXPIRED); } catch (CertificateNotYetValidException e) { - return CertVerifyResultAndroid.VERIFY_NOT_YET_VALID; + return new AndroidCertVerifyResult(CertVerifyStatusAndroid.VERIFY_NOT_YET_VALID); } catch (CertificateException e) { - return CertVerifyResultAndroid.VERIFY_FAILED; + return new AndroidCertVerifyResult(CertVerifyStatusAndroid.VERIFY_FAILED); } synchronized (sLock) { + List<X509Certificate> verifiedChain; try { - sDefaultTrustManager.checkServerTrusted(serverCertificates, authType); - return CertVerifyResultAndroid.VERIFY_OK; + verifiedChain = sDefaultTrustManager.checkServerTrusted(serverCertificates, + authType, host); } catch (CertificateException eDefaultManager) { try { - sTestTrustManager.checkServerTrusted(serverCertificates, authType); - return CertVerifyResultAndroid.VERIFY_OK; + verifiedChain = sTestTrustManager.checkServerTrusted(serverCertificates, + authType, host); } catch (CertificateException eTestManager) { // 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 new AndroidCertVerifyResult( + CertVerifyStatusAndroid.VERIFY_NO_TRUSTED_ROOT); } } + + boolean isIssuedByKnownRoot = false; + if (verifiedChain.size() > 0) { + X509Certificate root = verifiedChain.get(verifiedChain.size() - 1); + isIssuedByKnownRoot = sSystemTrustRoots.contains( + new Pair<X500Principal, PublicKey>(root.getSubjectX500Principal(), + root.getPublicKey())); + } + + return new AndroidCertVerifyResult(CertVerifyStatusAndroid.VERIFY_OK, + isIssuedByKnownRoot, verifiedChain); } } diff --git a/net/android/net_jni_registrar.cc b/net/android/net_jni_registrar.cc index f7712bb..94c849a 100644 --- a/net/android/net_jni_registrar.cc +++ b/net/android/net_jni_registrar.cc @@ -18,6 +18,7 @@ namespace net { namespace android { static base::android::RegistrationMethod kNetRegisteredMethods[] = { + { "AndroidCertVerifyResult", net::android::RegisterCertVerifyResult }, { "AndroidKeyStore", net::android::RegisterKeyStore }, { "AndroidNetworkLibrary", net::android::RegisterNetworkLibrary }, { "GURLUtils", net::RegisterGURLUtils }, diff --git a/net/android/network_library.cc b/net/android/network_library.cc index 2407100..37ab18e 100644 --- a/net/android/network_library.cc +++ b/net/android/network_library.cc @@ -23,9 +23,12 @@ using base::android::ToJavaByteArray; namespace net { namespace android { -CertVerifyResultAndroid VerifyX509CertChain( - const std::vector<std::string>& cert_chain, - const std::string& auth_type) { +void VerifyX509CertChain(const std::vector<std::string>& cert_chain, + const std::string& auth_type, + const std::string& host, + CertVerifyStatusAndroid* status, + bool* is_issued_by_known_root, + std::vector<std::string>* verified_chain) { JNIEnv* env = AttachCurrentThread(); ScopedJavaLocalRef<jobjectArray> chain_byte_array = @@ -36,10 +39,20 @@ CertVerifyResultAndroid VerifyX509CertChain( ConvertUTF8ToJavaString(env, auth_type); DCHECK(!auth_string.is_null()); - jint result = Java_AndroidNetworkLibrary_verifyServerCertificates( - env, chain_byte_array.obj(), auth_string.obj()); + ScopedJavaLocalRef<jstring> host_string = + ConvertUTF8ToJavaString(env, host); + DCHECK(!host_string.is_null()); - return static_cast<CertVerifyResultAndroid>(result); + ScopedJavaLocalRef<jobject> result = + Java_AndroidNetworkLibrary_verifyServerCertificates( + env, chain_byte_array.obj(), auth_string.obj(), host_string.obj()); + if (ClearException(env)) { + *status = android::VERIFY_FAILED; + return; + } + + ExtractCertVerifyResult(result.obj(), + status, is_issued_by_known_root, verified_chain); } void AddTestRootCertificate(const uint8* cert, size_t len) { diff --git a/net/android/network_library.h b/net/android/network_library.h index 6bdc1ae..5d4d73d 100644 --- a/net/android/network_library.h +++ b/net/android/network_library.h @@ -21,9 +21,12 @@ namespace android { // |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. -CertVerifyResultAndroid VerifyX509CertChain( - const std::vector<std::string>& cert_chain, - const std::string& auth_type); +void VerifyX509CertChain(const std::vector<std::string>& cert_chain, + const std::string& auth_type, + const std::string& host, + CertVerifyStatusAndroid* status, + bool* is_issued_by_known_root, + std::vector<std::string>* verified_chain); // 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/cert/cert_verify_proc.cc b/net/cert/cert_verify_proc.cc index 01c2720..798d902 100644 --- a/net/cert/cert_verify_proc.cc +++ b/net/cert/cert_verify_proc.cc @@ -261,21 +261,16 @@ int CertVerifyProc::Verify(X509Certificate* cert, rv = MapCertStatusToNetError(verify_result->cert_status); } -#if !defined(OS_ANDROID) // Flag certificates from publicly-trusted CAs that are issued to intranet // hosts. While the CA/Browser Forum Baseline Requirements (v1.1) permit // these to be issued until 1 November 2015, they represent a real risk for // the deployment of gTLDs and are being phased out ahead of the hard // deadline. - // - // TODO(ppi): is_issued_by_known_root is incorrect on Android. Once this is - // fixed, re-enable this check for Android. crbug.com/116838 if (verify_result->is_issued_by_known_root && IsHostnameNonUnique(hostname)) { verify_result->cert_status |= CERT_STATUS_NON_UNIQUE_NAME; // CERT_STATUS_NON_UNIQUE_NAME will eventually become a hard error. For // now treat it as a warning and do not map it to an error return value. } -#endif return rv; } diff --git a/net/cert/cert_verify_proc_android.cc b/net/cert/cert_verify_proc_android.cc index bca62bc..590bb79 100644 --- a/net/cert/cert_verify_proc_android.cc +++ b/net/cert/cert_verify_proc_android.cc @@ -8,9 +8,13 @@ #include <vector> #include "base/logging.h" +#include "base/sha1.h" +#include "base/strings/string_piece.h" +#include "crypto/sha2.h" #include "net/android/cert_verify_result_android.h" #include "net/android/network_library.h" #include "net/base/net_errors.h" +#include "net/cert/asn1_util.h" #include "net/cert/cert_status_flags.h" #include "net/cert/cert_verify_result.h" #include "net/cert/x509_certificate.h" @@ -22,11 +26,16 @@ namespace { // Returns true if the certificate verification call was successful (regardless // of its result), i.e. if |verify_result| was set. Otherwise returns false. bool VerifyFromAndroidTrustManager(const std::vector<std::string>& cert_bytes, + const std::string& hostname, CertVerifyResult* verify_result) { + android::CertVerifyStatusAndroid status; + std::vector<std::string> verified_chain; + // TODO(joth): Fetch the authentication type from SSL rather than hardcode. - android::CertVerifyResultAndroid android_result = - android::VerifyX509CertChain(cert_bytes, "RSA"); - switch (android_result) { + android::VerifyX509CertChain(cert_bytes, "RSA", hostname, + &status, &verify_result->is_issued_by_known_root, + &verified_chain); + switch (status) { case android::VERIFY_FAILED: return false; case android::VERIFY_OK: @@ -49,6 +58,35 @@ bool VerifyFromAndroidTrustManager(const std::vector<std::string>& cert_bytes, verify_result->cert_status |= CERT_STATUS_INVALID; break; } + + // Save the verified chain. + if (!verified_chain.empty()) { + std::vector<base::StringPiece> verified_chain_pieces(verified_chain.size()); + for (size_t i = 0; i < verified_chain.size(); i++) { + verified_chain_pieces[i] = base::StringPiece(verified_chain[i]); + } + scoped_refptr<X509Certificate> verified_cert = + X509Certificate::CreateFromDERCertChain(verified_chain_pieces); + if (verified_cert) + verify_result->verified_cert = verified_cert; + } + + // Extract the public key hashes. + for (size_t i = 0; i < verified_chain.size(); i++) { + base::StringPiece spki_bytes; + if (!asn1::ExtractSPKIFromDERCert(verified_chain[i], &spki_bytes)) + continue; + + HashValue sha1(HASH_VALUE_SHA1); + base::SHA1HashBytes(reinterpret_cast<const uint8*>(spki_bytes.data()), + spki_bytes.size(), sha1.data()); + verify_result->public_key_hashes.push_back(sha1); + + HashValue sha256(HASH_VALUE_SHA256); + crypto::SHA256HashString(spki_bytes, sha256.data(), crypto::kSHA256Length); + verify_result->public_key_hashes.push_back(sha256); + } + return true; } @@ -99,7 +137,7 @@ int CertVerifyProcAndroid::VerifyInternal( std::vector<std::string> cert_bytes; if (!GetChainDEREncodedBytes(cert, &cert_bytes)) return ERR_CERT_INVALID; - if (!VerifyFromAndroidTrustManager(cert_bytes, verify_result)) { + if (!VerifyFromAndroidTrustManager(cert_bytes, hostname, verify_result)) { NOTREACHED(); return ERR_FAILED; } @@ -111,11 +149,6 @@ int CertVerifyProcAndroid::VerifyInternal( // flag. All of the above require specific support from the platform, missing // in the Java APIs. See also: http://crbug.com/116838 - // Until the required support is available in the platform, we don't know if - // the trust root at the end of the chain was standard or user-added, so we - // mark all correctly verified certificates as issued by a known root. - verify_result->is_issued_by_known_root = true; - return OK; } diff --git a/net/cert/cert_verify_proc_unittest.cc b/net/cert/cert_verify_proc_unittest.cc index 71a0c0e..9d7e750 100644 --- a/net/cert/cert_verify_proc_unittest.cc +++ b/net/cert/cert_verify_proc_unittest.cc @@ -29,6 +29,8 @@ #include "base/win/windows_version.h" #elif defined(OS_MACOSX) && !defined(OS_IOS) #include "base/mac/mac_util.h" +#elif defined(OS_ANDROID) +#include "base/android/build_info.h" #endif using base::HexEncode; @@ -83,6 +85,26 @@ int WellKnownCaCertVerifyProc::VerifyInternal( return OK; } +bool SupportsReturningVerifiedChain() { +#if defined(OS_ANDROID) + // Before API level 17, Android does not expose the APIs necessary to get at + // the verified certificate chain. + if (base::android::BuildInfo::GetInstance()->sdk_int() < 17) + return false; +#endif + return true; +} + +bool SupportsDetectingKnownRoots() { +#if defined(OS_ANDROID) + // Before API level 17, Android does not expose the APIs necessary to get at + // the verified certificate chain and detect known roots. + if (base::android::BuildInfo::GetInstance()->sdk_int() < 17) + return false; +#endif + return true; +} + } // namespace class CertVerifyProcTest : public testing::Test { @@ -398,14 +420,15 @@ TEST_F(CertVerifyProcTest, RejectWeakKeys) { // provided by servers. See CertVerifyProcTest.CybertrustGTERoot for further // details. #define MAYBE_ExtraneousMD5RootCert DISABLED_ExtraneousMD5RootCert -#elif defined(USE_OPENSSL) || defined(OS_ANDROID) -// Disabled for OpenSSL / Android - Android and OpenSSL do not attempt to find -// a minimal certificate chain, thus prefer the MD5 root over the SHA-1 root. -#define MAYBE_ExtraneousMD5RootCert DISABLED_ExtraneousMD5RootCert #else #define MAYBE_ExtraneousMD5RootCert ExtraneousMD5RootCert #endif TEST_F(CertVerifyProcTest, MAYBE_ExtraneousMD5RootCert) { + if (!SupportsReturningVerifiedChain()) { + LOG(INFO) << "Skipping this test in this platform."; + return; + } + base::FilePath certs_dir = GetTestCertsDirectory(); scoped_refptr<X509Certificate> server_cert = @@ -554,13 +577,12 @@ TEST_F(CertVerifyProcTest, NameConstraintsOk) { EXPECT_EQ(0U, verify_result.cert_status); } -#if defined(OS_ANDROID) -// Disabled because Android isn't filling in SPKI hashes: crbug.com/116838. -#define MAYBE_NameConstraintsFailure DISABLED_NameConstraintsFailure -#else -#define MAYBE_NameConstraintsFailure NameConstraintsFailure -#endif -TEST_F(CertVerifyProcTest, MAYBE_NameConstraintsFailure) { +TEST_F(CertVerifyProcTest, NameConstraintsFailure) { + if (!SupportsReturningVerifiedChain()) { + LOG(INFO) << "Skipping this test in this platform."; + return; + } + CertificateList ca_cert_list = CreateCertificateListFromFile(GetTestCertsDirectory(), "root_ca_cert.pem", @@ -591,8 +613,12 @@ TEST_F(CertVerifyProcTest, MAYBE_NameConstraintsFailure) { verify_result.cert_status & CERT_STATUS_NAME_CONSTRAINT_VIOLATION); } -// The certse.pem certificate has been revoked. crbug.com/259723. TEST_F(CertVerifyProcTest, TestKnownRoot) { + if (!SupportsDetectingKnownRoots()) { + LOG(INFO) << "Skipping this test in this platform."; + return; + } + base::FilePath certs_dir = GetTestCertsDirectory(); CertificateList certs = CreateCertificateListFromFile( certs_dir, "satveda.pem", X509Certificate::FORMAT_AUTO); @@ -622,6 +648,11 @@ TEST_F(CertVerifyProcTest, TestKnownRoot) { // The certse.pem certificate has been revoked. crbug.com/259723. TEST_F(CertVerifyProcTest, PublicKeyHashes) { + if (!SupportsReturningVerifiedChain()) { + LOG(INFO) << "Skipping this test in this platform."; + return; + } + base::FilePath certs_dir = GetTestCertsDirectory(); CertificateList certs = CreateCertificateListFromFile( certs_dir, "satveda.pem", X509Certificate::FORMAT_AUTO); @@ -717,6 +748,11 @@ TEST_F(CertVerifyProcTest, InvalidKeyUsage) { // used to ensure that the actual, verified chain is being returned by // Verify(). TEST_F(CertVerifyProcTest, VerifyReturnChainBasic) { + if (!SupportsReturningVerifiedChain()) { + LOG(INFO) << "Skipping this test in this platform."; + return; + } + base::FilePath certs_dir = GetTestCertsDirectory(); CertificateList certs = CreateCertificateListFromFile( certs_dir, "x509_verify_results.chain.pem", @@ -759,19 +795,16 @@ TEST_F(CertVerifyProcTest, VerifyReturnChainBasic) { certs[2]->os_cert_handle())); } -#if defined(OS_ANDROID) -// TODO(ppi): Disabled because is_issued_by_known_root is incorrect on Android. -// Once this is fixed, re-enable this check for android. crbug.com/116838 -#define MAYBE_IntranetHostsRejected DISABLED_IntranetHostsRejected -#else -#define MAYBE_IntranetHostsRejected IntranetHostsRejected -#endif - // Test that certificates issued for 'intranet' names (that is, containing no // known public registry controlled domain information) issued by well-known // CAs are flagged appropriately, while certificates that are issued by // internal CAs are not flagged. -TEST_F(CertVerifyProcTest, MAYBE_IntranetHostsRejected) { +TEST_F(CertVerifyProcTest, IntranetHostsRejected) { + if (!SupportsDetectingKnownRoots()) { + LOG(INFO) << "Skipping this test in this platform."; + return; + } + CertificateList cert_list = CreateCertificateListFromFile( GetTestCertsDirectory(), "ok_cert.pem", X509Certificate::FORMAT_AUTO); @@ -802,6 +835,11 @@ TEST_F(CertVerifyProcTest, MAYBE_IntranetHostsRejected) { // of intermediate certificates are combined, it's possible that order may // not be maintained. TEST_F(CertVerifyProcTest, VerifyReturnChainProperlyOrdered) { + if (!SupportsReturningVerifiedChain()) { + LOG(INFO) << "Skipping this test in this platform."; + return; + } + base::FilePath certs_dir = GetTestCertsDirectory(); CertificateList certs = CreateCertificateListFromFile( certs_dir, "x509_verify_results.chain.pem", @@ -848,6 +886,11 @@ TEST_F(CertVerifyProcTest, VerifyReturnChainProperlyOrdered) { // Test that Verify() filters out certificates which are not related to // or part of the certificate chain being verified. TEST_F(CertVerifyProcTest, VerifyReturnChainFiltersUnrelatedCerts) { + if (!SupportsReturningVerifiedChain()) { + LOG(INFO) << "Skipping this test in this platform."; + return; + } + base::FilePath certs_dir = GetTestCertsDirectory(); CertificateList certs = CreateCertificateListFromFile( certs_dir, "x509_verify_results.chain.pem", @@ -946,6 +989,32 @@ TEST_F(CertVerifyProcTest, AdditionalTrustAnchors) { EXPECT_FALSE(verify_result.is_issued_by_additional_trust_anchor); } +// Tests that certificates issued by user-supplied roots are not flagged as +// issued by a known root. This should pass whether or not the platform supports +// detecting known roots. +TEST_F(CertVerifyProcTest, IsIssuedByKnownRootIgnoresTestRoots) { + // Load root_ca_cert.pem into the test root store. + TestRootCerts* root_certs = TestRootCerts::GetInstance(); + root_certs->AddFromFile( + GetTestCertsDirectory().AppendASCII("root_ca_cert.pem")); + + CertificateList cert_list = CreateCertificateListFromFile( + GetTestCertsDirectory(), "ok_cert.pem", + X509Certificate::FORMAT_AUTO); + ASSERT_EQ(1U, cert_list.size()); + scoped_refptr<X509Certificate> cert(cert_list[0]); + + // Verification should pass. + int flags = 0; + CertVerifyResult verify_result; + int error = Verify( + cert.get(), "127.0.0.1", flags, NULL, empty_cert_list_, &verify_result); + EXPECT_EQ(OK, error); + EXPECT_EQ(0U, verify_result.cert_status); + // But should not be marked as a known root. + EXPECT_FALSE(verify_result.is_issued_by_known_root); +} + #if defined(OS_MACOSX) && !defined(OS_IOS) // Tests that, on OS X, issues with a cross-certified Baltimore CyberTrust // Root can be successfully worked around once Apple completes removing the diff --git a/net/net.gyp b/net/net.gyp index 876b61e..0ad14b5 100644 --- a/net/net.gyp +++ b/net/net.gyp @@ -60,8 +60,9 @@ 'net_resources', ], 'sources': [ + 'android/cert_verify_result_android.cc', 'android/cert_verify_result_android.h', - 'android/cert_verify_result_android_list.h', + 'android/cert_verify_status_android_list.h', 'android/gurl_utils.cc', 'android/gurl_utils.h', 'android/keystore.cc', @@ -2958,6 +2959,7 @@ 'target_name': 'net_jni_headers', 'type': 'none', 'sources': [ + 'android/java/src/org/chromium/net/AndroidCertVerifyResult.java', 'android/java/src/org/chromium/net/AndroidKeyStore.java', 'android/java/src/org/chromium/net/AndroidNetworkLibrary.java', 'android/java/src/org/chromium/net/GURLUtils.java', @@ -2991,7 +2993,7 @@ }, 'dependencies': [ '../base/base.gyp:base', - 'cert_verify_result_android_java', + 'cert_verify_status_android_java', 'certificate_mime_types_java', 'net_errors_java', 'private_key_types_java', @@ -3044,14 +3046,14 @@ 'includes': [ '../build/android/java_cpp_template.gypi' ], }, { - 'target_name': 'cert_verify_result_android_java', + 'target_name': 'cert_verify_status_android_java', 'type': 'none', 'sources': [ - 'android/java/CertVerifyResultAndroid.template', + 'android/java/CertVerifyStatusAndroid.template', ], 'variables': { 'package_name': 'org/chromium/net', - 'template_deps': ['android/cert_verify_result_android_list.h'], + 'template_deps': ['android/cert_verify_status_android_list.h'], }, 'includes': [ '../build/android/java_cpp_template.gypi' ], }, |