am b74a55a3: am f5170c41: am 7a61721e: am 546250aa: am ed30deae: Relax permissions on package-specific paths.

* commit 'b74a55a36392d514f27952e7e2bf4af809ab48ae':
  Relax permissions on package-specific paths.

4 files changed, 114 insertions, 26 deletions

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());