Phase out keymaster fallback support
Keystore uses two different keymaster devices.
One device is provided by the OEM providing
hardware/trust zone backed functionality. The other
is a pure software implementation of keymaster.
The latter was used when a "hardware" implementation
failed generating or importing keys with certain
parameters.
This tolerance of misbehaving "hardware" implementations
had the effect that this behavior has done unnoticed for
too long. Therefore, we are phasing out the fallback
device.
This patch ensures that on devices with hardware
implementations supporting keymaster 2.0 and higher
there will be no fallback device papering over failures
in the underlying keymaster implementation.
Test: given a faulty KM2.0 implementation, import and generation
of keys with otherwise supported parameters returns an error
Change-Id: I8c2118e72558c326031368df13e836c3ef6b1da1
diff --git a/keystore/key_store_service.cpp b/keystore/key_store_service.cpp
index 6d17749..c2d98bb 100644
--- a/keystore/key_store_service.cpp
+++ b/keystore/key_store_service.cpp
@@ -652,8 +652,11 @@
if (!error.isOk()) {
ALOGE("Failed to generate key -> falling back to software keymaster");
usingFallback = true;
- auto& fallback = mKeyStore->getFallbackDevice();
- rc = KS_HANDLE_HIDL_ERROR(fallback->generateKey(params, hidl_cb));
+ auto fallback = mKeyStore->getFallbackDevice();
+ if (!fallback.isOk()) {
+ return error;
+ }
+ rc = KS_HANDLE_HIDL_ERROR(fallback.value()->generateKey(params, hidl_cb));
if (!rc.isOk()) {
return rc;
}
@@ -795,8 +798,11 @@
if (!error.isOk()) {
ALOGE("Failed to import key -> falling back to software keymaster");
usingFallback = true;
- auto& fallback = mKeyStore->getFallbackDevice();
- rc = KS_HANDLE_HIDL_ERROR(fallback->importKey(params, format, keyData, hidlCb));
+ auto fallback = mKeyStore->getFallbackDevice();
+ if (!fallback.isOk()) {
+ return error;
+ }
+ rc = KS_HANDLE_HIDL_ERROR(fallback.value()->importKey(params, format, keyData, hidlCb));
// possible hidl error
if (!rc.isOk()) {
return rc;
diff --git a/keystore/keystore.cpp b/keystore/keystore.cpp
index 2d9de5d..02c39df 100644
--- a/keystore/keystore.cpp
+++ b/keystore/keystore.cpp
@@ -40,8 +40,10 @@
using namespace keystore;
-KeyStore::KeyStore(Entropy* entropy, const km_device_t& device, const km_device_t& fallback)
- : mEntropy(entropy), mDevice(device), mFallbackDevice(fallback) {
+KeyStore::KeyStore(Entropy* entropy, const km_device_t& device, const km_device_t& fallback,
+ bool allowNewFallback)
+ : mEntropy(entropy), mDevice(device), mFallbackDevice(fallback),
+ mAllowNewFallback(allowNewFallback) {
memset(&mMetaData, '\0', sizeof(mMetaData));
}
diff --git a/keystore/keystore.h b/keystore/keystore.h
index 210d99d..8ff8899 100644
--- a/keystore/keystore.h
+++ b/keystore/keystore.h
@@ -24,24 +24,38 @@
#include <utils/Vector.h>
#include "blob.h"
+#include "include/keystore/keymaster_tags.h"
typedef struct {
uint32_t uid;
const uint8_t* filename;
} grant_t;
+using ::keystore::NullOr;
+
class KeyStore {
typedef ::android::sp<::android::hardware::keymaster::V3_0::IKeymasterDevice> km_device_t;
public:
- KeyStore(Entropy* entropy, const km_device_t& device, const km_device_t& fallback);
+ KeyStore(Entropy* entropy, const km_device_t& device, const km_device_t& fallback,
+ bool allowNewFallback);
~KeyStore();
km_device_t& getDevice() { return mDevice; }
- km_device_t& getFallbackDevice() { return mFallbackDevice; }
+ NullOr<km_device_t&> getFallbackDevice() {
+ // we only return the fallback device if the creation of new fallback key blobs is
+ // allowed. (also see getDevice below)
+ if (mAllowNewFallback) {
+ return mFallbackDevice;
+ } else {
+ return {};
+ }
+ }
km_device_t& getDevice(const Blob& blob) {
+ // We return a device, based on the nature of the blob to provide backward
+ // compatibility with old key blobs generated using the fallback device.
return blob.isFallback() ? mFallbackDevice : mDevice;
}
@@ -119,6 +133,7 @@
km_device_t mDevice;
km_device_t mFallbackDevice;
+ bool mAllowNewFallback;
android::Vector<UserState*> mMasterKeys;
diff --git a/keystore/keystore_main.cpp b/keystore/keystore_main.cpp
index c5b36fd..1416a2b 100644
--- a/keystore/keystore_main.cpp
+++ b/keystore/keystore_main.cpp
@@ -29,6 +29,8 @@
#include "keystore.h"
#include "permissions.h"
#include "legacy_keymaster_device_wrapper.h"
+#include "include/keystore/keystore_hidl_support.h"
+#include "include/keystore/keystore_return_types.h"
/* KeyStore is a secured storage for key-value pairs. In this implementation,
* each file stores one key-value pair. Keys are encoded in file names, and
@@ -68,7 +70,22 @@
return -1;
}
- KeyStore keyStore(&entropy, dev, fallback);
+ bool allowNewFallbackDevice = false;
+
+ keystore::KeyStoreServiceReturnCode rc;
+ rc = KS_HANDLE_HIDL_ERROR(dev->getHardwareFeatures(
+ [&] (bool, bool, bool, bool supportsAttestation) {
+ // Attestation support indicates the hardware is keymaster 2.0 or higher.
+ // For these devices we will not allow the fallback device for import or generation
+ // of keys. The fallback device is only used for legacy keys present on the device.
+ allowNewFallbackDevice = !supportsAttestation;
+ }));
+
+ if (!rc.isOk()) {
+ return -1;
+ }
+
+ KeyStore keyStore(&entropy, dev, fallback, allowNewFallbackDevice);
keyStore.initialize();
android::sp<android::IServiceManager> sm = android::defaultServiceManager();
android::sp<keystore::KeyStoreService> service = new keystore::KeyStoreService(&keyStore);