Merge "Modify PerformanceTests to test for renames and greater size" into mainline-prod
diff --git a/jni/FuseDaemon.cpp b/jni/FuseDaemon.cpp
old mode 100644
new mode 100755
index b6deb60..234d5ff
--- a/jni/FuseDaemon.cpp
+++ b/jni/FuseDaemon.cpp
@@ -517,8 +517,8 @@
if (pkg == ".nomedia") {
return true;
}
- if (!mp->IsUidForPackage(pkg, uid)) {
- PLOG(WARNING) << "Invalid other package file access from " << pkg << "(: " << path;
+ if (!mp->isUidAllowedAccessToDataOrObbPath(uid, path)) {
+ PLOG(WARNING) << "Invalid other package file access from " << uid << "(: " << path;
return false;
}
}
@@ -1383,7 +1383,8 @@
// Ignore lookup errors on
// 1. non-existing files returned from MediaProvider database.
// 2. path that doesn't match FuseDaemon UID and calling uid.
- if (error_code == ENOENT || error_code == EPERM || error_code == EACCES) continue;
+ if (error_code == ENOENT || error_code == EPERM || error_code == EACCES
+ || error_code == EIO) continue;
fuse_reply_err(req, error_code);
return;
}
diff --git a/jni/MediaProviderWrapper.cpp b/jni/MediaProviderWrapper.cpp
index b0db1ca..fb72961 100644
--- a/jni/MediaProviderWrapper.cpp
+++ b/jni/MediaProviderWrapper.cpp
@@ -149,11 +149,12 @@
return res;
}
-bool isUidForPackageInternal(JNIEnv* env, jobject media_provider_object,
- jmethodID mid_is_uid_for_package, const string& pkg, uid_t uid) {
- ScopedLocalRef<jstring> j_pkg(env, env->NewStringUTF(pkg.c_str()));
- bool res = env->CallBooleanMethod(media_provider_object, mid_is_uid_for_package, j_pkg.get(),
- uid);
+bool isUidAllowedAccessToDataOrObbPathInternal(JNIEnv* env, jobject media_provider_object,
+ jmethodID mid_is_uid_allowed_path_access_, uid_t uid,
+ const string& path) {
+ ScopedLocalRef<jstring> j_path(env, env->NewStringUTF(path.c_str()));
+ bool res = env->CallBooleanMethod(media_provider_object, mid_is_uid_allowed_path_access_, uid,
+ j_path.get());
if (CheckForJniException(env)) {
return false;
@@ -277,8 +278,9 @@
/*is_static*/ false);
mid_rename_ = CacheMethod(env, "rename", "(Ljava/lang/String;Ljava/lang/String;I)I",
/*is_static*/ false);
- mid_is_uid_for_package_ = CacheMethod(env, "isUidForPackage", "(Ljava/lang/String;I)Z",
- /*is_static*/ false);
+ mid_is_uid_allowed_access_to_data_or_obb_path_ =
+ CacheMethod(env, "isUidAllowedAccessToDataOrObbPath", "(ILjava/lang/String;)Z",
+ /*is_static*/ false);
mid_on_file_created_ = CacheMethod(env, "onFileCreated", "(Ljava/lang/String;)V",
/*is_static*/ false);
mid_should_allow_lookup_ = CacheMethod(env, "shouldAllowLookup", "(II)Z",
@@ -400,13 +402,14 @@
forWrite);
}
-bool MediaProviderWrapper::IsUidForPackage(const string& pkg, uid_t uid) {
+bool MediaProviderWrapper::isUidAllowedAccessToDataOrObbPath(uid_t uid, const string& path) {
if (shouldBypassMediaProvider(uid)) {
return true;
}
JNIEnv* env = MaybeAttachCurrentThread();
- return isUidForPackageInternal(env, media_provider_object_, mid_is_uid_for_package_, pkg, uid);
+ return isUidAllowedAccessToDataOrObbPathInternal(
+ env, media_provider_object_, mid_is_uid_allowed_access_to_data_or_obb_path_, uid, path);
}
int MediaProviderWrapper::Rename(const string& old_path, const string& new_path, uid_t uid) {
diff --git a/jni/MediaProviderWrapper.h b/jni/MediaProviderWrapper.h
index 4e931e8..ce5a5b1 100644
--- a/jni/MediaProviderWrapper.h
+++ b/jni/MediaProviderWrapper.h
@@ -137,14 +137,19 @@
int IsOpendirAllowed(const std::string& path, uid_t uid, bool forWrite);
/**
- * Determines if the given package name matches its uid
- * or has special access to priv-app directories
+ * Determines if one of the follows is true:
+ * 1. The package name of the given private path matches the given uid,
+ then this uid has access to private-app directories for this package.
+ * 2. The calling uid has special access to private-app directories:
+ * * DownloadProvider and ExternalStorageProvider has access to private
+ * app directories.
+ * * Installer apps have access to Android/obb directories
*
- * @param pkg the package name of the app
* @param uid UID of the app
+ * @param path the private path that the UID wants to access
* @return true if it matches, otherwise return false.
*/
- bool IsUidForPackage(const std::string& pkg, uid_t uid);
+ bool isUidAllowedAccessToDataOrObbPath(uid_t uid, const std::string& path);
/**
* Renames a file or directory to new path.
@@ -202,7 +207,7 @@
jmethodID mid_is_opendir_allowed_;
jmethodID mid_get_files_in_dir_;
jmethodID mid_rename_;
- jmethodID mid_is_uid_for_package_;
+ jmethodID mid_is_uid_allowed_access_to_data_or_obb_path_;
jmethodID mid_on_file_created_;
jmethodID mid_should_allow_lookup_;
jmethodID mid_is_app_clone_user_;
diff --git a/src/com/android/providers/media/LocalCallingIdentity.java b/src/com/android/providers/media/LocalCallingIdentity.java
index 4d3700b..ba22182 100644
--- a/src/com/android/providers/media/LocalCallingIdentity.java
+++ b/src/com/android/providers/media/LocalCallingIdentity.java
@@ -23,6 +23,7 @@
import static com.android.providers.media.util.PermissionUtils.checkIsLegacyStorageGranted;
import static com.android.providers.media.util.PermissionUtils.checkPermissionDelegator;
+import static com.android.providers.media.util.PermissionUtils.checkPermissionInstallPackages;
import static com.android.providers.media.util.PermissionUtils.checkPermissionManager;
import static com.android.providers.media.util.PermissionUtils.checkPermissionReadAudio;
import static com.android.providers.media.util.PermissionUtils.checkPermissionReadImages;
@@ -214,7 +215,9 @@
public static final int PERMISSION_WRITE_VIDEO = 1 << 20;
public static final int PERMISSION_WRITE_IMAGES = 1 << 21;
- public static final int PERMISSION_IS_SYSTEM_GALLERY = 1 <<22;
+ public static final int PERMISSION_IS_SYSTEM_GALLERY = 1 << 22;
+ public static final int PERMISSION_INSTALL_PACKAGES = 1 << 23;
+ public static final int PERMISSION_WRITE_EXTERNAL_STORAGE = 1 << 24;
private int hasPermission;
private int hasPermissionResolved;
@@ -256,6 +259,10 @@
case PERMISSION_IS_LEGACY_WRITE:
return isLegacyWriteInternal();
+ case PERMISSION_WRITE_EXTERNAL_STORAGE:
+ return checkPermissionWriteStorage(
+ context, pid, uid, getPackageName(), attributionTag);
+
case PERMISSION_READ_AUDIO:
return checkPermissionReadAudio(
context, pid, uid, getPackageName(), attributionTag);
@@ -277,6 +284,9 @@
case PERMISSION_IS_SYSTEM_GALLERY:
return checkWriteImagesOrVideoAppOps(
context, uid, getPackageName(), attributionTag);
+ case PERMISSION_INSTALL_PACKAGES:
+ return checkPermissionInstallPackages(
+ context, pid, uid, getPackageName(), attributionTag);
default:
return false;
}
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index 50aefbb..bb93287 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -39,6 +39,7 @@
import static com.android.providers.media.DatabaseHelper.EXTERNAL_DATABASE_NAME;
import static com.android.providers.media.DatabaseHelper.INTERNAL_DATABASE_NAME;
+import static com.android.providers.media.LocalCallingIdentity.PERMISSION_INSTALL_PACKAGES;
import static com.android.providers.media.LocalCallingIdentity.PERMISSION_IS_DELEGATOR;
import static com.android.providers.media.LocalCallingIdentity.PERMISSION_IS_LEGACY_GRANTED;
import static com.android.providers.media.LocalCallingIdentity.PERMISSION_IS_LEGACY_READ;
@@ -52,6 +53,7 @@
import static com.android.providers.media.LocalCallingIdentity.PERMISSION_READ_IMAGES;
import static com.android.providers.media.LocalCallingIdentity.PERMISSION_READ_VIDEO;
import static com.android.providers.media.LocalCallingIdentity.PERMISSION_WRITE_AUDIO;
+import static com.android.providers.media.LocalCallingIdentity.PERMISSION_WRITE_EXTERNAL_STORAGE;
import static com.android.providers.media.LocalCallingIdentity.PERMISSION_WRITE_IMAGES;
import static com.android.providers.media.LocalCallingIdentity.PERMISSION_WRITE_VIDEO;
import static com.android.providers.media.scan.MediaScanner.REASON_DEMAND;
@@ -69,6 +71,7 @@
import static com.android.providers.media.util.FileUtils.extractVolumePath;
import static com.android.providers.media.util.FileUtils.getAbsoluteSanitizedPath;
import static com.android.providers.media.util.FileUtils.isDataOrObbPath;
+import static com.android.providers.media.util.FileUtils.isObbOrChildPath;
import static com.android.providers.media.util.FileUtils.isDownload;
import static com.android.providers.media.util.FileUtils.sanitizePath;
import static com.android.providers.media.util.Logging.LOGV;
@@ -328,7 +331,7 @@
private static final int sUserId = UserHandle.myUserId();
- // WARNING/TODO: This will be replaced by signature APIs in S
+ // WARNING/TODO (b/173505864): This will be replaced by signature APIs in S
private static final String DOWNLOADS_PROVIDER_AUTHORITY = "downloads";
@GuardedBy("mShouldRedactThreadIds")
@@ -779,7 +782,8 @@
break;
case FileColumns.MEDIA_TYPE_PLAYLIST:
- consumer.accept(Audio.Playlists.Members.getContentUri(volumeName, id));
+ consumer.accept(ContentUris.withAppendedId(
+ MediaStore.Audio.Playlists.getContentUri(volumeName), id));
break;
}
@@ -1672,20 +1676,22 @@
clearLocalCallingIdentity(getCachedCallingIdentityForFuse(uid));
try {
- if (isPrivatePackagePathNotOwnedByCaller(path)) {
- return new String[] {""};
- }
-
- // Do not allow apps to list Android/data or Android/obb dirs. Installer and
- // MOUNT_EXTERNAL_ANDROID_WRITABLE apps won't be blocked by this, as their OBB dirs
- // are mounted to lowerfs directly.
- if (isDataOrObbPath(path)) {
+ if (isPrivatePackagePathNotAccessibleByCaller(path)) {
return new String[] {""};
}
if (shouldBypassFuseRestrictions(/*forWrite*/ false, path)) {
return new String[] {"/"};
}
+
+ // Do not allow apps to list Android/data or Android/obb dirs.
+ // On primary volumes, apps that get special access to these directories get it via
+ // mount views of lowerfs. On secondary volumes, such apps would return early from
+ // shouldBypassFuseRestrictions above (except for MTP apps b/174347304).
+ if (isDataOrObbPath(path)) {
+ return new String[] {""};
+ }
+
// Legacy apps that made is this far don't have the right storage permission and hence
// are not allowed to access anything other than their external app directory
if (isCallingPackageRequestingLegacy()) {
@@ -2234,8 +2240,8 @@
clearLocalCallingIdentity(getCachedCallingIdentityForFuse(uid));
try {
- if (isPrivatePackagePathNotOwnedByCaller(oldPath)
- || isPrivatePackagePathNotOwnedByCaller(newPath)) {
+ if (isPrivatePackagePathNotAccessibleByCaller(oldPath)
+ || isPrivatePackagePathNotAccessibleByCaller(newPath)) {
return OsConstants.EACCES;
}
@@ -3595,6 +3601,7 @@
return attachedVolume;
}
+ final DatabaseHelper helper = getDatabaseForUri(uri);
switch (match) {
case AUDIO_PLAYLISTS_ID:
case AUDIO_PLAYLISTS_ID_MEMBERS: {
@@ -3618,8 +3625,8 @@
// files on disk to ensure that we can reliably migrate between
// devices and recover from database corruption
final long id = addPlaylistMembers(playlistUri, initialValues);
- final ContentResolver resolver = getContext().getContentResolver();
- resolver.notifyChange(playlistUri, null, ContentResolver.NOTIFY_INSERT);
+ acceptWithExpansion(helper::notifyInsert, resolvedVolumeName, playlistId,
+ FileColumns.MEDIA_TYPE_PLAYLIST, false);
return ContentUris.withAppendedId(MediaStore.Audio.Playlists.Members
.getContentUri(originalVolumeName, playlistId), id);
}
@@ -3701,7 +3708,6 @@
long rowId = -1;
Uri newUri = null;
- final DatabaseHelper helper = getDatabaseForUri(uri);
final SQLiteQueryBuilder qb = getQueryBuilder(TYPE_INSERT, match, uri, extras, null);
switch (match) {
@@ -4634,6 +4640,7 @@
count = 1;
}
+ final DatabaseHelper helper = getDatabaseForUri(uri);
switch (match) {
case AUDIO_PLAYLISTS_ID_MEMBERS_ID:
extras.putString(QUERY_ARG_SQL_SELECTION,
@@ -4649,14 +4656,13 @@
// devices and recover from database corruption
int numOfRemovedPlaylistMembers = removePlaylistMembers(playlistUri, extras);
if (numOfRemovedPlaylistMembers > 0) {
- final ContentResolver resolver = getContext().getContentResolver();
- resolver.notifyChange(playlistUri, null, ContentResolver.NOTIFY_DELETE);
+ acceptWithExpansion(helper::notifyDelete, volumeName, playlistId,
+ FileColumns.MEDIA_TYPE_PLAYLIST, false);
}
return numOfRemovedPlaylistMembers;
}
}
- final DatabaseHelper helper = getDatabaseForUri(uri);
final SQLiteQueryBuilder qb = getQueryBuilder(TYPE_DELETE, match, uri, extras, null);
{
@@ -5336,6 +5342,7 @@
final int targetSdkVersion = getCallingPackageTargetSdkVersion();
final boolean allowHidden = isCallingPackageAllowedHidden();
final int match = matchUri(uri, allowHidden);
+ final DatabaseHelper helper = getDatabaseForUri(uri);
switch (match) {
case AUDIO_PLAYLISTS_ID_MEMBERS_ID:
@@ -5378,13 +5385,12 @@
addPlaylistMembers(playlistUri, values);
}
- final ContentResolver resolver = getContext().getContentResolver();
- resolver.notifyChange(playlistUri, null, ContentResolver.NOTIFY_UPDATE);
+ acceptWithExpansion(helper::notifyUpdate, volumeName, playlistId,
+ FileColumns.MEDIA_TYPE_PLAYLIST, false);
return 1;
}
}
- final DatabaseHelper helper = getDatabaseForUri(uri);
final SQLiteQueryBuilder qb = getQueryBuilder(TYPE_UPDATE, match, uri, extras, null);
// Give callers interacting with a specific media item a chance to
@@ -6700,7 +6706,7 @@
* <li>the calling identity is an app targeting Q or older versions AND is requesting legacy
* storage
* <li>the calling identity holds {@code MANAGE_EXTERNAL_STORAGE}
- * <li>the calling identity owns filePath (eg /Android/data/com.foo)
+ * <li>the calling identity owns or has access to the filePath (eg /Android/data/com.foo)
* <li>the calling identity has permission to write images and the given file is an image file
* <li>the calling identity has permission to write video and the given file is an video file
* </ul>
@@ -6716,9 +6722,8 @@
return true;
}
- // Files under the apps own private directory
- final String appSpecificDir = extractPathOwnerPackageName(filePath);
- if (appSpecificDir != null && isCallingIdentitySharedPackageName(appSpecificDir)) {
+ // Check if the caller has access to private app directories.
+ if (isUidAllowedAccessToDataOrObbPathForFuse(mCallingIdentity.get().uid, filePath)) {
return true;
}
@@ -6733,9 +6738,10 @@
/**
* Returns true if the passed in path is an application-private data directory
- * (such as Android/data/com.foo or Android/obb/com.foo) that does not belong to the caller.
+ * (such as Android/data/com.foo or Android/obb/com.foo) that does not belong to the caller and
+ * the caller does not have special access.
*/
- private boolean isPrivatePackagePathNotOwnedByCaller(String path) {
+ private boolean isPrivatePackagePathNotAccessibleByCaller(String path) {
// Files under the apps own private directory
final String appSpecificDir = extractPathOwnerPackageName(path);
@@ -6749,9 +6755,7 @@
if (relativePath.startsWith("Android/media")) {
return false;
}
-
- // This is a private-package path; return true if not owned by the caller
- return !isCallingIdentitySharedPackageName(appSpecificDir);
+ return !isUidAllowedAccessToDataOrObbPathForFuse(mCallingIdentity.get().uid, path);
}
private boolean shouldBypassDatabaseAndSetDirtyForFuse(int uid, String path) {
@@ -7017,7 +7021,7 @@
clearLocalCallingIdentity(getCachedCallingIdentityForFuse(uid));
try {
- if (isPrivatePackagePathNotOwnedByCaller(path)) {
+ if (isPrivatePackagePathNotAccessibleByCaller(path)) {
Log.e(TAG, "Can't open a file in another app's external directory!");
return OsConstants.ENOENT;
}
@@ -7219,7 +7223,7 @@
clearLocalCallingIdentity(getCachedCallingIdentityForFuse(uid));
try {
- if (isPrivatePackagePathNotOwnedByCaller(path)) {
+ if (isPrivatePackagePathNotAccessibleByCaller(path)) {
Log.e(TAG, "Can't create a file in another app's external directory");
return OsConstants.ENOENT;
}
@@ -7343,7 +7347,7 @@
final LocalCallingIdentity token =
clearLocalCallingIdentity(getCachedCallingIdentityForFuse(uid));
try {
- if (isPrivatePackagePathNotOwnedByCaller(path)) {
+ if (isPrivatePackagePathNotAccessibleByCaller(path)) {
Log.e(TAG, "Can't delete a file in another app's external directory!");
return OsConstants.ENOENT;
}
@@ -7407,7 +7411,7 @@
try {
// App dirs are not indexed, so we don't create an entry for the file.
- if (isPrivatePackagePathNotOwnedByCaller(path)) {
+ if (isPrivatePackagePathNotAccessibleByCaller(path)) {
Log.e(TAG, "Can't modify another app's external directory!");
return OsConstants.EACCES;
}
@@ -7461,21 +7465,23 @@
if ("/storage/emulated".equals(path)) {
return OsConstants.EPERM;
}
- if (isPrivatePackagePathNotOwnedByCaller(path)) {
+ if (isPrivatePackagePathNotAccessibleByCaller(path)) {
Log.e(TAG, "Can't access another app's external directory!");
return OsConstants.ENOENT;
}
- // Do not allow apps to open Android/data or Android/obb dirs. Installer and
- // MOUNT_EXTERNAL_ANDROID_WRITABLE apps won't be blocked by this, as their OBB dirs
- // are mounted to lowerfs directly.
- if (isDataOrObbPath(path)) {
- return OsConstants.EACCES;
- }
-
if (shouldBypassFuseRestrictions(forWrite, path)) {
return 0;
}
+
+ // Do not allow apps to open Android/data or Android/obb dirs.
+ // On primary volumes, apps that get special access to these directories get it via
+ // mount views of lowerfs. On secondary volumes, such apps would return early from
+ // shouldBypassFuseRestrictions above (except for MTP apps b/174347304).
+ if (isDataOrObbPath(path)) {
+ return OsConstants.EACCES;
+ }
+
// Legacy apps that made is this far don't have the right storage permission and hence
// are not allowed to access anything other than their external app directory
if (isCallingPackageRequestingLegacy()) {
@@ -7509,24 +7515,87 @@
}
@Keep
- public boolean isUidForPackageForFuse(@NonNull String packageName, int uid) {
+ public boolean isUidAllowedAccessToDataOrObbPathForFuse(int uid, String path) {
final LocalCallingIdentity token =
clearLocalCallingIdentity(getCachedCallingIdentityForFuse(uid));
try {
- return isCallingIdentitySharedPackageName(packageName) ||
- isCallingIdentityAllowedPrivAppAccess(uid);
+ // Files under the apps own private directory
+ final String appSpecificDir = extractPathOwnerPackageName(path);
+
+ if (appSpecificDir != null && isCallingIdentitySharedPackageName(appSpecificDir)) {
+ return true;
+ }
+ // This is a private-package path; return true if accessible by the caller
+ return isUidAllowedSpecialPrivatePathAccess(uid, path);
} finally {
restoreLocalCallingIdentity(token);
}
}
/**
- * External Storage Provider and Download Provider can access priv app directories.
+ * @return true iff the caller has installer privileges which gives write access to obb dirs.
+ * <p> Assumes that {@code mCallingIdentity} has been properly set to reflect the calling
+ * package.
+ */
+ private boolean isCallingIdentityAllowedInstallerAccess(int uid) {
+ final boolean hasWrite = mCallingIdentity.get().
+ hasPermission(PERMISSION_WRITE_EXTERNAL_STORAGE);
+
+ if (!hasWrite) {
+ return false;
+ }
+
+ // We're only willing to give out installer access if they also hold
+ // runtime permission; this is a firm CDD requirement
+ final boolean hasInstall = mCallingIdentity.get().
+ hasPermission(PERMISSION_INSTALL_PACKAGES);
+
+ if (hasInstall) {
+ return true;
+ }
+ // OPSTR_REQUEST_INSTALL_PACKAGES is granted/denied per package but vold can't
+ // update mountpoints of a specific package. So, check the appop for all packages
+ // sharing the uid and allow same level of storage access for all packages even if
+ // one of the packages has the appop granted.
+ // To maintain consistency of access in primary volume and secondary volumes use the same
+ // logic as we do for Zygote.MOUNT_EXTERNAL_INSTALLER view.
+ // TODO (b/173600911): Improve performance by caching this info.
+ for (String uidPackageName : mCallingIdentity.get().getSharedPackageNames()) {
+ if (mAppOpsManager.unsafeCheckOpRawNoThrow(AppOpsManager.OPSTR_REQUEST_INSTALL_PACKAGES,
+ uid, uidPackageName) == AppOpsManager.MODE_ALLOWED) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private boolean isCallingIdentityDownloadProvider(int uid) {
+ return uid == mDownloadsAuthorityAppId;
+ }
+
+ private boolean isCallingIdentityExternalStorageProvider(int uid) {
+ return uid == mExternalStorageAuthorityAppId;
+ }
+
+ /**
+ * ExternalStorageProvider and DownloadProvider can access all private-app directories.
+ * Installer apps can only access private-app directories on Android/obb.
+ * TODO (b/174347304): Allow MTP apps special access.
*
* @param uid UID of the calling package
*/
- private boolean isCallingIdentityAllowedPrivAppAccess(int uid) {
- return (uid == mExternalStorageAuthorityAppId) || (uid == mDownloadsAuthorityAppId);
+ private boolean isUidAllowedSpecialPrivatePathAccess(int uid, String path) {
+ final LocalCallingIdentity token =
+ clearLocalCallingIdentity(getCachedCallingIdentityForFuse(uid));
+ try {
+ if (isCallingIdentityDownloadProvider(uid) ||
+ isCallingIdentityExternalStorageProvider(uid)) {
+ return true;
+ }
+ return (isObbOrChildPath(path) && isCallingIdentityAllowedInstallerAccess(uid));
+ } finally {
+ restoreLocalCallingIdentity(token);
+ }
}
private boolean checkCallingPermissionGlobal(Uri uri, boolean forWrite) {
diff --git a/src/com/android/providers/media/scan/ModernMediaScanner.java b/src/com/android/providers/media/scan/ModernMediaScanner.java
index 6d4eb00..c5d5e55 100644
--- a/src/com/android/providers/media/scan/ModernMediaScanner.java
+++ b/src/com/android/providers/media/scan/ModernMediaScanner.java
@@ -168,7 +168,8 @@
// |excludeDirs * 2| < 1000 which is the max SQL expression size
// Because we add |excludeDir| and |excludeDir/| in the SQL expression to match dir and subdirs
// See SQLITE_MAX_EXPR_DEPTH in sqlite3.c
- private static final int MAX_EXCLUDE_DIRS = 450;
+ @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+ static final int MAX_EXCLUDE_DIRS = 450;
private static final Pattern PATTERN_VISIBLE = Pattern.compile(
"(?i)^/storage/[^/]+(?:/[0-9]+)?(?:/Android/sandbox/([^/]+))?$");
diff --git a/src/com/android/providers/media/util/FileUtils.java b/src/com/android/providers/media/util/FileUtils.java
index 42237ce..a1f24c8 100644
--- a/src/com/android/providers/media/util/FileUtils.java
+++ b/src/com/android/providers/media/util/FileUtils.java
@@ -925,6 +925,12 @@
public static final Pattern PATTERN_DATA_OR_OBB_PATH = Pattern.compile(
"(?i)^/storage/[^/]+/(?:[0-9]+/)?Android/(?:data|obb)/?$");
+ /**
+ * Regex that matches Android/obb paths.
+ */
+ public static final Pattern PATTERN_OBB_OR_CHILD_PATH = Pattern.compile(
+ "(?i)^/storage/[^/]+/(?:[0-9]+/)?Android/(?:obb)(/?.*)");
+
@VisibleForTesting
public static final String[] DEFAULT_FOLDER_NAMES = {
Environment.DIRECTORY_MUSIC,
@@ -1055,6 +1061,15 @@
}
/**
+ * Returns true if relative path is Android/obb path.
+ */
+ public static boolean isObbOrChildPath(String path) {
+ if (path == null) return false;
+ final Matcher m = PATTERN_OBB_OR_CHILD_PATH.matcher(path);
+ return m.matches();
+ }
+
+ /**
* Returns the name of the top level directory, or null if the path doesn't go through the
* external storage directory.
*/
diff --git a/src/com/android/providers/media/util/PermissionUtils.java b/src/com/android/providers/media/util/PermissionUtils.java
index adbe0e2..9593b18 100644
--- a/src/com/android/providers/media/util/PermissionUtils.java
+++ b/src/com/android/providers/media/util/PermissionUtils.java
@@ -17,6 +17,7 @@
package com.android.providers.media.util;
import static android.Manifest.permission.BACKUP;
+import static android.Manifest.permission.INSTALL_PACKAGES;
import static android.Manifest.permission.MANAGE_EXTERNAL_STORAGE;
import static android.Manifest.permission.READ_EXTERNAL_STORAGE;
import static android.Manifest.permission.UPDATE_DEVICE_STATS;
@@ -185,6 +186,12 @@
generateAppOpMessage(packageName, sOpDescription.get()));
}
+ public static boolean checkPermissionInstallPackages(@NonNull Context context, int pid, int uid,
+ @NonNull String packageName, @Nullable String attributionTag) {
+ return checkPermissionForDataDelivery(context, INSTALL_PACKAGES, pid,
+ uid, packageName, attributionTag, null);
+ }
+
/**
* Returns {@code true} if the given package has write images or write video app op, which
* indicates the package is a system gallery.
diff --git a/tests/client/src/com/android/providers/media/client/ClientPlaylistTest.java b/tests/client/src/com/android/providers/media/client/ClientPlaylistTest.java
index 9b2fadd..9aaaae2 100644
--- a/tests/client/src/com/android/providers/media/client/ClientPlaylistTest.java
+++ b/tests/client/src/com/android/providers/media/client/ClientPlaylistTest.java
@@ -123,22 +123,24 @@
mValues.put(MediaColumns.MIME_TYPE, mMimeType);
final Uri playlistUri = mContentResolver.insert(mExternalPlaylists, mValues);
+ final Uri externalVolumePlaylistUri = getExternalVolumePlaylistUri(
+ ContentUris.parseId(playlistUri));
+
+ final TestContentObserverHelper obs = TestContentObserverHelper.create(
+ Arrays.asList(playlistUri, externalVolumePlaylistUri),
+ ContentResolver.NOTIFY_INSERT);
final Uri membersUri = MediaStore.Audio.Playlists.Members
.getContentUri(VOLUME_EXTERNAL_PRIMARY, ContentUris.parseId(playlistUri));
- final TestContentObserver obs = TestContentObserver.create(
- playlistUri, ContentResolver.NOTIFY_INSERT);
// Inserting without ordering will always append
mValues.clear();
mValues.put(Playlists.Members.AUDIO_ID, mRed);
Uri resultUri = mContentResolver.insert(membersUri, mValues);
obs.waitForChange();
- assertEquals(ContentUris.withAppendedId(membersUri, 1), resultUri);
mValues.put(Playlists.Members.AUDIO_ID, mGreen);
resultUri = mContentResolver.insert(membersUri, mValues);
obs.waitForChange();
- assertEquals(ContentUris.withAppendedId(membersUri, 2), resultUri);
assertMembers(Arrays.asList(
Pair.create(mRed, 1),
Pair.create(mGreen, 2)), queryMembers(membersUri));
@@ -149,7 +151,6 @@
mValues.put(Playlists.Members.PLAY_ORDER, 1);
resultUri = mContentResolver.insert(membersUri, mValues);
obs.waitForChange();
- assertEquals(ContentUris.withAppendedId(membersUri, 1), resultUri);
assertMembers(Arrays.asList(
Pair.create(mBlue, 1),
Pair.create(mRed, 2),
@@ -163,10 +164,12 @@
final long playlistId = createPlaylist(mRed, mGreen, mBlue);
Uri playlistUri = ContentUris.withAppendedId(
MediaStore.Audio.Playlists.getContentUri(VOLUME_EXTERNAL), playlistId);
+ Uri externalVolumePlaylistUri = getExternalVolumePlaylistUri(playlistId);
final Uri membersUri = Playlists.Members.getContentUri(VOLUME_EXTERNAL_PRIMARY, playlistId);
- TestContentObserver obs = TestContentObserver.create(
- playlistUri, ContentResolver.NOTIFY_UPDATE);
+ TestContentObserverHelper obs = TestContentObserverHelper.create(
+ Arrays.asList(playlistUri, externalVolumePlaylistUri),
+ ContentResolver.NOTIFY_UPDATE);
// Simple move forwards
@@ -187,9 +190,6 @@
Pair.create(mGreen, 2),
Pair.create(mBlue, 3)), queryMembers(membersUri));
- playlistUri = ContentUris.withAppendedId(
- MediaStore.Audio.Playlists.getContentUri(VOLUME_EXTERNAL_PRIMARY), playlistId);
- obs = TestContentObserver.create(playlistUri, ContentResolver.NOTIFY_UPDATE);
// Advanced moves using query args
mValues.clear();
mValues.put(Playlists.Members.PLAY_ORDER, 1);
@@ -220,8 +220,12 @@
final long playlistId = createPlaylist(mRed, mGreen, mBlue);
final Uri membersUri = Playlists.Members.getContentUri(VOLUME_EXTERNAL_PRIMARY, playlistId);
- final TestContentObserver obs = TestContentObserver.create(
- membersUri, ContentResolver.NOTIFY_DELETE);
+ final Uri playlistUri = ContentUris.withAppendedId(
+ MediaStore.Audio.Playlists.getContentUri(VOLUME_EXTERNAL_PRIMARY), playlistId);
+ final Uri externalVolumePlaylistUri = getExternalVolumePlaylistUri(playlistId);
+ final TestContentObserverHelper obs = TestContentObserverHelper.create(
+ Arrays.asList(playlistUri, externalVolumePlaylistUri),
+ ContentResolver.NOTIFY_DELETE);
// Simple delete in middle, duplicates are okay
int count = mContentResolver.delete(membersUri, Playlists.Members.PLAY_ORDER + "=?",
@@ -291,8 +295,11 @@
mValues.clear();
mValues.put(MediaColumns.DISPLAY_NAME, "Playlist " + System.nanoTime());
mValues.put(MediaColumns.MIME_TYPE, mMimeType);
- final TestContentObserver obs = TestContentObserver.create(
- mExternalPlaylists,ContentResolver.NOTIFY_INSERT);
+ final Uri externalVolumePlaylistUri = MediaStore.Audio.Playlists
+ .getContentUri(VOLUME_EXTERNAL_PRIMARY);
+ final TestContentObserverHelper obs = TestContentObserverHelper.create(
+ Arrays.asList(mExternalPlaylists, externalVolumePlaylistUri),
+ ContentResolver.NOTIFY_INSERT);
final Uri playlist = mContentResolver.insert(mExternalPlaylists, mValues);
obs.waitForChange();
@@ -313,6 +320,11 @@
return ContentUris.parseId(playlist);
}
+ private static Uri getExternalVolumePlaylistUri(long playlistId) {
+ return ContentUris.withAppendedId(
+ MediaStore.Audio.Playlists.getContentUri(VOLUME_EXTERNAL), playlistId);
+ }
+
private void assertMembers(List<Pair<Long, Integer>> expected,
List<Pair<Long, Integer>> actual) {
assertEquals(expected.toString(), actual.toString());
@@ -335,6 +347,39 @@
}
/**
+ * Observer that will wait for a specific change event to be delivered on all the given uris.
+ */
+ private static class TestContentObserverHelper {
+ private List<TestContentObserver> observers;
+
+ private TestContentObserverHelper(List<TestContentObserver> observers) {
+ this.observers = observers;
+ }
+
+ private static TestContentObserverHelper create(List<Uri> uris, int flags) {
+ List<TestContentObserver> observers = new ArrayList();
+ for (Uri uri : uris) {
+ final TestContentObserver observer = TestContentObserver.create(uri, flags);
+ observers.add(observer);
+ }
+ final TestContentObserverHelper obsWrapper = new TestContentObserverHelper(observers);
+ return obsWrapper;
+ }
+
+ private void waitForChange() {
+ for (TestContentObserver observer : observers) {
+ observer.waitForChange();
+ }
+ }
+
+ private void unregister() {
+ for (TestContentObserver observer : observers) {
+ observer.unregister();
+ }
+ }
+ }
+
+ /**
* Observer that will wait for a specific change event to be delivered.
*/
public static class TestContentObserver extends ContentObserver {
@@ -359,7 +404,7 @@
public static TestContentObserver create(Uri uri, int flags) {
final TestContentObserver obs = new TestContentObserver(flags);
InstrumentationRegistry.getContext().getContentResolver()
- .registerContentObserver(uri, true, obs);
+ .registerContentObserver(uri, true, obs);
return obs;
}
diff --git a/tests/client/src/com/android/providers/media/client/DownloadProviderTest.java b/tests/client/src/com/android/providers/media/client/DownloadProviderTest.java
new file mode 100644
index 0000000..fc58590
--- /dev/null
+++ b/tests/client/src/com/android/providers/media/client/DownloadProviderTest.java
@@ -0,0 +1,237 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.providers.media.client;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import static org.junit.Assert.assertTrue;
+
+import android.app.UiAutomation;
+import android.content.ContentResolver;
+import android.content.Context;
+import android.os.ParcelFileDescriptor;
+import android.os.storage.StorageManager;
+import android.provider.MediaStore;
+import android.util.Log;
+
+import androidx.test.InstrumentationRegistry;
+import androidx.test.runner.AndroidJUnit4;
+
+import com.google.common.io.ByteStreams;
+
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.function.Supplier;
+
+/**
+ * Verify DownloadProvider's access to app's private files on primary and public volumes.
+ * DownloadProvider and Media Provider client tests use the same shared UID
+ * `android:sharedUserId="android.media"` which helps us test DownloadProvider's behavior here
+ */
+@RunWith(AndroidJUnit4.class)
+public class DownloadProviderTest {
+
+ private static final String TAG = "DownloadProviderTest";
+ private static final String OTHER_PKG_NAME = "com.example.foo";
+ private static final long POLLING_TIMEOUT_MILLIS = TimeUnit.SECONDS.toMillis(1);
+ private static final long POLLING_SLEEP_MILLIS = 100;
+
+ @BeforeClass
+ public static void setUp() throws Exception {
+ createNewPublicVolume();
+ }
+
+ @AfterClass
+ public static void tearDown() throws Exception {
+ deletePublicVolumes();
+ }
+
+ private static void deletePublicVolumes() throws Exception {
+ executeShellCommand("sm set-virtual-disk false");
+ }
+
+ @Test
+ public void testCanReadWriteOtherAppPrivateFiles() throws Exception {
+ List<File> otherPackageDirs = createOtherPackageExternalFilesDir();
+ List<File> otherPackagePrivateFiles = createOtherPackagePrivateFile(otherPackageDirs);
+ for (File privateFile: otherPackagePrivateFiles) {
+ assertTrue(canOpenForWrite(privateFile));
+ }
+ deleteOtherPackageExternalFiles(otherPackageDirs);
+ }
+
+ @Test
+ public void testCanOpenOtherAppPrivateDir() throws Exception {
+ List<File> otherPackageDirs = createOtherPackageExternalFilesDir();
+ for (File privateDir: otherPackageDirs) {
+ String[] dirContents = privateDir.list();
+ assertThat(dirContents).asList().containsExactly("files");
+ }
+ deleteOtherPackageExternalFiles(otherPackageDirs);
+ }
+
+ private List<File> createOtherPackageExternalFilesDir() throws Exception {
+ final Context context = InstrumentationRegistry.getTargetContext();
+ Set<String> volNames = MediaStore.getExternalVolumeNames(context);
+ List<File> otherPackageDirs = new ArrayList();
+
+ for (String volName : volNames) {
+ File volPath = getVolumePath(context, volName);
+ // List of private external package files for other package on the same volume
+ List<String> otherPackageDirsOnSameVolume = new ArrayList();
+ final String otherPackageDataDir = volPath.getAbsolutePath() + "/Android/data/"
+ + OTHER_PKG_NAME;
+ otherPackageDirsOnSameVolume.add(otherPackageDataDir);
+ final String otherPackageObbDir = volPath.getAbsolutePath() + "/Android/obb/"
+ + OTHER_PKG_NAME;
+ otherPackageDirsOnSameVolume.add(otherPackageObbDir);
+
+ for (String dir: otherPackageDirsOnSameVolume) {
+ otherPackageDirs.add(new File(dir));
+ final String otherPackageExternalFilesDir = dir + "/files";
+ executeShellCommand("mkdir -p " + otherPackageExternalFilesDir + " -m 2770");
+ // Need to wait for the directory to be created, as the rest of the test depends on
+ // the dir to be created. A race condition can cause the test to be flaky.
+ pollForDirectoryToBeCreated(new File(otherPackageExternalFilesDir));
+ }
+ }
+ return otherPackageDirs;
+ }
+
+ private List<File> createOtherPackagePrivateFile(List<File> otherPackageDirs) throws Exception {
+ List<File> otherPackagePrivateFiles = new ArrayList();
+ for (File otherPackageDir : otherPackageDirs) {
+ final String otherPackagePrivateFile = otherPackageDir + "/files/test.txt";
+ otherPackagePrivateFiles.add(new File(otherPackagePrivateFile));
+ executeShellCommand("touch " + otherPackagePrivateFile);
+ }
+ return otherPackagePrivateFiles;
+ }
+
+ private void deleteOtherPackageExternalFiles(List<File> otherPackageDirs) throws Exception {
+ for (File dir: otherPackageDirs) {
+ executeShellCommand("rm -r " + dir.getAbsolutePath());
+ // Need to wait for the directory to be deleted, as it is flaky sometimes due to the
+ // race condition in rm and the next mkdir for the next test.
+ pollForDirectoryToBeDeleted(dir);
+ }
+ }
+
+ /**
+ * Returns whether we can open the file.
+ */
+ private static boolean canOpenForWrite(File file) {
+ try (FileOutputStream fis = new FileOutputStream(file)) {
+ return true;
+ } catch (IOException expected) {
+ return false;
+ }
+ }
+
+ private static File getVolumePath(Context context, String volumeName) {
+ return context.getSystemService(StorageManager.class)
+ .getStorageVolume(MediaStore.Files.getContentUri(volumeName)).getDirectory();
+ }
+
+ private static String executeShellCommand(String cmd) throws IOException {
+ UiAutomation uiAutomation = InstrumentationRegistry.getInstrumentation().getUiAutomation();
+ ParcelFileDescriptor pfd = uiAutomation.executeShellCommand(cmd);
+ try (FileInputStream output = new FileInputStream(pfd.getFileDescriptor())) {
+ return new String(ByteStreams.toByteArray(output));
+ }
+ }
+
+ private static void pollForCondition(Supplier<Boolean> condition, String errorMessage)
+ throws Exception {
+ for (int i = 0; i < POLLING_TIMEOUT_MILLIS / POLLING_SLEEP_MILLIS; i++) {
+ if (condition.get()) {
+ return;
+ }
+ Thread.sleep(POLLING_SLEEP_MILLIS);
+ }
+ throw new TimeoutException(errorMessage);
+ }
+
+ /**
+ * Polls for directory to be created
+ */
+ private static void pollForDirectoryToBeCreated(File dir) throws Exception {
+ pollForCondition(
+ () -> dir.exists(),
+ "Timed out while waiting for dir " + dir + " to be created");
+ }
+
+ /**
+ * Polls for directory to be deleted
+ */
+ private static void pollForDirectoryToBeDeleted(File dir) throws Exception {
+ pollForCondition(
+ () -> !dir.exists(),
+ "Timed out while waiting for dir " + dir + " to be deleted");
+ }
+
+ /**
+ * Creates a new virtual public volume and returns the volume's name.
+ */
+ private static void createNewPublicVolume() throws Exception {
+ executeShellCommand("sm set-force-adoptable on");
+ executeShellCommand("sm set-virtual-disk true");
+ pollForCondition(() -> partitionDisk(), "Timed out while waiting for"
+ + " disk partitioning");
+ pollForCondition(() -> isPublicVolumeMounted(), "Timed out while waiting for"
+ + " the public volume to mount");
+ }
+
+ private static boolean isPublicVolumeMounted() {
+ try {
+ final String publicVolume = executeShellCommand("sm list-volumes public").trim();
+ return publicVolume != null && publicVolume.contains("mounted");
+ } catch (Exception e) {
+ return false;
+ }
+ }
+
+ private static boolean partitionDisk() {
+ try {
+ final String listDisks = executeShellCommand("sm list-disks").trim();
+ if (listDisks.length() > 0) {
+ executeShellCommand("sm partition " + listDisks + " public");
+ return true;
+ }
+ return false;
+ } catch (Exception e) {
+ return false;
+ }
+ }
+}
diff --git a/tests/src/com/android/providers/media/scan/ModernMediaScannerTest.java b/tests/src/com/android/providers/media/scan/ModernMediaScannerTest.java
index f5109f1..4b4618d 100644
--- a/tests/src/com/android/providers/media/scan/ModernMediaScannerTest.java
+++ b/tests/src/com/android/providers/media/scan/ModernMediaScannerTest.java
@@ -18,6 +18,7 @@
import static com.android.providers.media.scan.MediaScanner.REASON_UNKNOWN;
import static com.android.providers.media.scan.MediaScannerTest.stage;
+import static com.android.providers.media.scan.ModernMediaScanner.MAX_EXCLUDE_DIRS;
import static com.android.providers.media.scan.ModernMediaScanner.shouldScanPathAndIsPathHidden;
import static com.android.providers.media.scan.ModernMediaScanner.isFileAlbumArt;
import static com.android.providers.media.scan.ModernMediaScanner.parseOptional;
@@ -743,12 +744,12 @@
@Test
public void testScan_Nomedia_Dir() throws Exception {
- final File red = new File(mDir, "red");
- final File blue = new File(mDir, "blue");
- red.mkdirs();
- blue.mkdirs();
- stage(R.raw.test_image, new File(red, "red.jpg"));
- stage(R.raw.test_image, new File(blue, "blue.jpg"));
+ final File redDir = new File(mDir, "red");
+ final File blueDir = new File(mDir, "blue");
+ redDir.mkdirs();
+ blueDir.mkdirs();
+ stage(R.raw.test_image, new File(redDir, "red.jpg"));
+ stage(R.raw.test_image, new File(blueDir, "blue.jpg"));
mModern.scanDirectory(mDir, REASON_UNKNOWN);
@@ -756,7 +757,7 @@
assertQueryCount(2, MediaStore.Images.Media.EXTERNAL_CONTENT_URI);
// Hide one directory, rescan, and confirm hidden
- final File redNomedia = new File(red, ".nomedia");
+ final File redNomedia = new File(redDir, ".nomedia");
redNomedia.createNewFile();
mModern.scanDirectory(mDir, REASON_UNKNOWN);
assertQueryCount(1, MediaStore.Images.Media.EXTERNAL_CONTENT_URI);
@@ -768,6 +769,35 @@
}
@Test
+ public void testScan_MaxExcludeNomediaDirs_DoesNotThrowException() throws Exception {
+ // Create MAX_EXCLUDE_DIRS + 50 nomedia dirs in mDir
+ // (Need to add 50 as MAX_EXCLUDE_DIRS is a safe limit;
+ // 499 would have been too close to the exception limit)
+ // Mark them as non-dirty so that they are excluded from scans
+ for (int i = 0 ; i < (MAX_EXCLUDE_DIRS + 50) ; i++) {
+ createCleanNomediaDir(mDir);
+ }
+
+ final File redDir = new File(mDir, "red");
+ redDir.mkdirs();
+ stage(R.raw.test_image, new File(redDir, "red.jpg"));
+
+ assertQueryCount(0, MediaStore.Images.Media.EXTERNAL_CONTENT_URI);
+ mModern.scanDirectory(mDir, REASON_UNKNOWN);
+ assertQueryCount(1, MediaStore.Images.Media.EXTERNAL_CONTENT_URI);
+ }
+
+ private void createCleanNomediaDir(File dir) throws Exception {
+ final File nomediaDir = new File(dir, "test_" + System.nanoTime());
+ nomediaDir.mkdirs();
+ final File nomedia = new File(nomediaDir, ".nomedia");
+ nomedia.createNewFile();
+
+ FileUtils.setDirectoryDirty(nomediaDir, false);
+ assertThat(FileUtils.isDirectoryDirty(nomediaDir)).isFalse();
+ }
+
+ @Test
public void testScan_Nomedia_File() throws Exception {
final File image = new File(mDir, "image.jpg");
final File nomedia = new File(mDir, ".nomedia");