Merge "Add signature checking to AppCompatOverridesService." (52a66639) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/compat/overrides/AppCompatOverridesParser.java

+68 −3

Original line number	Diff line number	Diff line
		@@ -16,19 +16,24 @@

		package com.android.server.compat.overrides;

		import static android.content.pm.PackageManager.CERT_INPUT_SHA256;
		import static android.content.pm.PackageManager.MATCH_ANY_USER;

		import static java.util.Collections.emptyMap;
		import static java.util.Collections.emptySet;

		import android.annotation.Nullable;
		import android.app.compat.PackageOverride;
		import android.content.pm.ApplicationInfo;
		import android.content.pm.PackageManager;
		import android.util.ArrayMap;
		import android.util.ArraySet;
		import android.util.KeyValueListParser;
		import android.util.Pair;
		import android.util.Slog;

		import libcore.util.HexEncoding;

		import java.util.Arrays;
		import java.util.Comparator;
		import java.util.List;
		@@ -178,7 +183,9 @@ final class AppCompatOverridesParser {
		* overrides, and returns a map from change ID to {@link PackageOverride} instances to add.
		*
		* <p>Each change override is in the following format:
		* '<change-id>:<min-version-code?>:<max-version-code?>:<enabled>'.
		* '<signature?>~<change-id>:<min-version-code?>:<max-version-code?>:<enabled>'.
		*
		* <p>The signature is optional, and will only be enforced if included.
		*
		* <p>If there are multiple overrides that should be added with the same change ID, the one
		* that best fits the given {@code versionCode} is added.
		@@ -187,14 +194,27 @@ final class AppCompatOverridesParser {
		*
		* <p>If a change override entry in {@code configStr} is invalid, it will be ignored.
		*/
		static Map<Long, PackageOverride> parsePackageOverrides(String configStr, long versionCode,
		Map<Long, PackageOverride> parsePackageOverrides(String configStr, String packageName,
		long versionCode,
		Set<Long> changeIdsToSkip) {
		if (configStr.isEmpty()) {
		return emptyMap();
		}
		PackageOverrideComparator comparator = new PackageOverrideComparator(versionCode);
		Map<Long, PackageOverride> overridesToAdd = new ArrayMap<>();
		for (String overrideEntryString : configStr.split(",")) {

		Pair<String, String> signatureAndConfig = extractSignatureFromConfig(configStr);
		if (signatureAndConfig == null) {
		return emptyMap();
		}
		final String signature = signatureAndConfig.first;
		final String overridesConfig = signatureAndConfig.second;

		if (!verifySignature(packageName, signature)) {
		return emptyMap();
		}

		for (String overrideEntryString : overridesConfig.split(",")) {
		List<String> changeIdAndVersions = Arrays.asList(overrideEntryString.split(":", 4));
		if (changeIdAndVersions.size() != 4) {
		Slog.w(TAG, "Invalid change override entry: " + overrideEntryString);
		@@ -251,6 +271,51 @@ final class AppCompatOverridesParser {
		return overridesToAdd;
		}

		/**
		* Extracts the signature from the config string if one exists.
		*
		* @param configStr String in the form of <signature?>~<overrideConfig>
		*/
		@Nullable
		private static Pair<String, String> extractSignatureFromConfig(String configStr) {
		final List<String> signatureAndConfig = Arrays.asList(configStr.split("~"));

		if (signatureAndConfig.size() == 1) {
		// The config string doesn't contain a signature.
		return Pair.create("", configStr);
		}

		if (signatureAndConfig.size() > 2) {
		Slog.w(TAG, "Only one signature per config is supported. Config: " + configStr);
		return null;
		}

		return Pair.create(signatureAndConfig.get(0), signatureAndConfig.get(1));
		}

		/**
		* Verifies that the specified package was signed with a particular signature.
		*
		* @param packageName The package to check.
		* @param signature The optional signature to verify. If empty, we return true.
		* @return Whether the package is signed with that signature.
		*/
		private boolean verifySignature(String packageName, String signature) {
		try {
		final boolean signatureValid = signature.isEmpty()
		\|\| mPackageManager.hasSigningCertificate(packageName,
		HexEncoding.decode(signature), CERT_INPUT_SHA256);

		if (!signatureValid) {
		Slog.w(TAG, packageName + " did not have expected signature: " + signature);
		}
		return signatureValid;
		} catch (IllegalArgumentException e) {
		Slog.w(TAG, "Unable to verify signature " + signature + " for " + packageName, e);
		return false;
		}
		}

		/**
		* A {@link Comparator} that compares @link PackageOverride} instances with respect to a
		* specified {@code versionCode} as follows:

services/core/java/com/android/server/compat/overrides/AppCompatOverridesService.java

+3 −3

Original line number	Diff line number	Diff line
		@@ -196,8 +196,8 @@ public final class AppCompatOverridesService {
		private void applyPackageOverrides(String configStr, String packageName, long versionCode,
		Set<Long> ownedChangeIds, Set<Long> changeIdsToSkip,
		boolean removeOtherOwnedOverrides) {
		Map<Long, PackageOverride> overridesToAdd = AppCompatOverridesParser.parsePackageOverrides(
		configStr, versionCode, changeIdsToSkip);
		Map<Long, PackageOverride> overridesToAdd = mOverridesParser.parsePackageOverrides(
		configStr, packageName, versionCode, changeIdsToSkip);
		putPackageOverrides(packageName, overridesToAdd);

		if (!removeOtherOwnedOverrides) {
		@@ -426,5 +426,5 @@ public final class AppCompatOverridesService {
		break;
		}
		}
		};
		}
		}

services/tests/mockingservicestests/src/com/android/server/compat/overrides/AppCompatOverridesParserTest.java

+88 −12

Original line number	Diff line number	Diff line
		@@ -16,6 +16,8 @@

		package com.android.server.compat.overrides;

		import static android.content.pm.PackageManager.CERT_INPUT_SHA256;

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

		import static org.mockito.ArgumentMatchers.anyInt;
		@@ -31,6 +33,8 @@ import android.util.ArraySet;

		import androidx.test.filters.SmallTest;

		import libcore.util.HexEncoding;

		import org.junit.Before;
		import org.junit.Test;
		import org.junit.runner.RunWith;
		@@ -182,16 +186,18 @@ public class AppCompatOverridesParserTest {

		@Test
		public void parsePackageOverrides_emptyConfigNoOwnedChangeIds_returnsEmpty() {
		Map<Long, PackageOverride> result = AppCompatOverridesParser.parsePackageOverrides(
		/* configStr= / "", / versionCode= / 0, / changeIdsToSkip= */ emptySet());
		Map<Long, PackageOverride> result = mParser.parsePackageOverrides(
		/* configStr= / "", PACKAGE_1, / versionCode= / 0, / changeIdsToSkip= */
		emptySet());

		assertThat(result).isEmpty();
		}

		@Test
		public void parsePackageOverrides_configWithSingleOverride_returnsOverride() {
		Map<Long, PackageOverride> result = AppCompatOverridesParser.parsePackageOverrides(
		/* configStr= / "123:::true", / versionCode= / 5, / changeIdsToSkip= */
		Map<Long, PackageOverride> result = mParser.parsePackageOverrides(
		/* configStr= / "123:::true", PACKAGE_1, / versionCode= */
		5, /* changeIdsToSkip= */
		emptySet());

		assertThat(result).hasSize(1);
		@@ -201,10 +207,10 @@ public class AppCompatOverridesParserTest {

		@Test
		public void parsePackageOverrides_configWithMultipleOverrides_returnsOverrides() {
		Map<Long, PackageOverride> result = AppCompatOverridesParser.parsePackageOverrides(
		Map<Long, PackageOverride> result = mParser.parsePackageOverrides(
		/* configStr= */ "910:3:4:false,78:10::false,12:::false,34:1:2:true,34:10::true,"
		+ "56::2:true,56:3:4:false,34:4:8:true,78:6:7:true,910:5::true,"
		+ "1112::5:true,56:6::true,1112:6:7:false", /* versionCode= */
		+ "1112::5:true,56:6::true,1112:6:7:false", PACKAGE_1, /* versionCode= */
		5, /* changeIdsToSkip= */ emptySet());

		assertThat(result).hasSize(6);
		@@ -228,8 +234,9 @@ public class AppCompatOverridesParserTest {
		@Test
		public void parsePackageOverrides_changeIdsToSkipSpecified_returnsWithoutChangeIdsToSkip() {
		ArraySet<Long> changeIdsToSkip = new ArraySet<>(Arrays.asList(34L, 56L));
		Map<Long, PackageOverride> result = AppCompatOverridesParser.parsePackageOverrides(
		/* configStr= / "12:::true,56:3:7:true", / versionCode= */ 5, changeIdsToSkip);
		Map<Long, PackageOverride> result = mParser.parsePackageOverrides(
		/* configStr= / "12:::true,56:3:7:true", PACKAGE_1, / versionCode= */ 5,
		changeIdsToSkip);

		assertThat(result).hasSize(1);
		assertThat(result.get(12L)).isEqualTo(
		@@ -239,8 +246,77 @@ public class AppCompatOverridesParserTest {
		@Test
		public void parsePackageOverrides_changeIdsToSkipContainsAllIds_returnsEmpty() {
		ArraySet<Long> changeIdsToSkip = new ArraySet<>(Arrays.asList(12L, 34L));
		Map<Long, PackageOverride> result = AppCompatOverridesParser.parsePackageOverrides(
		/* configStr= / "12:::true", / versionCode= */ 5, changeIdsToSkip);
		Map<Long, PackageOverride> result = mParser.parsePackageOverrides(
		/* configStr= / "12:::true", PACKAGE_1, / versionCode= */ 5, changeIdsToSkip);

		assertThat(result).isEmpty();
		}

		@Test
		public void parsePackageOverrides_signatureInvalid() {
		when(mPackageManager.hasSigningCertificate(PACKAGE_1, HexEncoding.decode("aa"),
		CERT_INPUT_SHA256)).thenReturn(false);

		Map<Long, PackageOverride> result = mParser.parsePackageOverrides(
		/* configStr= */ "aa~12:::true,56:::true", PACKAGE_1,
		/* versionCode= / 0, / changeIdsToSkip= */ emptySet());

		assertThat(result).isEmpty();
		}

		@Test
		public void parsePackageOverrides_signatureInvalid_oddNumberOfCharacters() {
		// Valid hex encoding should always be an even number of characters.
		Map<Long, PackageOverride> result = mParser.parsePackageOverrides(
		/* configStr= */ "a~12:::true,56:::true", PACKAGE_1,
		/* versionCode= / 0, / changeIdsToSkip= */ emptySet());

		assertThat(result).isEmpty();
		}

		@Test
		public void parsePackageOverrides_signatureValid() {
		when(mPackageManager.hasSigningCertificate(PACKAGE_1, HexEncoding.decode("bb"),
		CERT_INPUT_SHA256)).thenReturn(true);

		Map<Long, PackageOverride> result = mParser.parsePackageOverrides(
		/* configStr= */ "bb~12:::true,56:::false", PACKAGE_1,
		/* versionCode= / 0, / changeIdsToSkip= */ emptySet());

		assertThat(result.keySet()).containsExactly(12L, 56L);
		}

		@Test
		public void parsePackageOverrides_emptySignature() {
		Map<Long, PackageOverride> result = mParser.parsePackageOverrides(
		/* configStr= */ "~12:::true,56:::false", PACKAGE_1,
		/* versionCode= / 0, / changeIdsToSkip= */ emptySet());

		assertThat(result.keySet()).containsExactly(12L, 56L);
		}

		@Test
		public void parsePackageOverrides_multipleSignatures() {
		when(mPackageManager.hasSigningCertificate(PACKAGE_1, HexEncoding.decode("aa"),
		CERT_INPUT_SHA256)).thenReturn(true);
		when(mPackageManager.hasSigningCertificate(PACKAGE_1, HexEncoding.decode("bb"),
		CERT_INPUT_SHA256)).thenReturn(true);

		Map<Long, PackageOverride> result = mParser.parsePackageOverrides(
		/* configStr= */ "aa~bb~12:::true,56:::false", PACKAGE_1,
		/* versionCode= /0, / changeIdsToSkip= */ emptySet());

		assertThat(result).isEmpty();
		}

		@Test
		public void parsePackageOverrides_signatureOnly() {
		when(mPackageManager.hasSigningCertificate(PACKAGE_1, HexEncoding.decode("aa"),
		CERT_INPUT_SHA256)).thenReturn(true);

		Map<Long, PackageOverride> result = mParser.parsePackageOverrides(
		/* configStr= */ "aa~", PACKAGE_1,
		/* versionCode= / 0, / changeIdsToSkip= */ emptySet());

		assertThat(result).isEmpty();
		}
		@@ -248,9 +324,9 @@ public class AppCompatOverridesParserTest {
		@Test
		public void parsePackageOverrides_someOverridesAreInvalid_returnsWithoutInvalidOverrides() {
		// We add a valid entry before and after the invalid ones to make sure they are applied.
		Map<Long, PackageOverride> result = AppCompatOverridesParser.parsePackageOverrides(
		Map<Long, PackageOverride> result = mParser.parsePackageOverrides(
		/* configStr= */ "12:::True,56:1:2:FALSE,56:3:true,78:4:8:true:,C1:::true,910:::no,"
		+ "1112:1:ten:true,1112:one:10:true,,1314:7:3:false,34:::",
		+ "1112:1:ten:true,1112:one:10:true,,1314:7:3:false,34:::", PACKAGE_1,
		/* versionCode= / 5, / changeIdsToSkip= */ emptySet());

		assertThat(result).hasSize(2);