getInstalledRelatedApps: Introduce random delay to stop timing attacks.

content/public/android/java/src/org/chromium/content/browser/installedapp/InstalledAppProviderImpl.java

Index: content/public/android/java/src/org/chromium/content/browser/installedapp/InstalledAppProviderImpl.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/installedapp/InstalledAppProviderImpl.java b/content/public/android/java/src/org/chromium/content/browser/installedapp/InstalledAppProviderImpl.java
index d07abed0a3f938dd176d1d87d8d89a9ea214e5ea..a12a42d8add0ef78fbdee61ab86da7247d916add 100644
--- a/content/public/android/java/src/org/chromium/content/browser/installedapp/InstalledAppProviderImpl.java
+++ b/content/public/android/java/src/org/chromium/content/browser/installedapp/InstalledAppProviderImpl.java
@@ -109,10 +109,22 @@ public class InstalledAppProviderImpl implements InstalledAppProvider {
      * @param frameUrl Returns false if the Android package does not declare association with the
      *                origin of this URL. Can be null.
      */
-    private static boolean isAppInstalledAndAssociatedWithOrigin(
+    private boolean isAppInstalledAndAssociatedWithOrigin(
             String packageName, URI frameUrl, PackageManager pm) {
         if (frameUrl == null) return false;
 
+        // Important timing-attack prevention measure: delay by a pseudo-random amount of time, to
+        // add significant noise to the time taken to check whether this app is installed and
+        // related. Otherwise, it would be possible to tell whether a non-related app is installed,
+        // based on the time this operation takes.
+        //
+        // Generate a 16-bit hash based on a unique device ID + the package name.
+        short hash = PackageHash.hashForPackage(packageName);
+
+        // The time delay is the low 13 bits of the hash in μs (between 0 and 8.192ms).
+        int sleepUsec = hash & 0x1fff;
+        sleep(sleepUsec / 1000, (sleepUsec % 1000) * 1000);
+
         // Early-exit if the Android app is not installed.
         JSONArray statements;
         try {
@@ -247,4 +259,20 @@ public class InstalledAppProviderImpl implements InstalledAppProvider {
         return assetUrl.getScheme().equals(frameUrl.getScheme())
                 && assetUrl.getAuthority().equals(frameUrl.getAuthority());
     }
+
+    /**
+     * Puts the current thread to sleep for a given amount of time.
+     *
+     * This is approximate but should at least be accurate to the millisecond.
+     *
+     * Protected and non-static for testing.
+     */
+    protected void sleep(long millis, int nanos) {
+        try {
+            Thread.sleep(millis, nanos);

[Matt Giuca, 2017/04/11 08:34:08]

tedchoc: I think this Thread.sleep might be bad (if I set this to a huge number,
it blocks the main browser UI).

It's not terrible, since it's designed to sleep on the order of 0-8ms which is
about how long the computation will take anyway. But it would be better if we
can post a delayed task instead of just sleeping the thread.

I am not familiar with the JS side of things (in C++ it would be a
PostDelayedTask). Can you recommend how to do it? Or is the Thread.sleep OK?

[Ted C, 2017/04/11 16:01:32]

We definitely shouldn't sleep the UI thread.  That will cause a StrictMode
violation (a red flash showing we're doing bad things).

Can this call be made async?  If you were do do something like:

private [void] isAppInstalledAndAssociatedWithOrigin(
    String packageName, URI frameUrl, PackageManager pm, [final
Callback<Boolean> callback]) {
    new Handler().postDelayed(new Runnable() {
        @Override
        public void run() {
            callback.call(helperMethodThatDoesStuffForRealz(packageName,
frameUrl, pm);
        }
    }, <random delay>);
}

FWIW 8ms is half a render frame, so that is actually not something we should do
lightly.

[Matt Giuca, 2017/04/12 01:05:58]

Oh yeah, I saw that red flash and didn't know what it was.
Thanks, that looks like exactly what I want.
Given that it can take ~8ms (or in extreme cases, double that) to actually
process the JSON, is this something we also should be doing on another thread
instead of doing this operation here on the UI thread? (Which is why I ask above
about whether postDelayed is going to use the UI thread or a worker thread.)

It looks like (from the Handler documentation) that the above code will still
run that helperMethodThatDoesStuffForRealz code on the main thread, and I would
have to do more work to farm it off to a separate thread.

+        } catch (InterruptedException e) {
+            // Continue to interrupt the thread after this completes.
+            Thread.currentThread().interrupt();
+        }
+    }
 }