From 6f753b39e4fb69280a3ef013e37dc88398975489 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Mon, 1 Aug 2016 10:24:24 -0600 Subject: Enforce calling identity before clearing. When opening a downloaded file, enforce that the caller can actually see the requested download before clearing their identity to read internal columns. Bug: 30537115 Change-Id: I01bbad7997e5e908bfb19f5d576860a24f59f295 (cherry picked from commit 8be3a92eb0b4105a9ed748be5a937ce79145f565) --- src/com/android/providers/downloads/DownloadProvider.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java index d9acc789..667a81df 100644 --- a/src/com/android/providers/downloads/DownloadProvider.java +++ b/src/com/android/providers/downloads/DownloadProvider.java @@ -1232,6 +1232,19 @@ public final class DownloadProvider extends ContentProvider { logVerboseOpenFileInfo(uri, mode); } + // Perform normal query to enforce caller identity access before + // clearing it to reach internal-only columns + final Cursor probeCursor = query(uri, new String[] { + Downloads.Impl._DATA }, null, null, null); + try { + if ((probeCursor == null) || (probeCursor.getCount() == 0)) { + throw new FileNotFoundException( + "No file found for " + uri + " as UID " + Binder.getCallingUid()); + } + } finally { + IoUtils.closeQuietly(probeCursor); + } + final Cursor cursor = queryCleared(uri, new String[] { Downloads.Impl._DATA, Downloads.Impl.COLUMN_STATUS, Downloads.Impl.COLUMN_DESTINATION, Downloads.Impl.COLUMN_MEDIA_SCANNED }, null, -- cgit v1.2.3 From 3f2cf47caf2cb2eafcf94bf127098e1179b7325e Mon Sep 17 00:00:00 2001 From: Adam Seaton Date: Fri, 26 Aug 2016 21:13:20 +0000 Subject: Revert "Enforce calling identity before clearing." This reverts commit 8be3a92eb0b4105a9ed748be5a937ce79145f565. Change-Id: I10401d57239b868f8e3514f81a0e20486838e29c (cherry picked from commit b440ceb00fd46c9233723066c680a538067fbf82) --- src/com/android/providers/downloads/DownloadProvider.java | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java index 667a81df..d9acc789 100644 --- a/src/com/android/providers/downloads/DownloadProvider.java +++ b/src/com/android/providers/downloads/DownloadProvider.java @@ -1232,19 +1232,6 @@ public final class DownloadProvider extends ContentProvider { logVerboseOpenFileInfo(uri, mode); } - // Perform normal query to enforce caller identity access before - // clearing it to reach internal-only columns - final Cursor probeCursor = query(uri, new String[] { - Downloads.Impl._DATA }, null, null, null); - try { - if ((probeCursor == null) || (probeCursor.getCount() == 0)) { - throw new FileNotFoundException( - "No file found for " + uri + " as UID " + Binder.getCallingUid()); - } - } finally { - IoUtils.closeQuietly(probeCursor); - } - final Cursor cursor = queryCleared(uri, new String[] { Downloads.Impl._DATA, Downloads.Impl.COLUMN_STATUS, Downloads.Impl.COLUMN_DESTINATION, Downloads.Impl.COLUMN_MEDIA_SCANNED }, null, -- cgit v1.2.3 From 1f66449c3bd4328116b2b2377f1a2d284e669578 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Fri, 16 Sep 2016 12:12:17 -0600 Subject: Enforce calling identity before clearing. When opening a downloaded file, enforce that the caller can actually see the requested download before clearing their identity to read internal columns. However, this means that we can no longer return the "my_downloads" paths: if those Uris were shared beyond the app that requested the download, access would be denied. Instead, we need to switch to using "all_downloads" Uris so that permission grants can be issued to third-party viewer apps. Since an app requesting a download doesn't normally have permission to "all_downloads" paths, we issue narrow grants toward the owner of each download, both at device boot and when new downloads are started. Bug: 30537115, 30945409 Change-Id: If944aada020878a91c363963728d0da9f6fae3ea (cherry picked from commit 7c1af8c62c8bdf6e8de5a00c1927daf9fd9c03d1) --- .../providers/downloads/DownloadProvider.java | 44 ++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java index d9acc789..13ab5455 100644 --- a/src/com/android/providers/downloads/DownloadProvider.java +++ b/src/com/android/providers/downloads/DownloadProvider.java @@ -462,6 +462,19 @@ public final class DownloadProvider extends ContentProvider { if (appInfo != null) { mDefContainerUid = appInfo.uid; } + + // Grant access permissions for all known downloads to the owning apps + final SQLiteDatabase db = mOpenHelper.getReadableDatabase(); + final Cursor cursor = db.query(DB_TABLE, new String[] { + Downloads.Impl._ID, Constants.UID }, null, null, null, null, null); + try { + while (cursor.moveToNext()) { + grantAllDownloadsPermission(cursor.getLong(0), cursor.getInt(1)); + } + } finally { + cursor.close(); + } + // start the DownloadService class. don't wait for the 1st download to be issued. // saves us by getting some initialization code in DownloadService out of the way. Context context = getContext(); @@ -687,6 +700,7 @@ public final class DownloadProvider extends ContentProvider { } insertRequestHeaders(db, rowID, values); + grantAllDownloadsPermission(rowID, Binder.getCallingUid()); notifyContentChanged(uri, match); // Always start service to handle notifications and/or scanning @@ -1193,6 +1207,7 @@ public final class DownloadProvider extends ContentProvider { try { while (cursor.moveToNext()) { final long id = cursor.getLong(0); + revokeAllDownloadsPermission(id); DownloadStorageProvider.onDownloadProviderDelete(getContext(), id); final String path = cursor.getString(1); @@ -1232,6 +1247,19 @@ public final class DownloadProvider extends ContentProvider { logVerboseOpenFileInfo(uri, mode); } + // Perform normal query to enforce caller identity access before + // clearing it to reach internal-only columns + final Cursor probeCursor = query(uri, new String[] { + Downloads.Impl._DATA }, null, null, null); + try { + if ((probeCursor == null) || (probeCursor.getCount() == 0)) { + throw new FileNotFoundException( + "No file found for " + uri + " as UID " + Binder.getCallingUid()); + } + } finally { + IoUtils.closeQuietly(probeCursor); + } + final Cursor cursor = queryCleared(uri, new String[] { Downloads.Impl._DATA, Downloads.Impl.COLUMN_STATUS, Downloads.Impl.COLUMN_DESTINATION, Downloads.Impl.COLUMN_MEDIA_SCANNED }, null, @@ -1413,4 +1441,20 @@ public final class DownloadProvider extends ContentProvider { to.put(key, defaultValue); } } + + private void grantAllDownloadsPermission(long id, int uid) { + final String[] packageNames = getContext().getPackageManager().getPackagesForUid(uid); + if (packageNames == null || packageNames.length == 0) return; + + // We only need to grant to the first package, since the + // platform internally tracks based on UIDs + final Uri uri = ContentUris.withAppendedId(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, id); + getContext().grantUriPermission(packageNames[0], uri, + Intent.FLAG_GRANT_READ_URI_PERMISSION | Intent.FLAG_GRANT_WRITE_URI_PERMISSION); + } + + private void revokeAllDownloadsPermission(long id) { + final Uri uri = ContentUris.withAppendedId(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, id); + getContext().revokeUriPermission(uri, ~0); + } } -- cgit v1.2.3