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/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;