Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 47641072 authored by Svet Ganov's avatar Svet Ganov
Browse files

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: If7044c83268560972a8112a6fbf853cdd61a55b3
parent 5eacc12e

services/core/java/com/android/server/appop/AppOpsService.java

+1 −1
Original line number Diff line number Diff line
@@ -201,7 +201,7 @@ public class AppOpsService extends IAppOpsService.Stub { 
    @VisibleForTesting

    final SparseArray<UidState> mUidStates = new SparseArray<>();



    private final HistoricalRegistry mHistoricalRegistry = new HistoricalRegistry(this);

    final HistoricalRegistry mHistoricalRegistry = new HistoricalRegistry(this);



    long mLastRealtime;

services/core/java/com/android/server/appop/HistoricalRegistry.java

+134 −47
Original line number Diff line number Diff line
@@ -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) {

    }



    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,6 +324,13 @@ final class HistoricalRegistry { 
    void getHistoricalOpsFromDiskRaw(int uid, @NonNull String packageName,

            @Nullable String[] opNames, long beginTimeMillis, long endTimeMillis,

            @OpFlags int flags, @NonNull RemoteCallback callback) {

        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);
@@ -309,6 +338,8 @@ final class HistoricalRegistry { 
                payload.putParcelable(AppOpsManager.KEY_HISTORICAL_OPS, result);

                callback.sendResult(payload);

            }

        }

    }



    void getHistoricalOps(int uid, @NonNull String packageName,

            @Nullable String[] opNames, long beginTimeMillis, long endTimeMillis,
@@ -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,18 +560,24 @@ 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;

            mNextPersistDueTimeMillis = System.currentTimeMillis();

            mPendingWrites.clear();

        }

        mPersistence.clearHistoryDLocked();

        Persistence.clearHistoryDLocked();

    }



    private @NonNull HistoricalOps getUpdatedPendingHistoricalOpsMLocked(long now) {
@@ -639,7 +712,7 @@ final class HistoricalRegistry { 
            mIntervalCompressionMultiplier = intervalCompressionMultiplier;

        }



        private final AtomicDirectory mHistoricalAppOpsDir = new AtomicDirectory(

        private static final AtomicDirectory sHistoricalAppOpsDir = new AtomicDirectory(

                new File(new File(Environment.getDataSystemDirectory(), "appops"), "history"));



        private File generateFile(@NonNull File baseDir, int depth) {
@@ -663,8 +736,8 @@ final class HistoricalRegistry { 
            persistHistoricalOpsDLocked(historicalOps);

        }



        void clearHistoryDLocked() {

            mHistoricalAppOpsDir.delete();

        static void clearHistoryDLocked() {

            sHistoricalAppOpsDir.delete();

        }



        void persistHistoricalOpsDLocked(@NonNull List<HistoricalOps> ops) {
@@ -673,8 +746,8 @@ final class HistoricalRegistry { 
                enforceOpsWellFormed(ops);

            }

            try {

                final File newBaseDir = mHistoricalAppOpsDir.startWrite();

                final File oldBaseDir = mHistoricalAppOpsDir.getBackupDirectory();

                final File newBaseDir = sHistoricalAppOpsDir.startWrite();

                final File oldBaseDir = sHistoricalAppOpsDir.getBackupDirectory();

                final HistoricalFilesInvariant filesInvariant;

                if (DEBUG) {

                    filesInvariant = new HistoricalFilesInvariant();
@@ -686,10 +759,10 @@ final class HistoricalRegistry { 
                if (DEBUG) {

                    filesInvariant.stopTracking(newBaseDir);

                }

                mHistoricalAppOpsDir.finishWrite();

                sHistoricalAppOpsDir.finishWrite();

            } catch (Throwable t) {

                wtf("Failed to write historical app ops, restoring backup", t, null);

                mHistoricalAppOpsDir.failWrite();

                sHistoricalAppOpsDir.failWrite();

            }

        }
@@ -715,22 +788,36 @@ final class HistoricalRegistry { 
        long getLastPersistTimeMillisDLocked() {

            File baseDir = null;

            try {

                baseDir = mHistoricalAppOpsDir.startRead();

                baseDir = sHistoricalAppOpsDir.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();

                sHistoricalAppOpsDir.finishRead();

            } catch (Throwable e) {

                wtf("Error reading historical app ops. Deleting history.", e, baseDir);

                mHistoricalAppOpsDir.delete();

                sHistoricalAppOpsDir.delete();

            }

            return 0;

        }
@@ -755,7 +842,7 @@ final class HistoricalRegistry { 
                long filterBeginTimeMillis, long filterEndTimeMillis, @OpFlags int filterFlags) {

            File baseDir = null;

            try {

                baseDir = mHistoricalAppOpsDir.startRead();

                baseDir = sHistoricalAppOpsDir.startRead();

                final HistoricalFilesInvariant filesInvariant;

                if (DEBUG) {

                    filesInvariant = new HistoricalFilesInvariant();
@@ -770,11 +857,11 @@ final class HistoricalRegistry { 
                if (DEBUG) {

                    filesInvariant.stopTracking(baseDir);

                }

                mHistoricalAppOpsDir.finishRead();

                sHistoricalAppOpsDir.finishRead();

                return ops;

            } catch (Throwable t) {

                wtf("Error reading historical app ops. Deleting history.", t, baseDir);

                mHistoricalAppOpsDir.delete();

                sHistoricalAppOpsDir.delete();

            }

            return null;

        }
@@ -1241,7 +1328,7 @@ final class HistoricalRegistry { 


        private void writeHistoricalOpsDLocked(@Nullable List<HistoricalOps> allOps,

                long intervalOverflowMillis, @NonNull File file) throws IOException {

            final FileOutputStream output = mHistoricalAppOpsDir.openWrite(file);

            final FileOutputStream output = sHistoricalAppOpsDir.openWrite(file);

            try {

                final XmlSerializer serializer = Xml.newSerializer();

                serializer.setOutput(output, StandardCharsets.UTF_8.name());
@@ -1263,9 +1350,9 @@ final class HistoricalRegistry { 
                }

                serializer.endTag(null, TAG_HISTORY);

                serializer.endDocument();

                mHistoricalAppOpsDir.closeWrite(output);

                sHistoricalAppOpsDir.closeWrite(output);

            } catch (IOException e) {

                mHistoricalAppOpsDir.failWrite(output);

                sHistoricalAppOpsDir.failWrite(output);

                throw e;

            }

        }

services/tests/mockingservicestests/src/com/android/server/appop/AppOpsServiceTest.java

+29 −18
Original line number Diff line number Diff line
@@ -35,6 +35,7 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.when; 
import static com.google.common.truth.Truth.assertThat;

import static com.google.common.truth.Truth.assertWithMessage;



import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.anyString;

import static org.mockito.ArgumentMatchers.eq;
@@ -44,12 +45,14 @@ import android.app.ActivityManager; 
import android.app.AppOpsManager;

import android.app.AppOpsManager.OpEntry;

import android.app.AppOpsManager.PackageOps;

import android.content.ContentResolver;

import android.content.Context;

import android.content.pm.PackageManagerInternal;

import android.os.Handler;

import android.os.HandlerThread;

import android.os.Process;

import android.os.RemoteCallback;

import android.provider.Settings;



import androidx.test.InstrumentationRegistry;

import androidx.test.filters.SmallTest;
@@ -93,24 +96,9 @@ public class AppOpsServiceTest { 
    private long mTestStartMillis;

    private StaticMockitoSession mMockingSession;



    @Before

    public void mockPackageManagerInternalGetApplicationInfo() {

        mMockingSession = mockitoSession()

                .strictness(Strictness.LENIENT)

                .spyStatic(LocalServices.class)

                .startMocking();



        // Mock LocalServices.getService(PackageManagerInternal.class).getApplicationInfo dependency

        // needed by AppOpsService

        PackageManagerInternal mockPackageManagerInternal = mock(PackageManagerInternal.class);

        when(mockPackageManagerInternal.getApplicationInfo(eq(sMyPackageName), anyInt(), anyInt(),

                anyInt())).thenReturn(sContext.getApplicationInfo());

        doReturn(mockPackageManagerInternal).when(

                () -> LocalServices.getService(PackageManagerInternal.class));

    }



    private void setupAppOpsService() {

        mAppOpsService = new AppOpsService(mAppOpsFile, mHandler);

        mAppOpsService.mHistoricalRegistry.systemReady(sContext.getContentResolver());

        mAppOpsService.mContext = spy(sContext);



        // Always approve all permission checks
@@ -131,7 +119,10 @@ public class AppOpsServiceTest { 
        mHandler = new Handler(handlerThread.getLooper());

        mMyUid = Process.myUid();



        initializeStaticMocks();



        setupAppOpsService();



        mTestStartMillis = System.currentTimeMillis();

    }
@@ -140,6 +131,26 @@ public class AppOpsServiceTest { 
        mMockingSession.finishMocking();

    }



    private void initializeStaticMocks() {

        mMockingSession = mockitoSession()

                .strictness(Strictness.LENIENT)

                .spyStatic(LocalServices.class)

                .spyStatic(Settings.Global.class)

                .startMocking();



        // Mock LocalServices.getService(PackageManagerInternal.class).getApplicationInfo dependency

        // needed by AppOpsService

        PackageManagerInternal mockPackageManagerInternal = mock(PackageManagerInternal.class);

        when(mockPackageManagerInternal.getApplicationInfo(eq(sMyPackageName), anyInt(), anyInt(),

                anyInt())).thenReturn(sContext.getApplicationInfo());

        doReturn(mockPackageManagerInternal).when(

                () -> LocalServices.getService(PackageManagerInternal.class));



        // Mock behavior to use specific Settings.Global.APPOP_HISTORY_PARAMETERS

        doReturn(null).when(() -> Settings.Global.getString(any(ContentResolver.class),

                eq(Settings.Global.APPOP_HISTORY_PARAMETERS)));

    }



    @Test

    public void testGetOpsForPackage_noOpsLogged() {

        assertThat(getLoggedOps()).isNull();
@@ -344,7 +355,7 @@ public class AppOpsServiceTest { 
    }



    @Test

    public void testUidProcStateChange_cachedToFgs() throws Exception {

    public void testUidProcStateChange_cachedToFgs() {

        setupProcStateTests();



        mAppOpsService.updateUidProcState(mMyUid, ActivityManager.PROCESS_STATE_CACHED_EMPTY);
@@ -357,7 +368,7 @@ public class AppOpsServiceTest { 
    }



    @Test

    public void testUidProcStateChange_cachedToFgsLocation() throws Exception {

    public void testUidProcStateChange_cachedToFgsLocation() {

        setupProcStateTests();



        mAppOpsService.updateUidProcState(mMyUid, ActivityManager.PROCESS_STATE_CACHED_EMPTY);