Fix a bug with the settings for skipping integrity check for verifiers.

Currently we read it at the start and cache it. This will cause any change in
settings to have effect only after restart. This is probably not what we want.

Bug: 150220476
Test: atest AppIntegrityManagerServiceImplTest
Change-Id: Icf4fc0e83e3563544b022c722e893a98f3512f25

services/core/java/com/android/server/integrity/AppIntegrityManagerServiceImpl.java

@@ -114,8 +114,6 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
     private final RuleEvaluationEngine mEvaluationEngine;
     private final IntegrityFileManager mIntegrityFileManager;
 
-    private final boolean mCheckIntegrityForRuleProviders;
-
     /** Create an instance of {@link AppIntegrityManagerServiceImpl}. */
     public static AppIntegrityManagerServiceImpl create(Context context) {
         HandlerThread handlerThread = new HandlerThread("AppIntegrityManagerServiceHandler");
@@ -126,13 +124,7 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
                 LocalServices.getService(PackageManagerInternal.class),
                 RuleEvaluationEngine.getRuleEvaluationEngine(),
                 IntegrityFileManager.getInstance(),
-                handlerThread.getThreadHandler(),
-                Settings.Global.getInt(
-                        context.getContentResolver(),
-                        Settings.Global.INTEGRITY_CHECK_INCLUDES_RULE_PROVIDER,
-                        0)
-                        == 1
-        );
+                handlerThread.getThreadHandler());
     }
 
     @VisibleForTesting
@@ -141,14 +133,12 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
             PackageManagerInternal packageManagerInternal,
             RuleEvaluationEngine evaluationEngine,
             IntegrityFileManager integrityFileManager,
