Update net/android/network_library.cc with fresher code.

net/android/java/org/chromium/net/AndroidNetworkLibrary.java

Index: net/android/java/org/chromium/net/AndroidNetworkLibrary.java
diff --git a/net/android/java/org/chromium/net/AndroidNetworkLibrary.java b/net/android/java/org/chromium/net/AndroidNetworkLibrary.java
new file mode 100644
index 0000000000000000000000000000000000000000..3eaa11c8c46e5df3342f53d5cd610977492bd26f
--- /dev/null
+++ b/net/android/java/org/chromium/net/AndroidNetworkLibrary.java
@@ -0,0 +1,130 @@
+// Copyright (c) 2012 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 android.content.ActivityNotFoundException;
+import android.content.Context;
+import android.content.Intent;
+import android.util.Log;
+
+import org.chromium.base.CalledByNative;
+import org.chromium.base.CalledByNativeUnchecked;
+
+import java.io.ByteArrayInputStream;
+import java.net.URLConnection;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.cert.CertificateException;
+import java.security.cert.CertificateFactory;
+import java.security.cert.X509Certificate;
+import java.util.concurrent.atomic.AtomicReference;
+
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import javax.net.ssl.X509TrustManager;
+
+// This class implements net utilities required by the net component.
+class AndroidNetworkLibrary {
+    private static final String TAG = "AndroidNetworkLibrary";
+
+    // Stores the key pair into the CertInstaller application.
+    @CalledByNative
+    static public boolean storeKeyPair(Context context, byte[] public_key, byte[] private_key) {
+        // This is based on android.security.Credentials.install()
+        // TODO(joth): Use KeyChain API instead of hard-coding constants here: http://b/5859651

[Ryan Sleevi, 2012/04/20 20:55:39]

Public bug for public code?

[Yaron, 2012/04/23 22:21:29]

Done.

+        try {
+            Intent intent = new Intent("android.credentials.INSTALL");
+            intent.setClassName("com.android.certinstaller",
+                    "com.android.certinstaller.CertInstallerMain");
+            intent.putExtra("KEY", private_key);
+            intent.putExtra("PKEY", public_key);
+            intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
+            context.startActivity(intent);
+            return true;
+        } catch (ActivityNotFoundException e) {
+            Log.w(TAG, "could not store certificate: " + e);
+        }
+        return false;
+    }
+
+    // Get the mime type (if any) that is associated with the file extension.
+    // Returns null if no corresponding mime type exists.
+    @CalledByNative
+    static public String getMimeTypeFromExtension(String extension) {
+        return URLConnection.guessContentTypeFromName("foo." + extension);
+    }
+
+    /**
+     * Validate the server's certificate chain is trusted.
+     * @param certChain The bytes for certificates, typically in ASN.1 DER encoded
+     * certificates format.

[Ryan Sleevi, 2012/04/20 20:55:39]

nit: The ASN.1 DER encoded bytes for certificates

I don't see where 'typically' makes sense?

[Yaron, 2012/04/23 22:21:29]

Done.

+     * @param authType The authentication type for the cert chain

[Ryan Sleevi, 2012/04/20 20:55:39]

I don't know if this explanation really adds much. Is this the policy for which
the server certificate is being validated against (eg: TLS server vs... XMPP
server?) 

[Yaron, 2012/04/23 22:21:29]

Done.

+     * @return true if the server is trusted
+     * @throws CertificateException,KeyStoreException,NoSuchAlgorithmException on error
+     * initializing the TrustManager or reading the certChain
+     */
+    @CalledByNativeUnchecked
+    public static boolean verifyServerCertificates(byte[][] certChain, String authType)
+            throws CertificateException, KeyStoreException, NoSuchAlgorithmException {
+        if (certChain == null || certChain.length == 0 || certChain[0] == null) {
+            throw new IllegalArgumentException("bad certificate chain");

[Ryan Sleevi, 2012/04/20 20:55:39]

nit: We try to write meaning error messages

[Yaron, 2012/04/23 22:21:29]

Done.

+        }
+
+        ensureInitialized();
+        X509Certificate[] serverCertificates = new X509Certificate[certChain.length];
+        for (int i = 0; i < certChain.length; ++i) {
+            serverCertificates[i] =
+                (X509Certificate) sCertificateFactory.get().generateCertificate(

[Ryan Sleevi, 2012/04/20 20:55:39]

sCertificateFactory may be NULL here if ensureInitialized failed.

Should you be wrapping the larger code in a try / catch with a return false? Is
that antithetical to the Java way?

[Yaron, 2012/04/23 22:21:29]

How does ensureInitialized fail? If CertificateFactory.getInstance fails? Seems
like that's a bug that we'd deal with as a crash report instead of silently
failing later, doesn't it?

[Ryan Sleevi, 2012/04/23 22:27:18]

If the crash report is meaningful, sure. I just wanted to make sure to point out
the possibility that it might be NULL here, and make sure that was acceptable.

+                        new ByteArrayInputStream(certChain[i]));
+        }
+
+        try {
+            sDefaultTrustManager.get().checkServerTrusted(serverCertificates, authType);
+            return true;
+        } catch (CertificateException e) {
+            Log.i(TAG, "failed to validate the certificate chain, error: " +
+                    e.getMessage());
+        }
+        return false;
+    }
+
+    // Default sources of authentication trust decisions and certificate object creation.
+    private static AtomicReference<X509TrustManager> sDefaultTrustManager =
+        new AtomicReference<X509TrustManager>();
+    private static AtomicReference<CertificateFactory> sCertificateFactory =
+        new AtomicReference<CertificateFactory>();
+
+    private static void ensureInitialized()

[Ryan Sleevi, 2012/04/20 20:55:39]

nit: Document, especially since this can throw?

[Yaron, 2012/04/23 22:21:29]

This is private but done. Also not really sure about a "meaningful" comment..

+            throws CertificateException, KeyStoreException, NoSuchAlgorithmException {
+        // There could be a begin race creating two instances of these objects, which
+        // is harmless save for a bit of wasted effort.
+        if (sDefaultTrustManager.get() == null) {
+            sDefaultTrustManager.compareAndSet(null, createDefaultTrustManager());

[Ryan Sleevi, 2012/04/20 20:55:39]

If createDefaultTrustManager() returns null, then you'll needlessly be spinning
between this, createDefaultTrustManager, and findX509TrustManager, right?

Should you instead throw at this point?

[Yaron, 2012/04/23 22:21:29]

I don't see that. It looks like verifyServerCertificates would crash when it
calls checkServerTrusted

[joth, 2012/04/24 08:43:26]

FWIW... my understanding now is AtomicReference just adds needless confusion, as
(a) a plain old java reference is atomic anyway, and (b) using AtomicReference
doesn't avoid the risk that thread B gets a partially constructed TrustManager
due to instruction reorder. (c.f. perils of double checked locking).

Careful usage of volatile reference can solve this. Or using synchronization.
But I think http://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom
is the nicest pattern.

+        }
+        if (sCertificateFactory.get() == null) {
+            sCertificateFactory.compareAndSet(null, CertificateFactory.getInstance("X.509"));
+        }
+    }
+
+    private static X509TrustManager createDefaultTrustManager()

[Ryan Sleevi, 2012/04/20 20:55:39]

nit: Document, especially since this can throw?

[Yaron, 2012/04/23 22:21:29]

Done.

+            throws KeyStoreException, NoSuchAlgorithmException {
+        String algorithm = TrustManagerFactory.getDefaultAlgorithm();
+        TrustManagerFactory tmf = TrustManagerFactory.getInstance(algorithm);
+        tmf.init((KeyStore) null);
+        TrustManager[] tms = tmf.getTrustManagers();
+        X509TrustManager trustManager = findX509TrustManager(tms);
+        return trustManager;
+    }
+
+    private static X509TrustManager findX509TrustManager(TrustManager[] tms) {
+        for (TrustManager tm : tms) {
+            if (tm instanceof X509TrustManager) {
+                return (X509TrustManager)tm;
+            }
+        }
+        return null;
+    }
+}