Hash keys with MD5; track IBinders not IInterface

Using a plaintext password doesn't work unless it's a certain length, so
just hash the plaintext password with MD5 to make it the right length
for the twofish encryption.

Tracking the IInterface doesn't make much sense since it's different
each time, so track the IBinder instead. That way we can unlinkToDeath
the binder when the last thing it's holding onto goes away.

Change-Id: Id828d25b4d74f27e9d8b4bfb3909c964469cc473

3 files changed, 67 insertions, 37 deletions

diff --git a/packages/DefaultContainerService/src/com/android/defcontainer/DefaultContainerService.java b/packages/DefaultContainerService/src/com/android/defcontainer/DefaultContainerService.java
index eb86277..ce10f5b 100644
--- a/packages/DefaultContainerService/src/com/android/defcontainer/DefaultContainerService.java
+++ b/packages/DefaultContainerService/src/com/android/defcontainer/DefaultContainerService.java
@@ -159,7 +159,7 @@ public class DefaultContainerService extends IntentService {
             try {
                 return ObbScanner.getObbInfo(filename);
             } catch (IOException e) {
-                Log.d(TAG, "Couldn't get OBB info", e);
+                Log.d(TAG, "Couldn't get OBB info for " + filename);
                 return null;
             }
         }
diff --git a/services/java/com/android/server/MountService.java b/services/java/com/android/server/MountService.java
index 62a9894..a4fe8bb 100644
--- a/services/java/com/android/server/MountService.java
+++ b/services/java/com/android/server/MountService.java
@@ -17,6 +17,7 @@
 package com.android.server;
 
 import com.android.internal.app.IMediaContainerService;
+import com.android.internal.util.HexDump;
 import com.android.server.am.ActivityManagerService;
 
 import android.content.BroadcastReceiver;
@@ -44,11 +45,13 @@ import android.os.storage.IMountServiceListener;
 import android.os.storage.IMountShutdownObserver;
 import android.os.storage.IObbActionListener;
 import android.os.storage.StorageResultCode;
+import android.security.MessageDigest;
 import android.util.Slog;
 
 import java.io.FileDescriptor;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -157,15 +160,18 @@ class MountService extends IMountService.Stub
      * OBBs.
      */
     final private Map<Integer, Integer> mObbUidUsage = new HashMap<Integer, Integer>();
-    final private Map<IObbActionListener, List<ObbState>> mObbMounts = new HashMap<IObbActionListener, List<ObbState>>();
+    final private Map<IBinder, List<ObbState>> mObbMounts = new HashMap<IBinder, List<ObbState>>();
     final private Map<String, ObbState> mObbPathToStateMap = new HashMap<String, ObbState>();
 
     class ObbState implements IBinder.DeathRecipient {
-        public ObbState(String filename, IObbActionListener token, int callerUid) {
+        public ObbState(String filename, IObbActionListener token, int callerUid)
+                throws RemoteException {
             this.filename = filename;
             this.token = token;
             this.callerUid = callerUid;
             mounted = false;
+
+            getBinder().linkToDeath(this, 0);
         }
 
         // OBB source filename
@@ -180,14 +186,18 @@ class MountService extends IMountService.Stub
         // Whether this is mounted currently.
         boolean mounted;
 
+        public IBinder getBinder() {
+            return token.asBinder();
+        }
+
         @Override
         public void binderDied() {
             ObbAction action = new UnmountObbAction(this, true);
             mObbActionHandler.sendMessage(mObbActionHandler.obtainMessage(OBB_RUN_ACTION, action));
+        }
 
-            removeObbState(this);
-
-            token.asBinder().unlinkToDeath(this, 0);
+        public void cleanUp() {
+            getBinder().unlinkToDeath(this, 0);
         }
 
         @Override
@@ -204,7 +214,6 @@ class MountService extends IMountService.Stub
             sb.append('}');
             return sb.toString();
         }
-
     }
 
     // OBB Action Handler
@@ -1556,7 +1565,8 @@ class MountService extends IMountService.Stub
         return false;
     }
 
