From 031b6050b17358538e27211c0cdb1021235290e5 Mon Sep 17 00:00:00 2001 From: Shawn Willden Date: Tue, 28 Mar 2017 00:40:57 +0000 Subject: [PATCH] Revert "Add auth token parsing to IKeymasterDevice.hal" This reverts commit 62f63c7ddbd08737e298a97975754225e5da0126. Reason for revert: b/36637075 Bug: 36637075 Change-Id: Ie0e8d0b480047a7c68f266e7e5d8a31722f85128 --- keymaster/3.0/IKeymasterDevice.hal | 16 ------- keymaster/3.0/default/KeymasterDevice.cpp | 58 +---------------------- keymaster/3.0/default/KeymasterDevice.h | 2 - keymaster/3.0/types.hal | 18 ++++--- 4 files changed, 13 insertions(+), 81 deletions(-) diff --git a/keymaster/3.0/IKeymasterDevice.hal b/keymaster/3.0/IKeymasterDevice.hal index 0c59e6c725..26647655b3 100644 --- a/keymaster/3.0/IKeymasterDevice.hal +++ b/keymaster/3.0/IKeymasterDevice.hal @@ -58,22 +58,6 @@ interface IKeymasterDevice { bool supportsAttestation, bool supportsAllDigests, string keymasterName, string keymasterAuthorName); - /** - * Parses a hardware authentication token blob to extract the details needed to determine if the - * token is applicable to a given keymaster operation. This method is intended to be - * implemented by the HAL, without requiring a call into the trusted hardware. It is not - * necessary for this method to verify that the values in the blob are correct. - * - * @param token The token blob provided by the authentication app. - * - * @return error ErrorCode::OK or, if the blob is invalid, ErrorCode::INVALD_ARGUMENT if the - * blob is the wrong size, the wrong version, or incorrectly structured. - * - * @return tokenInfo Information extracted from the auth token. - */ - parseHardwareAuthToken(vec token) - generates(ErrorCode error, HardwareAuthTokenInfo tokenInfo); - /** * Adds entropy to the RNG used by keymaster. Entropy added through this method is guaranteed * not to be the only source of entropy used, and the mixing function is required to be secure, diff --git a/keymaster/3.0/default/KeymasterDevice.cpp b/keymaster/3.0/default/KeymasterDevice.cpp index b2baa2bc9c..720b946099 100644 --- a/keymaster/3.0/default/KeymasterDevice.cpp +++ b/keymaster/3.0/default/KeymasterDevice.cpp @@ -33,8 +33,6 @@ namespace implementation { using ::keymaster::SoftKeymasterDevice; -namespace { - class SoftwareOnlyHidlKeymasterEnforcement : public ::keymaster::KeymasterEnforcement { public: SoftwareOnlyHidlKeymasterEnforcement() : KeymasterEnforcement(64, 64) {} @@ -62,7 +60,7 @@ class SoftwareOnlyHidlKeymasterContext : public ::keymaster::SoftKeymasterContex std::unique_ptr<::keymaster::KeymasterEnforcement> enforcement_; }; -int keymaster0_device_initialize(const hw_module_t* mod, keymaster2_device_t** dev) { +static int keymaster0_device_initialize(const hw_module_t* mod, keymaster2_device_t** dev) { assert(mod->module_api_version < KEYMASTER_MODULE_API_VERSION_1_0); ALOGI("Found keymaster0 module %s, version %x", mod->name, mod->module_api_version); @@ -139,7 +137,7 @@ err: return rc; } -int keymaster2_device_initialize(const hw_module_t* mod, keymaster2_device_t** dev) { +static int keymaster2_device_initialize(const hw_module_t* mod, keymaster2_device_t** dev) { assert(mod->module_api_version >= KEYMASTER_MODULE_API_VERSION_2_0); ALOGI("Found keymaster2 module %s, version %x", mod->name, mod->module_api_version); @@ -193,30 +191,6 @@ static int keymaster_device_initialize(keymaster2_device_t** dev, uint32_t* vers } } -template struct choose_ntoh; - -template struct choose_ntoh { - inline static IntType ntoh(const IntType& value) { - IntType result = 0; - const unsigned char* inbytes = reinterpret_cast(&value); - unsigned char* outbytes = reinterpret_cast(&result); - for (int i = sizeof(IntType) - 1; i >= 0; --i) { - *(outbytes++) = inbytes[i]; - } - return result; - } -}; - -template struct choose_ntoh { - inline static IntType hton(const IntType& value) { return value; } -}; - -template inline IntType ntoh(const IntType& value) { - return choose_ntoh::ntoh(value); -} - -} // anonymous namespace - KeymasterDevice::~KeymasterDevice() { if (keymaster_device_) keymaster_device_->common.close(&keymaster_device_->common); } @@ -402,34 +376,6 @@ Return KeymasterDevice::getHardwareFeatures(getHardwareFeatures_cb _hidl_c return Void(); } -Return KeymasterDevice::parseHardwareAuthToken(const hidl_vec& token, - parseHardwareAuthToken_cb _hidl_cb) { - HardwareAuthTokenInfo parsedToken; - if (token.size() != sizeof(hw_auth_token_t)) { - ALOGE("Received auth token of length %zu, expected %zu", token.size(), - sizeof(hw_auth_token_t)); - _hidl_cb(ErrorCode::INVALID_ARGUMENT, parsedToken); - return Void(); - } - - const hw_auth_token_t* authToken = reinterpret_cast(token.data()); - if (authToken->version != 0) { - ALOGE("Auth token version %u, expected version ", authToken->version); - _hidl_cb(ErrorCode::INVALID_ARGUMENT, parsedToken); - return Void(); - } - - parsedToken.challenge = authToken->challenge; - parsedToken.userId = authToken->user_id; - parsedToken.authenticatorId = authToken->authenticator_id; - parsedToken.authenticatorType = - static_cast(ntoh(authToken->authenticator_type)); - parsedToken.timestamp = ntoh(authToken->timestamp); - - _hidl_cb(ErrorCode::OK, parsedToken); - return Void(); -} - Return KeymasterDevice::addRngEntropy(const hidl_vec& data) { if (!data.size()) return ErrorCode::OK; return legacy_enum_conversion( diff --git a/keymaster/3.0/default/KeymasterDevice.h b/keymaster/3.0/default/KeymasterDevice.h index 8198eca96c..e048d5bb5b 100644 --- a/keymaster/3.0/default/KeymasterDevice.h +++ b/keymaster/3.0/default/KeymasterDevice.h @@ -53,8 +53,6 @@ class KeymasterDevice : public IKeymasterDevice { // Methods from ::android::hardware::keymaster::V3_0::IKeymasterDevice follow. Return getHardwareFeatures(getHardwareFeatures_cb _hidl_cb); - Return parseHardwareAuthToken(const hidl_vec& token, - parseHardwareAuthToken_cb _hidl_cb); Return addRngEntropy(const hidl_vec& data) override; Return generateKey(const hidl_vec& keyParams, generateKey_cb _hidl_cb) override; diff --git a/keymaster/3.0/types.hal b/keymaster/3.0/types.hal index 3ff4145902..1f4a0cc017 100644 --- a/keymaster/3.0/types.hal +++ b/keymaster/3.0/types.hal @@ -400,15 +400,19 @@ struct KeyCharacteristics { }; /** - * Data used to describe an authentication. This data is retrieved from an authentication token by - * calling the parseHardwareAuthToken method of the HAL. + * Data used to prove successful authentication. */ -struct HardwareAuthTokenInfo { +struct HardwareAuthToken { uint64_t challenge; - uint64_t userId; // Secure User ID, not Android user ID. - uint64_t authenticatorId; // Secure authenticator ID. - HardwareAuthenticatorType authenticatorType; - uint64_t timestamp; + uint64_t userId; // Secure User ID, not Android user ID. + uint64_t authenticatorId; // Secure authenticator ID. + uint32_t authenticatorType; // HardwareAuthenticatorType, in network order. + uint64_t timestamp; // In network order. + uint8_t[32] hmac; // HMAC is computed over 0 || challenge || user_id || + // authenticator_id || authenticator_type || timestamp, with a + // prefixed 0 byte (which was a version field in Keymaster1 and + // Keymaster2) and the fields packed (no padding; so you probably + // can't just compute over the bytes of the struct). }; enum SecurityLevel : uint32_t {