Commment signed webapks working and verified.

chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkValidator.java

 // Copyright 2016 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.webapk.lib.client;
 
 import static org.chromium.webapk.lib.common.WebApkConstants.WEBAPK_PACKAGE_PREFIX;
+import static org.chromium.webapk.lib.common.WebApkMetaDataKeys.START_URL;
 
 import android.content.Context;
 import android.content.Intent;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.PackageManager.NameNotFoundException;
 import android.content.pm.ResolveInfo;
 import android.content.pm.Signature;
+import android.text.TextUtils;
 import android.util.Log;
 
 import org.chromium.base.annotations.SuppressFBWarnings;
 
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.nio.MappedByteBuffer;
+import java.nio.channels.FileChannel;
+import java.security.KeyFactory;
+import java.security.PublicKey;
+import java.security.spec.X509EncodedKeySpec;
 import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
 
 /**
  * Checks whether a URL belongs to a WebAPK, and whether a WebAPK is signed by the WebAPK Minting
  * Server.
  */
 public class WebApkValidator {
+    private static final String TAG = "WebApkValidator";
+    private static final String KEY_FACTORY = "EC"; // aka "ECDSA"
 
-    private static final String TAG = "WebApkValidator";
     private static byte[] sExpectedSignature;
+    private static byte[] sCommentSignedPublicKeyBytes;
+    private static PublicKey sCommentSignedPublicKey;
 
     /**
-     * Queries the PackageManager to determine whether a WebAPK can handle the URL. Ignores
-     * whether the user has selected a default handler for the URL and whether the default
-     * handler is the WebAPK.
+     * Queries the PackageManager to determine whether a WebAPK can handle the URL. Ignores whether
+     * the user has selected a default handler for the URL and whether the default handler is the
+     * WebAPK.
      *
-     * NOTE(yfriedman): This can fail if multiple WebAPKs can match the supplied url.
+     * <p>NOTE(yfriedman): This can fail if multiple WebAPKs can match the supplied url.
      *
      * @param context The application context.
      * @param url The url to check.
      * @return Package name of WebAPK which can handle the URL. Null if the url should not be
      * handled by a WebAPK.
      */
     public static String queryWebApkPackage(Context context, String url) {
         return findWebApkPackage(context, resolveInfosForUrl(context, url));
     }
 
     /**
-     * Queries the PackageManager to determine whether a WebAPK can handle the URL. Ignores
-     * whether the user has selected a default handler for the URL and whether the default
-     * handler is the WebAPK.
+     * Queries the PackageManager to determine whether a WebAPK can handle the URL. Ignores whether
+     * the user has selected a default handler for the URL and whether the default handler is the
+     * WebAPK.
      *
-     * NOTE: This can fail if multiple WebAPKs can match the supplied url.
+     * <p>NOTE: This can fail if multiple WebAPKs can match the supplied url.
      *
      * @param context The application context.
      * @param url The url to check.
      * @return Resolve Info of a WebAPK which can handle the URL. Null if the url should not be
-     * handled by a WebAPK.
+     *     handled by a WebAPK.
      */
     public static ResolveInfo queryResolveInfo(Context context, String url) {
         return findResolveInfo(context, resolveInfosForUrl(context, url));
     }
 
     /**
      * Fetches the list of resolve infos from the PackageManager that can handle the URL.
      *
      * @param context The application context.
      * @param url The url to check.
@@ skipping 15 matching lines @@
             selector.setComponent(null);
         }
         return context.getPackageManager().queryIntentActivities(
                 intent, PackageManager.GET_RESOLVED_FILTER);
     }
 
     /**
      * @param context The context to use to check whether WebAPK is valid.
      * @param infos The ResolveInfos to search.
      * @return Package name of the ResolveInfo which corresponds to a WebAPK. Null if none of the
-     * ResolveInfos corresponds to a WebAPK.
+     *     ResolveInfos corresponds to a WebAPK.
      */
     public static String findWebApkPackage(Context context, List<ResolveInfo> infos) {
         ResolveInfo resolveInfo = findResolveInfo(context, infos);
         if (resolveInfo != null) {
             return resolveInfo.activityInfo.packageName;
         }
         return null;
     }
 
     /**
@@ skipping 12 matching lines @@
         return null;
     }
 
     /**
      * Returns whether the provided WebAPK is installed and passes signature checks.
      * @param context A context
      * @param webappPackageName The package name to check
      * @return true iff the WebAPK is installed and passes security checks
      */
     public static boolean isValidWebApk(Context context, String webappPackageName) {
-        if (sExpectedSignature == null) {
-            Log.wtf(TAG, "WebApk validation failure - expected signature not set."
-                    + "missing call to WebApkValidator.initWithBrowserHostSignature");
+        if (sExpectedSignature == null || sCommentSignedPublicKeyBytes == null) {
+            Log.wtf(TAG,
+                    "WebApk validation failure - expected signature not set."
+                            + "missing call to WebApkValidator.initWithBrowserHostSignature");
         }
-        if (!webappPackageName.startsWith(WEBAPK_PACKAGE_PREFIX)) {
-            return false;
-        }
-        // check signature
         PackageInfo packageInfo = null;
         try {
             packageInfo = context.getPackageManager().getPackageInfo(webappPackageName,
-                    PackageManager.GET_SIGNATURES);
+                    PackageManager.GET_SIGNATURES | PackageManager.GET_META_DATA);
         } catch (NameNotFoundException e) {
             e.printStackTrace();
             Log.d(TAG, "WebApk not found");
             return false;
         }
+        if (isNotWebApkQuick(packageInfo)) {
+            return false;
+        }
+        if (verifyV1WebApk(packageInfo, webappPackageName)) {
+            return true;
+        }
 
-        final Signature[] arrSignatures = packageInfo.signatures;
-        if (arrSignatures != null && arrSignatures.length == 2) {
-            for (Signature signature : arrSignatures) {
-                if (Arrays.equals(sExpectedSignature, signature.toByteArray())) {
-                    Log.d(TAG, "WebApk valid - signature match!");
-                    return true;
+        return verifyCommentSignedWebApk(packageInfo);

[Yaron, 2017/04/04 15:43:29]

can we guard this behind an about:flag. Sam pointed out that it would be nice to
have this defaulted to off since it would enable us to offer developer
preview/publicly creation service while fleshing out launch-blocking work.

[ScottK, 2017/04/04 21:03:03]

I don't think we need a flag as the existing of a "webapk:" comment is, in
effect, the flag.
I think that adding a flag will just add extra steps for testing/testers.

[Yaron, 2017/04/06 15:34:46]

Explained off-thread why the flag might be advantageous in the short-term. Let's
decide that so we can move forward here

[Yaron, 2017/04/06 15:51:50]

Discussed with Sam off-thread and he proposed a good compromise: let's add a
flag for just non-org.chromium.webapk webapks. Then we can decouple the signing
logic from webapk policy

[ScottK, 2017/04/07 21:57:08]

Added a new flag `any-webapk-package-name'

+    }
+
+    /** Determine quickly whether this is definitely not a WebAPK */
+    private static boolean isNotWebApkQuick(PackageInfo packageInfo) {
+        if (packageInfo.signatures == null || packageInfo.signatures.length == 0
+                || packageInfo.signatures.length > 2) {
+            // Wrong number of signatures want 1 or 2.
+            return true;
+        }
+        if (packageInfo.applicationInfo == null || packageInfo.applicationInfo.metaData == null) {
+            Log.e(TAG, "no application info, or metaData retrieved.");
+            return true;
+        }
+        // Having the startURL in AndroidManifest.xml is a strong signal.
+        String startUrl = packageInfo.applicationInfo.metaData.getString(START_URL);
+        return TextUtils.isEmpty(startUrl);
+    }
+
+    private static boolean verifyV1WebApk(PackageInfo packageInfo, String webappPackageName) {
+        if (packageInfo.signatures.length != 2
+                || !webappPackageName.startsWith(WEBAPK_PACKAGE_PREFIX)) {
+            return false;
+        }
+        for (Signature signature : packageInfo.signatures) {
+            if (Arrays.equals(sExpectedSignature, signature.toByteArray())) {
+                Log.d(TAG, "WebApk valid - signature match!");
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /** Verify that the comment signed webapk matches the public key. */
+    private static boolean verifyCommentSignedWebApk(PackageInfo packageInfo) {
+        PublicKey commentSignedPublicKey;
+        try {
+            commentSignedPublicKey = getCommentSignedPublicKey();
+        } catch (Exception e) {
+            Log.e(TAG, "WebApk failed to get Public Key", e);
+            return false;
+        }
+        if (commentSignedPublicKey == null) {
+            Log.e(TAG, "WebApk validation failure - unable to decode public key");
+            return false;
+        }
+        if (packageInfo.applicationInfo == null || packageInfo.applicationInfo.sourceDir == null) {
+            Log.e(TAG, "WebApk validation failure - missing applicationInfo sourcedir");
+            return false;
+        }
+
+        String packageFilename = packageInfo.applicationInfo.sourceDir;
+        RandomAccessFile file = null;
+        FileChannel inChannel = null;
+        try {
+            file = new RandomAccessFile(packageFilename, "r");

[Yaron, 2017/04/04 15:43:29]

this is potentially doing file io on the main thread so if we need to do it, you
need to add a StrictMode exemptoin (e.g.
https://cs.chromium.org/chromium/src/ui/android/java/src/org/chromium/ui/UiUt...)

Along with other Strictmode violations, we should add a histogram to monitor
this.

[ScottK, 2017/04/04 21:03:02]

I've added a file StrictMode allowThreadDiskReads.
I was unable to add the histogram because chrome/android/webapk/DEPS doesn't
allow anything from //base.
Maybe in another CL?

+            inChannel = file.getChannel();
+
+            MappedByteBuffer buf =
+                    inChannel.map(FileChannel.MapMode.READ_ONLY, 0, inChannel.size());
+            buf.load();
+
+            WebApkVerifySignature v = new WebApkVerifySignature(buf);
+            int result = v.read();
+            if (result != WebApkVerifySignature.ERROR_OK) {
+                Log.e(TAG, String.format("Failure reading %s: %s", packageFilename, result));
+                return false;
+            }
+            result = v.verifySignature(commentSignedPublicKey);
+            Log.d(TAG, "File " + packageFilename + ": " + result);

[Yaron, 2017/04/04 15:43:29]

remove log statement

[ScottK, 2017/04/04 21:03:02]

I think in the short term this might be useful to make sure it's getting here
and why it failed to verify (as the result is swallowed), what do you think?
Added a TODO.

+            return result == WebApkVerifySignature.ERROR_OK;
+        } catch (Exception e) {
+            Log.e(TAG, "WebApk file error for file " + packageFilename, e);
+            return false;
+        } finally {
+            if (inChannel != null) {
+                try {
+                    inChannel.close();
+                } catch (IOException e) {
+                }
+            }
+            if (file != null) {
+                try {
+                    file.close();
+                } catch (IOException e) {
                 }
             }
         }
-        Log.d(TAG, "WebApk invalid");
-        return false;
     }
 
     /**
-     * Initializes the WebApkValidator with the expected signature that WebAPKs must be signed
-     * with for the current host.
-     * @param expectedSignature
+     * Initializes the WebApkValidator with the expected signature that WebAPKs must be signed with
+     * for the current host.
+     * @param expectedSignature Old WebAPK RSA signature.

[Yaron, 2017/04/04 15:43:29]

Please avoid using "old" and "new". They are two different signing schemes but
V1 is unlikely to go away

[ScottK, 2017/04/04 21:03:03]

I think that v1 *is* likely to go away, as we start creating WebAPKs on the WAM
server that are singly signed (with the comment as the dual-signing).
Eventually, we can have clank force a re-minting of WebApks that use the v1
signing scheme.

+     * @param v2PublicKeyBytes New comment signed public key bytes as x509 encoded public key.
      */
     @SuppressFBWarnings("EI_EXPOSE_STATIC_REP2")
-    public static void initWithBrowserHostSignature(byte[] expectedSignature) {
-        if (sExpectedSignature != null) {
-            return;
+    public static void initWithBrowserHostSignature(
+            byte[] expectedSignature, byte[] v2PublicKeyBytes) {
+        if (sExpectedSignature == null) {
+            sExpectedSignature = expectedSignature;
         }
-        sExpectedSignature = expectedSignature;
+        if (sCommentSignedPublicKeyBytes == null) {
+            sCommentSignedPublicKeyBytes = v2PublicKeyBytes;
+        }
+    }
+
+    /**
+     * Lazy evaluate the creation of the Public Key as the KeyFactories may not yet be initialized.
+     * @return The decoded PublicKey or null
+     */
+    private static PublicKey getCommentSignedPublicKey() throws Exception {
+        if (sCommentSignedPublicKey == null) {
+            sCommentSignedPublicKey =
+                    KeyFactory.getInstance(KEY_FACTORY)
+                            .generatePublic(new X509EncodedKeySpec(sCommentSignedPublicKeyBytes));
+        }
+        return sCommentSignedPublicKey;
     }
 }