Index: chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkValidator.java |
diff --git a/chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkValidator.java b/chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkValidator.java |
index 14d656ffb7c440a998c69acc4cf8401e2f68feba..1f556aab4b359570c4e8de5bf178fb0866a20adf 100644 |
--- a/chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkValidator.java |
+++ b/chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkValidator.java |
@@ -5,6 +5,7 @@ |
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; |
@@ -15,6 +16,16 @@ import android.content.pm.ResolveInfo; |
import android.content.pm.Signature; |
import android.util.Log; |
+import java.io.IOException; |
+import java.io.RandomAccessFile; |
+import java.nio.MappedByteBuffer; |
+import java.nio.channels.FileChannel; |
+import java.security.KeyFactory; |
+import java.security.NoSuchAlgorithmException; |
+import java.security.PublicKey; |
+import java.security.SignatureException; |
+import java.security.spec.InvalidKeySpecException; |
+import java.security.spec.X509EncodedKeySpec; |
import java.util.Arrays; |
import java.util.LinkedList; |
import java.util.List; |
@@ -24,16 +35,19 @@ import java.util.List; |
* Server. |
*/ |
public class WebApkValidator { |
- |
private static final String TAG = "WebApkValidator"; |
+ private static final String KEY_FACTORY = "EC"; // aka "ECDSA" |
+ |
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. |
@@ -45,16 +59,16 @@ public class WebApkValidator { |
} |
/** |
- * 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)); |
@@ -90,7 +104,7 @@ public class WebApkValidator { |
* @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); |
@@ -118,51 +132,175 @@ public class WebApkValidator { |
/** |
* 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 (!webappPackageName.startsWith(WEBAPK_PACKAGE_PREFIX)) { |
- return false; |
+ long now = System.nanoTime(); |
+ if (sExpectedSignature == null || sCommentSignedPublicKeyBytes == null) { |
+ Log.wtf(TAG, |
+ "WebApk validation failure - expected signature not set." |
+ + "missing call to WebApkValidator.initWithBrowserHostSignature"); |
} |
- // check signature |
PackageInfo packageInfo = null; |
try { |
packageInfo = context.getPackageManager().getPackageInfo(webappPackageName, |
- PackageManager.GET_SIGNATURES); |
+ PackageManager.GET_SIGNATURES | PackageManager.GET_META_DATA); |
pkotwicz
2017/03/26 01:37:04
Is this call slower when called with PackageManage
ScottK
2017/03/27 20:27:56
Doesn't seem like much, although I only tested twi
pkotwicz
2017/03/28 04:41:34
That seems negligible
|
} catch (NameNotFoundException e) { |
e.printStackTrace(); |
Log.d(TAG, "WebApk not found"); |
return false; |
} |
+ if (isNotWebApkQuick(packageInfo)) { |
+ logTime(now, "get packageInfo Quick"); |
+ return false; |
+ } |
+ |
+ if (isV1WebApk(packageInfo, webappPackageName)) { |
+ logTime(now, "isV1WebApk"); |
+ return foundV1Signature(packageInfo); |
+ } |
+ |
+ logTime(now, "isCommentSignedWebApk"); |
pkotwicz
2017/03/26 01:37:04
I think that it is useful to have a histogram whic
ScottK
2017/03/27 20:27:56
I've removed the timing logs.
|
+ try { |
+ return verifyCommentSignedWebApk(packageInfo); |
+ } catch (IOException e) { |
+ Log.e(TAG, "WebApk IOException", e); |
+ } catch (SignatureException e) { |
+ Log.e(TAG, "WebApk SignatureException", e); |
+ } catch (IllegalArgumentException e) { |
+ Log.e(TAG, "WebApk IllegalArgument", e); |
+ } |
+ return false; |
+ } |
+ |
+ /** 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) { |
pkotwicz
2017/03/26 01:37:04
I don't think that this check is useful anymore. I
ScottK
2017/03/27 20:27:56
If they have no signatures, that's bad.
If they ha
pkotwicz
2017/03/28 04:41:34
We should let the user sign their APK however many
ScottK
2017/03/28 20:56:21
You can't upload an APK with no signatures and I'm
pkotwicz
2017/03/31 03:59:23
I think that the checks that you have for the amou
ScottK
2017/04/03 17:44:17
So the META-INF checks are only checked if it's a
pkotwicz
2017/04/04 18:19:36
I think that is reasonable. You have a separate ch
|
+ // 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 (startUrl == null || startUrl.isEmpty()); |
+ } |
- 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; |
- } |
+ private static boolean isV1WebApk(PackageInfo packageInfo, String webappPackageName) { |
+ return packageInfo.signatures.length == 2 |
+ && webappPackageName.startsWith(WEBAPK_PACKAGE_PREFIX); |
+ } |
+ |
+ private static boolean foundV1Signature(PackageInfo packageInfo) { |
pkotwicz
2017/03/26 01:37:04
You can probably merge isV1WebApk() and foundV1Sig
ScottK
2017/03/27 20:27:55
Done.
|
+ for (Signature signature : packageInfo.signatures) { |
+ if (Arrays.equals(sExpectedSignature, signature.toByteArray())) { |
+ Log.d(TAG, "WebApk valid - signature match!"); |
+ return true; |
} |
} |
- Log.d(TAG, "WebApk invalid"); |
return false; |
} |
+ private static long logTime(long start, String message) { |
+ long now = System.nanoTime(); |
+ Log.d(TAG, String.format("%f ms: %s", (now - start) / 1E6, message)); |
+ return now; |
+ } |
+ |
+ /** Verify that the comment signed webapk matches the public key. */ |
+ private static boolean verifyCommentSignedWebApk(PackageInfo packageInfo) |
+ throws IOException, SignatureException, IllegalArgumentException { |
+ long now = System.nanoTime(); |
+ |
+ PublicKey CommentSignedPublicKey = getCommentSignedPublicKey(); |
+ 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; |
+ MappedByteBuffer buf = null; |
+ WebApkVerifySignature.Error verified; |
+ try { |
+ file = new RandomAccessFile(packageFilename, "r"); |
+ inChannel = file.getChannel(); |
+ buf = inChannel.map(FileChannel.MapMode.READ_ONLY, 0, inChannel.size()); |
+ buf.load(); |
+ |
+ now = logTime(now, "opening file " + packageFilename); |
+ |
+ WebApkVerifySignature v = new WebApkVerifySignature(buf); |
+ verified = v.read(); |
+ |
+ now = logTime(now, "read"); |
+ |
+ if (verified != WebApkVerifySignature.Error.OK) { |
+ Log.e(TAG, String.format("Failure reading %s: %s", packageFilename, verified)); |
+ return false; |
+ } |
+ verified = v.verifySignature(sCommentSignedPublicKey); |
+ Log.d(TAG, "File " + packageFilename + ": " + verified); |
+ } finally { |
+ if (buf != null) { |
+ buf.clear(); |
+ } |
+ if (inChannel != null) { |
+ inChannel.close(); |
+ } |
+ if (file != null) { |
+ file.close(); |
+ } |
+ logTime(now, "verify"); |
+ } |
+ return verified == WebApkVerifySignature.Error.OK; |
+ } |
+ |
/** |
- * Initializes the WebApkValidator with the expected signature that WebAPKs must be signed |
- * with for the current host. |
+ * Initializes the WebApkValidator with the expected signature that WebAPKs must be signed with |
+ * for the current host. |
+ * |
* @param expectedSignature |
+ * @param v2PublicKeyBytes |
*/ |
- public static void initWithBrowserHostSignature(byte[] expectedSignature) { |
- if (sExpectedSignature != null) { |
- return; |
+ public static void initWithBrowserHostSignature( |
+ byte[] expectedSignature, byte[] v2PublicKeyBytes) { |
+ if (sExpectedSignature == null) { |
+ sExpectedSignature = Arrays.copyOf(expectedSignature, expectedSignature.length); |
+ } |
+ if (sCommentSignedPublicKeyBytes == null) { |
+ sCommentSignedPublicKeyBytes = Arrays.copyOf(v2PublicKeyBytes, v2PublicKeyBytes.length); |
+ } |
pkotwicz
2017/03/26 01:37:04
Is there a reason that we are copying the array?
ScottK
2017/03/27 20:27:56
The original code has Arrays.copyOf() for sExpecte
ScottK
2017/03/28 20:56:21
The try bots didn't like this change. I think the
|
+ } |
+ |
+ /** |
+ * 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() { |
+ if (sCommentSignedPublicKey == null) { |
+ try { |
+ sCommentSignedPublicKey = KeyFactory.getInstance(KEY_FACTORY) |
+ .generatePublic(new X509EncodedKeySpec( |
+ sCommentSignedPublicKeyBytes)); |
+ } catch (InvalidKeySpecException e) { |
pkotwicz
2017/03/26 01:37:04
Nit: You can probably catch the generic Exception
ScottK
2017/03/27 20:27:56
done.
|
+ Log.e(TAG, "Failed to understand public key: " + e.getMessage()); |
+ } catch (NoSuchAlgorithmException e) { |
+ Log.e(TAG, "Failed to find KeyFactory: " + e.getMessage()); |
+ } |
} |
- sExpectedSignature = Arrays.copyOf(expectedSignature, expectedSignature.length); |
+ return sCommentSignedPublicKey; |
} |
} |