From ed30deae5fe5b9de142b44933001c9b098c47712 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Mon, 13 Jul 2015 10:25:58 -0700
Subject: Relax permissions on package-specific paths.

Normally apps must hold the WRITE_EXTERNAL_STORAGE permission in
order to use DownloadManager.  However, now that the platform has
relaxed permissions on package-specific directories, we relax the
DownloadManager check in a similar way.  This also opens up using
DownloadManager to save files on secondary external storage devices.

Fix bug so that we now check the relevant volume state when thinking
about resuming a download.

Bug: 22135060
Change-Id: If439340ea48789ea167f49709b5b69a4f0883150
---
 AndroidManifest.xml                                |  1 +
 .../android/providers/downloads/DownloadInfo.java  | 14 +++-
 .../providers/downloads/DownloadProvider.java      | 47 +++++++++----
 src/com/android/providers/downloads/Helpers.java   | 78 +++++++++++++++++++---
 .../android/providers/downloads/StorageUtils.java  |  1 +
 5 files changed, 115 insertions(+), 26 deletions(-)

diff --git a/AndroidManifest.xml b/AndroidManifest.xml
index df65cdaa..0104493d 100644
--- a/AndroidManifest.xml
+++ b/AndroidManifest.xml
@@ -56,6 +56,7 @@
     <uses-permission android:name="android.permission.MODIFY_NETWORK_ACCOUNTING" />
     <uses-permission android:name="android.permission.CLEAR_APP_CACHE" />
     <uses-permission android:name="android.permission.WAKE_LOCK" />
+    <uses-permission android:name="android.permission.UPDATE_APP_OPS_STATS" />
 
     <application android:process="android.process.media"
                  android:label="@string/app_label"
diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index 2423c0d7..bee5c4a9 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -16,6 +16,8 @@
 
 package com.android.providers.downloads;
 
+import static com.android.providers.downloads.Constants.TAG;
+
 import android.app.DownloadManager;
 import android.content.ContentResolver;
 import android.content.ContentUris;
@@ -31,12 +33,14 @@ import android.os.Environment;
 import android.provider.Downloads;
 import android.provider.Downloads.Impl;
 import android.text.TextUtils;
+import android.util.Log;
 import android.util.Pair;
 
 import com.android.internal.annotations.GuardedBy;
 import com.android.internal.util.IndentingPrintWriter;
 
 import java.io.CharArrayWriter;
+import java.io.File;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -344,7 +348,15 @@ public class DownloadInfo {
                 return restartTime(now) <= now;
             case Downloads.Impl.STATUS_DEVICE_NOT_FOUND_ERROR:
                 // is the media mounted?
-                return Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED);
+                final Uri uri = Uri.parse(mUri);
+                if (ContentResolver.SCHEME_FILE.equals(uri.getScheme())) {
+                    final File file = new File(uri.getPath());
+                    return Environment.MEDIA_MOUNTED
+                            .equals(Environment.getExternalStorageState(file));
+                } else {
+                    Log.w(TAG, "Expected file URI on external storage: " + mUri);
+                    return false;
+                }
             case Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR:
                 // avoids repetition of retrying download
                 return false;
diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java
index 2087c227..b080d703 100644
--- a/src/com/android/providers/downloads/DownloadProvider.java
+++ b/src/com/android/providers/downloads/DownloadProvider.java
@@ -16,6 +16,7 @@
 
 package com.android.providers.downloads;
 
+import android.app.AppOpsManager;
 import android.app.DownloadManager;
 import android.app.DownloadManager.Request;
 import android.content.ContentProvider;
@@ -34,7 +35,6 @@ import android.database.sqlite.SQLiteDatabase;
 import android.database.sqlite.SQLiteOpenHelper;
 import android.net.Uri;
 import android.os.Binder;
-import android.os.Environment;
 import android.os.Handler;
 import android.os.HandlerThread;
 import android.os.ParcelFileDescriptor;
@@ -47,12 +47,12 @@ import android.text.TextUtils;
 import android.text.format.DateUtils;
 import android.util.Log;
 
