From 56ba912b30e91a2c20174f693c4e2b0d634cfb90 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Mon, 19 Apr 2021 19:10:47 +0100 Subject: [PATCH] Align KeyMint AIDL with usage - Make HardwareAuthToken nullable on begin() - Drop unused vestigial performOperation() entrypoint - Drop unused Tag::BLOB_USAGE_REQUIREMENTS Test: TreeHugger, VtsKeyMintAidlTargetTest (CF) Change-Id: I577ac04d843ee6d03cbfb99e56ef3e69eb034532 --- .../security/keymint/IKeyMintDevice.aidl | 3 +-- .../hardware/security/keymint/Tag.aidl | 1 - .../security/keymint/IKeyMintDevice.aidl | 23 ++----------------- .../hardware/security/keymint/Tag.aidl | 17 +------------- .../vts/functional/KeyMintAidlTestBase.cpp | 5 ++-- .../aidl/vts/functional/KeyMintTest.cpp | 10 -------- .../aidl/vts/performance/KeyMintBenchmark.cpp | 3 +-- 7 files changed, 7 insertions(+), 55 deletions(-) diff --git a/security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/IKeyMintDevice.aidl b/security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/IKeyMintDevice.aidl index bf3099920f..3f75af6dd0 100644 --- a/security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/IKeyMintDevice.aidl +++ b/security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/IKeyMintDevice.aidl @@ -44,10 +44,9 @@ interface IKeyMintDevice { void deleteKey(in byte[] keyBlob); void deleteAllKeys(); void destroyAttestationIds(); - android.hardware.security.keymint.BeginResult begin(in android.hardware.security.keymint.KeyPurpose purpose, in byte[] keyBlob, in android.hardware.security.keymint.KeyParameter[] params, in android.hardware.security.keymint.HardwareAuthToken authToken); + android.hardware.security.keymint.BeginResult begin(in android.hardware.security.keymint.KeyPurpose purpose, in byte[] keyBlob, in android.hardware.security.keymint.KeyParameter[] params, in @nullable android.hardware.security.keymint.HardwareAuthToken authToken); void deviceLocked(in boolean passwordOnly, in @nullable android.hardware.security.secureclock.TimeStampToken timestampToken); void earlyBootEnded(); byte[] convertStorageKeyToEphemeral(in byte[] storageKeyBlob); - byte[] performOperation(in byte[] request); const int AUTH_TOKEN_MAC_LENGTH = 32; } diff --git a/security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/Tag.aidl b/security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/Tag.aidl index 7591318289..e310b4448f 100644 --- a/security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/Tag.aidl +++ b/security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/Tag.aidl @@ -48,7 +48,6 @@ enum Tag { RSA_PUBLIC_EXPONENT = 1342177480, INCLUDE_UNIQUE_ID = 1879048394, RSA_OAEP_MGF_DIGEST = 536871115, - BLOB_USAGE_REQUIREMENTS = 268435757, BOOTLOADER_ONLY = 1879048494, ROLLBACK_RESISTANCE = 1879048495, HARDWARE_TYPE = 268435760, diff --git a/security/keymint/aidl/android/hardware/security/keymint/IKeyMintDevice.aidl b/security/keymint/aidl/android/hardware/security/keymint/IKeyMintDevice.aidl index 1c503c2913..455df4edcd 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/IKeyMintDevice.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/IKeyMintDevice.aidl @@ -257,9 +257,6 @@ interface IKeyMintDevice { * * o Tag::ORIGIN with the value KeyOrigin::GENERATED. * - * o Tag::BLOB_USAGE_REQUIREMENTS with the appropriate value (see KeyBlobUsageRequirements in - * Tag.aidl). - * * o Tag::OS_VERSION, Tag::OS_PATCHLEVEL, Tag::VENDOR_PATCHLEVEL and Tag::BOOT_PATCHLEVEL with * appropriate values. * @@ -713,9 +710,7 @@ interface IKeyMintDevice { * contain a tag Tag::NONCE. If Tag::NONCE is provided for a key without * Tag:CALLER_NONCE, ErrorCode::CALLER_NONCE_PROHIBITED must be returned. * - * @param inAuthToken Authentication token. Callers that provide no token must set all numeric - * fields to zero and the MAC must be an empty vector. TODO: make this field nullable. - * b/173483024. + * @param inAuthToken Authentication token. * * @return BeginResult as output, which contains the challenge, KeyParameters which haves * additional data from the operation initialization, notably to return the IV or nonce @@ -723,7 +718,7 @@ interface IKeyMintDevice { * which is used to perform update(), finish() or abort() operations. */ BeginResult begin(in KeyPurpose purpose, in byte[] keyBlob, in KeyParameter[] params, - in HardwareAuthToken authToken); + in @nullable HardwareAuthToken authToken); /** * Called by client to notify the IKeyMintDevice that the device is now locked, and keys with @@ -783,18 +778,4 @@ interface IKeyMintDevice { * place of the input storageKeyBlob */ byte[] convertStorageKeyToEphemeral(in byte[] storageKeyBlob); - - /** - * Called by the client to perform a KeyMint operation. - * - * This method is added primarily as a placeholder. Details will be fleshed before the KeyMint - * V1 interface is frozen. Until then, implementations must return ErrorCode::UNIMPLEMENTED. - * - * @param request is an encrypted buffer containing a description of the operation the client - * wishes to perform. Structure, content and encryption are TBD. - * - * @return an encrypted buffer containing the result of the operation. Structure, content and - * encryption are TBD. - */ - byte[] performOperation(in byte[] request); } diff --git a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl index cde1fc0f60..1639f385fd 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl @@ -203,22 +203,7 @@ enum Tag { */ RSA_OAEP_MGF_DIGEST = (2 << 28) /* TagType:ENUM_REP */ | 203, - /** - * TODO(seleneh) this tag needs to be deleted from all codes. - * - * Tag::BLOB_USAGE_REQUIREMENTS specifies the necessary system environment conditions for the - * generated key to be used. Possible values are defined by the KeyBlobUsageRequirements enum. - * - * This tag is specified by the caller during key generation or import to require that the key - * is usable in the specified condition. If the caller specifies Tag::BLOB_USAGE_REQUIREMENTS - * with value KeyBlobUsageRequirements::STANDALONE the IKeyMintDevice must return a key blob - * that can be used without file system support. This is critical for devices with encrypted - * disks, where the file system may not be available until after a KeyMint key is used to - * decrypt the disk. - * - * Must be hardware-enforced. - */ - BLOB_USAGE_REQUIREMENTS = (1 << 28) /* TagType:ENUM */ | 301, + // Tag 301 reserved /** * Tag::BOOTLOADER_ONLY specifies only the bootloader can use the key. diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp index 59cb57b75b..ad24364418 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp @@ -119,7 +119,6 @@ char nibble2hex[16] = {'0', '1', '2', '3', '4', '5', '6', '7', // Attestations don't contain everything in key authorization lists, so we need to filter the key // lists to produce the lists that we expect to match the attestations. auto kTagsToFilter = { - Tag::BLOB_USAGE_REQUIREMENTS, // Tag::CREATION_DATETIME, // Tag::EC_CURVE, Tag::HARDWARE_TYPE, @@ -348,7 +347,7 @@ ErrorCode KeyMintAidlTestBase::Begin(KeyPurpose purpose, const vector& SCOPED_TRACE("Begin"); Status result; BeginResult out; - result = keymint_->begin(purpose, key_blob, in_params.vector_data(), HardwareAuthToken(), &out); + result = keymint_->begin(purpose, key_blob, in_params.vector_data(), std::nullopt, &out); if (result.isOk()) { *out_params = out.params; @@ -366,7 +365,7 @@ ErrorCode KeyMintAidlTestBase::Begin(KeyPurpose purpose, const vector& Status result; BeginResult out; - result = keymint_->begin(purpose, key_blob, in_params.vector_data(), HardwareAuthToken(), &out); + result = keymint_->begin(purpose, key_blob, in_params.vector_data(), std::nullopt, &out); if (result.isOk()) { *out_params = out.params; diff --git a/security/keymint/aidl/vts/functional/KeyMintTest.cpp b/security/keymint/aidl/vts/functional/KeyMintTest.cpp index 09cdab1a48..c44526c021 100644 --- a/security/keymint/aidl/vts/functional/KeyMintTest.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintTest.cpp @@ -5302,16 +5302,6 @@ TEST_P(UnlockedDeviceRequiredTest, DISABLED_KeysBecomeUnusable) { INSTANTIATE_KEYMINT_AIDL_TEST(UnlockedDeviceRequiredTest); -using PerformOperationTest = KeyMintAidlTestBase; - -TEST_P(PerformOperationTest, RequireUnimplemented) { - vector response; - auto result = keymint_->performOperation({} /* request */, &response); - ASSERT_EQ(GetReturnErrorCode(result), ErrorCode::UNIMPLEMENTED); -} - -INSTANTIATE_KEYMINT_AIDL_TEST(PerformOperationTest); - } // namespace aidl::android::hardware::security::keymint::test int main(int argc, char** argv) { diff --git a/security/keymint/aidl/vts/performance/KeyMintBenchmark.cpp b/security/keymint/aidl/vts/performance/KeyMintBenchmark.cpp index 6c795f5ac0..54b6fdc36b 100644 --- a/security/keymint/aidl/vts/performance/KeyMintBenchmark.cpp +++ b/security/keymint/aidl/vts/performance/KeyMintBenchmark.cpp @@ -228,8 +228,7 @@ class KeyMintBenchmarkTest { AuthorizationSet* out_params) { Status result; BeginResult out; - result = keymint_->begin(purpose, key_blob_, in_params.vector_data(), HardwareAuthToken(), - &out); + result = keymint_->begin(purpose, key_blob_, in_params.vector_data(), std::nullopt, &out); if (result.isOk()) { *out_params = out.params; op_ = out.operation;