FUSE readdir: enforce restrictions based on target SDK
Legacy app with storage permission will bypass FUSE and files list will
be obtained from lower file system instead of MediaProvider database.
Legacy app with no permissions can only list files in its external media
directory.
Bug: 144990065
Test: atest -c LegacyAccessHostTest#testListFiles_hasR
atest -c LegacyAccessHostTest#testReadOnlyExternalStorage_hasR
Change-Id: Ic867c8d885ab8ee471891d254dee73a499f8c8fd
diff --git a/jni/MediaProviderWrapper.cpp b/jni/MediaProviderWrapper.cpp
index 48ee19f..8cbdcca 100644
--- a/jni/MediaProviderWrapper.cpp
+++ b/jni/MediaProviderWrapper.cpp
@@ -176,6 +176,21 @@
}
int de_count = env->GetArrayLength(files_list.get());
+ if (de_count == 1) {
+ ScopedLocalRef<jstring> j_d_name(env,
+ (jstring)env->GetObjectArrayElement(files_list.get(), 0));
+ ScopedUtfChars d_name(env, j_d_name.get());
+ if (d_name.c_str() == nullptr) {
+ LOG(ERROR) << "Error reading file name returned from MediaProvider at index " << 0;
+ directory_entries.push_back(std::make_shared<DirectoryEntry>("", EFAULT));
+ return directory_entries;
+ } else if (d_name.c_str()[0] == '\0') {
+ // Calling package has no storage permissions.
+ directory_entries.push_back(std::make_shared<DirectoryEntry>("", EPERM));
+ return directory_entries;
+ }
+ }
+
for (int i = 0; i < de_count; i++) {
ScopedLocalRef<jstring> j_d_name(env,
(jstring)env->GetObjectArrayElement(files_list.get(), i));
@@ -184,8 +199,7 @@
if (d_name.c_str() == nullptr) {
LOG(ERROR) << "Error reading file name returned from MediaProvider at index " << i;
directory_entries.resize(0);
- directory_entries.insert(directory_entries.begin(),
- std::make_shared<DirectoryEntry>("", EFAULT));
+ directory_entries.push_back(std::make_shared<DirectoryEntry>("", EFAULT));
break;
}
directory_entries.push_back(std::make_shared<DirectoryEntry>(d_name.c_str(), DT_REG));
diff --git a/jni/ReaddirHelper.cpp b/jni/ReaddirHelper.cpp
index fc97ee7..1fd1e7b 100644
--- a/jni/ReaddirHelper.cpp
+++ b/jni/ReaddirHelper.cpp
@@ -43,8 +43,7 @@
if (errno) {
PLOG(ERROR) << "DEBUG: readdir(): readdir failed with %d" << errno;
directory_entries->resize(0);
- directory_entries->insert(directory_entries->begin(),
- std::make_shared<DirectoryEntry>("", errno));
+ directory_entries->push_back(std::make_shared<DirectoryEntry>("", errno));
}
break;
}
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index ce08c18..d0b74c8 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -973,8 +973,11 @@
* @return a list of file names in the given directory path.
* An empty list is returned if no files are visible to the calling app or the given directory
* does not have any files.
- * A list with ["/"] is returned if the path is not indexed by MediaProvider database and
- * file names should be obtained from lower file system.
+ * A list with ["/"] is returned if the path is not indexed by MediaProvider database or
+ * calling package is a legacy app and has appropriate storage permissions for the given path.
+ * In both scenarios file names should be obtained from lower file system.
+ * A list with empty string[""] is returned if the package requesting legacy behavior doesn't
+ * have storage permissions for the given path.
* Directory names are always obtained from lower file system.
*
* Called from JNI in jni/MediaProviderWrapper.cpp
@@ -984,7 +987,21 @@
final LocalCallingIdentity token = clearLocalCallingIdentity(
LocalCallingIdentity.fromExternal(getContext(), uid));
try {
- //TODO(b/144990065): enforce based on target SDK
+ if (shouldBypassFuseRestrictions(/*forWrite*/ false)) {
+ return new String[] {"/"};
+ }
+
+ // Allow legacy app without storage permissions to list files only in its external
+ // media directory.
+ if (isCallingPackageRequestingLegacy()) {
+ final String appSpecificDir = extractPathOwnerPackageName(path);
+ if ((appSpecificDir != null &&
+ isCallingIdentitySharedPackageName(appSpecificDir))) {
+ return new String[] {"/"};
+ } else {
+ return new String[] {""};
+ }
+ }
// Get relative path for the contents of given directory.
String relativePath = extractRelativePathForDirectory(path);
diff --git a/tests/jni/FuseDaemonTest/host/src/com/android/tests/fused/host/LegacyAccessHostTest.java b/tests/jni/FuseDaemonTest/host/src/com/android/tests/fused/host/LegacyAccessHostTest.java
index f88deb3..8342568 100644
--- a/tests/jni/FuseDaemonTest/host/src/com/android/tests/fused/host/LegacyAccessHostTest.java
+++ b/tests/jni/FuseDaemonTest/host/src/com/android/tests/fused/host/LegacyAccessHostTest.java
@@ -19,6 +19,7 @@
import static org.junit.Assert.assertTrue;
import static com.google.common.truth.Truth.assertThat;
+import com.android.tradefed.device.ITestDevice;
import com.android.tradefed.testtype.DeviceJUnit4ClassRunner;
import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test;
@@ -65,9 +66,27 @@
}
}
- private void createFile(String filePath) throws Exception {
- executeShellCommand("touch " + filePath);
- assertThat(getDevice().doesFileExist(filePath)).isTrue();
+ /**
+ * Creates a file {@code filePath} in shell and may bypass Media Provider restrictions for
+ * creating file.
+ */
+ private void createFileAsShell(String filePath, boolean bypassFuse) throws Exception {
+ if (bypassFuse) {
+ // Run shell as root to bypass Media Provider.
+ final ITestDevice device = getDevice();
+ final boolean isAdbRoot = device.isAdbRoot() ? true : false;
+ if (!isAdbRoot) {
+ device.enableAdbRoot();
+ }
+ executeShellCommand("touch " + filePath);
+ if (!isAdbRoot) {
+ device.disableAdbRoot();
+ }
+ assertThat(getDevice().doesFileExist(filePath)).isTrue();
+ } else {
+ executeShellCommand("touch " + filePath);
+ assertThat(getDevice().doesFileExist(filePath)).isTrue();
+ }
}
@Before
@@ -102,7 +121,7 @@
@Test
public void testReadOnlyExternalStorage_hasR() throws Exception {
revokePermissions("android.permission.WRITE_EXTERNAL_STORAGE");
- createFile(SHELL_FILE);
+ createFileAsShell(SHELL_FILE, /*bypassFuse*/ true);
try {
runDeviceTest("testReadOnlyExternalStorage_hasR");
} finally {
@@ -114,11 +133,23 @@
public void testCantAccessExternalStorage() throws Exception {
revokePermissions("android.permission.WRITE_EXTERNAL_STORAGE",
"android.permission.READ_EXTERNAL_STORAGE");
- createFile(SHELL_FILE);
+ createFileAsShell(SHELL_FILE, /*bypassFuse*/ true);
try {
runDeviceTest("testCantAccessExternalStorage");
} finally {
executeShellCommand("rm " + SHELL_FILE);
}
}
+
+ @Test
+ public void testListFiles_hasR() throws Exception {
+ revokePermissions("android.permission.WRITE_EXTERNAL_STORAGE");
+ createFileAsShell(SHELL_FILE, /*bypassFuse*/ true);
+ try {
+ runDeviceTest("testListFiles_hasR");
+ } finally {
+ executeShellCommand("rm " + SHELL_FILE);
+ }
+
+ }
}
diff --git a/tests/jni/FuseDaemonTest/legacy/src/com/android/tests/fused/legacy/LegacyFileAccessTest.java b/tests/jni/FuseDaemonTest/legacy/src/com/android/tests/fused/legacy/LegacyFileAccessTest.java
index 39efda1..597c947 100644
--- a/tests/jni/FuseDaemonTest/legacy/src/com/android/tests/fused/legacy/LegacyFileAccessTest.java
+++ b/tests/jni/FuseDaemonTest/legacy/src/com/android/tests/fused/legacy/LegacyFileAccessTest.java
@@ -37,6 +37,7 @@
import java.io.File;
import java.io.FileDescriptor;
import java.io.IOException;
+import java.util.Arrays;
import java.util.concurrent.TimeUnit;
/**
@@ -161,11 +162,29 @@
// try to list a directory and fail
assertThat(new File(Environment.getExternalStorageDirectory(),
Environment.DIRECTORY_MUSIC).list()).isNull();
+ assertThat(Environment.getExternalStorageDirectory().list()).isNull();
// However, even without permissions, we can access our own external dir
file = new File(InstrumentationRegistry.getContext().getExternalFilesDir(null),
"LegacyFileAccessTest");
- assertCanCreateFile(file);
+ try {
+ assertThat(file.createNewFile()).isTrue();
+ assertThat(Arrays.asList(file.getParentFile().list()))
+ .containsExactly("LegacyFileAccessTest");
+ } finally {
+ file.delete();
+ }
+
+ // we can access our own external media directory without permissions.
+ file = new File(InstrumentationRegistry.getContext().getExternalMediaDirs()[0],
+ "LegacyFileAccessTest");
+ try {
+ assertThat(file.createNewFile()).isTrue();
+ assertThat(Arrays.asList(file.getParentFile().list()))
+ .containsExactly("LegacyFileAccessTest");
+ } finally {
+ file.delete();
+ }
}
// test read storage permission
@@ -211,6 +230,19 @@
.mkdir()).isFalse();
}
+ /*
+ * Test that legacy app with storage permission can list all files
+ */
+ @Test
+ public void testListFiles_hasR() throws Exception {
+ pollForPermission(Manifest.permission.READ_EXTERNAL_STORAGE, /*granted*/ true);
+ pollForPermission(Manifest.permission.WRITE_EXTERNAL_STORAGE, /*granted*/ false);
+
+ // can list a non-media file created by other package.
+ assertThat(Arrays.asList(Environment.getExternalStorageDirectory().list()))
+ .contains("LegacyAccessHostTest_shell");
+ }
+
private static void assertCanCreateFile(File file) throws IOException {
if (file.exists()) {
file.delete();