-            Handler handler,
-            boolean checkIntegrityForRuleProviders) {
+            Handler handler) {
         mContext = context;
         mPackageManagerInternal = packageManagerInternal;
         mEvaluationEngine = evaluationEngine;
         mIntegrityFileManager = integrityFileManager;
         mHandler = handler;
-        mCheckIntegrityForRuleProviders = checkIntegrityForRuleProviders;
 
         IntentFilter integrityVerificationFilter = new IntentFilter();
         integrityVerificationFilter.addAction(ACTION_PACKAGE_NEEDS_INTEGRITY_VERIFICATION);
@@ -259,7 +249,7 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
             String installerPackageName = getInstallerPackageName(intent);
 
             // Skip integrity verification if the verifier is doing the install.
-            if (!mCheckIntegrityForRuleProviders
+            if (!integrityCheckIncludesRuleProvider()
                     && isRuleProvider(installerPackageName)) {
                 Slog.i(TAG, "Verifier doing the install. Skipping integrity check.");
                 mPackageManagerInternal.setIntegrityVerificationResult(
@@ -271,8 +261,6 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
             List<String> installerCertificates =
                     getInstallerCertificateFingerprint(installerPackageName);
 
-            Slog.w(TAG, appCertificates.toString());
-
             AppInstallMetadata.Builder builder = new AppInstallMetadata.Builder();
 
             builder.setPackageName(getPackageNameNormalized(packageName));
@@ -631,4 +619,12 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
         return getAllowedRuleProviders().stream()
                 .anyMatch(ruleProvider -> ruleProvider.equals(installerPackageName));
     }
+
+    private boolean integrityCheckIncludesRuleProvider() {
+        return Settings.Global.getInt(
+                mContext.getContentResolver(),
+                Settings.Global.INTEGRITY_CHECK_INCLUDES_RULE_PROVIDER,
+                0)
+                == 1;
+    }
 }

services/tests/servicestests/src/com/android/server/integrity/AppIntegrityManagerServiceImplTest.java

@@ -60,6 +60,7 @@ import android.content.res.Resources;
 import android.net.Uri;
 import android.os.Handler;
 import android.os.Message;
+import android.provider.Settings;
 
 import androidx.test.InstrumentationRegistry;
 
@@ -119,7 +120,6 @@ public class AppIntegrityManagerServiceImplTest {
     private static final String PLAY_STORE_PKG = "com.android.vending";
     private static final String ADB_INSTALLER = "adb";
     private static final String PLAY_STORE_CERT = "play_store_cert";
-    private static final String ADB_CERT = "";
 
     @org.junit.Rule
     public MockitoRule mMockitoRule = MockitoJUnit.rule();
@@ -137,11 +137,12 @@ public class AppIntegrityManagerServiceImplTest {
     @Mock
     Handler mHandler;
 
+    private final Context mRealContext = InstrumentationRegistry.getTargetContext();
+
     private PackageManager mSpyPackageManager;
     private File mTestApk;
     private File mTestApkTwoCerts;
 
-    private final Context mRealContext = InstrumentationRegistry.getTargetContext();
     // under test
     private AppIntegrityManagerServiceImpl mService;
 
@@ -163,8 +164,7 @@ public class AppIntegrityManagerServiceImplTest {
                         mPackageManagerInternal,
                         mRuleEvaluationEngine,
                         mIntegrityFileManager,
-                        mHandler,
-                        /* checkIntegrityForRuleProviders= */ true);
+                        mHandler);
 
         mSpyPackageManager = spy(mRealContext.getPackageManager());
         // setup mocks to prevent NPE
@@ -172,6 +172,9 @@ public class AppIntegrityManagerServiceImplTest {
         when(mMockContext.getResources()).thenReturn(mMockResources);
         when(mMockResources.getStringArray(anyInt())).thenReturn(new String[]{});
         when(mIntegrityFileManager.initialized()).thenReturn(true);
+        // These are needed to override the Settings.Global.get result.
+        when(mMockContext.getContentResolver()).thenReturn(mRealContext.getContentResolver());
+        setIntegrityCheckIncludesRuleProvider(true);
     }
 
     @After
@@ -201,6 +204,7 @@ public class AppIntegrityManagerServiceImplTest {
     @Test
     public void updateRuleSet_notSystemApp() throws Exception {
         whitelistUsAsRuleProvider();
+        makeUsSystemApp(false);
         Rule rule =
                 new Rule(
                         new AtomicFormula.BooleanAtomicFormula(AtomicFormula.PRE_INSTALLED, true),
@@ -411,14 +415,7 @@ public class AppIntegrityManagerServiceImplTest {
     public void verifierAsInstaller_skipIntegrityVerification() throws Exception {
         whitelistUsAsRuleProvider();
         makeUsSystemApp();
-        mService =
-                new AppIntegrityManagerServiceImpl(
-                        mMockContext,
-                        mPackageManagerInternal,
-                        mRuleEvaluationEngine,
-                        mIntegrityFileManager,
-                        mHandler,
-                        /* checkIntegrityForRuleProviders= */ false);
+        setIntegrityCheckIncludesRuleProvider(false);
         ArgumentCaptor<BroadcastReceiver> broadcastReceiverCaptor =
                 ArgumentCaptor.forClass(BroadcastReceiver.class);
         verify(mMockContext, atLeastOnce())
@@ -460,12 +457,21 @@ public class AppIntegrityManagerServiceImplTest {
     }
 
     private void makeUsSystemApp() throws Exception {
+        makeUsSystemApp(true);
+    }
+
+    private void makeUsSystemApp(boolean isSystemApp) throws Exception {
         PackageInfo packageInfo =
                 mRealContext.getPackageManager().getPackageInfo(TEST_FRAMEWORK_PACKAGE, 0);
-        packageInfo.applicationInfo.flags |= ApplicationInfo.FLAG_SYSTEM;
+        if (isSystemApp) {
+            packageInfo.applicationInfo.flags |= ApplicationInfo.FLAG_SYSTEM;
+        } else {
+            packageInfo.applicationInfo.flags &= ~ApplicationInfo.FLAG_SYSTEM;
+        }
         doReturn(packageInfo)
                 .when(mSpyPackageManager)
                 .getPackageInfo(eq(TEST_FRAMEWORK_PACKAGE), anyInt());
+        when(mMockContext.getPackageManager()).thenReturn(mSpyPackageManager);
     }
 
     private Intent makeVerificationIntent() throws Exception {
@@ -492,4 +498,13 @@ public class AppIntegrityManagerServiceImplTest {
         intent.putExtra(Intent.EXTRA_LONG_VERSION_CODE, VERSION_CODE);
         return intent;
     }
+
+    private void setIntegrityCheckIncludesRuleProvider(boolean shouldInclude) throws Exception {
+        int value = shouldInclude ? 1 : 0;
+        Settings.Global.putInt(mRealContext.getContentResolver(),
+                Settings.Global.INTEGRITY_CHECK_INCLUDES_RULE_PROVIDER, value);
+        assertThat(Settings.Global.getInt(mRealContext.getContentResolver(),
+                Settings.Global.INTEGRITY_CHECK_INCLUDES_RULE_PROVIDER, -1) == 1).isEqualTo(
+                shouldInclude);
+    }
 }