From 401b2603516a64d3ee7804e270c966828e6b454a Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Thu, 14 Dec 2017 22:15:20 -0700 Subject: [PATCH] Delay touching disks when secure keyguard showing. We've tried our best to protect against malicious storage devices with limited SELinux domains, but let's be even more paranoid and refuse to look at disks inserted while a secure keyguard is showing. We'll gladly scan them right away once the user confirms their credentials. Test: builds, boots, manual testing Bug: 68054513 Change-Id: I37fd6c25bbd6631fa4ba3f84e19384d746a22498 --- VoldNativeService.cpp | 7 +++ VoldNativeService.h | 2 + VolumeManager.cpp | 97 +++++++++++++++++++++++++----------- VolumeManager.h | 8 +++ binder/android/os/IVold.aidl | 2 + 5 files changed, 87 insertions(+), 29 deletions(-) diff --git a/VoldNativeService.cpp b/VoldNativeService.cpp index e8e151f..9d8b990 100644 --- a/VoldNativeService.cpp +++ b/VoldNativeService.cpp @@ -285,6 +285,13 @@ binder::Status VoldNativeService::onUserStopped(int32_t userId) { return translate(VolumeManager::Instance()->onUserStopped(userId)); } +binder::Status VoldNativeService::onSecureKeyguardStateChanged(bool isShowing) { + ENFORCE_UID(AID_SYSTEM); + ACQUIRE_LOCK; + + return translate(VolumeManager::Instance()->onSecureKeyguardStateChanged(isShowing)); +} + binder::Status VoldNativeService::partition(const std::string& diskId, int32_t partitionType, int32_t ratio) { ENFORCE_UID(AID_SYSTEM); diff --git a/VoldNativeService.h b/VoldNativeService.h index d107138..1359d90 100644 --- a/VoldNativeService.h +++ b/VoldNativeService.h @@ -43,6 +43,8 @@ public: binder::Status onUserStarted(int32_t userId); binder::Status onUserStopped(int32_t userId); + binder::Status onSecureKeyguardStateChanged(bool isShowing); + binder::Status partition(const std::string& diskId, int32_t partitionType, int32_t ratio); binder::Status forgetPartition(const std::string& partGuid, const std::string& fsUuid); diff --git a/VolumeManager.cpp b/VolumeManager.cpp index e078c0d..0936ed0 100644 --- a/VolumeManager.cpp +++ b/VolumeManager.cpp @@ -90,6 +90,9 @@ VolumeManager *VolumeManager::Instance() { VolumeManager::VolumeManager() { mDebug = false; mNextObbId = 0; + // For security reasons, assume that a secure keyguard is + // showing until we hear otherwise + mSecureKeyguardShowing = true; } VolumeManager::~VolumeManager() { @@ -116,23 +119,13 @@ int VolumeManager::updateVirtualDisk() { auto disk = new android::vold::Disk("virtual", buf.st_rdev, "virtual", android::vold::Disk::Flags::kAdoptable | android::vold::Disk::Flags::kSd); - disk->create(); mVirtualDisk = std::shared_ptr(disk); - mDisks.push_back(mVirtualDisk); + handleDiskAdded(mVirtualDisk); } } else { if (mVirtualDisk != nullptr) { dev_t device = mVirtualDisk->getDevice(); - - auto i = mDisks.begin(); - while (i != mDisks.end()) { - if ((*i)->getDevice() == device) { - (*i)->destroy(); - i = mDisks.erase(i); - } else { - ++i; - } - } + handleDiskRemoved(device); Loop::destroyByDevice(mVirtualDiskPath.c_str()); mVirtualDisk = nullptr; @@ -217,8 +210,7 @@ void VolumeManager::handleBlockEvent(NetlinkEvent *evt) { auto disk = new android::vold::Disk(eventPath, device, source->getNickname(), flags); - disk->create(); - mDisks.push_back(std::shared_ptr(disk)); + handleDiskAdded(std::shared_ptr(disk)); break; } } @@ -226,24 +218,11 @@ void VolumeManager::handleBlockEvent(NetlinkEvent *evt) { } case NetlinkEvent::Action::kChange: { LOG(DEBUG) << "Disk at " << major << ":" << minor << " changed"; - for (const auto& disk : mDisks) { - if (disk->getDevice() == device) { - disk->readMetadata(); - disk->readPartitions(); - } - } + handleDiskChanged(device); break; } case NetlinkEvent::Action::kRemove: { - auto i = mDisks.begin(); - while (i != mDisks.end()) { - if ((*i)->getDevice() == device) { - (*i)->destroy(); - i = mDisks.erase(i); - } else { - ++i; - } - } + handleDiskRemoved(device); break; } default: { @@ -253,6 +232,51 @@ void VolumeManager::handleBlockEvent(NetlinkEvent *evt) { } } +void VolumeManager::handleDiskAdded(const std::shared_ptr& disk) { + // For security reasons, if secure keyguard is showing, wait + // until the user unlocks the device to actually touch it + if (mSecureKeyguardShowing) { + LOG(INFO) << "Found disk at " << disk->getEventPath() + << " but delaying scan due to secure keyguard"; + mPendingDisks.push_back(disk); + } else { + disk->create(); + mDisks.push_back(disk); + } +} + +void VolumeManager::handleDiskChanged(dev_t device) { + for (const auto& disk : mDisks) { + if (disk->getDevice() == device) { + disk->readMetadata(); + disk->readPartitions(); + } + } + + // For security reasons, we ignore all pending disks, since + // we'll scan them once the device is unlocked +} + +void VolumeManager::handleDiskRemoved(dev_t device) { + auto i = mDisks.begin(); + while (i != mDisks.end()) { + if ((*i)->getDevice() == device) { + (*i)->destroy(); + i = mDisks.erase(i); + } else { + ++i; + } + } + auto j = mPendingDisks.begin(); + while (j != mPendingDisks.end()) { + if ((*j)->getDevice() == device) { + j = mPendingDisks.erase(j); + } else { + ++j; + } + } +} + void VolumeManager::addDiskSource(const std::shared_ptr& diskSource) { std::lock_guard lock(mLock); mDiskSources.push_back(diskSource); @@ -367,6 +391,20 @@ int VolumeManager::onUserStopped(userid_t userId) { return 0; } +int VolumeManager::onSecureKeyguardStateChanged(bool isShowing) { + mSecureKeyguardShowing = isShowing; + if (!mSecureKeyguardShowing) { + // Now that secure keyguard has been dismissed, process + // any pending disks + for (const auto& disk : mPendingDisks) { + disk->create(); + mDisks.push_back(disk); + } + mPendingDisks.clear(); + } + return 0; +} + int VolumeManager::setPrimary(const std::shared_ptr& vol) { mPrimary = vol; for (userid_t userId : mStartedUsers) { @@ -554,6 +592,7 @@ int VolumeManager::shutdown() { disk->destroy(); } mDisks.clear(); + mPendingDisks.clear(); android::vold::sSleepOnUnmount = true; return 0; } diff --git a/VolumeManager.h b/VolumeManager.h index 5baa7ce..fb455d8 100644 --- a/VolumeManager.h +++ b/VolumeManager.h @@ -94,6 +94,8 @@ public: int onUserStarted(userid_t userId); int onUserStopped(userid_t userId); + int onSecureKeyguardStateChanged(bool isShowing); + int setPrimary(const std::shared_ptr& vol); int remountUid(uid_t uid, const std::string& mode); @@ -132,6 +134,10 @@ private: int linkPrimary(userid_t userId); + void handleDiskAdded(const std::shared_ptr& disk); + void handleDiskChanged(dev_t device); + void handleDiskRemoved(dev_t device); + std::mutex mLock; std::mutex mCryptLock; @@ -139,6 +145,7 @@ private: std::list> mDiskSources; std::list> mDisks; + std::list> mPendingDisks; std::list> mObbVolumes; std::unordered_map mAddedUsers; @@ -150,6 +157,7 @@ private: std::shared_ptr mPrimary; int mNextObbId; + bool mSecureKeyguardShowing; }; #endif diff --git a/binder/android/os/IVold.aidl b/binder/android/os/IVold.aidl index c3f5029..d073be2 100644 --- a/binder/android/os/IVold.aidl +++ b/binder/android/os/IVold.aidl @@ -33,6 +33,8 @@ interface IVold { void onUserStarted(int userId); void onUserStopped(int userId); + void onSecureKeyguardStateChanged(boolean isShowing); + void partition(@utf8InCpp String diskId, int partitionType, int ratio); void forgetPartition(@utf8InCpp String partGuid, @utf8InCpp String fsUuid);