[DO NOT MERGE] Handle config override via settings correctly

Initialize persistence only after we can read the settings
provider as we need to take into account the current config
potentially set as an override in settings vs using the
hard coded defaults.

Make the last persist time more robust to ensure it always
termines to guard against other unforeseen cases wehere the
persistet files don't match what is expected under the current
config.

Test: atest CtsAppOpsTestCases
Test: atest android.appsecurity.cts.AppOpsTest

bug:134093967

Change-Id: I211d267ccca044093d83dc5928f0531afa47791e

1 files changed, 117 insertions, 30 deletions

diff --git a/services/core/java/com/android/server/appop/HistoricalRegistry.java b/services/core/java/com/android/server/appop/HistoricalRegistry.java
index 69a1c9f584cb..805cac755fd5 100644
--- a/services/core/java/com/android/server/appop/HistoricalRegistry.java
+++ b/services/core/java/com/android/server/appop/HistoricalRegistry.java
@@ -108,6 +108,12 @@ import java.util.concurrent.TimeUnit;
  * must be called with the mInMemoryLock, xxxDMLocked suffix means the method
  * must be called with the mOnDiskLock and mInMemoryLock locks acquired in that
  * exact order.
+ * <p>
+ * INITIALIZATION: We can initialize persistence only after the system is ready
+ * as we need to check the optional configuration override from the settings
+ * database which is not initialized at the time the app ops service is created.
+ * This means that all entry points that touch persistence should be short
+ * circuited via isPersistenceInitialized() check.
  */
 // TODO (bug:122218838): Make sure we handle start of epoch time
 // TODO (bug:122218838): Validate changed time is handled correctly
