diff --git a/core/java/android/os/storage/IStorageManager.aidl b/core/java/android/os/storage/IStorageManager.aidl index 9385402c3d72..6c0a1f99e112 100644 --- a/core/java/android/os/storage/IStorageManager.aidl +++ b/core/java/android/os/storage/IStorageManager.aidl @@ -54,13 +54,13 @@ interface IStorageManager { */ void shutdown(IStorageShutdownObserver observer) = 19; /** - * Mounts an Opaque Binary Blob (OBB) with the specified decryption key and - * only allows the calling process's UID access to the contents. - * StorageManagerService will call back to the supplied IObbActionListener to inform - * it of the terminal state of the call. + * Mounts an Opaque Binary Blob (OBB). Only allows the calling process's UID + * access to the contents. StorageManagerService will call back to the + * supplied IObbActionListener to inform it of the terminal state of the + * call. */ - void mountObb(in String rawPath, in String canonicalPath, in String key, - IObbActionListener token, int nonce, in ObbInfo obbInfo) = 21; + void mountObb(in String rawPath, in String canonicalPath, IObbActionListener token, + int nonce, in ObbInfo obbInfo) = 21; /** * Unmounts an Opaque Binary Blob (OBB). When the force flag is specified, * any program using it will be forcibly killed to unmount the image. diff --git a/core/java/android/os/storage/StorageManager.java b/core/java/android/os/storage/StorageManager.java index 77c794cd17a8..39f87d558098 100644 --- a/core/java/android/os/storage/StorageManager.java +++ b/core/java/android/os/storage/StorageManager.java @@ -665,9 +665,7 @@ public class StorageManager { } /** - * Mount an Opaque Binary Blob (OBB) file. If a key is - * specified, it is supplied to the mounting process to be used in any - * encryption used in the OBB. + * Mount an Opaque Binary Blob (OBB) file. *

* The OBB will remain mounted for as long as the StorageManager reference * is held by the application. As soon as this reference is lost, the OBBs @@ -680,19 +678,22 @@ public class StorageManager { * application's OBB that shares its UID. * * @param rawPath the path to the OBB file - * @param key secret used to encrypt the OBB; may be null if no - * encryption was used on the OBB. + * @param key must be null. Previously, some Android device + * implementations accepted a non-null key to mount + * an encrypted OBB file. However, this never worked reliably and + * is no longer supported. * @param listener will receive the success or failure of the operation * @return whether the mount call was successfully queued or not */ public boolean mountObb(String rawPath, String key, OnObbStateChangeListener listener) { Preconditions.checkNotNull(rawPath, "rawPath cannot be null"); + Preconditions.checkArgument(key == null, "mounting encrypted OBBs is no longer supported"); Preconditions.checkNotNull(listener, "listener cannot be null"); try { final String canonicalPath = new File(rawPath).getCanonicalPath(); final int nonce = mObbActionListener.addListener(listener); - mStorageManager.mountObb(rawPath, canonicalPath, key, mObbActionListener, nonce, + mStorageManager.mountObb(rawPath, canonicalPath, mObbActionListener, nonce, getObbInfo(canonicalPath)); return true; } catch (IOException e) { diff --git a/core/tests/coretests/res/raw/obb_enc_file100_orig1.obb b/core/tests/coretests/res/raw/obb_enc_file100_orig1.obb deleted file mode 100644 index 373b8e4bd01f..000000000000 Binary files a/core/tests/coretests/res/raw/obb_enc_file100_orig1.obb and /dev/null differ diff --git a/core/tests/coretests/res/raw/obb_enc_file100_orig3.obb b/core/tests/coretests/res/raw/obb_enc_file100_orig3.obb deleted file mode 100644 index aa531d8b35bd..000000000000 Binary files a/core/tests/coretests/res/raw/obb_enc_file100_orig3.obb and /dev/null differ diff --git a/core/tests/coretests/src/android/os/storage/StorageManagerBaseTest.java b/core/tests/coretests/src/android/os/storage/StorageManagerBaseTest.java index 16dcff5e0c8c..e56c0ad0138a 100644 --- a/core/tests/coretests/src/android/os/storage/StorageManagerBaseTest.java +++ b/core/tests/coretests/src/android/os/storage/StorageManagerBaseTest.java @@ -46,11 +46,7 @@ public class StorageManagerBaseTest extends InstrumentationTestCase { protected static String OBB_FILE_1_CONTENTS_1 = "OneToOneThousandInts.bin"; protected static String OBB_FILE_2 = "obb_file2.obb"; protected static String OBB_FILE_3 = "obb_file3.obb"; - protected static String OBB_FILE_1_PASSWORD = "password1"; - protected static String OBB_FILE_1_ENCRYPTED = "obb_enc_file100_orig1.obb"; protected static String OBB_FILE_2_UNSIGNED = "obb_file2_nosign.obb"; - protected static String OBB_FILE_3_PASSWORD = "password3"; - protected static String OBB_FILE_3_ENCRYPTED = "obb_enc_file100_orig3.obb"; protected static String OBB_FILE_3_BAD_PACKAGENAME = "obb_file3_bad_packagename.obb"; protected static boolean FORCE = true; @@ -180,22 +176,21 @@ public class StorageManagerBaseTest extends InstrumentationTestCase { * Mounts an OBB file * * @param obbFilePath The full path to the OBB file to mount - * @param key (optional) The key to use to unencrypt the OBB; pass null for no encryption * @param expectedState The expected state resulting from trying to mount the OBB * @return A {@link String} representing the normalized path to OBB file that was mounted */ - protected String mountObb(String obbFilePath, String key, int expectedState) { - return doMountObb(obbFilePath, key, expectedState); + protected String mountObb(String obbFilePath, int expectedState) { + return doMountObb(obbFilePath, expectedState); } /** - * Mounts an OBB file with default options (no encryption, mounting succeeds) + * Mounts an OBB file with default options. * * @param obbFilePath The full path to the OBB file to mount * @return A {@link String} representing the normalized path to OBB file that was mounted */ protected String mountObb(String obbFilePath) { - return doMountObb(obbFilePath, null, OnObbStateChangeListener.MOUNTED); + return doMountObb(obbFilePath, OnObbStateChangeListener.MOUNTED); } /** @@ -232,13 +227,13 @@ public class StorageManagerBaseTest extends InstrumentationTestCase { * @return true if the listener was signaled of a state change by the system; else a fail() * is triggered if we timed out */ - protected String doMountObb_noThrow(String obbFilePath, String key, int expectedState) { - Log.i(LOG_TAG, "doMountObb() on " + obbFilePath + " using key: " + key); + protected String doMountObb_noThrow(String obbFilePath, int expectedState) { + Log.i(LOG_TAG, "doMountObb() on " + obbFilePath); assertTrue ("Null path was passed in for OBB file!", obbFilePath != null); assertTrue ("Null path was passed in for OBB file!", obbFilePath != null); ObbListener obbListener = new ObbListener(); - boolean success = mSm.mountObb(obbFilePath, key, obbListener); + boolean success = mSm.mountObb(obbFilePath, null, obbListener); success &= obbFilePath.equals(doWaitForObbStateChange(obbListener)); success &= (expectedState == obbListener.state()); @@ -260,17 +255,16 @@ public class StorageManagerBaseTest extends InstrumentationTestCase { * Mounts an OBB file without throwing and synchronously waits for it to finish mounting * * @param obbFilePath The full path to the OBB file to mount - * @param key (optional) The key to use to unencrypt the OBB; pass null for no encryption * @param expectedState The expected state resulting from trying to mount the OBB * @return A {@link String} representing the actual normalized path to OBB file that was * mounted, or null if the mounting failed */ - protected String doMountObb(String obbFilePath, String key, int expectedState) { - Log.i(LOG_TAG, "doMountObb() on " + obbFilePath + " using key: " + key); + protected String doMountObb(String obbFilePath, int expectedState) { + Log.i(LOG_TAG, "doMountObb() on " + obbFilePath); assertTrue ("Null path was passed in for OBB file!", obbFilePath != null); ObbListener obbListener = new ObbListener(); - assertTrue("mountObb call failed", mSm.mountObb(obbFilePath, key, obbListener)); + assertTrue("mountObb call failed", mSm.mountObb(obbFilePath, null, obbListener)); assertTrue("Failed to get OBB mount status change for file: " + obbFilePath, doWaitForObbStateChange(obbListener)); assertEquals("OBB mount state not what was expected!", expectedState, diff --git a/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java b/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java index 62f2ac28a601..ecd2f76a5160 100644 --- a/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java +++ b/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java @@ -82,58 +82,6 @@ public class StorageManagerIntegrationTest extends StorageManagerBaseTest { } } - /** - * Tests mounting a single encrypted OBB file and verifies its contents. - */ - @LargeTest - public void testMountSingleEncryptedObb() throws Exception { - final File file = createObbFile(OBB_FILE_3_ENCRYPTED, R.raw.obb_enc_file100_orig3); - String filePath = file.getAbsolutePath(); - mountObb(filePath, OBB_FILE_3_PASSWORD, OnObbStateChangeListener.MOUNTED); - verifyObb3Contents(filePath); - unmountObb(filePath, DONT_FORCE); - } - - /** - * Tests mounting a single encrypted OBB file using an invalid password. - */ - @LargeTest - public void testMountSingleEncryptedObbInvalidPassword() throws Exception { - final File file = createObbFile("bad password@$%#@^*(!&)", R.raw.obb_enc_file100_orig3); - String filePath = file.getAbsolutePath(); - mountObb(filePath, OBB_FILE_1_PASSWORD, OnObbStateChangeListener.ERROR_COULD_NOT_MOUNT); - } - - /** - * Tests simultaneously mounting 2 encrypted OBBs with different keys and verifies contents. - */ - @LargeTest - public void testMountTwoEncryptedObb() throws Exception { - File file3 = null; - File file1 = null; - try { - file3 = createObbFile(OBB_FILE_3_ENCRYPTED, R.raw.obb_enc_file100_orig3); - String filePath3 = file3.getAbsolutePath(); - mountObb(filePath3, OBB_FILE_3_PASSWORD, OnObbStateChangeListener.MOUNTED); - verifyObb3Contents(filePath3); - - file1 = createObbFile(OBB_FILE_1_ENCRYPTED, R.raw.obb_enc_file100_orig1); - String filePath1 = file1.getAbsolutePath(); - mountObb(filePath1, OBB_FILE_1_PASSWORD, OnObbStateChangeListener.MOUNTED); - verifyObb1Contents(filePath1); - - unmountObb(filePath3, DONT_FORCE); - unmountObb(filePath1, DONT_FORCE); - } finally { - if (file3 != null) { - file3.delete(); - } - if (file1 != null) { - file1.delete(); - } - } - } - /** * Tests mounting a single OBB that isn't signed. */ @@ -142,7 +90,7 @@ public class StorageManagerIntegrationTest extends StorageManagerBaseTest { final File file = createObbFile(OBB_FILE_2_UNSIGNED, R.raw.obb_file2_nosign); String filePath = file.getAbsolutePath(); try { - mountObb(filePath, OBB_FILE_2_UNSIGNED, OnObbStateChangeListener.ERROR_INTERNAL); + mountObb(filePath, OnObbStateChangeListener.ERROR_INTERNAL); fail("mountObb should've failed with an exception"); } catch (IllegalArgumentException e) { // Expected @@ -156,8 +104,7 @@ public class StorageManagerIntegrationTest extends StorageManagerBaseTest { public void testMountBadPackageNameObb() throws Exception { final File file = createObbFile(OBB_FILE_3_BAD_PACKAGENAME, R.raw.obb_file3_bad_packagename); String filePath = file.getAbsolutePath(); - mountObb(filePath, OBB_FILE_3_BAD_PACKAGENAME, - OnObbStateChangeListener.ERROR_PERMISSION_DENIED); + mountObb(filePath, OnObbStateChangeListener.ERROR_PERMISSION_DENIED); } /** @@ -169,7 +116,7 @@ public class StorageManagerIntegrationTest extends StorageManagerBaseTest { String filePath = file.getAbsolutePath(); mountObb(filePath); verifyObb1Contents(filePath); - mountObb(filePath, null, OnObbStateChangeListener.ERROR_ALREADY_MOUNTED); + mountObb(filePath, OnObbStateChangeListener.ERROR_ALREADY_MOUNTED); verifyObb1Contents(filePath); unmountObb(filePath, DONT_FORCE); } diff --git a/libs/storage/IMountService.cpp b/libs/storage/IMountService.cpp index fd6e6e932ebc..055dbb2b5c5e 100644 --- a/libs/storage/IMountService.cpp +++ b/libs/storage/IMountService.cpp @@ -442,14 +442,13 @@ public: reply.readExceptionCode(); } - void mountObb(const String16& rawPath, const String16& canonicalPath, const String16& key, + void mountObb(const String16& rawPath, const String16& canonicalPath, const sp& token, int32_t nonce, const sp& obbInfo) { Parcel data, reply; data.writeInterfaceToken(IMountService::getInterfaceDescriptor()); data.writeString16(rawPath); data.writeString16(canonicalPath); - data.writeString16(key); data.writeStrongBinder(IInterface::asBinder(token)); data.writeInt32(nonce); obbInfo->writeToParcel(&data); diff --git a/libs/storage/include/storage/IMountService.h b/libs/storage/include/storage/IMountService.h index 2463e023efc1..5b07318f4432 100644 --- a/libs/storage/include/storage/IMountService.h +++ b/libs/storage/include/storage/IMountService.h @@ -64,8 +64,8 @@ public: virtual void shutdown(const sp& observer) = 0; virtual void finishMediaUpdate() = 0; virtual void mountObb(const String16& rawPath, const String16& canonicalPath, - const String16& key, const sp& token, - const int32_t nonce, const sp& obbInfo) = 0; + const sp& token, const int32_t nonce, + const sp& obbInfo) = 0; virtual void unmountObb(const String16& filename, const bool force, const sp& token, const int32_t nonce) = 0; virtual bool isObbMounted(const String16& filename) = 0; diff --git a/native/android/storage_manager.cpp b/native/android/storage_manager.cpp index 22725254fef6..9e0a6eb476d3 100644 --- a/native/android/storage_manager.cpp +++ b/native/android/storage_manager.cpp @@ -140,8 +140,7 @@ public: } } - void mountObb(const char* rawPath, const char* key, AStorageManager_obbCallbackFunc func, - void* data) { + void mountObb(const char* rawPath, AStorageManager_obbCallbackFunc func, void* data) { // Resolve path before sending to MountService char canonicalPath[PATH_MAX]; if (realpath(rawPath, canonicalPath) == NULL) { @@ -158,9 +157,7 @@ public: ObbCallback* cb = registerObbCallback(func, data); String16 rawPath16(rawPath); String16 canonicalPath16(canonicalPath); - String16 key16(key); - mMountService->mountObb(rawPath16, canonicalPath16, key16, mObbActionListener, - cb->nonce, obbInfo); + mMountService->mountObb(rawPath16, canonicalPath16, mObbActionListener, cb->nonce, obbInfo); } void unmountObb(const char* filename, const bool force, AStorageManager_obbCallbackFunc func, void* data) { @@ -207,7 +204,11 @@ void AStorageManager_delete(AStorageManager* mgr) { void AStorageManager_mountObb(AStorageManager* mgr, const char* filename, const char* key, AStorageManager_obbCallbackFunc cb, void* data) { - mgr->mountObb(filename, key, cb, data); + if (key != nullptr && key[0] != '\0') { + ALOGE("mounting encrypted OBBs is no longer supported"); + return; + } + mgr->mountObb(filename, cb, data); } void AStorageManager_unmountObb(AStorageManager* mgr, const char* filename, const int force, diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java index 9546496fd8e8..9266bb431aa5 100644 --- a/services/core/java/com/android/server/StorageManagerService.java +++ b/services/core/java/com/android/server/StorageManagerService.java @@ -173,9 +173,6 @@ import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; import java.io.PrintWriter; -import java.math.BigInteger; -import java.security.GeneralSecurityException; -import java.security.spec.KeySpec; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -194,10 +191,6 @@ import java.util.concurrent.TimeoutException; import java.util.regex.Matcher; import java.util.regex.Pattern; -import javax.crypto.SecretKey; -import javax.crypto.SecretKeyFactory; -import javax.crypto.spec.PBEKeySpec; - /** * Service responsible for various storage media. Connects to {@code vold} to * watch for and manage dynamically added storage, such as SD cards and USB mass @@ -3073,8 +3066,8 @@ class StorageManagerService extends IStorageManager.Stub } @Override - public void mountObb(String rawPath, String canonicalPath, String key, - IObbActionListener token, int nonce, ObbInfo obbInfo) { + public void mountObb(String rawPath, String canonicalPath, IObbActionListener token, + int nonce, ObbInfo obbInfo) { Objects.requireNonNull(rawPath, "rawPath cannot be null"); Objects.requireNonNull(canonicalPath, "canonicalPath cannot be null"); Objects.requireNonNull(token, "token cannot be null"); @@ -3083,7 +3076,7 @@ class StorageManagerService extends IStorageManager.Stub final int callingUid = Binder.getCallingUid(); final ObbState obbState = new ObbState(rawPath, canonicalPath, callingUid, token, nonce, null); - final ObbAction action = new MountObbAction(obbState, key, callingUid, obbInfo); + final ObbAction action = new MountObbAction(obbState, callingUid, obbInfo); mObbActionHandler.sendMessage(mObbActionHandler.obtainMessage(OBB_RUN_ACTION, action)); if (DEBUG_OBB) @@ -4428,13 +4421,11 @@ class StorageManagerService extends IStorageManager.Stub } class MountObbAction extends ObbAction { - private final String mKey; private final int mCallingUid; private ObbInfo mObbInfo; - MountObbAction(ObbState obbState, String key, int callingUid, ObbInfo obbInfo) { + MountObbAction(ObbState obbState, int callingUid, ObbInfo obbInfo) { super(obbState); - mKey = key; mCallingUid = callingUid; mObbInfo = obbInfo; } @@ -4457,29 +4448,8 @@ class StorageManagerService extends IStorageManager.Stub "Attempt to mount OBB which is already mounted: " + mObbInfo.filename); } - final String hashedKey; - final String binderKey; - if (mKey == null) { - hashedKey = "none"; - binderKey = ""; - } else { - try { - SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1"); - - KeySpec ks = new PBEKeySpec(mKey.toCharArray(), mObbInfo.salt, - PBKDF2_HASH_ROUNDS, CRYPTO_ALGORITHM_KEY_SIZE); - SecretKey key = factory.generateSecret(ks); - BigInteger bi = new BigInteger(key.getEncoded()); - hashedKey = bi.toString(16); - binderKey = hashedKey; - } catch (GeneralSecurityException e) { - throw new ObbException(ERROR_INTERNAL, e); - } - } - try { - mObbState.volId = mVold.createObb(mObbState.canonicalPath, binderKey, - mObbState.ownerGid); + mObbState.volId = mVold.createObb(mObbState.canonicalPath, mObbState.ownerGid); mVold.mount(mObbState.volId, 0, -1, null); if (DEBUG_OBB)