Merge "Remove broken code for mounting encrypted OBB files" am: a4f24f08b0
Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/2003052
Change-Id: Id027c555af21e46922a1c0ce29195d95bb7bb876
diff --git a/core/java/android/os/storage/IStorageManager.aidl b/core/java/android/os/storage/IStorageManager.aidl
index 9385402c..6c0a1f9 100644
--- a/core/java/android/os/storage/IStorageManager.aidl
+++ b/core/java/android/os/storage/IStorageManager.aidl
@@ -54,13 +54,13 @@
*/
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 77c794c..39f87d5 100644
--- a/core/java/android/os/storage/StorageManager.java
+++ b/core/java/android/os/storage/StorageManager.java
@@ -665,9 +665,7 @@
}
/**
- * Mount an Opaque Binary Blob (OBB) file. If a <code>key</code> 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.
* <p>
* 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 @@
* 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 <code>null</code> if no
- * encryption was used on the OBB.
+ * @param key must be <code>null</code>. Previously, some Android device
+ * implementations accepted a non-<code>null</code> 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 373b8e4..0000000
--- a/core/tests/coretests/res/raw/obb_enc_file100_orig1.obb
+++ /dev/null
Binary files 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 aa531d8..0000000
--- a/core/tests/coretests/res/raw/obb_enc_file100_orig3.obb
+++ /dev/null
Binary files differ
diff --git a/core/tests/coretests/src/android/os/storage/StorageManagerBaseTest.java b/core/tests/coretests/src/android/os/storage/StorageManagerBaseTest.java
index 16dcff5..e56c0ad 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 @@
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 @@
* 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 @@
* @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 @@
* 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 62f2ac2..ecd2f76 100644
--- a/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java
+++ b/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java
@@ -83,58 +83,6 @@
}
/**
- * 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.
*/
@LargeTest
@@ -142,7 +90,7 @@
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 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 @@
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 fd6e6e9..055dbb2 100644
--- a/libs/storage/IMountService.cpp
+++ b/libs/storage/IMountService.cpp
@@ -442,14 +442,13 @@
reply.readExceptionCode();
}
- void mountObb(const String16& rawPath, const String16& canonicalPath, const String16& key,
+ void mountObb(const String16& rawPath, const String16& canonicalPath,
const sp<IObbActionListener>& token, int32_t nonce, const sp<ObbInfo>& 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 2463e02..5b07318 100644
--- a/libs/storage/include/storage/IMountService.h
+++ b/libs/storage/include/storage/IMountService.h
@@ -64,8 +64,8 @@
virtual void shutdown(const sp<IMountShutdownObserver>& observer) = 0;
virtual void finishMediaUpdate() = 0;
virtual void mountObb(const String16& rawPath, const String16& canonicalPath,
- const String16& key, const sp<IObbActionListener>& token,
- const int32_t nonce, const sp<ObbInfo>& obbInfo) = 0;
+ const sp<IObbActionListener>& token, const int32_t nonce,
+ const sp<ObbInfo>& obbInfo) = 0;
virtual void unmountObb(const String16& filename, const bool force,
const sp<IObbActionListener>& 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 2272525..9e0a6eb 100644
--- a/native/android/storage_manager.cpp
+++ b/native/android/storage_manager.cpp
@@ -140,8 +140,7 @@
}
}
- 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 @@
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_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 9546496..9266bb4 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.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.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 @@
}
@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 @@
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 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 @@
"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)