Don't provide read logs for shell-initiated installations.

Only if the application is profileable.

Bug: 158238023
Fixes: 158238023
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest PackageParserTest
Change-Id: I8575830ec3f29850297fdbfbaa157072d6350a28
Merged-In: I8575830ec3f29850297fdbfbaa157072d6350a28

11 files changed, 169 insertions, 19 deletions

diff --git a/core/java/android/content/pm/PackageParser.java b/core/java/android/content/pm/PackageParser.java
index c8dd4d9d9d51..885ffbac3d30 100644
--- a/core/java/android/content/pm/PackageParser.java
+++ b/core/java/android/content/pm/PackageParser.java
@@ -205,6 +205,7 @@ public class PackageParser {
     public static final String TAG_USES_PERMISSION_SDK_M = "uses-permission-sdk-m";
     public static final String TAG_USES_SDK = "uses-sdk";
     public static final String TAG_USES_SPLIT = "uses-split";
+    public static final String TAG_PROFILEABLE = "profileable";
 
     public static final String METADATA_MAX_ASPECT_RATIO = "android.max_aspect";
     public static final String METADATA_SUPPORTS_SIZE_CHANGES = "android.supports_size_changes";
@@ -459,6 +460,9 @@ public class PackageParser {
         public final SigningDetails signingDetails;
         public final boolean coreApp;
         public final boolean debuggable;
+        // This does not represent the actual manifest structure since the 'profilable' tag
+        // could be used with attributes other than 'shell'. Extend if necessary.
+        public final boolean profilableByShell;
         public final boolean multiArch;
         public final boolean use32bitAbi;
         public final boolean extractNativeLibs;
@@ -470,15 +474,13 @@ public class PackageParser {
         public final int overlayPriority;
 
         public ApkLite(String codePath, String packageName, String splitName,
-                boolean isFeatureSplit,
-                String configForSplit, String usesSplitName, boolean isSplitRequired,
-                int versionCode, int versionCodeMajor,
-                int revisionCode, int installLocation, List<VerifierInfo> verifiers,
-                SigningDetails signingDetails, boolean coreApp,
-                boolean debuggable, boolean multiArch, boolean use32bitAbi,
-                boolean useEmbeddedDex, boolean extractNativeLibs, boolean isolatedSplits,
-                String targetPackageName, boolean overlayIsStatic, int overlayPriority,
-                int minSdkVersion, int targetSdkVersion) {
+                boolean isFeatureSplit, String configForSplit, String usesSplitName,
+                boolean isSplitRequired, int versionCode, int versionCodeMajor, int revisionCode,
+                int installLocation, List<VerifierInfo> verifiers, SigningDetails signingDetails,
+                boolean coreApp, boolean debuggable, boolean profilableByShell, boolean multiArch,
+                boolean use32bitAbi, boolean useEmbeddedDex, boolean extractNativeLibs,
+                boolean isolatedSplits, String targetPackageName, boolean overlayIsStatic,
+                int overlayPriority, int minSdkVersion, int targetSdkVersion) {
             this.codePath = codePath;
             this.packageName = packageName;
             this.splitName = splitName;
@@ -493,6 +495,7 @@ public class PackageParser {
             this.verifiers = verifiers.toArray(new VerifierInfo[verifiers.size()]);
             this.coreApp = coreApp;
             this.debuggable = debuggable;
+            this.profilableByShell = profilableByShell;
             this.multiArch = multiArch;
             this.use32bitAbi = use32bitAbi;
             this.useEmbeddedDex = useEmbeddedDex;
@@ -1573,6 +1576,7 @@ public class PackageParser {
         int revisionCode = 0;
         boolean coreApp = false;
         boolean debuggable = false;
+        boolean profilableByShell = false;
         boolean multiArch = false;
         boolean use32bitAbi = false;
         boolean extractNativeLibs = true;
@@ -1638,6 +1642,10 @@ public class PackageParser {
                     final String attr = attrs.getAttributeName(i);
                     if ("debuggable".equals(attr)) {
                         debuggable = attrs.getAttributeBooleanValue(i, false);
+                        if (debuggable) {
+                            // Debuggable implies profileable
+                            profilableByShell = true;
+                        }
                     }
                     if ("multiArch".equals(attr)) {
                         multiArch = attrs.getAttributeBooleanValue(i, false);
@@ -1690,6 +1698,13 @@ public class PackageParser {
                         minSdkVersion = attrs.getAttributeIntValue(i, DEFAULT_MIN_SDK_VERSION);
                     }
                 }
+            } else if (TAG_PROFILEABLE.equals(parser.getName())) {
+                for (int i = 0; i < attrs.getAttributeCount(); ++i) {
+                    final String attr = attrs.getAttributeName(i);
+                    if ("shell".equals(attr)) {
+                        profilableByShell = attrs.getAttributeBooleanValue(i, profilableByShell);
+                    }
+                }
             }
         }
 
@@ -1707,8 +1722,9 @@ public class PackageParser {
         return new ApkLite(codePath, packageSplit.first, packageSplit.second, isFeatureSplit,
                 configForSplit, usesSplitName, isSplitRequired, versionCode, versionCodeMajor,
                 revisionCode, installLocation, verifiers, signingDetails, coreApp, debuggable,
-                multiArch, use32bitAbi, useEmbeddedDex, extractNativeLibs, isolatedSplits,
-                targetPackage, overlayIsStatic, overlayPriority, minSdkVersion, targetSdkVersion);
+                profilableByShell, multiArch, use32bitAbi, useEmbeddedDex, extractNativeLibs,
+                isolatedSplits, targetPackage, overlayIsStatic, overlayPriority, minSdkVersion,
+                targetSdkVersion);
     }
 
     /**
diff --git a/core/java/android/content/pm/parsing/ApkLiteParseUtils.java b/core/java/android/content/pm/parsing/ApkLiteParseUtils.java
index d2172d3741d1..c3e9402a389e 100644
--- a/core/java/android/content/pm/parsing/ApkLiteParseUtils.java
+++ b/core/java/android/content/pm/parsing/ApkLiteParseUtils.java
@@ -20,7 +20,6 @@ import static android.content.pm.PackageManager.INSTALL_PARSE_FAILED_BAD_PACKAGE
 import static android.content.pm.PackageManager.INSTALL_PARSE_FAILED_MANIFEST_MALFORMED;
 import static android.os.Trace.TRACE_TAG_PACKAGE_MANAGER;
 
-import android.compat.annotation.UnsupportedAppUsage;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.PackageParser;
@@ -303,6 +302,7 @@ public class ApkLiteParseUtils {
         int revisionCode = 0;
         boolean coreApp = false;
         boolean debuggable = false;
+        boolean profilableByShell = false;
         boolean multiArch = false;
         boolean use32bitAbi = false;
         boolean extractNativeLibs = true;
@@ -379,6 +379,10 @@ public class ApkLiteParseUtils {
                     switch (attr) {
                         case "debuggable":
                             debuggable = attrs.getAttributeBooleanValue(i, false);
+                            if (debuggable) {
+                                // Debuggable implies profileable
+                                profilableByShell = true;
+                            }
                             break;
                         case "multiArch":
                             multiArch = attrs.getAttributeBooleanValue(i, false);
@@ -431,6 +435,13 @@ public class ApkLiteParseUtils {
                         minSdkVersion = attrs.getAttributeIntValue(i, DEFAULT_MIN_SDK_VERSION);
                     }
                 }
+            } else if (PackageParser.TAG_PROFILEABLE.equals(parser.getName())) {
+                for (int i = 0; i < attrs.getAttributeCount(); ++i) {
+                    final String attr = attrs.getAttributeName(i);
+                    if ("shell".equals(attr)) {
+                        profilableByShell = attrs.getAttributeBooleanValue(i, profilableByShell);
+                    }
+                }
             }
         }
 
@@ -445,12 +456,13 @@ public class ApkLiteParseUtils {
             overlayPriority = 0;
         }
 
-        return input.success(new PackageParser.ApkLite(codePath, packageSplit.first,
-                packageSplit.second, isFeatureSplit, configForSplit, usesSplitName, isSplitRequired,
-                versionCode, versionCodeMajor, revisionCode, installLocation, verifiers,
-                signingDetails, coreApp, debuggable, multiArch, use32bitAbi, useEmbeddedDex,
-                extractNativeLibs, isolatedSplits, targetPackage, overlayIsStatic, overlayPriority,
-                minSdkVersion, targetSdkVersion));
+        return input.success(
+                new PackageParser.ApkLite(codePath, packageSplit.first, packageSplit.second,
+                        isFeatureSplit, configForSplit, usesSplitName, isSplitRequired, versionCode,
+                        versionCodeMajor, revisionCode, installLocation, verifiers, signingDetails,
+                        coreApp, debuggable, profilableByShell, multiArch, use32bitAbi,
+                        useEmbeddedDex, extractNativeLibs, isolatedSplits, targetPackage,
+                        overlayIsStatic, overlayPriority, minSdkVersion, targetSdkVersion));
     }
 
     public static ParseResult<Pair<String, String>> parsePackageSplitNames(ParseInput input,
diff --git a/core/java/android/os/incremental/IIncrementalService.aidl b/core/java/android/os/incremental/IIncrementalService.aidl
index 220ce22ded5c..61e6a05fce37 100644
--- a/core/java/android/os/incremental/IIncrementalService.aidl
+++ b/core/java/android/os/incremental/IIncrementalService.aidl
@@ -111,6 +111,11 @@ interface IIncrementalService {
     void deleteStorage(int storageId);
 
     /**
+     * Permanently disable readlogs reporting for a storage given its ID.
+     */
+    void disableReadLogs(int storageId);
+
+    /**
      * Setting up native library directories and extract native libs onto a storage if needed.
      */
     boolean configureNativeBinaries(int storageId, in @utf8InCpp String apkFullPath, in @utf8InCpp String libDirRelativePath, in @utf8InCpp String abi, boolean extractNativeLibs);
diff --git a/core/java/android/os/incremental/IncrementalFileStorages.java b/core/java/android/os/incremental/IncrementalFileStorages.java
index 863d86ef88c9..31ccf95ba16f 100644
--- a/core/java/android/os/incremental/IncrementalFileStorages.java
+++ b/core/java/android/os/incremental/IncrementalFileStorages.java
@@ -153,6 +153,13 @@ public final class IncrementalFileStorages {
     }
 
     /**
+     * Permanently disables readlogs.
+     */
+    public void disableReadLogs() {
+        mDefaultStorage.disableReadLogs();
+    }
+
+    /**
      * Resets the states and unbinds storage instances for an installation session.
      * TODO(b/136132412): make sure unnecessary binds are removed but useful storages are kept
      */
diff --git a/core/java/android/os/incremental/IncrementalStorage.java b/core/java/android/os/incremental/IncrementalStorage.java
index 6200a38fe13c..ca6114f29b9c 100644
--- a/core/java/android/os/incremental/IncrementalStorage.java
+++ b/core/java/android/os/incremental/IncrementalStorage.java
@@ -418,6 +418,17 @@ public final class IncrementalStorage {
     private static final int INCFS_MAX_ADD_DATA_SIZE = 128;
 
     /**
+     * Permanently disable readlogs collection.
+     */
+    public void disableReadLogs() {
+        try {
+            mService.disableReadLogs(mId);
+        } catch (RemoteException e) {
+            e.rethrowFromSystemServer();
+        }
+    }
+
+    /**
      * Deserialize and validate v4 signature bytes.
      */
     private static void validateV4Signature(@Nullable byte[] v4signatureBytes)
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index 7ab05c4f762c..de8ad6b7db13 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -2108,6 +2108,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                 baseApk = apk;
             }
 
+            // Validate and add Dex Metadata (.dm).
             final File dexMetadataFile = DexMetadataHelper.findDexMetadataForFile(addedFile);
             if (dexMetadataFile != null) {
                 if (!FileUtils.isValidExtFilename(dexMetadataFile.getName())) {
@@ -2295,6 +2296,13 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
             throw new PackageManagerException(INSTALL_FAILED_MISSING_SPLIT,
                     "Missing split for " + mPackageName);
         }
+
+        final boolean isInstallerShell = (mInstallerUid == Process.SHELL_UID);
+        if (isInstallerShell && isIncrementalInstallation() && mIncrementalFileStorages != null) {
+            if (!baseApk.debuggable && !baseApk.profilableByShell) {
+                mIncrementalFileStorages.disableReadLogs();
+            }
+        }
     }
 
     private void resolveAndStageFile(File origFile, File targetFile)
diff --git a/services/incremental/BinderIncrementalService.cpp b/services/incremental/BinderIncrementalService.cpp
index e790a196ad64..7132706c4ef1 100644
--- a/services/incremental/BinderIncrementalService.cpp
+++ b/services/incremental/BinderIncrementalService.cpp
@@ -165,6 +165,11 @@ binder::Status BinderIncrementalService::deleteStorage(int32_t storageId) {
     return ok();
 }
 
+binder::Status BinderIncrementalService::disableReadLogs(int32_t storageId) {
+    mImpl.disableReadLogs(storageId);
+    return ok();
+}
+
 binder::Status BinderIncrementalService::makeDirectory(int32_t storageId, const std::string& path,
                                                        int32_t* _aidl_return) {
     *_aidl_return = mImpl.makeDir(storageId, path);
diff --git a/services/incremental/BinderIncrementalService.h b/services/incremental/BinderIncrementalService.h
index 68549f5a8ff8..10154946d3ee 100644
--- a/services/incremental/BinderIncrementalService.h
+++ b/services/incremental/BinderIncrementalService.h
@@ -74,7 +74,7 @@ public:
                                    std::vector<uint8_t>* _aidl_return) final;
     binder::Status startLoading(int32_t storageId, bool* _aidl_return) final;
     binder::Status deleteStorage(int32_t storageId) final;
-
+    binder::Status disableReadLogs(int32_t storageId) final;
     binder::Status configureNativeBinaries(int32_t storageId, const std::string& apkFullPath,
                                            const std::string& libDirRelativePath,
                                            const std::string& abi, bool extractNativeLibs,
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index 885f4d2d34d7..3450c3ae9fb3 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -60,6 +60,7 @@ struct Constants {
     static constexpr auto storagePrefix = "st"sv;
     static constexpr auto mountpointMdPrefix = ".mountpoint."sv;
     static constexpr auto infoMdName = ".info"sv;
+    static constexpr auto readLogsDisabledMarkerName = ".readlogs_disabled"sv;
     static constexpr auto libDir = "lib"sv;
     static constexpr auto libSuffix = ".so"sv;
     static constexpr auto blockSize = 4096;
@@ -172,6 +173,13 @@ std::string makeBindMdName() {
 
     return name;
 }
+
+static bool checkReadLogsDisabledMarker(std::string_view root) {
+    const auto markerPath = path::c_str(path::join(root, constants().readLogsDisabledMarkerName));
+    struct stat st;
+    return (::stat(markerPath, &st) == 0);
+}
+
 } // namespace
 
 IncrementalService::IncFsMount::~IncFsMount() {
@@ -618,6 +626,32 @@ StorageId IncrementalService::findStorageId(std::string_view path) const {
     return it->second->second.storage;
 }
 
+void IncrementalService::disableReadLogs(StorageId storageId) {
+    std::unique_lock l(mLock);
+    const auto ifs = getIfsLocked(storageId);
+    if (!ifs) {
+        LOG(ERROR) << "disableReadLogs failed, invalid storageId: " << storageId;
+        return;
+    }
+    if (!ifs->readLogsEnabled()) {
+        return;
+    }
+    ifs->disableReadLogs();
+    l.unlock();
+
+    const auto metadata = constants().readLogsDisabledMarkerName;
+    if (auto err = mIncFs->makeFile(ifs->control,
+                                    path::join(ifs->root, constants().mount,
+                                               constants().readLogsDisabledMarkerName),
+                                    0777, idFromMetadata(metadata), {})) {
+        //{.metadata = {metadata.data(), (IncFsSize)metadata.size()}})) {
+        LOG(ERROR) << "Failed to make marker file for storageId: " << storageId;
+        return;
+    }
+
+    setStorageParams(storageId, /*enableReadLogs=*/false);
+}
+
 int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLogs) {
     const auto ifs = getIfs(storageId);
     if (!ifs) {
@@ -627,6 +661,11 @@ int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLog
 
     const auto& params = ifs->dataLoaderStub->params();
     if (enableReadLogs) {
+        if (!ifs->readLogsEnabled()) {
+            LOG(ERROR) << "setStorageParams failed, readlogs disabled for storageId: " << storageId;
+            return -EPERM;
+        }
+
         if (auto status = mAppOpsManager->checkPermission(kDataUsageStats, kOpUsage,
                                                           params.packageName.c_str());
             !status.isOk()) {
@@ -1072,6 +1111,11 @@ std::unordered_set<std::string_view> IncrementalService::adoptMountedInstances()
                                                 std::move(control), *this);
         cleanupFiles.release(); // ifs will take care of that now
 
+        // Check if marker file present.
+        if (checkReadLogsDisabledMarker(root)) {
+            ifs->disableReadLogs();
+        }
+
         std::vector<std::pair<std::string, metadata::BindPoint>> permanentBindPoints;
         auto d = openDir(root);
         while (auto e = ::readdir(d.get())) {
@@ -1243,6 +1287,11 @@ bool IncrementalService::mountExistingImage(std::string_view root) {
     ifs->mountId = mount.storage().id();
     mNextId = std::max(mNextId, ifs->mountId + 1);
 
+    // Check if marker file present.
+    if (checkReadLogsDisabledMarker(mountTarget)) {
+        ifs->disableReadLogs();
+    }
+
     // DataLoader params
     DataLoaderParamsParcel dataLoaderParams;
     {
diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h
index 918531b7921c..a6cc94639c8a 100644
--- a/services/incremental/IncrementalService.h
+++ b/services/incremental/IncrementalService.h
@@ -94,6 +94,10 @@ public:
         Permanent = 1,
     };
 
+    enum StorageFlags {
+        ReadLogsEnabled = 1,
+    };
+
     static FileId idFromMetadata(std::span<const uint8_t> metadata);
     static inline FileId idFromMetadata(std::span<const char> metadata) {
         return idFromMetadata({(const uint8_t*)metadata.data(), metadata.size()});
@@ -116,6 +120,7 @@ public:
     int unbind(StorageId storage, std::string_view target);
     void deleteStorage(StorageId storage);
 
+    void disableReadLogs(StorageId storage);
     int setStorageParams(StorageId storage, bool enableReadLogs);
 
     int makeFile(StorageId storage, std::string_view path, int mode, FileId id,
@@ -264,6 +269,7 @@ private:
         const std::string root;
         Control control;
         /*const*/ MountId mountId;
+        int32_t flags = StorageFlags::ReadLogsEnabled;
         StorageMap storages;
         BindMap bindPoints;
         DataLoaderStubPtr dataLoaderStub;
@@ -282,6 +288,9 @@ private:
 
         StorageMap::iterator makeStorage(StorageId id);
 
+        void disableReadLogs() { flags &= ~StorageFlags::ReadLogsEnabled; }
+        int32_t readLogsEnabled() const { return (flags & StorageFlags::ReadLogsEnabled); }
+
         static void cleanupFilesystem(std::string_view root);
     };
 
diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp
index 26b5094a795a..1ae9e256c9f4 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -929,6 +929,34 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccess) {
     ASSERT_GE(mDataLoader->setStorageParams(true), 0);
 }
 
+TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndDisabled) {
+    mVold->mountIncFsSuccess();
+    mIncFs->makeFileSuccess();
+    mVold->bindMountSuccess();
+    mVold->setIncFsMountOptionsSuccess();
+    mDataLoaderManager->bindToDataLoaderSuccess();
+    mDataLoaderManager->getDataLoaderSuccess();
+    mAppOpsManager->checkPermissionSuccess();
+    EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_));
+    EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
+    // Enabling and then disabling readlogs.
+    EXPECT_CALL(*mVold, setIncFsMountOptions(_, true)).Times(1);
+    EXPECT_CALL(*mVold, setIncFsMountOptions(_, false)).Times(1);
+    // After setIncFsMountOptions succeeded expecting to start watching.
+    EXPECT_CALL(*mAppOpsManager, startWatchingMode(_, _, _)).Times(1);
+    // Not expecting callback removal.
+    EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(0);
+    TemporaryDir tempDir;
+    int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel),
+                                                       IncrementalService::CreateOptions::CreateNew,
+                                                       {}, {}, {});
+    ASSERT_GE(storageId, 0);
+    ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+    // Now disable.
+    mIncrementalService->disableReadLogs(storageId);
+    ASSERT_EQ(mDataLoader->setStorageParams(true), -EPERM);
+}
+
 TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndPermissionChanged) {
     mVold->mountIncFsSuccess();
     mIncFs->makeFileSuccess();