Merge "Fix a bug with the settings for skipping integrity check for verifiers." into rvc-dev am: 9651ab2de4

Change-Id: Id0803298de2c87845a631f8be92d21d5041555db

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

@@ -118,8 +118,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");
@@ -130,13 +128,7 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
                 LocalServices.getService(PackageManagerInternal.class),
                 RuleEvaluationEngine.getRuleEvaluationEngine(),
                 IntegrityFileManager.getInstance(),
-                handlerThread.getThreadHandler(),
+                handlerThread.getThreadHandler());
-                Settings.Global.getInt(
-                        context.getContentResolver(),
-                        Settings.Global.INTEGRITY_CHECK_INCLUDES_RULE_PROVIDER,
-                        0)
-                        == 1
-        );
     }
 
     @VisibleForTesting
@@ -145,14 +137,12 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
             PackageManagerInternal packageManagerInternal,
             RuleEvaluationEngine evaluationEngine,
             IntegrityFileManager integrityFileManager,
-            Handler handler,
+            Handler handler) {
-            boolean checkIntegrityForRuleProviders) {
         mContext = context;
         mPackageManagerInternal = packageManagerInternal;
         mEvaluationEngine = evaluationEngine;
         mIntegrityFileManager = integrityFileManager;
         mHandler = handler;
-        mCheckIntegrityForRuleProviders = checkIntegrityForRuleProviders;
 
         IntentFilter integrityVerificationFilter = new IntentFilter();
         integrityVerificationFilter.addAction(ACTION_PACKAGE_NEEDS_INTEGRITY_VERIFICATION);
@@ -263,7 +253,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(
@@ -275,8 +265,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));
@@ -635,4 +623,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,
+                        mHandler);
-                        /* checkIntegrityForRuleProviders= */ true);
 
         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 =
+        setIntegrityCheckIncludesRuleProvider(false);
-                new AppIntegrityManagerServiceImpl(
-                        mMockContext,
-                        mPackageManagerInternal,
-                        mRuleEvaluationEngine,
-                        mIntegrityFileManager,
-                        mHandler,
-                        /* checkIntegrityForRuleProviders= */ 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);
+        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);
+    }
 }