@@ -177,14 +183,33 @@ final class HistoricalRegistry {
 
     // Object managing persistence (read/write)
     @GuardedBy("mOnDiskLock")
-    private Persistence mPersistence = new Persistence(mBaseSnapshotInterval,
-            mIntervalCompressionMultiplier);
+    private Persistence mPersistence;
 
     HistoricalRegistry(@NonNull Object lock) {
         mInMemoryLock = lock;
-        if (mMode != AppOpsManager.HISTORICAL_MODE_DISABLED) {
-            synchronized (mOnDiskLock) {
-                synchronized (mInMemoryLock) {
+    }
+
+    void systemReady(@NonNull ContentResolver resolver) {
+        final Uri uri = Settings.Global.getUriFor(Settings.Global.APPOP_HISTORY_PARAMETERS);
+        resolver.registerContentObserver(uri, false, new ContentObserver(
+                FgThread.getHandler()) {
+            @Override
+            public void onChange(boolean selfChange) {
+                updateParametersFromSetting(resolver);
+            }
+        });
+
+        updateParametersFromSetting(resolver);
+
+        synchronized (mOnDiskLock) {
+            synchronized (mInMemoryLock) {
+                if (mMode != AppOpsManager.HISTORICAL_MODE_DISABLED) {
+                    // Can be uninitialized if there is no config in the settings table.
+                    if (!isPersistenceInitializedMLocked()) {
+                        mPersistence = new Persistence(mBaseSnapshotInterval,
+                                mIntervalCompressionMultiplier);
+                    }
+
                     // When starting always adjust history to now.
                     final long lastPersistTimeMills =
                             mPersistence.getLastPersistTimeMillisDLocked();
@@ -197,16 +222,8 @@ final class HistoricalRegistry {
         }
     }
 
-    void systemReady(@NonNull ContentResolver resolver) {
-        updateParametersFromSetting(resolver);
-        final Uri uri = Settings.Global.getUriFor(Settings.Global.APPOP_HISTORY_PARAMETERS);
-        resolver.registerContentObserver(uri, false, new ContentObserver(
-                FgThread.getHandler()) {
-            @Override
-            public void onChange(boolean selfChange) {
-                updateParametersFromSetting(resolver);
-            }
-        });
+    private boolean isPersistenceInitializedMLocked() {
+        return mPersistence != null;
     }
 
     private void updateParametersFromSetting(@NonNull ContentResolver resolver) {
@@ -274,6 +291,11 @@ final class HistoricalRegistry {
                 makeRelativeToEpochStart(currentOps, nowMillis);
                 currentOps.accept(visitor);
 
+                if(isPersistenceInitializedMLocked()) {
+                    Slog.e(LOG_TAG, "Interaction before persistence initialized");
+                    return;
+                }
+
                 final List<HistoricalOps> ops = mPersistence.readHistoryDLocked();
                 if (ops != null) {
                     // TODO (bug:122218838): Make sure this is properly dumped
@@ -302,12 +324,21 @@ final class HistoricalRegistry {
     void getHistoricalOpsFromDiskRaw(int uid, @NonNull String packageName,
             @Nullable String[] opNames, long beginTimeMillis, long endTimeMillis,
             @OpFlags int flags, @NonNull RemoteCallback callback) {
-        final HistoricalOps result = new HistoricalOps(beginTimeMillis, endTimeMillis);
-        mPersistence.collectHistoricalOpsDLocked(result, uid, packageName, opNames,
-                beginTimeMillis, endTimeMillis, flags);
-        final Bundle payload = new Bundle();
-        payload.putParcelable(AppOpsManager.KEY_HISTORICAL_OPS, result);
-        callback.sendResult(payload);
+        synchronized (mOnDiskLock) {
+            synchronized (mInMemoryLock) {
+                if (!isPersistenceInitializedMLocked()) {
+                    Slog.e(LOG_TAG, "Interaction before persistence initialized");
+                    callback.sendResult(new Bundle());
+                    return;
+                }
+                final HistoricalOps result = new HistoricalOps(beginTimeMillis, endTimeMillis);
+                mPersistence.collectHistoricalOpsDLocked(result, uid, packageName, opNames,
+                        beginTimeMillis, endTimeMillis, flags);
+                final Bundle payload = new Bundle();
+                payload.putParcelable(AppOpsManager.KEY_HISTORICAL_OPS, result);
+                callback.sendResult(payload);
+            }
+        }
     }
 
     void getHistoricalOps(int uid, @NonNull String packageName,
@@ -331,6 +362,12 @@ final class HistoricalRegistry {
             boolean collectOpsFromDisk;
 
             synchronized (mInMemoryLock) {
+                if (!isPersistenceInitializedMLocked()) {
+                    Slog.e(LOG_TAG, "Interaction before persistence initialized");
+                    callback.sendResult(new Bundle());
+                    return;
+                }
+
                 currentOps = getUpdatedPendingHistoricalOpsMLocked(currentTimeMillis);
                 if (!(inMemoryAdjBeginTimeMillis >= currentOps.getEndTimeMillis()
                         || inMemoryAdjEndTimeMillis <= currentOps.getBeginTimeMillis())) {
@@ -374,6 +411,10 @@ final class HistoricalRegistry {
             @UidState int uidState, @OpFlags int flags) {
         synchronized (mInMemoryLock) {
             if (mMode == AppOpsManager.HISTORICAL_MODE_ENABLED_ACTIVE) {
+                if (!isPersistenceInitializedMLocked()) {
+                    Slog.e(LOG_TAG, "Interaction before persistence initialized");
+                    return;
+                }
                 getUpdatedPendingHistoricalOpsMLocked(System.currentTimeMillis())
                         .increaseAccessCount(op, uid, packageName, uidState, flags, 1);
             }
@@ -384,6 +425,10 @@ final class HistoricalRegistry {
             @UidState int uidState, @OpFlags int flags) {
         synchronized (mInMemoryLock) {
             if (mMode == AppOpsManager.HISTORICAL_MODE_ENABLED_ACTIVE) {
+                if (!isPersistenceInitializedMLocked()) {
+                    Slog.e(LOG_TAG, "Interaction before persistence initialized");
+                    return;
+                }
                 getUpdatedPendingHistoricalOpsMLocked(System.currentTimeMillis())
                         .increaseRejectCount(op, uid, packageName, uidState, flags, 1);
             }
@@ -394,6 +439,10 @@ final class HistoricalRegistry {
             @UidState int uidState, @OpFlags int flags, long increment) {
         synchronized (mInMemoryLock) {
             if (mMode == AppOpsManager.HISTORICAL_MODE_ENABLED_ACTIVE) {
+                if (!isPersistenceInitializedMLocked()) {
+                    Slog.e(LOG_TAG, "Interaction before persistence initialized");
+                    return;
+                }
                 getUpdatedPendingHistoricalOpsMLocked(System.currentTimeMillis())
                         .increaseAccessDuration(op, uid, packageName, uidState, flags, increment);
             }
@@ -404,6 +453,8 @@ final class HistoricalRegistry {
             long baseSnapshotInterval, long intervalCompressionMultiplier) {
         synchronized (mOnDiskLock) {
             synchronized (mInMemoryLock) {
+                // NOTE: We allow this call if persistence is not initialized as
+                // it is a part of the persistence initialization process.
                 boolean resampleHistory = false;
                 Slog.i(LOG_TAG, "New history parameters: mode:"
                         + AppOpsManager.historicalModeToString(mMode) + " baseSnapshotInterval:"
@@ -412,7 +463,7 @@ final class HistoricalRegistry {
                 if (mMode != mode) {
                     mMode = mode;
                     if (mMode == AppOpsManager.HISTORICAL_MODE_DISABLED) {
-                        clearHistoryOnDiskLocked();
+                        clearHistoryOnDiskDLocked();
                     }
                 }
                 if (mBaseSnapshotInterval != baseSnapshotInterval) {
@@ -433,6 +484,10 @@ final class HistoricalRegistry {
     void offsetHistory(long offsetMillis) {
         synchronized (mOnDiskLock) {
             synchronized (mInMemoryLock) {
+                if (!isPersistenceInitializedMLocked()) {
+                    Slog.e(LOG_TAG, "Interaction before persistence initialized");
+                    return;
+                }
                 final List<HistoricalOps> history = mPersistence.readHistoryDLocked();
                 clearHistory();
                 if (history != null) {
@@ -453,6 +508,10 @@ final class HistoricalRegistry {
     void addHistoricalOps(HistoricalOps ops) {
         final List<HistoricalOps> pendingWrites;
         synchronized (mInMemoryLock) {
+            if (!isPersistenceInitializedMLocked()) {
+                Slog.e(LOG_TAG, "Interaction before persistence initialized");
+                return;
+            }
             // The history files start from mBaseSnapshotInterval - take this into account.
             ops.offsetBeginAndEndTime(mBaseSnapshotInterval);
             mPendingWrites.offerFirst(ops);
@@ -468,6 +527,10 @@ final class HistoricalRegistry {
     }
 
     void resetHistoryParameters() {
+        if (!isPersistenceInitializedMLocked()) {
+            Slog.e(LOG_TAG, "Interaction before persistence initialized");
+            return;
+        }
         setHistoryParameters(DEFAULT_MODE, DEFAULT_SNAPSHOT_INTERVAL_MILLIS,
                 DEFAULT_COMPRESSION_STEP);
     }
@@ -475,6 +538,10 @@ final class HistoricalRegistry {
     void clearHistory(int uid, String packageName) {
         synchronized (mOnDiskLock) {
             synchronized (mInMemoryLock) {
+                if (!isPersistenceInitializedMLocked()) {
+                    Slog.e(LOG_TAG, "Interaction before persistence initialized");
+                    return;
+                }
                 if (mMode != AppOpsManager.HISTORICAL_MODE_ENABLED_ACTIVE) {
                     return;
                 }
@@ -493,11 +560,17 @@ final class HistoricalRegistry {
 
     void clearHistory() {
         synchronized (mOnDiskLock) {
-            clearHistoryOnDiskLocked();
+            synchronized (mInMemoryLock) {
+                if (!isPersistenceInitializedMLocked()) {
+                    Slog.e(LOG_TAG, "Interaction before persistence initialized");
+                    return;
+                }
+                clearHistoryOnDiskDLocked();
+            }
         }
     }
 
-    private void clearHistoryOnDiskLocked() {
+    private void clearHistoryOnDiskDLocked() {
         BackgroundThread.getHandler().removeMessages(MSG_WRITE_PENDING_HISTORY);
         synchronized (mInMemoryLock) {
             mCurrentHistoricalOps = null;
@@ -718,14 +791,28 @@ final class HistoricalRegistry {
                 baseDir = mHistoricalAppOpsDir.startRead();
                 final File[] files = baseDir.listFiles();
                 if (files != null && files.length > 0) {
-                    final Set<File> historyFiles = new ArraySet<>();
-                    Collections.addAll(historyFiles, files);
-                    for (int i = 0;; i++) {
-                        final File file = generateFile(baseDir, i);
-                        if (historyFiles.contains(file)) {
-                            return file.lastModified();
+                    File shortestFile = null;
+                    for (File candidate : files) {
+                        final String candidateName = candidate.getName();
+                        if (!candidateName.endsWith(HISTORY_FILE_SUFFIX)) {
+                            continue;
+                        }
+                        if (shortestFile == null) {
+                            shortestFile = candidate;
+                        } else if (candidateName.length() < shortestFile.getName().length()) {
+                            shortestFile = candidate;
                         }
                     }
+                    if (shortestFile == null) {
+                        return 0;
+                    }
+                    final String shortestNameNoExtension = shortestFile.getName()
+                            .replace(HISTORY_FILE_SUFFIX, "");
+                    try {
+                        return Long.parseLong(shortestNameNoExtension);
+                    } catch (NumberFormatException e) {
+                        return 0;
+                    }
                 }
                 mHistoricalAppOpsDir.finishRead();
             } catch (Throwable e) {