tequilaOS/platform_system_vold
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Remove broken code for mounting encrypted OBB files

Browse source 
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
This commit is contained in:
Eric Biggers 2022-03-01 21:19:18 +00:00
parent 537b76cd98
commit 7e79a43a72
10 changed files with 12 additions and 180 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
Android.bp
View file

@ -115,7 +115,6 @@ cc_library_static {
"Benchmark.cpp",
"Checkpoint.cpp",
"CryptoType.cpp",
"Devmapper.cpp",
"EncryptInplace.cpp",
"FileDeviceUtils.cpp",
"FsCrypt.cpp",

100
Devmapper.cpp
View file

@ -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;
}

31
Devmapper.h
View file

@ -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

7
VoldNativeService.cpp
View file

@ -414,16 +414,13 @@ binder::Status VoldNativeService::fixupAppDir(const std::string& path, int32_t a
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) {

4
VoldNativeService.h
View file

@ -73,8 +73,8 @@ class VoldNativeService : public BinderService<VoldNativeService>, public os::Bn
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,

8
VolumeManager.cpp
View file

@ -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 @@ int VolumeManager::start() {
// 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 @@ int VolumeManager::fixupAppDir(const std::string& path, int32_t appUid) {
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 @@ int VolumeManager::createObb(const std::string& sourcePath, const std::string& s
}
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);

3
VolumeManager.h
View file

@ -186,8 +186,7 @@ class VolumeManager {
// 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,

3
binder/android/os/IVold.aidl
View file

@ -60,8 +60,7 @@ interface IVold {
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);

30
model/ObbVolume.cpp
View file

@ -15,7 +15,6 @@
*/
#include "ObbVolume.h"
#include "Devmapper.h"
#include "Loop.h"
#include "Utils.h"
#include "VoldUtil.h"
@ -39,12 +38,10 @@ using android::base::StringPrintf;
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 @@ status_t ObbVolume::doCreate() {
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 @@ status_t ObbVolume::doMount() {
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";

5
model/ObbVolume.h
View file

@ -29,7 +29,7 @@ namespace vold {
*/
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 @@ class ObbVolume : public VolumeBase {
private:
std::string mSourcePath;
std::string mSourceKey;
gid_t mOwnerGid;
std::string mLoopPath;
std::string mDmPath;
std::string mMountPath;
DISALLOW_COPY_AND_ASSIGN(ObbVolume);
};