Set app.metadata file permission to 640
This should prevent apps from circumventing the GET_APP_METADATA
permission by reading the file directly if they are aware of the file
path.
Bug: 267823160
Test: atest android.packageinstaller.install.cts.InstallAppMetadataTest
Change-Id: I4aab10b48e62234bc252535ab2e2c8b9c77a7ac3
diff --git a/core/java/android/app/ApplicationPackageManager.java b/core/java/android/app/ApplicationPackageManager.java
index fc4e2db..6301ad7 100644
--- a/core/java/android/app/ApplicationPackageManager.java
+++ b/core/java/android/app/ApplicationPackageManager.java
@@ -93,6 +93,7 @@
import android.os.Handler;
import android.os.Looper;
import android.os.Message;
+import android.os.ParcelFileDescriptor;
import android.os.ParcelableException;
import android.os.PersistableBundle;
import android.os.Process;
@@ -126,9 +127,6 @@
import libcore.util.EmptyArray;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.lang.ref.WeakReference;
@@ -1233,25 +1231,23 @@
public PersistableBundle getAppMetadata(@NonNull String packageName)
throws NameNotFoundException {
PersistableBundle appMetadata = null;
- String path = null;
+ ParcelFileDescriptor pfd = null;
try {
- path = mPM.getAppMetadataPath(packageName, getUserId());
+ pfd = mPM.getAppMetadataFd(packageName, getUserId());
} catch (ParcelableException e) {
e.maybeRethrow(NameNotFoundException.class);
throw new RuntimeException(e);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
- if (path != null) {
- File file = new File(path);
- try (InputStream inputStream = new FileInputStream(file)) {
+ if (pfd != null) {
+ try (InputStream inputStream = new ParcelFileDescriptor.AutoCloseInputStream(pfd)) {
appMetadata = PersistableBundle.readFromStream(inputStream);
- } catch (FileNotFoundException e) {
- // ignore and return empty bundle if app metadata does not exist
} catch (IOException e) {
throw new RuntimeException(e);
}
}
+
return appMetadata != null ? appMetadata : new PersistableBundle();
}
diff --git a/core/java/android/content/pm/IPackageManager.aidl b/core/java/android/content/pm/IPackageManager.aidl
index dfaa065..133ebbb 100644
--- a/core/java/android/content/pm/IPackageManager.aidl
+++ b/core/java/android/content/pm/IPackageManager.aidl
@@ -159,7 +159,8 @@
*/
ParceledListSlice getInstalledPackages(long flags, in int userId);
- String getAppMetadataPath(String packageName, int userId);
+ @nullable ParcelFileDescriptor getAppMetadataFd(String packageName,
+ int userId);
/**
* This implements getPackagesHoldingPermissions via a "last returned row"
diff --git a/core/java/android/os/incremental/IIncrementalService.aidl b/core/java/android/os/incremental/IIncrementalService.aidl
index 8c9f2dd..278057c 100644
--- a/core/java/android/os/incremental/IIncrementalService.aidl
+++ b/core/java/android/os/incremental/IIncrementalService.aidl
@@ -83,7 +83,7 @@
/**
* Creates a file under a storage.
*/
- int makeFile(int storageId, in @utf8InCpp String path, in IncrementalNewFileParams params, in @nullable byte[] content);
+ int makeFile(int storageId, in @utf8InCpp String path, int mode, in IncrementalNewFileParams params, in @nullable byte[] content);
/**
* Creates a file under a storage. Content of the file is from a range inside another file.
diff --git a/core/java/android/os/incremental/IncrementalFileStorages.java b/core/java/android/os/incremental/IncrementalFileStorages.java
index 8ebc081..6f4a12f 100644
--- a/core/java/android/os/incremental/IncrementalFileStorages.java
+++ b/core/java/android/os/incremental/IncrementalFileStorages.java
@@ -153,7 +153,8 @@
final String apkName = apk.name;
final File targetFile = new File(mStageDir, apkName);
if (!targetFile.exists()) {
- mDefaultStorage.makeFile(apkName, apk.size, null, apk.metadata, apk.signature, null);
+ mDefaultStorage.makeFile(apkName, apk.size, 0777, null, apk.metadata,
+ apk.signature, null);
}
}
@@ -176,8 +177,10 @@
/**
* Creates file in default storage and sets its content.
*/
- public void makeFile(@NonNull String name, @NonNull byte[] content) throws IOException {
- mDefaultStorage.makeFile(name, content.length, UUID.randomUUID(), null, null, content);
+ public void makeFile(@NonNull String name, @NonNull byte[] content,
+ @NonNull int mode) throws IOException {
+ mDefaultStorage.makeFile(name, content.length, mode, UUID.randomUUID(),
+ null, null, content);
}
/**
diff --git a/core/java/android/os/incremental/IncrementalStorage.java b/core/java/android/os/incremental/IncrementalStorage.java
index a1ed253..9138409 100644
--- a/core/java/android/os/incremental/IncrementalStorage.java
+++ b/core/java/android/os/incremental/IncrementalStorage.java
@@ -173,11 +173,12 @@
*
* @param path Relative path of the new file.
* @param size Size of the new file in bytes.
+ * @param mode File access permission mode.
* @param metadata Metadata bytes.
* @param v4signatureBytes Serialized V4SignatureProto.
* @param content Optionally set file content.
*/
- public void makeFile(@NonNull String path, long size, @Nullable UUID id,
+ public void makeFile(@NonNull String path, long size, int mode, @Nullable UUID id,
@Nullable byte[] metadata, @Nullable byte[] v4signatureBytes, @Nullable byte[] content)
throws IOException {
try {
@@ -190,7 +191,7 @@
params.metadata = (metadata == null ? new byte[0] : metadata);
params.fileId = idToBytes(id);
params.signature = v4signatureBytes;
- int res = mService.makeFile(mId, path, params, content);
+ int res = mService.makeFile(mId, path, mode, params, content);
if (res != 0) {
throw new IOException("makeFile() failed with errno " + -res);
}
@@ -199,7 +200,6 @@
}
}
-
/**
* Creates a file in Incremental storage. The content of the file is mapped from a range inside
* a source file in the same storage.
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index e5e87af..c15cc01 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -314,6 +314,8 @@
/** Default byte size limit for app metadata */
private static final long DEFAULT_APP_METADATA_BYTE_SIZE_LIMIT = 32000;
+ private static final int APP_METADATA_FILE_ACCESS_MODE = 0640;
+
// TODO: enforce INSTALL_ALLOW_TEST
// TODO: enforce INSTALL_ALLOW_DOWNGRADE
@@ -1660,9 +1662,12 @@
Binder.restoreCallingIdentity(identity);
}
+ // If file is app metadata then set permission to 0640 to deny user read access since it
+ // might contain sensitive information.
+ int mode = name.equals(APP_METADATA_FILE_NAME) ? APP_METADATA_FILE_ACCESS_MODE : 0644;
ParcelFileDescriptor targetPfd = openTargetInternal(target.getAbsolutePath(),
- O_CREAT | O_WRONLY, 0644);
- Os.chmod(target.getAbsolutePath(), 0644);
+ O_CREAT | O_WRONLY, mode);
+ Os.chmod(target.getAbsolutePath(), mode);
// If caller specified a total length, allocate it for them. Free up
// cache space to grow, if needed.
@@ -3145,7 +3150,8 @@
getIncrementalFileStorages();
try {
incrementalFileStorages.makeFile(APP_METADATA_FILE_NAME,
- Files.readAllBytes(appMetadataFile.toPath()));
+ Files.readAllBytes(appMetadataFile.toPath()),
+ APP_METADATA_FILE_ACCESS_MODE);
} catch (IOException e) {
Slog.e(TAG, "Failed to write app metadata to incremental storage", e);
} finally {
@@ -3413,7 +3419,7 @@
if (!isIncrementalInstallation() || incrementalFileStorages == null) {
FileUtils.bytesToFile(absolutePath, bytes);
} else {
- incrementalFileStorages.makeFile(localPath, bytes);
+ incrementalFileStorages.makeFile(localPath, bytes, 0777);
}
}
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 97ee3c5..7337f69 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -131,6 +131,7 @@
import android.os.IBinder;
import android.os.Message;
import android.os.Parcel;
+import android.os.ParcelFileDescriptor;
import android.os.ParcelableException;
import android.os.PersistableBundle;
import android.os.Process;
@@ -5149,8 +5150,8 @@
}
@Override
- public String getAppMetadataPath(String packageName, int userId) {
- mContext.enforceCallingOrSelfPermission(GET_APP_METADATA, "getAppMetadataPath");
+ public ParcelFileDescriptor getAppMetadataFd(String packageName, int userId) {
+ mContext.enforceCallingOrSelfPermission(GET_APP_METADATA, "getAppMetadataFd");
final int callingUid = Binder.getCallingUid();
final Computer snapshot = snapshotComputer();
final PackageStateInternal ps = snapshot.getPackageStateForInstalledAndFiltered(
@@ -5159,7 +5160,12 @@
throw new ParcelableException(
new PackageManager.NameNotFoundException(packageName));
}
- return new File(ps.getPathString(), APP_METADATA_FILE_NAME).getAbsolutePath();
+ try {
+ File file = new File(ps.getPath(), APP_METADATA_FILE_NAME);
+ return ParcelFileDescriptor.open(file, ParcelFileDescriptor.MODE_READ_ONLY);
+ } catch (FileNotFoundException e) {
+ return null;
+ }
}
@Override
diff --git a/services/incremental/BinderIncrementalService.cpp b/services/incremental/BinderIncrementalService.cpp
index 45ca5cd..aff8e97 100644
--- a/services/incremental/BinderIncrementalService.cpp
+++ b/services/incremental/BinderIncrementalService.cpp
@@ -223,7 +223,7 @@
}
binder::Status BinderIncrementalService::makeFile(
- int32_t storageId, const std::string& path,
+ int32_t storageId, const std::string& path, int32_t mode,
const ::android::os::incremental::IncrementalNewFileParams& params,
const ::std::optional<::std::vector<uint8_t>>& content, int32_t* _aidl_return) {
auto [err, fileId, nfp] = toMakeFileParams(params);
@@ -232,7 +232,7 @@
return ok();
}
- *_aidl_return = mImpl.makeFile(storageId, path, 0777, fileId, nfp, toSpan(content));
+ *_aidl_return = mImpl.makeFile(storageId, path, mode, fileId, nfp, toSpan(content));
return ok();
}
binder::Status BinderIncrementalService::makeFileFromRange(int32_t storageId,
diff --git a/services/incremental/BinderIncrementalService.h b/services/incremental/BinderIncrementalService.h
index 39f1bcb..89ed70f 100644
--- a/services/incremental/BinderIncrementalService.h
+++ b/services/incremental/BinderIncrementalService.h
@@ -63,7 +63,7 @@
int32_t* _aidl_return) final;
binder::Status makeDirectories(int32_t storageId, const std::string& path,
int32_t* _aidl_return) final;
- binder::Status makeFile(int32_t storageId, const std::string& path,
+ binder::Status makeFile(int32_t storageId, const std::string& path, int32_t mode,
const IncrementalNewFileParams& params,
const ::std::optional<::std::vector<uint8_t>>& content,
int32_t* _aidl_return) final;