From d4417fb98233bf090755fb2eba580c8e33d1714b Mon Sep 17 00:00:00 2001 From: Shawn Willden Date: Thu, 23 Feb 2017 11:01:49 -0700 Subject: [PATCH] Add digest support and implementation name to getHardwareFeatures This is needed to support the keystore statistics gathering initiative. It will allow us to get information about what kinds of keymaster implementations exist in the ecosystem, and which ones fail in which ways. Bug: 36549319 Test: Will add to VTS tests Change-Id: I49ee4623656060d69a6de7723b11cd715150451a --- keymaster/3.0/IKeymasterDevice.hal | 14 ++++++- keymaster/3.0/default/KeymasterDevice.cpp | 51 ++++++++++++----------- keymaster/3.0/default/KeymasterDevice.h | 7 +++- 3 files changed, 43 insertions(+), 29 deletions(-) diff --git a/keymaster/3.0/IKeymasterDevice.hal b/keymaster/3.0/IKeymasterDevice.hal index 50a41ec6e1..26647655b3 100644 --- a/keymaster/3.0/IKeymasterDevice.hal +++ b/keymaster/3.0/IKeymasterDevice.hal @@ -43,10 +43,20 @@ interface IKeymasterDevice { * key attestation certificates, signed with a key injected in a secure * environment. CDD requires that all devices initially launched with Android O or * later must support hardware attestation. + * + * @return supportsAllDigests is true if the hardware supports all keymaster digest functions, + * namely ND-5, SHA-1, SHA-224, SHA-256, SHA-384 and SHA-512. CDD requires that all + * devices launched initially with Android O or later must support all digests. + * + * @return keymasterName is the name of the keymaster implementation. + * + * @return keymasterAuthorName is the name of the author of the keymaster implementation + * (generally this should be the name of an organization, not an individual.) */ getHardwareFeatures() - generates(bool isSecure, bool supportsEllipticCurve, - bool supportsSymmetricCryptography, bool supportsAttestation); + generates(bool isSecure, bool supportsEllipticCurve, bool supportsSymmetricCryptography, + bool supportsAttestation, bool supportsAllDigests, string keymasterName, + string keymasterAuthorName); /** * Adds entropy to the RNG used by keymaster. Entropy added through this method is guaranteed diff --git a/keymaster/3.0/default/KeymasterDevice.cpp b/keymaster/3.0/default/KeymasterDevice.cpp index 01f079554b..720b946099 100644 --- a/keymaster/3.0/default/KeymasterDevice.cpp +++ b/keymaster/3.0/default/KeymasterDevice.cpp @@ -102,7 +102,8 @@ err: return rc; } -static int keymaster1_device_initialize(const hw_module_t* mod, keymaster2_device_t** dev) { +static int keymaster1_device_initialize(const hw_module_t* mod, keymaster2_device_t** dev, + bool* supports_all_digests) { assert(mod->module_api_version >= KEYMASTER_MODULE_API_VERSION_1_0); ALOGI("Found keymaster1 module %s, version %x", mod->name, mod->module_api_version); @@ -126,6 +127,7 @@ static int keymaster1_device_initialize(const hw_module_t* mod, keymaster2_devic } // SoftKeymasterDevice will be deleted by keymaster_device_release() + *supports_all_digests = soft_keymaster->supports_all_digests(); *dev = soft_keymaster.release()->keymaster2_device(); return 0; @@ -157,7 +159,7 @@ err: } static int keymaster_device_initialize(keymaster2_device_t** dev, uint32_t* version, - bool* supports_ec) { + bool* supports_ec, bool* supports_all_digests) { const hw_module_t* mod; *supports_ec = true; @@ -173,6 +175,7 @@ static int keymaster_device_initialize(keymaster2_device_t** dev, uint32_t* vers if (mod->module_api_version < KEYMASTER_MODULE_API_VERSION_1_0) { *version = 0; + *supports_all_digests = false; int rc = keymaster0_device_initialize(mod, dev); if (rc == 0 && ((*dev)->flags & KEYMASTER_SUPPORTS_EC) == 0) { *supports_ec = false; @@ -180,9 +183,10 @@ static int keymaster_device_initialize(keymaster2_device_t** dev, uint32_t* vers return rc; } else if (mod->module_api_version == KEYMASTER_MODULE_API_VERSION_1_0) { *version = 1; - return keymaster1_device_initialize(mod, dev); + return keymaster1_device_initialize(mod, dev, supports_all_digests); } else { *version = 2; + *supports_all_digests = true; return keymaster2_device_initialize(mod, dev); } } @@ -353,7 +357,7 @@ static inline hidl_vec kmParamSet2Hidl(const keymaster_key_param_s // Methods from ::android::hardware::keymaster::V3_0::IKeymasterDevice follow. Return KeymasterDevice::getHardwareFeatures(getHardwareFeatures_cb _hidl_cb) { - bool is_secure = false; + bool is_secure = !(keymaster_device_->flags & KEYMASTER_SOFTWARE_ONLY); bool supports_symmetric_cryptography = false; bool supports_attestation = false; @@ -363,14 +367,12 @@ Return KeymasterDevice::getHardwareFeatures(getHardwareFeatures_cb _hidl_c /* Falls through */ case 1: supports_symmetric_cryptography = true; - /* Falls through */ - case 0: - is_secure = true; break; }; _hidl_cb(is_secure, hardware_supports_ec_, supports_symmetric_cryptography, - supports_attestation); + supports_attestation, hardware_supports_all_digests_, + keymaster_device_->common.module->name, keymaster_device_->common.module->author); return Void(); } @@ -519,21 +521,19 @@ Return KeymasterDevice::attestKey(const hidl_vec& keyToAttest, for (size_t i = 0; i < attestParams.size(); ++i) { switch (attestParams[i].tag) { - case Tag::ATTESTATION_ID_BRAND: - case Tag::ATTESTATION_ID_DEVICE: - case Tag::ATTESTATION_ID_PRODUCT: - case Tag::ATTESTATION_ID_SERIAL: - case Tag::ATTESTATION_ID_IMEI: - case Tag::ATTESTATION_ID_MEID: - case Tag::ATTESTATION_ID_MANUFACTURER: - case Tag::ATTESTATION_ID_MODEL: - // Device id attestation may only be supported if the device is able to permanently - // destroy its knowledge of the ids. This device is unable to do this, so it must - // never perform any device id attestation. - _hidl_cb(ErrorCode::CANNOT_ATTEST_IDS, resultCertChain); - return Void(); - default: - break; + case Tag::ATTESTATION_ID_BRAND: + case Tag::ATTESTATION_ID_DEVICE: + case Tag::ATTESTATION_ID_PRODUCT: + case Tag::ATTESTATION_ID_SERIAL: + case Tag::ATTESTATION_ID_IMEI: + case Tag::ATTESTATION_ID_MEID: + // Device id attestation may only be supported if the device is able to permanently + // destroy its knowledge of the ids. This device is unable to do this, so it must + // never perform any device id attestation. + _hidl_cb(ErrorCode::CANNOT_ATTEST_IDS, resultCertChain); + return Void(); + default: + break; } } @@ -703,7 +703,8 @@ IKeymasterDevice* HIDL_FETCH_IKeymasterDevice(const char* /* name */) { uint32_t version; bool supports_ec; - auto rc = keymaster_device_initialize(&dev, &version, &supports_ec); + bool supports_all_digests; + auto rc = keymaster_device_initialize(&dev, &version, &supports_ec, &supports_all_digests); if (rc) return nullptr; auto kmrc = ::keymaster::ConfigureDevice(dev); @@ -712,7 +713,7 @@ IKeymasterDevice* HIDL_FETCH_IKeymasterDevice(const char* /* name */) { return nullptr; } - return new KeymasterDevice(dev, version, supports_ec); + return new KeymasterDevice(dev, version, supports_ec, supports_all_digests); } } // namespace implementation diff --git a/keymaster/3.0/default/KeymasterDevice.h b/keymaster/3.0/default/KeymasterDevice.h index 382f45fcbf..e048d5bb5b 100644 --- a/keymaster/3.0/default/KeymasterDevice.h +++ b/keymaster/3.0/default/KeymasterDevice.h @@ -44,9 +44,11 @@ using ::android::sp; class KeymasterDevice : public IKeymasterDevice { public: - KeymasterDevice(keymaster2_device_t* dev, uint32_t hardware_version, bool hardware_supports_ec) + KeymasterDevice(keymaster2_device_t* dev, uint32_t hardware_version, bool hardware_supports_ec, + bool hardware_supports_all_digests) : keymaster_device_(dev), hardware_version_(hardware_version), - hardware_supports_ec_(hardware_supports_ec) {} + hardware_supports_ec_(hardware_supports_ec), + hardware_supports_all_digests_(hardware_supports_all_digests) {} virtual ~KeymasterDevice(); // Methods from ::android::hardware::keymaster::V3_0::IKeymasterDevice follow. @@ -85,6 +87,7 @@ class KeymasterDevice : public IKeymasterDevice { keymaster2_device_t* keymaster_device_; uint32_t hardware_version_; bool hardware_supports_ec_; + bool hardware_supports_all_digests_; }; extern "C" IKeymasterDevice* HIDL_FETCH_IKeymasterDevice(const char* name);