+import libcore.io.IoUtils;
+
 import com.android.internal.util.IndentingPrintWriter;
 import com.google.android.collect.Maps;
 import com.google.common.annotations.VisibleForTesting;
 
-import libcore.io.IoUtils;
-
 import java.io.File;
 import java.io.FileDescriptor;
 import java.io.FileNotFoundException;
@@ -555,11 +555,19 @@ public final class DownloadProvider extends ContentProvider {
                 dest = Downloads.Impl.DESTINATION_CACHE_PARTITION;
             }
             if (dest == Downloads.Impl.DESTINATION_FILE_URI) {
-                getContext().enforcePermission(
-                        android.Manifest.permission.WRITE_EXTERNAL_STORAGE,
-                        Binder.getCallingPid(), Binder.getCallingUid(),
-                        "need WRITE_EXTERNAL_STORAGE permission to use DESTINATION_FILE_URI");
                 checkFileUriDestination(values);
+
+            } else if (dest == Downloads.Impl.DESTINATION_EXTERNAL) {
+                getContext().enforceCallingOrSelfPermission(
+                        android.Manifest.permission.WRITE_EXTERNAL_STORAGE,
+                        "No permission to write");
+
+                final AppOpsManager appOps = getContext().getSystemService(AppOpsManager.class);
+                if (appOps.noteOp(AppOpsManager.OP_WRITE_EXTERNAL_STORAGE, Binder.getCallingUid(),
+                        getCallingPackage()) != AppOpsManager.MODE_ALLOWED) {
+                    throw new SecurityException("No permission to write");
+                }
+
             } else if (dest == Downloads.Impl.DESTINATION_SYSTEMCACHE_PARTITION) {
                 getContext().enforcePermission(
                         android.Manifest.permission.ACCESS_CACHE_FILESYSTEM,
@@ -706,14 +714,25 @@ public final class DownloadProvider extends ContentProvider {
         if (path == null) {
             throw new IllegalArgumentException("Invalid file URI: " + uri);
         }
-        try {
-            final String canonicalPath = new File(path).getCanonicalPath();
-            final String externalPath = Environment.getExternalStorageDirectory().getAbsolutePath();
-            if (!canonicalPath.startsWith(externalPath)) {
-                throw new SecurityException("Destination must be on external storage: " + uri);
+
+        final File file = new File(path);
+        if (Helpers.isFilenameValidInExternalPackage(getContext(), file, getCallingPackage())) {
+            // No permissions required for paths belonging to calling package
+            return;
+        } else if (Helpers.isFilenameValidInExternal(getContext(), file)) {
+            // Otherwise we require write permission
+            getContext().enforceCallingOrSelfPermission(
+                    android.Manifest.permission.WRITE_EXTERNAL_STORAGE,
+                    "No permission to write to " + file);
+
+            final AppOpsManager appOps = getContext().getSystemService(AppOpsManager.class);
+            if (appOps.noteOp(AppOpsManager.OP_WRITE_EXTERNAL_STORAGE, Binder.getCallingUid(),
+                    getCallingPackage()) != AppOpsManager.MODE_ALLOWED) {
+                throw new SecurityException("No permission to write to " + file);
             }
-        } catch (IOException e) {
-            throw new SecurityException("Problem resolving path: " + uri);
+
+        } else {
+            throw new SecurityException("Unsupported path " + file);
         }
     }
 
diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java
index 0aa49c0a..098d11b7 100644
--- a/src/com/android/providers/downloads/Helpers.java
+++ b/src/com/android/providers/downloads/Helpers.java
@@ -16,6 +16,10 @@
 
 package com.android.providers.downloads;
 
+import static android.os.Environment.buildExternalStorageAppCacheDirs;
+import static android.os.Environment.buildExternalStorageAppFilesDirs;
+import static android.os.Environment.buildExternalStorageAppMediaDirs;
+import static android.os.Environment.buildExternalStorageAppObbDirs;
 import static com.android.providers.downloads.Constants.TAG;
 
 import android.content.Context;
@@ -23,6 +27,9 @@ import android.net.Uri;
 import android.os.Environment;
 import android.os.FileUtils;
 import android.os.SystemClock;
+import android.os.UserHandle;
+import android.os.storage.StorageManager;
+import android.os.storage.StorageVolume;
 import android.provider.Downloads;
 import android.util.Log;
 import android.webkit.MimeTypeMap;
@@ -334,31 +341,80 @@ public class Helpers {
         throw new IOException("Failed to generate an available filename");
     }
 
+    static boolean isFilenameValid(Context context, File file) {
+        return isFilenameValid(context, file, true);
+    }
+
+    static boolean isFilenameValidInExternal(Context context, File file) {
+        return isFilenameValid(context, file, false);
+    }
+
+    /**
+     * Test if given file exists in one of the package-specific external storage
+     * directories that are always writable to apps, regardless of storage
+     * permission.
+     */
+    static boolean isFilenameValidInExternalPackage(Context context, File file,
+            String packageName) {
+        try {
+            file = file.getCanonicalFile();
+
+            if (containsCanonical(buildExternalStorageAppFilesDirs(packageName), file) ||
+                    containsCanonical(buildExternalStorageAppObbDirs(packageName), file) ||
+                    containsCanonical(buildExternalStorageAppCacheDirs(packageName), file) ||
+                    containsCanonical(buildExternalStorageAppMediaDirs(packageName), file)) {
+                return true;
+            }
+        } catch (IOException e) {
+            Log.w(TAG, "Failed to resolve canonical path: " + e);
+            return false;
+        }
+
+        Log.w(TAG, "Path appears to be invalid: " + file);
+        return false;
+    }
+
     /**
      * Checks whether the filename looks legitimate for security purposes. This
      * prevents us from opening files that aren't actually downloads.
      */
-    static boolean isFilenameValid(Context context, File file) {
-        final File[] whitelist;
+    static boolean isFilenameValid(Context context, File file, boolean allowInternal) {
         try {
             file = file.getCanonicalFile();
-            whitelist = new File[] {
-                    context.getFilesDir().getCanonicalFile(),
-                    context.getCacheDir().getCanonicalFile(),
-                    Environment.getDownloadCacheDirectory().getCanonicalFile(),
-                    Environment.getExternalStorageDirectory().getCanonicalFile(),
-            };
+
+            if (allowInternal) {
+                if (containsCanonical(context.getFilesDir(), file)
+                        || containsCanonical(context.getCacheDir(), file)
+                        || containsCanonical(Environment.getDownloadCacheDirectory(), file)) {
+                    return true;
+                }
+            }
+
+            final StorageVolume[] volumes = StorageManager.getVolumeList(UserHandle.myUserId());
+            for (StorageVolume volume : volumes) {
+                if (containsCanonical(volume.getPathFile(), file)) {
+                    return true;
+                }
+            }
         } catch (IOException e) {
             Log.w(TAG, "Failed to resolve canonical path: " + e);
             return false;
         }
 
-        for (File testDir : whitelist) {
-            if (FileUtils.contains(testDir, file)) {
+        Log.w(TAG, "Path appears to be invalid: " + file);
+        return false;
+    }
+
+    private static boolean containsCanonical(File dir, File file) throws IOException {
+        return FileUtils.contains(dir.getCanonicalFile(), file);
+    }
+
+    private static boolean containsCanonical(File[] dirs, File file) throws IOException {
+        for (File dir : dirs) {
+            if (containsCanonical(dir, file)) {
                 return true;
             }
         }
-
         return false;
     }
 
diff --git a/src/com/android/providers/downloads/StorageUtils.java b/src/com/android/providers/downloads/StorageUtils.java
index 1817c758..3bb57c8e 100644
--- a/src/com/android/providers/downloads/StorageUtils.java
+++ b/src/com/android/providers/downloads/StorageUtils.java
@@ -105,6 +105,7 @@ public class StorageUtils {
             throw e.rethrowAsIOException();
         }
 
+        // TODO: teach about evicting caches on adopted secondary storage devices
         final long dataDev = getDeviceId(Environment.getDataDirectory());
         final long cacheDev = getDeviceId(Environment.getDownloadCacheDirectory());
         final long externalDev = getDeviceId(Environment.getExternalStorageDirectory());
-- 
cgit v1.2.3