-    public void mountObb(String filename, String key, IObbActionListener token) {
+    public void mountObb(String filename, String key, IObbActionListener token)
+            throws RemoteException {
         waitForReady();
         warnOnNotMounted();
 
@@ -1589,13 +1599,22 @@ class MountService extends IMountService.Stub
             addObbState(obbState);
         }
 
+        final MessageDigest md;
         try {
-            token.asBinder().linkToDeath(obbState, 0);
-        } catch (RemoteException rex) {
-            Slog.e(TAG, "Failed to link to listener death");
+            md = MessageDigest.getInstance("MD5");
+        } catch (NoSuchAlgorithmException e) {
+            Slog.e(TAG, "Could not load MD5 algorithm", e);
+            try {
+                token.onObbResult(filename, Environment.MEDIA_UNMOUNTED);
+            } catch (RemoteException e1) {
+                Slog.d(TAG, "Could not send unmount notification for: " + filename);
+            }
+            return;
         }
 
-        ObbAction action = new MountObbAction(obbState, key);
+        String hashedKey = HexDump.toHexString(md.digest(key.getBytes()));
+
+        ObbAction action = new MountObbAction(obbState, hashedKey);
         mObbActionHandler.sendMessage(mObbActionHandler.obtainMessage(OBB_RUN_ACTION, action));
 
         if (DEBUG_OBB)
@@ -1625,7 +1644,7 @@ class MountService extends IMountService.Stub
 
             if (Binder.getCallingUid() != obbState.callerUid) {
                 throw new SecurityException("caller UID does not match original mount caller UID");
-            } else if (!token.asBinder().equals(obbState.token.asBinder())) {
+            } else if (!token.asBinder().equals(obbState.getBinder())) {
                 throw new SecurityException("caller does not match original mount caller");
             }
         }
