Merge "Resolve the issue that when there are no integrity rule files pushed, we do not evaluate the rules based on android manifest."

4 files changed, 123 insertions, 145 deletions

diff --git a/services/core/java/com/android/server/integrity/AppIntegrityManagerServiceImpl.java b/services/core/java/com/android/server/integrity/AppIntegrityManagerServiceImpl.java
index 214dcc0220ff..11fe15f0e12b 100644
--- a/services/core/java/com/android/server/integrity/AppIntegrityManagerServiceImpl.java
+++ b/services/core/java/com/android/server/integrity/AppIntegrityManagerServiceImpl.java
@@ -46,7 +46,6 @@ import android.os.Binder;
 import android.os.Bundle;
 import android.os.Handler;
 import android.os.HandlerThread;
-import android.os.RemoteException;
 import android.util.Slog;
 import android.util.StatsLog;
 
@@ -158,8 +157,7 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
 
     @Override
     public void updateRuleSet(
-            String version, ParceledListSlice<Rule> rules, IntentSender statusReceiver)
-            throws RemoteException {
+            String version, ParceledListSlice<Rule> rules, IntentSender statusReceiver) {
         String ruleProvider = getCallerPackageNameOrThrow();
 
         mHandler.post(
@@ -190,7 +188,7 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
     }
 
     @Override
-    public String getCurrentRuleSetVersion() throws RemoteException {
+    public String getCurrentRuleSetVersion() {
         getCallerPackageNameOrThrow();
 
         RuleMetadata ruleMetadata = mIntegrityFileManager.readMetadata();
@@ -200,7 +198,7 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
     }
 
     @Override
-    public String getCurrentRuleSetProvider() throws RemoteException {
+    public String getCurrentRuleSetProvider() {
         getCallerPackageNameOrThrow();
 
         RuleMetadata ruleMetadata = mIntegrityFileManager.readMetadata();
@@ -212,14 +210,6 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
     private void handleIntegrityVerification(Intent intent) {
         int verificationId = intent.getIntExtra(EXTRA_VERIFICATION_ID, -1);
 
-        // Fail early if we don't have any rules at all.
-        if (!mIntegrityFileManager.initialized()) {
-            Slog.i(TAG, "Rules not initialized. Skipping integrity check.");
-            mPackageManagerInternal.setIntegrityVerificationResult(
-                    verificationId, PackageManagerInternal.INTEGRITY_VERIFICATION_ALLOW);
-            return;
-        }
-
         try {
             Slog.i(TAG, "Received integrity verification intent " + intent.toString());
             Slog.i(TAG, "Extras " + intent.getExtras());
diff --git a/services/core/java/com/android/server/integrity/engine/RuleEvaluationEngine.java b/services/core/java/com/android/server/integrity/engine/RuleEvaluationEngine.java
index 1a6b3d8b8411..79e69e15ff67 100644
--- a/services/core/java/com/android/server/integrity/engine/RuleEvaluationEngine.java
+++ b/services/core/java/com/android/server/integrity/engine/RuleEvaluationEngine.java
@@ -76,6 +76,11 @@ public class RuleEvaluationEngine {
     }
 
     private List<Rule> loadRules(AppInstallMetadata appInstallMetadata) {
+        if (!mIntegrityFileManager.initialized()) {
+            Slog.w(TAG, "Integrity rule files are not available. Evaluating only manifest rules.");
+            return new ArrayList<>();
+        }
+
         try {
             return mIntegrityFileManager.readRules(appInstallMetadata);
         } catch (Exception e) {
diff --git a/services/tests/servicestests/src/com/android/server/integrity/AppIntegrityManagerServiceImplTest.java b/services/tests/servicestests/src/com/android/server/integrity/AppIntegrityManagerServiceImplTest.java
index 9a633931017e..770afb0a24d5 100644
--- a/services/tests/servicestests/src/com/android/server/integrity/AppIntegrityManagerServiceImplTest.java
+++ b/services/tests/servicestests/src/com/android/server/integrity/AppIntegrityManagerServiceImplTest.java
@@ -32,11 +32,9 @@ import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
-import static org.mockito.internal.verification.VerificationModeFactory.times;
 
 import android.content.BroadcastReceiver;
 import android.content.Context;
@@ -59,7 +57,6 @@ import android.os.Message;
 import androidx.test.InstrumentationRegistry;
 
 import com.android.internal.R;
-import com.android.server.LocalServices;
 import com.android.server.integrity.engine.RuleEvaluationEngine;
 import com.android.server.integrity.model.IntegrityCheckResult;
 import com.android.server.testutils.TestUtils;
@@ -147,29 +144,8 @@ public class AppIntegrityManagerServiceImplTest {
         when(mIntegrityFileManager.initialized()).thenReturn(true);
     }
 
-    // This is not a test of the class, but more of a safeguard that we don't block any install in
-    // the default case. This is needed because we don't have any emergency kill switch to disable
-    // this component.
-    @Test
-    public void default_allow() throws Exception {
-        LocalServices.removeServiceForTest(PackageManagerInternal.class);
-        LocalServices.addService(PackageManagerInternal.class, mPackageManagerInternal);
-        mService = AppIntegrityManagerServiceImpl.create(mMockContext);
-        ArgumentCaptor<BroadcastReceiver> broadcastReceiverCaptor =
-                ArgumentCaptor.forClass(BroadcastReceiver.class);
-        verify(mMockContext, times(2))
-                .registerReceiver(broadcastReceiverCaptor.capture(), any(), any(), any());
-        Intent intent = makeVerificationIntent();
-
-        broadcastReceiverCaptor.getValue().onReceive(mMockContext, intent);
-
-        // Since we are not mocking handler in this case, we must wait.
-        // 2 seconds should be a sensible timeout.
-        Thread.sleep(2000);
-        verify(mPackageManagerInternal)
-                .setIntegrityVerificationResult(
-                        1, PackageManagerInternal.INTEGRITY_VERIFICATION_ALLOW);
-    }
+    // TODO(b/148370598): Implement a test to validate that allow response is retuned when the test
+    //    request times out.
 
     @Test
     public void updateRuleSet_notAuthorized() throws Exception {
@@ -381,10 +357,10 @@ public class AppIntegrityManagerServiceImplTest {
         broadcastReceiverCaptor.getValue().onReceive(mMockContext, intent);
         runJobInHandler();
 
+        // The evaluation will still run since we still evaluate manifest based rules.
         verify(mPackageManagerInternal)
                 .setIntegrityVerificationResult(
                         1, PackageManagerInternal.INTEGRITY_VERIFICATION_ALLOW);
-        verify(mSpyPackageManager, never()).getPackageArchiveInfo(any(), anyInt());
     }
 
     @Test
@@ -432,8 +408,8 @@ public class AppIntegrityManagerServiceImplTest {
 
     private Intent makeVerificationIntent() throws Exception {
         PackageInfo packageInfo =
-                mRealContext.getPackageManager().getPackageInfo(TEST_FRAMEWORK_PACKAGE,
-                        PackageManager.GET_SIGNATURES);
+                mRealContext.getPackageManager()
+                        .getPackageInfo(TEST_FRAMEWORK_PACKAGE, PackageManager.GET_SIGNATURES);
         doReturn(packageInfo)
                 .when(mSpyPackageManager)
                 .getPackageInfo(eq(INSTALLER), anyInt());
diff --git a/services/tests/servicestests/src/com/android/server/integrity/engine/RuleEvaluationEngineTest.java b/services/tests/servicestests/src/com/android/server/integrity/engine/RuleEvaluationEngineTest.java
index 99157024bb66..26b20965fbf5 100644
--- a/services/tests/servicestests/src/com/android/server/integrity/engine/RuleEvaluationEngineTest.java
+++ b/services/tests/servicestests/src/com/android/server/integrity/engine/RuleEvaluationEngineTest.java
@@ -16,12 +16,12 @@
 
 package com.android.server.integrity.engine;
 
-import static org.junit.Assert.assertEquals;
+import static com.google.common.truth.Truth.assertThat;
+
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.when;
 
 import android.content.integrity.AppInstallMetadata;
-import android.content.integrity.Rule;
 
 import com.android.server.integrity.IntegrityFileManager;
 import com.android.server.integrity.model.IntegrityCheckResult;
@@ -36,7 +36,6 @@ import org.mockito.MockitoAnnotations;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 
 @RunWith(JUnit4.class)
@@ -68,33 +67,28 @@ public class RuleEvaluationEngineTest {
     public void testAllowedInstallers_empty() {
         Map<String, String> allowedInstallers = Collections.emptyMap();
 
-        assertEquals(
-                IntegrityCheckResult.Effect.ALLOW,
-                mEngine.evaluate(
-                        getAppInstallMetadataBuilder()
-                                .setInstallerName(INSTALLER_1)
-                                .setInstallerCertificate(INSTALLER_1_CERT)
-                                .build(),
-                        allowedInstallers)
-                        .getEffect());
-        assertEquals(
-                IntegrityCheckResult.Effect.ALLOW,
-                mEngine.evaluate(
-                        getAppInstallMetadataBuilder()
-                                .setInstallerName(INSTALLER_2)
-                                .setInstallerCertificate(INSTALLER_2_CERT)
-                                .build(),
-                        allowedInstallers)
-                        .getEffect());
-        assertEquals(
-                IntegrityCheckResult.Effect.ALLOW,
-                mEngine.evaluate(
-                        getAppInstallMetadataBuilder()
-                                .setInstallerName(RANDOM_INSTALLER)
-                                .setInstallerCertificate(RANDOM_INSTALLER_CERT)
-                                .build(),
-                        allowedInstallers)
-                        .getEffect());
+        AppInstallMetadata appInstallMetadata1 =
+                getAppInstallMetadataBuilder()
+                        .setInstallerName(INSTALLER_1)
+                        .setInstallerCertificate(INSTALLER_1_CERT)
+                        .build();
+        AppInstallMetadata appInstallMetadata2 =
+                getAppInstallMetadataBuilder()
+                        .setInstallerName(INSTALLER_2)
+                        .setInstallerCertificate(INSTALLER_2_CERT)
+                        .build();
+        AppInstallMetadata appInstallMetadata3 =
+                getAppInstallMetadataBuilder()
+                        .setInstallerName(RANDOM_INSTALLER)
+                        .setInstallerCertificate(RANDOM_INSTALLER_CERT)
+                        .build();
+
+        assertThat(mEngine.evaluate(appInstallMetadata1, allowedInstallers).getEffect())
+                .isEqualTo(IntegrityCheckResult.Effect.ALLOW);
+        assertThat(mEngine.evaluate(appInstallMetadata2, allowedInstallers).getEffect())
+                .isEqualTo(IntegrityCheckResult.Effect.ALLOW);
+        assertThat(mEngine.evaluate(appInstallMetadata3, allowedInstallers).getEffect())
+                .isEqualTo(IntegrityCheckResult.Effect.ALLOW);
     }
 
     @Test
@@ -102,87 +96,100 @@ public class RuleEvaluationEngineTest {
         Map<String, String> allowedInstallers =
                 Collections.singletonMap(INSTALLER_1, INSTALLER_1_CERT);
 
-        assertEquals(
-                IntegrityCheckResult.Effect.ALLOW,
-                mEngine.evaluate(
-                        getAppInstallMetadataBuilder()
-                                .setInstallerName(INSTALLER_1)
-                                .setInstallerCertificate(INSTALLER_1_CERT)
-                                .build(),
-                        allowedInstallers)
-                        .getEffect());
-        assertEquals(
-                IntegrityCheckResult.Effect.DENY,
-                mEngine.evaluate(
-                        getAppInstallMetadataBuilder()
-                                .setInstallerName(RANDOM_INSTALLER)
-                                .setInstallerCertificate(INSTALLER_1_CERT)
-                                .build(),
-                        allowedInstallers)
-                        .getEffect());
-        assertEquals(
-                IntegrityCheckResult.Effect.DENY,
-                mEngine.evaluate(
-                        getAppInstallMetadataBuilder()
-                                .setInstallerName(INSTALLER_1)
-                                .setInstallerCertificate(RANDOM_INSTALLER_CERT)
-                                .build(),
-                        allowedInstallers)
-                        .getEffect());
-        assertEquals(
-                IntegrityCheckResult.Effect.DENY,
-                mEngine.evaluate(
-                        getAppInstallMetadataBuilder()
-                                .setInstallerName(RANDOM_INSTALLER)
-                                .setInstallerCertificate(RANDOM_INSTALLER_CERT)
-                                .build(),
-                        allowedInstallers)
-                        .getEffect());
+        AppInstallMetadata appInstallMetadata1 =
+                getAppInstallMetadataBuilder()
+                        .setInstallerName(INSTALLER_1)
+                        .setInstallerCertificate(INSTALLER_1_CERT)
+                        .build();
+        assertThat(mEngine.evaluate(appInstallMetadata1, allowedInstallers).getEffect())
+                .isEqualTo(IntegrityCheckResult.Effect.ALLOW);
+
+        AppInstallMetadata appInstallMetadata2 =
+                getAppInstallMetadataBuilder()
+                        .setInstallerName(RANDOM_INSTALLER)
+                        .setInstallerCertificate(INSTALLER_1_CERT)
+                        .build();
+        assertThat(mEngine.evaluate(appInstallMetadata2, allowedInstallers).getEffect())
+                .isEqualTo(IntegrityCheckResult.Effect.DENY);
+
+        AppInstallMetadata appInstallMetadata3 =
+                getAppInstallMetadataBuilder()
+                        .setInstallerName(INSTALLER_1)
+                        .setInstallerCertificate(RANDOM_INSTALLER_CERT)
+                        .build();
+        assertThat(mEngine.evaluate(appInstallMetadata3, allowedInstallers).getEffect())
+                .isEqualTo(IntegrityCheckResult.Effect.DENY);
+
+        AppInstallMetadata appInstallMetadata4 =
+                getAppInstallMetadataBuilder()
+                        .setInstallerName(INSTALLER_1)
+                        .setInstallerCertificate(RANDOM_INSTALLER_CERT)
+                        .build();
+        assertThat(mEngine.evaluate(appInstallMetadata4, allowedInstallers).getEffect())
+                .isEqualTo(IntegrityCheckResult.Effect.DENY);
     }
 
     @Test
     public void testAllowedInstallers_multipleElement() {
-        List<Rule> rules = new ArrayList<>();
         Map<String, String> allowedInstallers = new HashMap<>(2);
         allowedInstallers.put(INSTALLER_1, INSTALLER_1_CERT);
         allowedInstallers.put(INSTALLER_2, INSTALLER_2_CERT);
 
-        assertEquals(
-                IntegrityCheckResult.Effect.ALLOW,
-                mEngine.evaluate(
-                        getAppInstallMetadataBuilder()
-                                .setInstallerName(INSTALLER_1)
-                                .setInstallerCertificate(INSTALLER_1_CERT)
-                                .build(),
-                        allowedInstallers)
-                        .getEffect());
-        assertEquals(
-                IntegrityCheckResult.Effect.ALLOW,
-                mEngine.evaluate(
-                        getAppInstallMetadataBuilder()
-                                .setInstallerName(INSTALLER_2)
-                                .setInstallerCertificate(INSTALLER_2_CERT)
-                                .build(),
-                        allowedInstallers)
-                        .getEffect());
-        assertEquals(
-                IntegrityCheckResult.Effect.DENY,
-                mEngine.evaluate(
-                        getAppInstallMetadataBuilder()
-                                .setInstallerName(INSTALLER_1)
-                                .setInstallerCertificate(INSTALLER_2_CERT)
-                                .build(),
-                        allowedInstallers)
-                        .getEffect());
-        assertEquals(
-                IntegrityCheckResult.Effect.DENY,
-                mEngine.evaluate(
-                        getAppInstallMetadataBuilder()
-                                .setInstallerName(INSTALLER_2)
-                                .setInstallerCertificate(INSTALLER_1_CERT)
-                                .build(),
-                        allowedInstallers)
-                        .getEffect());
+        AppInstallMetadata appInstallMetadata1 =
+                getAppInstallMetadataBuilder()
+                        .setInstallerName(INSTALLER_1)
+                        .setInstallerCertificate(INSTALLER_1_CERT)
+                        .build();
+        assertThat(mEngine.evaluate(appInstallMetadata1, allowedInstallers).getEffect())
+                .isEqualTo(IntegrityCheckResult.Effect.ALLOW);
+
+        AppInstallMetadata appInstallMetadata2 =
+                getAppInstallMetadataBuilder()
+                        .setInstallerName(INSTALLER_2)
+                        .setInstallerCertificate(INSTALLER_2_CERT)
+                        .build();
+        assertThat(mEngine.evaluate(appInstallMetadata2, allowedInstallers).getEffect())
+                .isEqualTo(IntegrityCheckResult.Effect.ALLOW);
+
+        AppInstallMetadata appInstallMetadata3 =
+                getAppInstallMetadataBuilder()
+                        .setInstallerName(INSTALLER_1)
+                        .setInstallerCertificate(INSTALLER_2_CERT)
+                        .build();
+        assertThat(mEngine.evaluate(appInstallMetadata3, allowedInstallers).getEffect())
+                .isEqualTo(IntegrityCheckResult.Effect.DENY);
+
+        AppInstallMetadata appInstallMetadata4 =
+                getAppInstallMetadataBuilder()
+                        .setInstallerName(INSTALLER_2)
+                        .setInstallerCertificate(INSTALLER_1_CERT)
+                        .build();
+        assertThat(mEngine.evaluate(appInstallMetadata4, allowedInstallers).getEffect())
+                .isEqualTo(IntegrityCheckResult.Effect.DENY);
+    }
+
+    @Test
+    public void manifestBasedRuleEvaluationWorksEvenWhenIntegrityFilesAreUnavailable() {
+        when(mIntegrityFileManager.initialized()).thenReturn(false);
+
+        Map<String, String> allowedInstallers =
+                Collections.singletonMap(INSTALLER_1, INSTALLER_1_CERT);
+
+        AppInstallMetadata appInstallMetadata1 =
+                getAppInstallMetadataBuilder()
+                        .setInstallerName(INSTALLER_1)
+                        .setInstallerCertificate(INSTALLER_1_CERT)
+                        .build();
+        assertThat(mEngine.evaluate(appInstallMetadata1, allowedInstallers).getEffect())
+                .isEqualTo(IntegrityCheckResult.Effect.ALLOW);
+
+        AppInstallMetadata appInstallMetadata2 =
+                getAppInstallMetadataBuilder()
+                        .setInstallerName(RANDOM_INSTALLER)
+                        .setInstallerCertificate(INSTALLER_1_CERT)
+                        .build();
+        assertThat(mEngine.evaluate(appInstallMetadata2, allowedInstallers).getEffect())
+                .isEqualTo(IntegrityCheckResult.Effect.DENY);
     }
 
     /** Returns a builder with all fields filled with some dummy data. */