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;