Check crc and time of secondary dex filesandroid-o-preview-1android-n-mr2-preview-2o-preview

Protect extracted dex files from modifications by checking their crc and
modification time. In case of change, proceed to a new extraction.

Those checks are replacing the check of the zip integrity by
opening it with a ZipFile.

Test: SupportMultidexHostTest (from tradefed)
Bug: 32159214
Change-Id: I09aa01550782f5f550bee6fc91709455e82c1057

2 files changed, 71 insertions, 52 deletions

diff --git a/library/src/android/support/multidex/MultiDex.java b/library/src/android/support/multidex/MultiDex.java
index fad50d7..d35da96 100644
--- a/library/src/android/support/multidex/MultiDex.java
+++ b/library/src/android/support/multidex/MultiDex.java
@@ -155,7 +155,8 @@ public final class MultiDex {
                 }
 
                 File dexDir = getDexDir(context, applicationInfo);
-                List<File> files = MultiDexExtractor.load(context, applicationInfo, dexDir, false);
+                List<? extends File> files =
+                    MultiDexExtractor.load(context, applicationInfo, dexDir, false);
                 installSecondaryDexes(loader, dexDir, files);
             }
 
@@ -217,7 +218,8 @@ public final class MultiDex {
         return isMultidexCapable;
     }
 
