Don't redact if caller is owner and add redaction test
Add check for owner package name when redacting a file, if the owner is
reading the file or of the reader has been granted write permission to
the corresponding URI, we don't redact.
Also add tests for redaction, however, no test for the URI permission
grant has been added, yet. Tracked in b/146346138.
Fixes: 145795132
Test: atest FuseDaemonHostTest
Change-Id: Ibf5900c53d27af9f17204f8f294f915c81b04614
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index 4bd282b..31aa54e 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -4980,7 +4980,38 @@
long[] res = new long[0];
try {
- if (isRedactionNeeded() && !shouldBypassFuseRestrictions(/*forWrite*/ false)) {
+ if (!isRedactionNeeded() || shouldBypassFuseRestrictions(/*forWrite*/ false)) {
+ return res;
+ }
+
+ final Uri contentUri = Files.getContentUri(MediaStore.getVolumeName(new File(path)));
+ final String[] projection = new String[]{
+ MediaColumns.OWNER_PACKAGE_NAME, MediaColumns._ID };
+ final String selection = MediaColumns.DATA + "=?";
+ final String[] selectionArgs = new String[] { path };
+ final String ownerPackageName;
+ final Uri item;
+ try (final Cursor c = queryForSingleItem(contentUri, projection, selection,
+ selectionArgs, null)) {
+ c.moveToFirst();
+ ownerPackageName = c.getString(0);
+ item = ContentUris.withAppendedId(contentUri, /*item id*/ c.getInt(1));
+ } catch (FileNotFoundException e) {
+ // Ideally, this shouldn't happen unless the file was deleted after we checked its
+ // existence and before we get to the redaction logic here. In this case we throw
+ // and fail the operation and FuseDaemon should handle this and fail the whole open
+ // operation gracefully.
+ throw new FileNotFoundException(
+ path + " not found while calculating redaction ranges: " + e.getMessage());
+ }
+
+ final boolean callerIsOwner = Objects.equals(getCallingPackageOrSelf(),
+ ownerPackageName);
+ final boolean callerHasUriPermission = getContext().checkUriPermission(
+ item, mCallingIdentity.get().pid, mCallingIdentity.get().uid,
+ Intent.FLAG_GRANT_WRITE_URI_PERMISSION) == PERMISSION_GRANTED;
+
+ if (!callerIsOwner && !callerHasUriPermission) {
res = getRedactionRanges(file).redactionRanges;
}
} finally {
diff --git a/tests/jni/FuseDaemonTest/FilePathAccessTestHelper/src/com/android/tests/fused/FilePathAccessTestHelper.java b/tests/jni/FuseDaemonTest/FilePathAccessTestHelper/src/com/android/tests/fused/FilePathAccessTestHelper.java
index a31137a..6e11fe8 100644
--- a/tests/jni/FuseDaemonTest/FilePathAccessTestHelper/src/com/android/tests/fused/FilePathAccessTestHelper.java
+++ b/tests/jni/FuseDaemonTest/FilePathAccessTestHelper/src/com/android/tests/fused/FilePathAccessTestHelper.java
@@ -18,6 +18,10 @@
import static com.android.tests.fused.lib.ReaddirTestHelper.CREATE_FILE_QUERY;
import static com.android.tests.fused.lib.ReaddirTestHelper.DELETE_FILE_QUERY;
import static com.android.tests.fused.lib.ReaddirTestHelper.READDIR_QUERY;
+import static com.android.tests.fused.lib.RedactionTestHelper.EXIF_METADATA_QUERY;
+import static com.android.tests.fused.lib.RedactionTestHelper.getExifMetadata;
+import static com.android.tests.fused.lib.TestUtils.INTENT_EXCEPTION;
+import static com.android.tests.fused.lib.TestUtils.INTENT_EXTRA_PATH;
import static com.android.tests.fused.lib.TestUtils.QUERY_TYPE;
import android.app.Activity;
@@ -25,13 +29,12 @@
import android.os.Bundle;
import android.util.Log;
-import java.io.IOException;
-import java.io.File;
-import java.util.ArrayList;
-
-
import com.android.tests.fused.lib.ReaddirTestHelper;
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+
/**
* App for FilePathAccessTest Functions.
*
@@ -53,15 +56,37 @@
case DELETE_FILE_QUERY:
createOrDeleteFile(queryType);
break;
+ case EXIF_METADATA_QUERY:
+ sendMetadata(queryType);
+ break;
case "null":
default:
Log.e(TAG, "Unknown query received from launcher app: " + queryType);
}
}
+ private void sendMetadata(String queryType) {
+ final Intent intent = new Intent(queryType);
+ if (getIntent().hasExtra(INTENT_EXTRA_PATH)) {
+ final String filePath = getIntent().getStringExtra(INTENT_EXTRA_PATH);
+ try {
+ if (EXIF_METADATA_QUERY.equals(queryType)) {
+ intent.putExtra(queryType, getExifMetadata(new File(filePath)));
+ }
+ } catch (Exception e) {
+ intent.putExtra(INTENT_EXCEPTION, e);
+ }
+ } else {
+ Log.e(TAG, "File path not set from launcher app");
+ intent.putExtra(INTENT_EXCEPTION, new IllegalStateException(
+ "File path not set from launcher app"));
+ }
+ sendBroadcast(intent);
+ }
+
private void sendDirectoryEntries(String queryType) {
- if (getIntent().hasExtra(queryType)) {
- final String directoryPath = getIntent().getStringExtra(queryType);
+ if (getIntent().hasExtra(INTENT_EXTRA_PATH)) {
+ final String directoryPath = getIntent().getStringExtra(INTENT_EXTRA_PATH);
ArrayList<String> directoryEntries = new ArrayList<String>();
if (queryType.equals(READDIR_QUERY)) {
directoryEntries = ReaddirTestHelper.readDirectory(directoryPath);
@@ -75,8 +100,8 @@
}
private void createOrDeleteFile(String queryType) {
- if (getIntent().hasExtra(queryType)) {
- final String filePath = getIntent().getStringExtra(queryType);
+ if (getIntent().hasExtra(INTENT_EXTRA_PATH)) {
+ final String filePath = getIntent().getStringExtra(INTENT_EXTRA_PATH);
final File file = new File(filePath);
boolean returnStatus = false;
try {
diff --git a/tests/jni/FuseDaemonTest/host/src/com/android/tests/fused/host/FuseDaemonHostTest.java b/tests/jni/FuseDaemonTest/host/src/com/android/tests/fused/host/FuseDaemonHostTest.java
index f0a3db2..3c859e3 100644
--- a/tests/jni/FuseDaemonTest/host/src/com/android/tests/fused/host/FuseDaemonHostTest.java
+++ b/tests/jni/FuseDaemonTest/host/src/com/android/tests/fused/host/FuseDaemonHostTest.java
@@ -113,4 +113,9 @@
public void testListFilesFromExternalMediaDirectory() throws Exception {
runDeviceTest("testListFilesFromExternalMediaDirectory");
}
+
+ @Test
+ public void testMetaDataRedaction() throws Exception {
+ runDeviceTest("testMetaDataRedaction");
+ }
}
\ No newline at end of file
diff --git a/tests/jni/FuseDaemonTest/libs/FuseDaemonTestLib/src/com/android/tests/fused/lib/RedactionTestHelper.java b/tests/jni/FuseDaemonTest/libs/FuseDaemonTestLib/src/com/android/tests/fused/lib/RedactionTestHelper.java
new file mode 100644
index 0000000..b3b3a4a
--- /dev/null
+++ b/tests/jni/FuseDaemonTest/libs/FuseDaemonTestLib/src/com/android/tests/fused/lib/RedactionTestHelper.java
@@ -0,0 +1,107 @@
+/**
+ * Copyright (C) 2019 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.tests.fused.lib;
+
+import static androidx.test.InstrumentationRegistry.getContext;
+
+import static org.junit.Assert.fail;
+
+import android.media.ExifInterface;
+
+import androidx.annotation.NonNull;
+import androidx.annotation.RawRes;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.HashMap;
+import java.util.Objects;
+
+/**
+ * Helper functions and utils for redactions tests
+ */
+public class RedactionTestHelper {
+ private static final String TAG = "RedactionTestHelper";
+
+ private static final String[] EXIF_GPS_TAGS = {
+ ExifInterface.TAG_GPS_ALTITUDE,
+ ExifInterface.TAG_GPS_DOP,
+ ExifInterface.TAG_GPS_DATESTAMP,
+ ExifInterface.TAG_GPS_LATITUDE,
+ ExifInterface.TAG_GPS_LATITUDE_REF,
+ ExifInterface.TAG_GPS_LONGITUDE,
+ ExifInterface.TAG_GPS_LONGITUDE_REF,
+ ExifInterface.TAG_GPS_PROCESSING_METHOD,
+ ExifInterface.TAG_GPS_TIMESTAMP,
+ ExifInterface.TAG_GPS_VERSION_ID,
+ };
+
+ public static final String EXIF_METADATA_QUERY = "com.android.tests.fused.exif";
+
+ @NonNull
+ public static HashMap<String, String> getExifMetadata(@NonNull File file) throws IOException {
+ final ExifInterface exif = new ExifInterface(file);
+ return dumpExifGpsTagsToMap(exif);
+ }
+
+ @NonNull
+ public static HashMap<String, String> getExifMetadataFromRawResource(@RawRes int resId)
+ throws IOException {
+ final ExifInterface exif;
+ try (InputStream in = getContext().getResources().openRawResource(resId)) {
+ exif = new ExifInterface(in);
+ }
+ return dumpExifGpsTagsToMap(exif);
+ }
+
+ public static void assertExifMetadataMatch(@NonNull HashMap<String, String> actual,
+ @NonNull HashMap<String, String> expected) {
+ for (String tag : EXIF_GPS_TAGS) {
+ assertMetadataEntryMatch(tag, actual.get(tag), expected.get(tag));
+ }
+ }
+
+ public static void assertExifMetadataMismatch(@NonNull HashMap<String, String> actual,
+ @NonNull HashMap<String, String> expected) {
+ for (String tag : EXIF_GPS_TAGS) {
+ assertMetadataEntryMismatch(tag, actual.get(tag), expected.get(tag));
+ }
+ }
+
+ private static void assertMetadataEntryMatch(String tag, String actual, String expected) {
+ if (!Objects.equals(actual, expected)) {
+ fail("Unexpected metadata mismatch for tag: " + tag + "\n"
+ + "expected:" + expected + "\n"
+ + "but was: " + actual);
+ }
+ }
+
+ private static void assertMetadataEntryMismatch(String tag, String actual, String expected) {
+ if (Objects.equals(actual, expected)) {
+ fail("Unexpected metadata match for tag: " + tag + "\n"
+ + "expected not to be:" + expected);
+ }
+ }
+
+ private static HashMap<String, String> dumpExifGpsTagsToMap(ExifInterface exif) {
+ final HashMap<String, String> res = new HashMap<>();
+ for (String tag : EXIF_GPS_TAGS) {
+ res.put(tag, exif.getAttribute(tag));
+ }
+ return res;
+ }
+}
diff --git a/tests/jni/FuseDaemonTest/libs/FuseDaemonTestLib/src/com/android/tests/fused/lib/TestUtils.java b/tests/jni/FuseDaemonTest/libs/FuseDaemonTestLib/src/com/android/tests/fused/lib/TestUtils.java
index 51abf9e..115e4c9 100644
--- a/tests/jni/FuseDaemonTest/libs/FuseDaemonTestLib/src/com/android/tests/fused/lib/TestUtils.java
+++ b/tests/jni/FuseDaemonTest/libs/FuseDaemonTestLib/src/com/android/tests/fused/lib/TestUtils.java
@@ -21,6 +21,7 @@
import static com.android.tests.fused.lib.ReaddirTestHelper.CREATE_FILE_QUERY;
import static com.android.tests.fused.lib.ReaddirTestHelper.DELETE_FILE_QUERY;
import static com.android.tests.fused.lib.ReaddirTestHelper.READDIR_QUERY;
+import static com.android.tests.fused.lib.RedactionTestHelper.EXIF_METADATA_QUERY;
import static com.google.common.truth.Truth.assertThat;
@@ -30,11 +31,17 @@
import android.app.ActivityManager;
import android.app.UiAutomation;
import android.content.BroadcastReceiver;
+import android.content.ContentResolver;
+import android.content.ContentUris;
import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
+import android.database.Cursor;
+import android.net.Uri;
+import android.provider.MediaStore;
import android.util.Log;
+import androidx.annotation.NonNull;
import androidx.test.InstrumentationRegistry;
import com.android.cts.install.lib.Install;
@@ -44,8 +51,10 @@
import com.google.common.io.ByteStreams;
+import java.io.File;
import java.io.FileInputStream;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.concurrent.CountDownLatch;
/**
@@ -55,6 +64,8 @@
static final String TAG = "FuseDaemonTest";
public static final String QUERY_TYPE = "com.android.tests.fused.queryType";
+ public static final String INTENT_EXTRA_PATH = "com.android.tests.fused.path";
+ public static final String INTENT_EXCEPTION = "com.android.tests.fused.exception";
private static final UiAutomation sUiAutomation = InstrumentationRegistry.getInstrumentation()
.getUiAutomation();
@@ -102,6 +113,20 @@
}
/**
+ * Makes the given {@code testApp} read the EXIF metadata from the given file and returns the
+ * result as an {@link HashMap}
+ */
+ public static HashMap<String, String> readExifMetadataFromTestApp(TestApp testApp,
+ String filePath) throws Exception {
+ HashMap<String, String> res =
+ getMetadataFromTestApp(testApp, filePath, EXIF_METADATA_QUERY);
+ if (res.containsKey(INTENT_EXCEPTION)) {
+ throw new IllegalStateException(res.get(INTENT_EXCEPTION));
+ }
+ return res;
+ }
+
+ /**
* Makes the given {@code testApp} create a file.
*/
public static boolean createFileAs(TestApp testApp, String path) throws Exception {
@@ -153,6 +178,24 @@
}
}
+ /**
+ * Queries {@link ContentResolver} for a file and returns the corresponding {@link Uri} for its
+ * entry in the database.
+ */
+ @NonNull
+ public static Uri getFileUri(@NonNull ContentResolver cr, @NonNull File file) {
+ final Uri contentUri = MediaStore.Files.getContentUri(MediaStore.VOLUME_EXTERNAL);
+ try (Cursor c = cr.query(contentUri,
+ /*projection*/ new String[] { MediaStore.MediaColumns._ID },
+ /*selection*/ MediaStore.MediaColumns.DATA + " = ?",
+ /*selectionArgs*/ new String[] { file.getAbsolutePath() },
+ /*sortOrder*/ null)) {
+ c.moveToFirst();
+ int id = c.getInt(0);
+ return ContentUris.withAppendedId(contentUri, id);
+ }
+ }
+
public static <T extends Exception> void assertThrows(Class<T> clazz, Operation<T> r)
throws Exception {
assertThrows(clazz, "", r);
@@ -197,7 +240,6 @@
private static void sendIntentToTestApp(TestApp testApp, String dirPath, String actionName,
BroadcastReceiver broadcastReceiver, CountDownLatch latch) throws Exception {
- final ArrayList<String> appOutputList = new ArrayList<String>();
final String packageName = testApp.getPackageName();
forceStopApp(packageName);
// Register broadcast receiver
@@ -211,13 +253,35 @@
intent.setPackage(packageName);
intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
intent.putExtra(QUERY_TYPE, actionName);
- intent.putExtra(actionName, dirPath);
+ intent.putExtra(INTENT_EXTRA_PATH, dirPath);
intent.addCategory(Intent.CATEGORY_LAUNCHER);
getContext().startActivity(intent);
latch.await();
getContext().unregisterReceiver(broadcastReceiver);
}
+ private static HashMap<String, String> getMetadataFromTestApp(TestApp testApp, String dirPath,
+ String actionName) throws Exception {
+ final CountDownLatch latch = new CountDownLatch(1);
+ final HashMap<String, String> appOutputList = new HashMap<>();
+ final BroadcastReceiver broadcastReceiver = new BroadcastReceiver() {
+ @Override
+ public void onReceive(Context context, Intent intent) {
+ if (intent.hasExtra(INTENT_EXCEPTION)) {
+ appOutputList.put(INTENT_EXCEPTION,
+ ((Exception)intent.getExtras().get(INTENT_EXCEPTION)).getMessage());
+ } else if(intent.hasExtra(actionName)) {
+ HashMap<String, String> res =
+ (HashMap<String, String>) intent.getExtras().get(actionName);
+ appOutputList.putAll(res);
+ }
+ latch.countDown();
+ }
+ };
+ sendIntentToTestApp(testApp, dirPath, actionName, broadcastReceiver, latch);
+ return appOutputList;
+ }
+
private static ArrayList<String> getContentsFromTestApp(TestApp testApp, String dirPath,
String actionName) throws Exception {
final CountDownLatch latch = new CountDownLatch(1);
diff --git a/tests/jni/FuseDaemonTest/res/raw/img_with_metadata.jpg b/tests/jni/FuseDaemonTest/res/raw/img_with_metadata.jpg
new file mode 100644
index 0000000..c9063f8
--- /dev/null
+++ b/tests/jni/FuseDaemonTest/res/raw/img_with_metadata.jpg
Binary files differ
diff --git a/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java b/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
index 86c7550..ca3eb7c 100644
--- a/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
+++ b/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
@@ -21,12 +21,16 @@
import static androidx.test.InstrumentationRegistry.getContext;
+import static com.android.tests.fused.lib.RedactionTestHelper.assertExifMetadataMatch;
+import static com.android.tests.fused.lib.RedactionTestHelper.assertExifMetadataMismatch;
+import static com.android.tests.fused.lib.RedactionTestHelper.getExifMetadata;
+import static com.android.tests.fused.lib.RedactionTestHelper.getExifMetadataFromRawResource;
import static com.android.tests.fused.lib.TestUtils.assertThrows;
import static com.android.tests.fused.lib.TestUtils.createFileAs;
import static com.android.tests.fused.lib.TestUtils.deleteFileAs;
-import static com.android.tests.fused.lib.TestUtils.executeShellCommand;
import static com.android.tests.fused.lib.TestUtils.installApp;
import static com.android.tests.fused.lib.TestUtils.listAs;
+import static com.android.tests.fused.lib.TestUtils.readExifMetadataFromTestApp;
import static com.android.tests.fused.lib.TestUtils.revokeReadExternalStorage;
import static com.android.tests.fused.lib.TestUtils.uninstallApp;
@@ -37,6 +41,7 @@
import android.content.ContentResolver;
import android.database.Cursor;
import android.os.Environment;
+import android.os.FileUtils;
import android.provider.MediaStore;
import android.system.ErrnoException;
import android.system.Os;
@@ -45,6 +50,9 @@
import androidx.test.runner.AndroidJUnit4;
+import com.android.cts.install.lib.TestApp;
+import com.android.tests.fused.lib.ReaddirTestHelper;
+
import com.google.common.io.ByteStreams;
import org.junit.Before;
@@ -55,12 +63,11 @@
import java.io.FileDescriptor;
import java.io.FileInputStream;
import java.io.FileOutputStream;
-import java.io.InputStream;
import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
import java.nio.ByteBuffer;
-
-import com.android.cts.install.lib.TestApp;
-import com.android.tests.fused.lib.ReaddirTestHelper;
+import java.util.HashMap;
@RunWith(AndroidJUnit4.class)
public class FilePathAccessTest {
@@ -540,6 +547,41 @@
}
}
+ @Test
+ public void testMetaDataRedaction() throws Exception {
+ File jpgFile = new File(PICTURES_DIR, "img_metadata.jpg");
+ try {
+ if (jpgFile.exists()) {
+ assertThat(jpgFile.delete()).isTrue();
+ }
+
+ HashMap<String, String> originalExif = getExifMetadataFromRawResource(
+ R.raw.img_with_metadata);
+
+ try (InputStream in = getContext().getResources().openRawResource(
+ R.raw.img_with_metadata);
+ OutputStream out = new FileOutputStream(jpgFile)) {
+ // Dump the image we have to external storage
+ FileUtils.copy(in, out);
+ }
+
+ HashMap<String, String> exif = getExifMetadata(jpgFile);
+ assertExifMetadataMatch(exif, originalExif);
+
+ installApp(TEST_APP_A, /*grantStoragePermissions*/ true);
+ HashMap<String, String> exifFromTestApp = readExifMetadataFromTestApp(TEST_APP_A,
+ jpgFile.getPath());
+ // Other apps shouldn't have access to the same metadata without explicit permission
+ assertExifMetadataMismatch(exifFromTestApp, originalExif);
+
+ // TODO(b/146346138): Test that if we give TEST_APP_A write URI permission,
+ // it would be able to access the metadata.
+ } finally {
+ jpgFile.delete();
+ uninstallApp(TEST_APP_A);
+ }
+ }
+
private static ContentResolver getContentResolver() {
return getContext().getContentResolver();
}
