Remove broken code for mounting encrypted OBB files
Mounting encrypted OBB files has never worked reliably across devices,
partly due to its reliance on Twofish encryption support in the kernel.
This is because Twofish support (CONFIG_CRYPTO_TWOFISH) has never been
required or even recommended for Android. It has never been enabled in
GKI, but even before GKI it wasn't required or recommended. Moreover,
this is now the only Android feature that still uses dm-crypt
(CONFIG_DM_CRYPT), and some devices don't have that enabled either.
Therefore, it appears that this feature is unused. That's perhaps not
surprising, considering that the documentation for OBBs
(https://developer.android.com/google/play/expansion-files) says that
they are deprecated, and also it explains OBBs as being app files that
are opaque to the platform; the ability of the platform to mount OBBs
that happen to be in a particular format is never mentioned. That means
that OBB mounting is probably rarely used even with unencrypted OBBs.
Finally, the usefulness of OBBs having their own encryption layer (in
addition to what the platform already provides via FBE) is not clear
either, especially with such an unusual choice of cipher.
To avoid the confusion that is being caused by having the broken code
for mounting encrypted OBBs still sitting around, let's remove it.
Test: atest StorageManagerTest # on Cuttlefish
Test: atest StorageManagerIntegrationTest # on Cuttlefish
Bug: 216475849
Change-Id: Iaef32cce90f95ea745ba2b143f89e66f533f3479
diff --git a/Android.bp b/Android.bp
index 1550264..38abdb9 100644
--- a/Android.bp
+++ b/Android.bp
@@ -115,7 +115,6 @@
"Benchmark.cpp",
"Checkpoint.cpp",
"CryptoType.cpp",
- "Devmapper.cpp",
"EncryptInplace.cpp",
"FileDeviceUtils.cpp",
"FsCrypt.cpp",
diff --git a/Devmapper.cpp b/Devmapper.cpp
deleted file mode 100644
index 00fb4b3..0000000
--- a/Devmapper.cpp
+++ /dev/null
@@ -1,100 +0,0 @@
-/*
- * Copyright (C) 2008 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#define ATRACE_TAG ATRACE_TAG_PACKAGE_MANAGER
-
-#include <errno.h>
-#include <fcntl.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-
-#include <sys/stat.h>
-#include <sys/types.h>
-
-#include <linux/kdev_t.h>
-
-#include <android-base/logging.h>
-#include <android-base/stringprintf.h>
-#include <android-base/strings.h>
-#include <libdm/dm.h>
-#include <utils/Trace.h>
-
-#include "Devmapper.h"
-
-using android::base::StringPrintf;
-using namespace android::dm;
-
-static const char* kVoldPrefix = "vold:";
-
-int Devmapper::create(const char* name_raw, const char* loopFile, const char* key,
- unsigned long numSectors, char* ubuffer, size_t len) {
- auto& dm = DeviceMapper::Instance();
- auto name_string = StringPrintf("%s%s", kVoldPrefix, name_raw);
-
- DmTable table;
- table.Emplace<DmTargetCrypt>(0, numSectors, "twofish", key, 0, loopFile, 0);
-
- if (!dm.CreateDevice(name_string, table)) {
- LOG(ERROR) << "Failed to create device-mapper device " << name_string;
- return -1;
- }
-
- std::string path;
- if (!dm.GetDmDevicePathByName(name_string, &path)) {
- LOG(ERROR) << "Failed to get device-mapper device path for " << name_string;
- return -1;
- }
- snprintf(ubuffer, len, "%s", path.c_str());
- return 0;
-}
-
-int Devmapper::destroy(const char* name_raw) {
- auto& dm = DeviceMapper::Instance();
-
- auto name_string = StringPrintf("%s%s", kVoldPrefix, name_raw);
- if (!dm.DeleteDevice(name_string)) {
- if (errno != ENXIO) {
- PLOG(ERROR) << "Failed DM_DEV_REMOVE";
- }
- return -1;
- }
- return 0;
-}
-
-int Devmapper::destroyAll() {
- ATRACE_NAME("Devmapper::destroyAll");
-
- auto& dm = DeviceMapper::Instance();
- std::vector<DeviceMapper::DmBlockDevice> devices;
- if (!dm.GetAvailableDevices(&devices)) {
- LOG(ERROR) << "Failed to get dm devices";
- return -1;
- }
-
- for (const auto& device : devices) {
- if (android::base::StartsWith(device.name(), kVoldPrefix)) {
- LOG(DEBUG) << "Tearing down stale dm device named " << device.name();
- if (!dm.DeleteDevice(device.name())) {
- if (errno != ENXIO) {
- PLOG(WARNING) << "Failed to destroy dm device named " << device.name();
- }
- }
- }
- }
- return 0;
-}
diff --git a/Devmapper.h b/Devmapper.h
deleted file mode 100644
index 9d4896e..0000000
--- a/Devmapper.h
+++ /dev/null
@@ -1,31 +0,0 @@
-/*
- * Copyright (C) 2008 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#ifndef _DEVMAPPER_H
-#define _DEVMAPPER_H
-
-#include <linux/dm-ioctl.h>
-#include <unistd.h>
-
-class Devmapper {
- public:
- static int create(const char* name, const char* loopFile, const char* key,
- unsigned long numSectors, char* buffer, size_t len);
- static int destroy(const char* name);
- static int destroyAll();
-};
-
-#endif
diff --git a/VoldNativeService.cpp b/VoldNativeService.cpp
index 1033af9..576717c 100644
--- a/VoldNativeService.cpp
+++ b/VoldNativeService.cpp
@@ -414,16 +414,13 @@
return translate(VolumeManager::Instance()->fixupAppDir(path, appUid));
}
-binder::Status VoldNativeService::createObb(const std::string& sourcePath,
- const std::string& sourceKey, int32_t ownerGid,
+binder::Status VoldNativeService::createObb(const std::string& sourcePath, int32_t ownerGid,
std::string* _aidl_return) {
ENFORCE_SYSTEM_OR_ROOT;
CHECK_ARGUMENT_PATH(sourcePath);
- CHECK_ARGUMENT_HEX(sourceKey);
ACQUIRE_LOCK;
- return translate(
- VolumeManager::Instance()->createObb(sourcePath, sourceKey, ownerGid, _aidl_return));
+ return translate(VolumeManager::Instance()->createObb(sourcePath, ownerGid, _aidl_return));
}
binder::Status VoldNativeService::destroyObb(const std::string& volId) {
diff --git a/VoldNativeService.h b/VoldNativeService.h
index 49bcbaa..58301f5 100644
--- a/VoldNativeService.h
+++ b/VoldNativeService.h
@@ -73,8 +73,8 @@
binder::Status setupAppDir(const std::string& path, int32_t appUid);
binder::Status fixupAppDir(const std::string& path, int32_t appUid);
- binder::Status createObb(const std::string& sourcePath, const std::string& sourceKey,
- int32_t ownerGid, std::string* _aidl_return);
+ binder::Status createObb(const std::string& sourcePath, int32_t ownerGid,
+ std::string* _aidl_return);
binder::Status destroyObb(const std::string& volId);
binder::Status createStubVolume(const std::string& sourcePath, const std::string& mountPath,
diff --git a/VolumeManager.cpp b/VolumeManager.cpp
index 9311321..bc51042 100644
--- a/VolumeManager.cpp
+++ b/VolumeManager.cpp
@@ -55,7 +55,6 @@
#include <fscrypt/fscrypt.h>
#include "AppFuseUtil.h"
-#include "Devmapper.h"
#include "FsCrypt.h"
#include "Loop.h"
#include "NetlinkManager.h"
@@ -179,7 +178,6 @@
// directories that we own, in case we crashed.
unmountAll();
- Devmapper::destroyAll();
Loop::destroyAll();
// Assume that we always have an emulated volume on internal
@@ -1075,8 +1073,8 @@
return setupAppDir(path, appUid, true /* fixupExistingOnly */);
}
-int VolumeManager::createObb(const std::string& sourcePath, const std::string& sourceKey,
- int32_t ownerGid, std::string* outVolId) {
+int VolumeManager::createObb(const std::string& sourcePath, int32_t ownerGid,
+ std::string* outVolId) {
int id = mNextObbId++;
std::string lowerSourcePath;
@@ -1114,7 +1112,7 @@
}
auto vol = std::shared_ptr<android::vold::VolumeBase>(
- new android::vold::ObbVolume(id, lowerSourcePath, sourceKey, ownerGid));
+ new android::vold::ObbVolume(id, lowerSourcePath, ownerGid));
vol->create();
mObbVolumes.push_back(vol);
diff --git a/VolumeManager.h b/VolumeManager.h
index 3573b1a..a8117c9 100644
--- a/VolumeManager.h
+++ b/VolumeManager.h
@@ -186,8 +186,7 @@
// Called before zygote starts to ensure dir exists so zygote can bind mount them.
int ensureAppDirsCreated(const std::vector<std::string>& paths, int32_t appUid);
- int createObb(const std::string& path, const std::string& key, int32_t ownerGid,
- std::string* outVolId);
+ int createObb(const std::string& path, int32_t ownerGid, std::string* outVolId);
int destroyObb(const std::string& volId);
int createStubVolume(const std::string& sourcePath, const std::string& mountPath,
diff --git a/binder/android/os/IVold.aidl b/binder/android/os/IVold.aidl
index c72ceea..cec0593 100644
--- a/binder/android/os/IVold.aidl
+++ b/binder/android/os/IVold.aidl
@@ -60,8 +60,7 @@
void fixupAppDir(@utf8InCpp String path, int appUid);
void ensureAppDirsCreated(in @utf8InCpp String[] paths, int appUid);
- @utf8InCpp String createObb(@utf8InCpp String sourcePath, @utf8InCpp String sourceKey,
- int ownerGid);
+ @utf8InCpp String createObb(@utf8InCpp String sourcePath, int ownerGid);
void destroyObb(@utf8InCpp String volId);
void fstrim(int fstrimFlags, IVoldTaskListener listener);
diff --git a/model/ObbVolume.cpp b/model/ObbVolume.cpp
index 21479c4..b64c1ba 100644
--- a/model/ObbVolume.cpp
+++ b/model/ObbVolume.cpp
@@ -15,7 +15,6 @@
*/
#include "ObbVolume.h"
-#include "Devmapper.h"
#include "Loop.h"
#include "Utils.h"
#include "VoldUtil.h"
@@ -39,12 +38,10 @@
namespace android {
namespace vold {
-ObbVolume::ObbVolume(int id, const std::string& sourcePath, const std::string& sourceKey,
- gid_t ownerGid)
+ObbVolume::ObbVolume(int id, const std::string& sourcePath, gid_t ownerGid)
: VolumeBase(Type::kObb) {
setId(StringPrintf("obb:%d", id));
mSourcePath = sourcePath;
- mSourceKey = sourceKey;
mOwnerGid = ownerGid;
}
@@ -55,36 +52,13 @@
PLOG(ERROR) << getId() << " failed to create loop";
return -1;
}
-
- if (!mSourceKey.empty()) {
- uint64_t nr_sec = 0;
- if (GetBlockDev512Sectors(mLoopPath, &nr_sec) != OK) {
- PLOG(ERROR) << getId() << " failed to get loop size";
- return -1;
- }
-
- char tmp[PATH_MAX];
- if (Devmapper::create(getId().c_str(), mLoopPath.c_str(), mSourceKey.c_str(), nr_sec, tmp,
- PATH_MAX)) {
- PLOG(ERROR) << getId() << " failed to create dm";
- return -1;
- }
- mDmPath = tmp;
- mMountPath = mDmPath;
- } else {
- mMountPath = mLoopPath;
- }
return OK;
}
status_t ObbVolume::doDestroy() {
- if (!mDmPath.empty() && Devmapper::destroy(getId().c_str())) {
- PLOG(WARNING) << getId() << " failed to destroy dm";
- }
if (!mLoopPath.empty() && Loop::destroyByDevice(mLoopPath.c_str())) {
PLOG(WARNING) << getId() << " failed to destroy loop";
}
- mDmPath.clear();
mLoopPath.clear();
return OK;
}
@@ -98,7 +72,7 @@
return -1;
}
// clang-format off
- if (android::vold::vfat::Mount(mMountPath, path, true, false, true,
+ if (android::vold::vfat::Mount(mLoopPath, path, true, false, true,
0, mOwnerGid, 0227, false)) {
// clang-format on
PLOG(ERROR) << getId() << " failed to mount";
diff --git a/model/ObbVolume.h b/model/ObbVolume.h
index 8f7ee94..bfcd3d2 100644
--- a/model/ObbVolume.h
+++ b/model/ObbVolume.h
@@ -29,7 +29,7 @@
*/
class ObbVolume : public VolumeBase {
public:
- ObbVolume(int id, const std::string& sourcePath, const std::string& sourceKey, gid_t ownerGid);
+ ObbVolume(int id, const std::string& sourcePath, gid_t ownerGid);
virtual ~ObbVolume();
protected:
@@ -40,12 +40,9 @@
private:
std::string mSourcePath;
- std::string mSourceKey;
gid_t mOwnerGid;
std::string mLoopPath;
- std::string mDmPath;
- std::string mMountPath;
DISALLOW_COPY_AND_ASSIGN(ObbVolume);
};