diff options
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. */ |