Pass volume key as a KeyBuffer
Not for security, but for consistency with the way we handle other
keys, and to move the length check to where it belongs.
Test: create private volume on Cuttlefish
Bug: 147814592
Change-Id: I10fc4896183d050ce25ff174faf78f525cf62930
diff --git a/cryptfs.cpp b/cryptfs.cpp
index 337bdc2..e00165f 100644
--- a/cryptfs.cpp
+++ b/cryptfs.cpp
@@ -71,6 +71,7 @@
using android::base::ParseUint;
using android::base::StringPrintf;
using android::fs_mgr::GetEntryForMountPoint;
+using android::vold::KeyBuffer;
using namespace android::dm;
using namespace std::chrono_literals;
@@ -1910,8 +1911,13 @@
* as any metadata is been stored in a separate, small partition. We
* assume it must be using our same crypt type and keysize.
*/
-int cryptfs_setup_ext_volume(const char* label, const char* real_blkdev, const unsigned char* key,
+int cryptfs_setup_ext_volume(const char* label, const char* real_blkdev, const KeyBuffer& key,
std::string* out_crypto_blkdev) {
+ if (key.size() != cryptfs_get_keysize()) {
+ SLOGE("Raw keysize %zu does not match crypt keysize %" PRIu32, key.size(),
+ cryptfs_get_keysize());
+ return -1;
+ }
uint64_t nr_sec = 0;
if (android::vold::GetBlockDev512Sectors(real_blkdev, &nr_sec) != android::OK) {
SLOGE("Failed to get size of %s: %s", real_blkdev, strerror(errno));
@@ -1929,7 +1935,8 @@
android::base::GetBoolProperty("ro.crypto.allow_encrypt_override", false))
flags |= CREATE_CRYPTO_BLK_DEV_FLAGS_ALLOW_ENCRYPT_OVERRIDE;
- return create_crypto_blk_dev(&ext_crypt_ftr, key, real_blkdev, out_crypto_blkdev, label, flags);
+ return create_crypto_blk_dev(&ext_crypt_ftr, reinterpret_cast<const unsigned char*>(key.data()),
+ real_blkdev, out_crypto_blkdev, label, flags);
}
/*
diff --git a/cryptfs.h b/cryptfs.h
index 98ba7d6..463db7f 100644
--- a/cryptfs.h
+++ b/cryptfs.h
@@ -25,6 +25,8 @@
#include <cutils/properties.h>
+#include "KeyBuffer.h"
+
#define CRYPT_FOOTER_OFFSET 0x4000
/* Return values for cryptfs_crypto_complete */
@@ -62,8 +64,8 @@
int cryptfs_enable(int type, const char* passwd, int no_ui);
int cryptfs_changepw(int type, const char* newpw);
int cryptfs_enable_default(int no_ui);
-int cryptfs_setup_ext_volume(const char* label, const char* real_blkdev, const unsigned char* key,
- std::string* out_crypto_blkdev);
+int cryptfs_setup_ext_volume(const char* label, const char* real_blkdev,
+ const android::vold::KeyBuffer& key, std::string* out_crypto_blkdev);
int cryptfs_revert_ext_volume(const char* label);
int cryptfs_getfield(const char* fieldname, char* value, int len);
int cryptfs_setfield(const char* fieldname, const char* value);
diff --git a/model/Disk.cpp b/model/Disk.cpp
index b66c336..bfaf2cd 100644
--- a/model/Disk.cpp
+++ b/model/Disk.cpp
@@ -216,7 +216,8 @@
LOG(DEBUG) << "Found key for GUID " << normalizedGuid;
- auto vol = std::shared_ptr<VolumeBase>(new PrivateVolume(device, keyRaw));
+ auto keyBuffer = KeyBuffer(keyRaw.begin(), keyRaw.end());
+ auto vol = std::shared_ptr<VolumeBase>(new PrivateVolume(device, keyBuffer));
if (mJustPartitioned) {
LOG(DEBUG) << "Device just partitioned; silently formatting";
vol->setSilent(true);
diff --git a/model/PrivateVolume.cpp b/model/PrivateVolume.cpp
index 7fd46a2..1653fae 100644
--- a/model/PrivateVolume.cpp
+++ b/model/PrivateVolume.cpp
@@ -43,7 +43,7 @@
static const unsigned int kMajorBlockMmc = 179;
-PrivateVolume::PrivateVolume(dev_t device, const std::string& keyRaw)
+PrivateVolume::PrivateVolume(dev_t device, const KeyBuffer& keyRaw)
: VolumeBase(Type::kPrivate), mRawDevice(device), mKeyRaw(keyRaw) {
setId(StringPrintf("private:%u,%u", major(device), minor(device)));
mRawDevPath = StringPrintf("/dev/block/vold/%s", getId().c_str());
@@ -64,19 +64,13 @@
if (CreateDeviceNode(mRawDevPath, mRawDevice)) {
return -EIO;
}
- if (mKeyRaw.size() != cryptfs_get_keysize()) {
- PLOG(ERROR) << getId() << " Raw keysize " << mKeyRaw.size()
- << " does not match crypt keysize " << cryptfs_get_keysize();
- return -EIO;
- }
// Recover from stale vold by tearing down any old mappings
cryptfs_revert_ext_volume(getId().c_str());
// TODO: figure out better SELinux labels for private volumes
- unsigned char* key = (unsigned char*)mKeyRaw.data();
- int res = cryptfs_setup_ext_volume(getId().c_str(), mRawDevPath.c_str(), key, &mDmDevPath);
+ int res = cryptfs_setup_ext_volume(getId().c_str(), mRawDevPath.c_str(), mKeyRaw, &mDmDevPath);
if (res != 0) {
PLOG(ERROR) << getId() << " failed to setup cryptfs";
return -EIO;
diff --git a/model/PrivateVolume.h b/model/PrivateVolume.h
index cb8e75d..819632b 100644
--- a/model/PrivateVolume.h
+++ b/model/PrivateVolume.h
@@ -37,7 +37,7 @@
*/
class PrivateVolume : public VolumeBase {
public:
- PrivateVolume(dev_t device, const std::string& keyRaw);
+ PrivateVolume(dev_t device, const KeyBuffer& keyRaw);
virtual ~PrivateVolume();
const std::string& getFsType() const { return mFsType; };
const std::string& getRawDevPath() const { return mRawDevPath; };
@@ -63,7 +63,7 @@
std::string mPath;
/* Encryption key as raw bytes */
- std::string mKeyRaw;
+ KeyBuffer mKeyRaw;
/* Filesystem type */
std::string mFsType;