Merge "Merge "Solve AppBackupUtils.signaturesMatch() policies" into oc-mr1-dev am: a743c01d00" into oc-mr1-dev-plus-aosp

am: 13889d75c7

Change-Id: Ifd1afbf731cfb10d2e37d86d0c5b1d64011eb9d6

3 files changed, 84 insertions, 145 deletions

diff --git a/services/backup/java/com/android/server/backup/BackupManagerService.java b/services/backup/java/com/android/server/backup/BackupManagerService.java
index cb4beccaa205..29d562b17deb 100644
--- a/services/backup/java/com/android/server/backup/BackupManagerService.java
+++ b/services/backup/java/com/android/server/backup/BackupManagerService.java
@@ -119,6 +119,7 @@ import android.util.StringBuilderPrinter;
 import com.android.internal.annotations.GuardedBy;
 import com.android.internal.backup.IBackupTransport;
 import com.android.internal.backup.IObbBackupService;
+import com.android.internal.util.ArrayUtils;
 import com.android.internal.util.DumpUtils;
 import com.android.server.AppWidgetBackupBridge;
 import com.android.server.EventLogTags;
@@ -8416,7 +8417,19 @@ if (MORE_DEBUG) Slog.v(TAG, "   + got " + nRead + "; now wanting " + (size - soF
 
     // ----- Restore handling -----
 
-    // Old style: directly match the stored vs on device signature blocks
+    /**
+     * Returns whether the signatures stored {@param storedSigs}, coming from the source apk, match
+     * the signatures of the apk installed on the device, the target apk. If the target resides in
+     * the system partition we return true. Otherwise it's considered a match if both conditions
+     * hold:
+     *
+     * <ul>
+     *   <li>Source and target have at least one signature each
+     *   <li>Target contains all signatures in source
+     * </ul>
+     *
+     * Note that if {@param target} is null we return false.
+     */
     static boolean signaturesMatch(Signature[] storedSigs, PackageInfo target) {
         if (target == null) {
             return false;
@@ -8428,26 +8441,24 @@ if (MORE_DEBUG) Slog.v(TAG, "   + got " + nRead + "; now wanting " + (size - soF
         // partition will be signed with the device's platform certificate, so on
         // different phones the same system app will have different signatures.)
         if ((target.applicationInfo.flags & ApplicationInfo.FLAG_SYSTEM) != 0) {
-            if (MORE_DEBUG) Slog.v(TAG, "System app " + target.packageName + " - skipping sig check");
+            if (MORE_DEBUG) {
+                Slog.v(TAG, "System app " + target.packageName + " - skipping sig check");
+            }
             return true;
         }
 
-        // Allow unsigned apps, but not signed on one device and unsigned on the other
-        // !!! TODO: is this the right policy?
         Signature[] deviceSigs = target.signatures;
-        if (MORE_DEBUG) Slog.v(TAG, "signaturesMatch(): stored=" + storedSigs
-                + " device=" + deviceSigs);
-        if ((storedSigs == null || storedSigs.length == 0)
-                && (deviceSigs == null || deviceSigs.length == 0)) {
-            return true;
+        if (MORE_DEBUG) {
+            Slog.v(TAG, "signaturesMatch(): stored=" + storedSigs + " device=" + deviceSigs);
         }
-        if (storedSigs == null || deviceSigs == null) {
+
+        // Don't allow unsigned apps on either end
+        if (ArrayUtils.isEmpty(storedSigs) || ArrayUtils.isEmpty(deviceSigs)) {
             return false;
         }
 
-        // !!! TODO: this demands that every stored signature match one
-        // that is present on device, and does not demand the converse.
-        // Is this this right policy?
+        // Signatures can be added over time, so the target-device apk needs to contain all the
+        // source-device apk signatures, but not necessarily the other way around.
         int nStored = storedSigs.length;
         int nDevice = deviceSigs.length;
 
diff --git a/services/backup/java/com/android/server/backup/utils/AppBackupUtils.java b/services/backup/java/com/android/server/backup/utils/AppBackupUtils.java
index c033d98e3896..4abf18add469 100644
--- a/services/backup/java/com/android/server/backup/utils/AppBackupUtils.java
+++ b/services/backup/java/com/android/server/backup/utils/AppBackupUtils.java
@@ -26,6 +26,8 @@ import android.content.pm.Signature;
 import android.os.Process;
 import android.util.Slog;
 
+import com.android.internal.util.ArrayUtils;
+
 /**
  * Utility methods wrapping operations on ApplicationInfo and PackageInfo.
  */
@@ -91,9 +93,18 @@ public class AppBackupUtils {
     }
 
     /**
-     * Old style: directly match the stored vs on device signature blocks.
+     * Returns whether the signatures stored {@param storedSigs}, coming from the source apk, match
+     * the signatures of the apk installed on the device, the target apk. If the target resides in
+     * the system partition we return true. Otherwise it's considered a match if both conditions
+     * hold:
+     *
+     * <ul>
+     *   <li>Source and target have at least one signature each
+     *   <li>Target contains all signatures in source
+     * </ul>
+     *
+     * Note that if {@param target} is null we return false.
      */
-    // TODO(b/37977154): Resolve questionable policies.
     public static boolean signaturesMatch(Signature[] storedSigs, PackageInfo target) {
         if (target == null) {
             return false;
@@ -111,24 +122,18 @@ public class AppBackupUtils {
             return true;
         }
 
-        // Allow unsigned apps, but not signed on one device and unsigned on the other
-        // TODO(b/37977154): is this the right policy?
         Signature[] deviceSigs = target.signatures;
         if (MORE_DEBUG) {
             Slog.v(TAG, "signaturesMatch(): stored=" + storedSigs + " device=" + deviceSigs);
         }
-        if ((storedSigs == null || storedSigs.length == 0)
-                && (deviceSigs == null || deviceSigs.length == 0)) {
-            return true;
-        }
-        // TODO(b/37977154): This allows empty stored signature, is this right?
-        if (storedSigs == null || deviceSigs == null) {
+
+        // Don't allow unsigned apps on either end
+        if (ArrayUtils.isEmpty(storedSigs) || ArrayUtils.isEmpty(deviceSigs)) {
             return false;
         }
 
-        // TODO(b/37977154): this demands that every stored signature match one
-        // that is present on device, and does not demand the converse.
-        // Is this this right policy?
+        // Signatures can be added over time, so the target-device apk needs to contain all the
+        // source-device apk signatures, but not necessarily the other way around.
         int nStored = storedSigs.length;
         int nDevice = deviceSigs.length;
 
diff --git a/services/tests/servicestests/src/com/android/server/backup/utils/AppBackupUtilsTest.java b/services/tests/servicestests/src/com/android/server/backup/utils/AppBackupUtilsTest.java
index 650681e2755b..db0ec0702b9f 100644
--- a/services/tests/servicestests/src/com/android/server/backup/utils/AppBackupUtilsTest.java
+++ b/services/tests/servicestests/src/com/android/server/backup/utils/AppBackupUtilsTest.java
@@ -31,8 +31,6 @@ import com.android.server.backup.RefactoredBackupManagerService;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
-import java.util.Random;
-
 @SmallTest
 @Presubmit
 @RunWith(AndroidJUnit4.class)
@@ -40,7 +38,10 @@ public class AppBackupUtilsTest {
     private static final String CUSTOM_BACKUP_AGENT_NAME = "custom.backup.agent";
     private static final String TEST_PACKAGE_NAME = "test_package";
 
-    private final Random mRandom = new Random(1000000009);
+    private static final Signature SIGNATURE_1 = generateSignature((byte) 1);
+    private static final Signature SIGNATURE_2 = generateSignature((byte) 2);
+    private static final Signature SIGNATURE_3 = generateSignature((byte) 3);
+    private static final Signature SIGNATURE_4 = generateSignature((byte) 4);
 
     @Test
     public void appIsEligibleForBackup_backupNotAllowed_returnsFalse() throws Exception {
@@ -220,7 +221,7 @@ public class AppBackupUtilsTest {
 
     @Test
     public void signaturesMatch_targetIsNull_returnsFalse() throws Exception {
-        boolean result = AppBackupUtils.signaturesMatch(new Signature[0], null);
+        boolean result = AppBackupUtils.signaturesMatch(new Signature[] {SIGNATURE_1}, null);
 
         assertThat(result).isFalse();
     }
@@ -237,61 +238,63 @@ public class AppBackupUtilsTest {
     }
 
     @Test
-    public void signaturesMatch_allowsUnsignedApps_bothSignaturesNull_returnsTrue()
+    public void signaturesMatch_disallowsUnsignedApps_storedSignatureNull_returnsFalse()
             throws Exception {
         PackageInfo packageInfo = new PackageInfo();
-        packageInfo.signatures = null;
+        packageInfo.signatures = new Signature[] {SIGNATURE_1};
         packageInfo.applicationInfo = new ApplicationInfo();
 
         boolean result = AppBackupUtils.signaturesMatch(null, packageInfo);
 
-        assertThat(result).isTrue();
+        assertThat(result).isFalse();
     }
 
     @Test
-    public void signaturesMatch_allowsUnsignedApps_bothSignaturesEmpty_returnsTrue()
+    public void signaturesMatch_disallowsUnsignedApps_storedSignatureEmpty_returnsFalse()
             throws Exception {
         PackageInfo packageInfo = new PackageInfo();
-        packageInfo.signatures = new Signature[0];
+        packageInfo.signatures = new Signature[] {SIGNATURE_1};
         packageInfo.applicationInfo = new ApplicationInfo();
 
         boolean result = AppBackupUtils.signaturesMatch(new Signature[0], packageInfo);
 
-        assertThat(result).isTrue();
+        assertThat(result).isFalse();
     }
 
+
     @Test
     public void
-    signaturesMatch_allowsUnsignedApps_storedSignatureNullTargetSignatureEmpty_returnsTrue()
+    signaturesMatch_disallowsUnsignedApps_targetSignatureEmpty_returnsFalse()
             throws Exception {
         PackageInfo packageInfo = new PackageInfo();
         packageInfo.signatures = new Signature[0];
         packageInfo.applicationInfo = new ApplicationInfo();
 
-        boolean result = AppBackupUtils.signaturesMatch(null, packageInfo);
+        boolean result = AppBackupUtils.signaturesMatch(new Signature[] {SIGNATURE_1},
+                packageInfo);
 
-        assertThat(result).isTrue();
+        assertThat(result).isFalse();
     }
 
     @Test
     public void
-    signaturesMatch_allowsUnsignedApps_storedSignatureEmptyTargetSignatureNull_returnsTrue()
+    signaturesMatch_disallowsUnsignedApps_targetSignatureNull_returnsFalse()
             throws Exception {
         PackageInfo packageInfo = new PackageInfo();
         packageInfo.signatures = null;
         packageInfo.applicationInfo = new ApplicationInfo();
 
-        boolean result = AppBackupUtils.signaturesMatch(new Signature[0], packageInfo);
+        boolean result = AppBackupUtils.signaturesMatch(new Signature[] {SIGNATURE_1},
+                packageInfo);
 
-        assertThat(result).isTrue();
+        assertThat(result).isFalse();
     }
 
     @Test
-    public void
-    signaturesMatch_disallowsAppsUnsignedOnOnlyOneDevice_storedSignatureIsNull_returnsFalse()
+    public void signaturesMatch_disallowsUnsignedApps_bothSignaturesNull_returnsFalse()
             throws Exception {
         PackageInfo packageInfo = new PackageInfo();
-        packageInfo.signatures = new Signature[]{generateRandomSignature()};
+        packageInfo.signatures = null;
         packageInfo.applicationInfo = new ApplicationInfo();
 
         boolean result = AppBackupUtils.signaturesMatch(null, packageInfo);
@@ -300,37 +303,25 @@ public class AppBackupUtilsTest {
     }
 
     @Test
-    public void
-    signaturesMatch_disallowsAppsUnsignedOnOnlyOneDevice_targetSignatureIsNull_returnsFalse()
+    public void signaturesMatch_disallowsUnsignedApps_bothSignaturesEmpty_returnsFalse()
             throws Exception {
         PackageInfo packageInfo = new PackageInfo();
-        packageInfo.signatures = null;
+        packageInfo.signatures = new Signature[0];
         packageInfo.applicationInfo = new ApplicationInfo();
 
-        boolean result = AppBackupUtils.signaturesMatch(new Signature[]{generateRandomSignature()},
-                packageInfo);
+        boolean result = AppBackupUtils.signaturesMatch(new Signature[0], packageInfo);
 
         assertThat(result).isFalse();
     }
 
     @Test
-    public void signaturesMatch_signaturesMatch_returnsTrue() throws Exception {
-        Signature signature1 = generateRandomSignature();
-        Signature signature2 = generateRandomSignature();
-        Signature signature3 = generateRandomSignature();
-        assertThat(signature1).isNotEqualTo(signature2);
-        assertThat(signature2).isNotEqualTo(signature3);
-        assertThat(signature1).isNotEqualTo(signature3);
-
-        Signature signature1Copy = new Signature(signature1.toByteArray());
-        Signature signature2Copy = new Signature(signature2.toByteArray());
-        Signature signature3Copy = new Signature(signature3.toByteArray());
-        assertThat(signature1Copy).isEqualTo(signature1);
-        assertThat(signature2Copy).isEqualTo(signature2);
-        assertThat(signature3Copy).isEqualTo(signature3);
+    public void signaturesMatch_equalSignatures_returnsTrue() throws Exception {
+        Signature signature1Copy = new Signature(SIGNATURE_1.toByteArray());
+        Signature signature2Copy = new Signature(SIGNATURE_2.toByteArray());
+        Signature signature3Copy = new Signature(SIGNATURE_3.toByteArray());
 
         PackageInfo packageInfo = new PackageInfo();
-        packageInfo.signatures = new Signature[]{signature1, signature2, signature3};
+        packageInfo.signatures = new Signature[]{SIGNATURE_1, SIGNATURE_2, SIGNATURE_3};
         packageInfo.applicationInfo = new ApplicationInfo();
 
         boolean result = AppBackupUtils.signaturesMatch(
@@ -341,20 +332,11 @@ public class AppBackupUtilsTest {
 
     @Test
     public void signaturesMatch_extraSignatureInTarget_returnsTrue() throws Exception {
-        Signature signature1 = generateRandomSignature();
-        Signature signature2 = generateRandomSignature();
-        Signature signature3 = generateRandomSignature();
-        assertThat(signature1).isNotEqualTo(signature2);
-        assertThat(signature2).isNotEqualTo(signature3);
-        assertThat(signature1).isNotEqualTo(signature3);
-
-        Signature signature1Copy = new Signature(signature1.toByteArray());
-        Signature signature2Copy = new Signature(signature2.toByteArray());
-        assertThat(signature1Copy).isEqualTo(signature1);
-        assertThat(signature2Copy).isEqualTo(signature2);
+        Signature signature1Copy = new Signature(SIGNATURE_1.toByteArray());
+        Signature signature2Copy = new Signature(SIGNATURE_2.toByteArray());
 
         PackageInfo packageInfo = new PackageInfo();
-        packageInfo.signatures = new Signature[]{signature1, signature2, signature3};
+        packageInfo.signatures = new Signature[]{SIGNATURE_1, SIGNATURE_2, SIGNATURE_3};
         packageInfo.applicationInfo = new ApplicationInfo();
 
         boolean result = AppBackupUtils.signaturesMatch(
@@ -365,96 +347,37 @@ public class AppBackupUtilsTest {
 
     @Test
     public void signaturesMatch_extraSignatureInStored_returnsFalse() throws Exception {
-        Signature signature1 = generateRandomSignature();
-        Signature signature2 = generateRandomSignature();
-        Signature signature3 = generateRandomSignature();
-        assertThat(signature1).isNotEqualTo(signature2);
-        assertThat(signature2).isNotEqualTo(signature3);
-        assertThat(signature1).isNotEqualTo(signature3);
-
-        Signature signature1Copy = new Signature(signature1.toByteArray());
-        Signature signature2Copy = new Signature(signature2.toByteArray());
-        assertThat(signature1Copy).isEqualTo(signature1);
-        assertThat(signature2Copy).isEqualTo(signature2);
+        Signature signature1Copy = new Signature(SIGNATURE_1.toByteArray());
+        Signature signature2Copy = new Signature(SIGNATURE_2.toByteArray());
 
         PackageInfo packageInfo = new PackageInfo();
         packageInfo.signatures = new Signature[]{signature1Copy, signature2Copy};
         packageInfo.applicationInfo = new ApplicationInfo();
 
         boolean result = AppBackupUtils.signaturesMatch(
-                new Signature[]{signature1, signature2, signature3}, packageInfo);
-
-        assertThat(result).isFalse();
-    }
-
-    @Test
-    public void signaturesMatch_emptyStoredSignatures_returnsTrue() throws Exception {
-        Signature signature1 = generateRandomSignature();
-        Signature signature2 = generateRandomSignature();
-        Signature signature3 = generateRandomSignature();
-        assertThat(signature1).isNotEqualTo(signature2);
-        assertThat(signature2).isNotEqualTo(signature3);
-        assertThat(signature1).isNotEqualTo(signature3);
-
-        PackageInfo packageInfo = new PackageInfo();
-        packageInfo.signatures = new Signature[]{signature1, signature2, signature3};
-        packageInfo.applicationInfo = new ApplicationInfo();
-
-        boolean result = AppBackupUtils.signaturesMatch(new Signature[0], packageInfo);
-
-        assertThat(result).isTrue();
-    }
-
-    @Test
-    public void signaturesMatch_emptyTargetSignatures_returnsFalse() throws Exception {
-        Signature signature1 = generateRandomSignature();
-        Signature signature2 = generateRandomSignature();
-        Signature signature3 = generateRandomSignature();
-        assertThat(signature1).isNotEqualTo(signature2);
-        assertThat(signature2).isNotEqualTo(signature3);
-        assertThat(signature1).isNotEqualTo(signature3);
-
-        PackageInfo packageInfo = new PackageInfo();
-        packageInfo.signatures = new Signature[0];
-        packageInfo.applicationInfo = new ApplicationInfo();
-
-        boolean result = AppBackupUtils.signaturesMatch(
-                new Signature[]{signature1, signature2, signature3}, packageInfo);
+                new Signature[]{SIGNATURE_1, SIGNATURE_2, SIGNATURE_3}, packageInfo);
 
         assertThat(result).isFalse();
     }
 
     @Test
     public void signaturesMatch_oneNonMatchingSignature_returnsFalse() throws Exception {
-        Signature signature1 = generateRandomSignature();
-        Signature signature2 = generateRandomSignature();
-        Signature signature3 = generateRandomSignature();
-        Signature signature4 = generateRandomSignature();
-        assertThat(signature1).isNotEqualTo(signature2);
-        assertThat(signature2).isNotEqualTo(signature3);
-        assertThat(signature1).isNotEqualTo(signature3);
-        assertThat(signature1).isNotEqualTo(signature4);
-        assertThat(signature2).isNotEqualTo(signature4);
-        assertThat(signature3).isNotEqualTo(signature4);
-
-        Signature signature1Copy = new Signature(signature1.toByteArray());
-        Signature signature2Copy = new Signature(signature2.toByteArray());
-        assertThat(signature1Copy).isEqualTo(signature1);
-        assertThat(signature2Copy).isEqualTo(signature2);
+        Signature signature1Copy = new Signature(SIGNATURE_1.toByteArray());
+        Signature signature2Copy = new Signature(SIGNATURE_2.toByteArray());
 
         PackageInfo packageInfo = new PackageInfo();
-        packageInfo.signatures = new Signature[]{signature1, signature2, signature3};
+        packageInfo.signatures = new Signature[]{SIGNATURE_1, SIGNATURE_2, SIGNATURE_3};
         packageInfo.applicationInfo = new ApplicationInfo();
 
         boolean result = AppBackupUtils.signaturesMatch(
-                new Signature[]{signature1Copy, signature2Copy, signature4}, packageInfo);
+                new Signature[]{signature1Copy, signature2Copy, SIGNATURE_4}, packageInfo);
 
         assertThat(result).isFalse();
     }
 
-    private Signature generateRandomSignature() {
+    private static Signature generateSignature(byte i) {
         byte[] signatureBytes = new byte[256];
-        mRandom.nextBytes(signatureBytes);
+        signatureBytes[0] = i;
         return new Signature(signatureBytes);
     }
 }