Relax permissions on package-specific paths.
Jeff Sharkey [Mon, 13 Jul 2015 17:25:58 +0000 (10:25 -0700)]
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
src/com/android/providers/downloads/DownloadInfo.java
src/com/android/providers/downloads/DownloadProvider.java
src/com/android/providers/downloads/Helpers.java
src/com/android/providers/downloads/StorageUtils.java

index df65cda..0104493 100644 (file)
@@ -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"
index 2423c0d..bee5c4a 100644 (file)
@@ -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;
index 2087c22..b080d70 100644 (file)
@@ -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);
         }
     }
 
index 0aa49c0..098d11b 100644 (file)
 
 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;
     }
 
index 1817c75..3bb57c8 100644 (file)
@@ -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());