-    private static void installSecondaryDexes(ClassLoader loader, File dexDir, List<File> files)
+    private static void installSecondaryDexes(ClassLoader loader, File dexDir,
+        List<? extends File> files)
             throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException,
             InvocationTargetException, NoSuchMethodException, IOException {
         if (!files.isEmpty()) {
@@ -374,7 +376,8 @@ public final class MultiDex {
      */
     private static final class V19 {
 
-        private static void install(ClassLoader loader, List<File> additionalClassPathEntries,
+        private static void install(ClassLoader loader,
+                List<? extends File> additionalClassPathEntries,
                 File optimizedDirectory)
                         throws IllegalArgumentException, IllegalAccessException,
                         NoSuchFieldException, InvocationTargetException, NoSuchMethodException {
@@ -439,7 +442,8 @@ public final class MultiDex {
      */
     private static final class V14 {
 
-        private static void install(ClassLoader loader, List<File> additionalClassPathEntries,
+        private static void install(ClassLoader loader,
+                List<? extends File> additionalClassPathEntries,
                 File optimizedDirectory)
                         throws IllegalArgumentException, IllegalAccessException,
                         NoSuchFieldException, InvocationTargetException, NoSuchMethodException {
@@ -473,7 +477,8 @@ public final class MultiDex {
      * Installer for platform versions 4 to 13.
      */
     private static final class V4 {
-        private static void install(ClassLoader loader, List<File> additionalClassPathEntries)
+        private static void install(ClassLoader loader,
+                List<? extends File> additionalClassPathEntries)
                         throws IllegalArgumentException, IllegalAccessException,
                         NoSuchFieldException, IOException {
             /* The patched class loader is expected to be a descendant of
@@ -490,7 +495,7 @@ public final class MultiDex {
             File[] extraFiles = new File[extraSize];
             ZipFile[] extraZips = new ZipFile[extraSize];
             DexFile[] extraDexs = new DexFile[extraSize];
-            for (ListIterator<File> iterator = additionalClassPathEntries.listIterator();
+            for (ListIterator<? extends File> iterator = additionalClassPathEntries.listIterator();
                     iterator.hasNext();) {
                 File additionalEntry = iterator.next();
                 String entryPath = additionalEntry.getAbsolutePath();
diff --git a/library/src/android/support/multidex/MultiDexExtractor.java b/library/src/android/support/multidex/MultiDexExtractor.java
index 998ccec..2d7402a 100644
--- a/library/src/android/support/multidex/MultiDexExtractor.java
+++ b/library/src/android/support/multidex/MultiDexExtractor.java
@@ -21,7 +21,6 @@ import android.content.SharedPreferences;
 import android.content.pm.ApplicationInfo;
 import android.os.Build;
 import android.util.Log;
-
 import java.io.BufferedOutputStream;
 import java.io.Closeable;
 import java.io.File;
@@ -36,7 +35,6 @@ import java.nio.channels.FileLock;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.zip.ZipEntry;
-import java.util.zip.ZipException;
 import java.util.zip.ZipFile;
 import java.util.zip.ZipOutputStream;
 
@@ -46,6 +44,17 @@ import java.util.zip.ZipOutputStream;
  */
 final class MultiDexExtractor {
 
+    /**
+     * Zip file containing one secondary dex file.
+     */
+    private static class ExtractedDex extends File {
+        public long crc = NO_VALUE;
+
+        public ExtractedDex(File dexDir, String fileName) {
+            super(dexDir, fileName);
+        }
+    }
+
     private static final String TAG = MultiDex.TAG;
 
     /**
@@ -63,6 +72,8 @@ final class MultiDexExtractor {
     private static final String KEY_TIME_STAMP = "timestamp";
     private static final String KEY_CRC = "crc";
     private static final String KEY_DEX_NUMBER = "dex.number";
+    private static final String KEY_DEX_CRC = "dex.crc.";
+    private static final String KEY_DEX_TIME = "dex.time.";
 
     /**
      * Size of reading buffers.
@@ -82,7 +93,7 @@ final class MultiDexExtractor {
      * @throws IOException if encounters a problem while reading or writing
      *         secondary dex files
      */
-    static List<File> load(Context context, ApplicationInfo applicationInfo, File dexDir,
+    static List<? extends File> load(Context context, ApplicationInfo applicationInfo, File dexDir,
             boolean forceReload) throws IOException {
         Log.i(TAG, "MultiDexExtractor.load(" + applicationInfo.sourceDir + ", " + forceReload + ")");
         final File sourceApk = new File(applicationInfo.sourceDir);
@@ -94,7 +105,7 @@ final class MultiDexExtractor {
         RandomAccessFile lockRaf = new RandomAccessFile(lockFile, "rw");
         FileChannel lockChannel = null;
         FileLock cacheLock = null;
-        List<File> files;
+        List<ExtractedDex> files;
         IOException releaseLockException = null;
         try {
             lockChannel = lockRaf.getChannel();
@@ -109,14 +120,12 @@ final class MultiDexExtractor {
                     Log.w(TAG, "Failed to reload existing extracted secondary dex files,"
                             + " falling back to fresh extraction", ioe);
                     files = performExtractions(sourceApk, dexDir);
-                    putStoredApkInfo(context,
-                            getTimeStamp(sourceApk), currentCrc, files.size() + 1);
-
+                    putStoredApkInfo(context, getTimeStamp(sourceApk), currentCrc, files);
                 }
             } else {
                 Log.i(TAG, "Detected that extraction must be performed.");
                 files = performExtractions(sourceApk, dexDir);
-                putStoredApkInfo(context, getTimeStamp(sourceApk), currentCrc, files.size() + 1);
+                putStoredApkInfo(context, getTimeStamp(sourceApk), currentCrc, files);
             }
         } finally {
             if (cacheLock != null) {
@@ -147,23 +156,35 @@ final class MultiDexExtractor {
      * Load previously extracted secondary dex files. Should be called only while owning the lock on
      * {@link #LOCK_FILENAME}.
      */
-    private static List<File> loadExistingExtractions(Context context, File sourceApk, File dexDir)
+    private static List<ExtractedDex> loadExistingExtractions(
+            Context context, File sourceApk, File dexDir)
             throws IOException {
         Log.i(TAG, "loading existing secondary dex files");
 
         final String extractedFilePrefix = sourceApk.getName() + EXTRACTED_NAME_EXT;
-        int totalDexNumber = getMultiDexPreferences(context).getInt(KEY_DEX_NUMBER, 1);
-        final List<File> files = new ArrayList<File>(totalDexNumber);
+        SharedPreferences multiDexPreferences = getMultiDexPreferences(context);
+        int totalDexNumber = multiDexPreferences.getInt(KEY_DEX_NUMBER, 1);
+        final List<ExtractedDex> files = new ArrayList<ExtractedDex>(totalDexNumber - 1);
 
         for (int secondaryNumber = 2; secondaryNumber <= totalDexNumber; secondaryNumber++) {
             String fileName = extractedFilePrefix + secondaryNumber + EXTRACTED_SUFFIX;
-            File extractedFile = new File(dexDir, fileName);
+            ExtractedDex extractedFile = new ExtractedDex(dexDir, fileName);
             if (extractedFile.isFile()) {
-                files.add(extractedFile);
-                if (!verifyZipFile(extractedFile)) {
-                    Log.i(TAG, "Invalid zip file: " + extractedFile);
-                    throw new IOException("Invalid ZIP file.");
+                extractedFile.crc = getZipCrc(extractedFile);
+                long expectedCrc =
+                        multiDexPreferences.getLong(KEY_DEX_CRC + secondaryNumber, NO_VALUE);
+                long expectedModTime =
+                        multiDexPreferences.getLong(KEY_DEX_TIME + secondaryNumber, NO_VALUE);
+                long lastModified = extractedFile.lastModified();
+                if ((expectedModTime != lastModified)
+                        || (expectedCrc != extractedFile.crc)) {
+                    throw new IOException("Invalid extracted dex: " + extractedFile +
+                            ", expected modification time: "
+                            + expectedModTime + ", modification time: "
+                            + lastModified + ", expected crc: "
+                            + expectedCrc + ", file crc: " + extractedFile.crc);
                 }
+                files.add(extractedFile);
             } else {
                 throw new IOException("Missing extracted secondary dex file '" +
                         extractedFile.getPath() + "'");
@@ -203,7 +224,7 @@ final class MultiDexExtractor {
         return computedValue;
     }
 
-    private static List<File> performExtractions(File sourceApk, File dexDir)
+    private static List<ExtractedDex> performExtractions(File sourceApk, File dexDir)
             throws IOException {
 
         final String extractedFilePrefix = sourceApk.getName() + EXTRACTED_NAME_EXT;
@@ -214,7 +235,7 @@ final class MultiDexExtractor {
         // while another had created it.
         prepareDexDir(dexDir, extractedFilePrefix);
 
-        List<File> files = new ArrayList<File>();
+        List<ExtractedDex> files = new ArrayList<ExtractedDex>();
 
         final ZipFile apk = new ZipFile(sourceApk);
         try {
@@ -224,7 +245,7 @@ final class MultiDexExtractor {
             ZipEntry dexFile = apk.getEntry(DEX_PREFIX + secondaryNumber + DEX_SUFFIX);
             while (dexFile != null) {
                 String fileName = extractedFilePrefix + secondaryNumber + EXTRACTED_SUFFIX;
-                File extractedFile = new File(dexDir, fileName);
+                ExtractedDex extractedFile = new ExtractedDex(dexDir, fileName);
                 files.add(extractedFile);
 
                 Log.i(TAG, "Extraction is needed for file " + extractedFile);
@@ -237,13 +258,19 @@ final class MultiDexExtractor {
                     // (dexFile) from the apk.
                     extract(apk, dexFile, extractedFile, extractedFilePrefix);
 
-                    // Verify that the extracted file is indeed a zip file.
-                    isExtractionSuccessful = verifyZipFile(extractedFile);
+                    // Read zip crc of extracted dex
+                    try {
+                        extractedFile.crc = getZipCrc(extractedFile);
+                        isExtractionSuccessful = true;
+                    } catch (IOException e) {
+                        isExtractionSuccessful = false;
+                        Log.w(TAG, "Failed to read crc from " + extractedFile.getAbsolutePath(), e);
+                    }
 
-                    // Log the sha1 of the extracted zip file
-                    Log.i(TAG, "Extraction " + (isExtractionSuccessful ? "success" : "failed") +
+                    // Log size and crc of the extracted zip file
+                    Log.i(TAG, "Extraction " + (isExtractionSuccessful ? "succeeded" : "failed") +
                             " - length " + extractedFile.getAbsolutePath() + ": " +
-                            extractedFile.length());
+                            extractedFile.length() + " - crc: " + extractedFile.crc);
                     if (!isExtractionSuccessful) {
                         // Delete the extracted file
                         extractedFile.delete();
@@ -277,12 +304,19 @@ final class MultiDexExtractor {
      * {@link #LOCK_FILENAME}.
      */
     private static void putStoredApkInfo(Context context, long timeStamp, long crc,
-            int totalDexNumber) {
+            List<ExtractedDex> extractedDexes) {
         SharedPreferences prefs = getMultiDexPreferences(context);
         SharedPreferences.Editor edit = prefs.edit();
         edit.putLong(KEY_TIME_STAMP, timeStamp);
         edit.putLong(KEY_CRC, crc);
-        edit.putInt(KEY_DEX_NUMBER, totalDexNumber);
+        edit.putInt(KEY_DEX_NUMBER, extractedDexes.size() + 1);
+
+        int extractedDexId = 2;
+        for (ExtractedDex dex : extractedDexes) {
+            edit.putLong(KEY_DEX_CRC + extractedDexId, dex.crc);
+            edit.putLong(KEY_DEX_TIME + extractedDexId, dex.lastModified());
+            extractedDexId++;
+        }
         /* Use commit() and not apply() as advised by the doc because we need synchronous writing of
          * the editor content and apply is doing an "asynchronous commit to disk".
          */
@@ -372,26 +406,6 @@ final class MultiDexExtractor {
     }
 
     /**
-     * Returns whether the file is a valid zip file.
-     */
-    private static boolean verifyZipFile(File file) {
-        try {
-            ZipFile zipFile = new ZipFile(file);
-            try {
-                zipFile.close();
-                return true;
-            } catch (IOException e) {
-                Log.w(TAG, "Failed to close zip file: " + file.getAbsolutePath());
-            }
-        } catch (ZipException ex) {
-            Log.w(TAG, "File " + file.getAbsolutePath() + " is not a valid zip file.", ex);
-        } catch (IOException ex) {
-            Log.w(TAG, "Got an IOException trying to open zip file: " + file.getAbsolutePath(), ex);
-        }
-        return false;
-    }
-
-    /**
      * Closes the given {@code Closeable}. Suppresses any IO exceptions.
      */
     private static void closeQuietly(Closeable closeable) {