@@ -1639,10 +1658,10 @@ class MountService extends IMountService.Stub
 
     private void addObbState(ObbState obbState) {
         synchronized (mObbMounts) {
-            List<ObbState> obbStates = mObbMounts.get(obbState.token);
+            List<ObbState> obbStates = mObbMounts.get(obbState.getBinder());
             if (obbStates == null) {
                 obbStates = new ArrayList<ObbState>();
-                mObbMounts.put(obbState.token, obbStates);
+                mObbMounts.put(obbState.getBinder(), obbStates);
             }
             obbStates.add(obbState);
             mObbPathToStateMap.put(obbState.filename, obbState);
@@ -1660,12 +1679,13 @@ class MountService extends IMountService.Stub
 
     private void removeObbState(ObbState obbState) {
         synchronized (mObbMounts) {
-            final List<ObbState> obbStates = mObbMounts.get(obbState.token);
+            final List<ObbState> obbStates = mObbMounts.get(obbState.getBinder());
             if (obbStates != null) {
                 obbStates.remove(obbState);
             }
             if (obbStates == null || obbStates.isEmpty()) {
-                mObbMounts.remove(obbState.token);
+                mObbMounts.remove(obbState.getBinder());
+                obbState.cleanUp();
             }
             mObbPathToStateMap.remove(obbState.filename);
 
@@ -1719,20 +1739,16 @@ class MountService extends IMountService.Stub
                             Slog.e(TAG, "Failed to bind to media container service");
                             action.handleError();
                             return;
-                        } else {
-                            // Once we bind to the service, the first
-                            // pending request will be processed.
-                            mActions.add(action);
-                        }
-                    } else {
-                        // Already bound to the service. Just make
-                        // sure we trigger off processing the first request.
-                        if (mActions.size() == 0) {
-                            mObbActionHandler.sendEmptyMessage(OBB_MCS_BOUND);
                         }
 
                         mActions.add(action);
+                        break;
                     }
+
+                    // Once we bind to the service, the first
+                    // pending request will be processed.
+                    mActions.add(action);
+                    mObbActionHandler.sendEmptyMessage(OBB_MCS_BOUND);
                     break;
                 }
                 case OBB_MCS_BOUND: {
@@ -1898,7 +1914,7 @@ class MountService extends IMountService.Stub
             mKey = key;
         }
 
-        public void handleExecute() throws IOException {
+        public void handleExecute() throws IOException, RemoteException {
             final ObbInfo obbInfo = getObbInfo();
 
             /*
@@ -2084,6 +2100,8 @@ class MountService extends IMountService.Stub
             sb.append(mObbState.callerUid);
             sb.append(",token=");
             sb.append(mObbState.token != null ? mObbState.token.toString() : "null");
+            sb.append(",binder=");
+            sb.append(mObbState.getBinder().toString());
             sb.append('}');
             return sb.toString();
         }
diff --git a/tools/obbtool/mkobb.sh b/tools/obbtool/mkobb.sh
index f4cae9a..1987696 100755
--- a/tools/obbtool/mkobb.sh
+++ b/tools/obbtool/mkobb.sh
@@ -21,7 +21,7 @@
 MOUNTDIR=/tmp
 
 # Presets. Changing these will probably break your OBB on the device
-CRYPTO=blowfish
+CRYPTO=twofish
 FS=vfat
 MKFS=mkfs.vfat
 LOSETUP=losetup
@@ -122,7 +122,12 @@ onexit() {
         rmdir ${temp_mount}
     fi
     if [ "x${loop_dev}" != "x" ]; then \
-        ${LOSETUPBIN} -d ${loop_dev}
+        if [ ${use_crypto} -eq 1 ]; then \
+            dmsetup remove -f ${loop_dev}
+            ${LOSETUPBIN} -d ${old_loop_dev}
+        else \
+            ${LOSETUPBIN} -d ${loop_dev}
+        fi
     fi
     if [ "x${tempfile}" != "x" -a -f "${tempfile}" ]; then \
         rm -f ${tempfile}
@@ -202,7 +207,7 @@ trap onexit ERR
 
 tempfile=$(tempfile -d ${outdir}) || ( echo "ERROR: couldn't create temporary file in ${outdir}"; exit 1 )
 
-block_count=`du --apparent-size --block-size=512 ${directory} | awk '{ print $1; }'`
+block_count=`du -s --apparent-size --block-size=512 ${directory} | awk '{ print $1; }'`
 if [ $? -ne 0 ]; then \
     echo "ERROR: Couldn't read size of input directory ${directory}"
     exit 1
@@ -216,12 +221,14 @@ fi
 
 loop_dev=$(${LOSETUPBIN} -f) || ( echo "ERROR: losetup wouldn't tell us the next unused device"; exit 1 )
 
+${LOSETUPBIN} ${loop_dev} ${tempfile} || ( echo "ERROR: couldn't create loopback device"; exit 1 )
+
 if [ ${use_crypto} -eq 1 ]; then \
-    keyfile=$(tempfile -d ${outdir}) || ( echo "ERROR: could not create temporary key file"; exit 1 )
-    ${LOSETUPBIN} -p 5 -e ${CRYPTO} ${loop_dev} ${tempfile} 5< ${keyfile} || ( echo "ERROR: couldn't create loopback device"; exit 1 )
-    rm -f ${keyfile}
-else \
-    ${LOSETUPBIN} ${loop_dev} ${tempfile} || ( echo "ERROR: couldn't create loopback device"; exit 1 )
+    hashed_key=`echo -n "${key}" | md5sum | awk '{ print $1 }'`
+    unique_dm_name=`basename ${tempfile}`
+    echo "0 `blockdev --getsize ${loop_dev}` crypt ${CRYPTO} ${hashed_key} 0 ${loop_dev} 0" | dmsetup create ${unique_dm_name}
+    old_loop_dev=${loop_dev}
+    loop_dev=/dev/mapper/${unique_dm_name}
 fi
 
 #
@@ -252,7 +259,12 @@ echo "Successfully created \`${filename}'"
 #
 umount ${temp_mount}
 rmdir ${temp_mount}
-${LOSETUPBIN} -d ${loop_dev}
+if [ ${use_crypto} -eq 1 ]; then \
+    dmsetup remove -f ${loop_dev}
+    ${LOSETUPBIN} -d ${old_loop_dev}
+else \
+    ${LOSETUPBIN} -d ${loop_dev}
+fi
 mv ${tempfile} ${filename}
 
 trap - ERR