[Cronet] Public key pinning for Java API

components/cronet/android/api/src/org/chromium/net/CronetEngine.java

 // Copyright 2015 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.Context;
 import android.support.annotation.IntDef;
+import android.util.Base64;
 import android.util.Log;
 
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
 
 import java.io.File;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.reflect.Constructor;
 import java.net.Proxy;
 import java.net.URL;
 import java.net.URLConnection;
 import java.net.URLStreamHandlerFactory;
+import java.util.Collection;
+import java.util.Date;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.Executor;
 
 /**
  * An engine to process {@link UrlRequest}s, which uses the best HTTP stack
  * available on the current platform.
  */
 public abstract class CronetEngine {
     /**
      * A builder for {@link CronetEngine}s, which allows runtime configuration of
      * {@code CronetEngine}. Configuration options are set on the builder and
@@ skipping 238 matching lines @@
          * ({@link #HTTP_CACHE_DISK}) is needed to take advantage of 0-RTT
          * connection establishment between sessions.
          *
          * @param host hostname of the server that supports QUIC.
          * @param port host of the server that supports QUIC.
          * @param alternatePort alternate port to use for QUIC.
          * @return the builder to facilitate chaining.
          */
         public Builder addQuicHint(String host, int port, int alternatePort) {
             if (host.contains("/")) {
                 throw new IllegalArgumentException("Illegal QUIC Hint Host: " + host);

[mef, 2015/11/06 18:02:16]

In the past there were some misuses of API with url passed instead of host.
Maybe we should have the same / similar check in addPublicKeyPins?

[kapishnikov, 2015/11/06 19:45:08]

I will add the proper check.

             }
             try {
                 JSONArray quicHints = mConfig.optJSONArray(CronetEngineBuilderList.QUIC_HINTS);
                 if (quicHints == null) {
                     quicHints = new JSONArray();
                     mConfig.put(CronetEngineBuilderList.QUIC_HINTS, quicHints);
                 }
 
                 JSONObject hint = new JSONObject();
                 hint.put(CronetEngineBuilderList.QUIC_HINT_HOST, host);
                 hint.put(CronetEngineBuilderList.QUIC_HINT_PORT, port);
                 hint.put(CronetEngineBuilderList.QUIC_HINT_ALT_PORT, alternatePort);
                 quicHints.put(hint);
             } catch (JSONException e) {
                 // Intentionally do nothing.
             }
             return this;
         }
 
         /**
+         * Adds public key pinning for a given host.
+         *
+         * @param hostName name of the host to which public keys should be pinned.
+         * @param pinsSha256 a collection of pins. Each pin is the SHA-256 cryptographic
+         *                   hash of DER-encoded ASN.1 representation of Subject Public

[mef, 2015/11/06 18:02:17]

pinning -> pins

[kapishnikov, 2015/11/06 19:45:08]

Done.

+         *                   Key Info (SPKI) of the host X.509 certificate. You can use

[mef, 2015/11/06 18:02:16]

Usually Chrome frowns upon use of 'you', 'we', etc in comments.

[kapishnikov, 2015/11/06 19:45:08]

Done.

+         *                   {@code security.cert.X509Certificate.getPublicKey().getEncoded()}
+         *                   to obtain DER-encoded ASN.1 representation of SPKI.
+         * @param includeSubdomains indicates whether the pinning policy should be applied to
+         *                          subdomains of {@code hostName}.
+         * @param expirationDate specifies the expiration date for the pins.
+         * @return the builder to facilitate chaining.
+         * @throws NullPointerException if one of the input parameters is null.
+         * @throws IllegalArgumentException if {@code pinsSha256} collection contains a byte array
+         *                                  that does not represent a valid SHA-256 hash.
+         *
+         */
+        public Builder addPublicKeyPins(String hostName, Collection<byte[]> pinsSha256,
+                boolean includeSubdomains, Date expirationDate) {
+            // Validate the input

[mef, 2015/11/06 18:02:16]

comments should end in period, but reasonably we don't need this comment.

[kapishnikov, 2015/11/06 19:45:08]

Done.

+            if (hostName == null) {
+                throw new NullPointerException("The hostname cannot be null");
+            }
+            if (pinsSha256 == null) {
+                throw new NullPointerException("The collection of SHA256 pins cannot be null");
+            }
+            try {
+                // Add HPKP_LIST JSON array element if it is not present.
+                JSONArray hpkpList = mConfig.optJSONArray(CronetEngineBuilderList.HPKP_LIST);
+                if (hpkpList == null) {
+                    hpkpList = new JSONArray();
+                    mConfig.put(CronetEngineBuilderList.HPKP_LIST, hpkpList);
+                }
+
+                // Convert the pin to BASE64 encoding.
+                Set<String> hashes = new HashSet<>(pinsSha256.size());
+                for (byte[] pinSha256 : pinsSha256) {
+                    hashes.add(convertSha256ToBase64WithPrefix(pinSha256));
+                }
+
+                // Add new element to HPKP_LIST JSON array.
+                JSONObject hpkp = new JSONObject();
+                hpkp.put(CronetEngineBuilderList.HPKP_HOST, hostName);
+                hpkp.put(CronetEngineBuilderList.HPKP_PIN_HASHES, new JSONArray(hashes));
+                hpkp.put(CronetEngineBuilderList.HPKP_INCLUDE_SUBDOMAINS, includeSubdomains);
+                // The expiration time is passed as a double, in seconds since January 1, 1970.
+                hpkp.put(CronetEngineBuilderList.HPKP_EXPIRATION_DATE,
+                        (double) expirationDate.getTime() / 1000);

[mef, 2015/11/06 18:02:16]

why double? Can it be long?

[kapishnikov, 2015/11/06 19:45:08]

Our current C++ JsonValueConverter
(https://code.google.com/p/chromium/codesearch#chromium/src/base/json/json_val...)
parser does not support java long (int64) value type. Probably we could add one.

+                hpkpList.put(hpkp);
+            } catch (JSONException e) {
+                Log.e(TAG, "Unable to add public key pins", e);

[mef, 2015/11/06 18:02:16]

hrm, why wouldn't we re-throw it? The builder is wrong at this point, right?

[kapishnikov, 2015/11/06 19:45:08]

I agree that re-throwing an exception would be the correct way to handle this
situation. However, I have noticed that in most cases we do not do it, e.g.
https://code.google.com/p/chromium/codesearch#chromium/src/components/cronet/...

The same applies to C++ code that I saw, e.g. when the host name is empty or
hash cannot be converted to SHA-256.

+            }
+            return this;
+        }
+
+        /**
+         * Converts a given SHA256 array of bytes to BASE64 encoding with the prefix. The format
+         * corresponds to the format that is expected by net::HashValue class.
+         *
+         * @param sha256 SHA256 bytes to convert to BASE64.
+         * @return the BASE64 conversion.
+         * @throws IllegalArgumentException if the provided pin is invalid.
+         */
+        private String convertSha256ToBase64WithPrefix(byte[] sha256) {
+            if (sha256 == null || sha256.length != 32) {
+                throw new IllegalArgumentException("The provided pin is invalid");

[mef, 2015/11/06 18:02:16]

provided -> public key

[kapishnikov, 2015/11/06 19:45:08]

Done.

+            }
+            return "sha256/" + Base64.encodeToString(sha256, Base64.NO_WRAP);
+        }
+
+        /**
          * Sets experimental QUIC connection options, overwriting any pre-existing
          * options. List of options is subject to change.
          *
          * @param quicConnectionOptions comma-separated QUIC options (for example
          * "PACE,IW10") to use if QUIC is enabled.
          * @return the builder to facilitate chaining.
          */
         public Builder setExperimentalQuicConnectionOptions(String quicConnectionOptions) {
             return putString(CronetEngineBuilderList.QUIC_OPTIONS, quicConnectionOptions);
         }
@@ skipping 348 matching lines @@
                 cronetEngine = possibleEngine;
             }
         } catch (ClassNotFoundException e) {
             // Leave as null.
         } catch (Exception e) {
             throw new IllegalStateException("Cannot instantiate: " + CRONET_URL_REQUEST_CONTEXT, e);
         }
         return cronetEngine;
     }
 }