commit 3d98f5d1599794010151d3abd92295d88684658e
author Paul Crowley <paulcrowley@google.com> Fri Feb 07 11:49:09 2020 -0800
committer Paul Crowley <paulcrowley@google.com> Wed Feb 12 14:26:38 2020 -0800
tree 83ac8c6be5902f30e28bcafa79a5b41e7b49dac6
parent 3dfb094cb26cf37e14b3bbf81e31248b913b3e41

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

model/Disk.cpp

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);

model/PrivateVolume.cpp

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;

model/PrivateVolume.h

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;