From d0bc4b9e6469c961f89b9b7ec67bd9c98b21f805 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Tue, 4 May 2021 09:22:10 +0100 Subject: [PATCH 1/2] KeyMint HAL: restore getKeyCharacteristics() docs Commit 40eb8f53ea3b ("KeyMint AIDL tweaks") removed references to getKeyCharacteristics() in docs, as it was a KeyMaster entrypoint that wasn't present on the KeyMint HAL. Commit dadb18dd290f ("Add getKeyCharacteristics method to KeyMint") added the getKeyCharacteristics() entrypoint to KeyMint, as it turned out it was needed after all. This commit restores references to getKeyCharacteristics() in the Tag.aidl documentation. Test: VtsAidlKeyMintTargetTest Change-Id: I860479554b85f4adfeddd4eee70a09cf5265c938 --- .../hardware/security/keymint/Tag.aidl | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl index 1e101abc7a..8fbc91a0ef 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl @@ -505,10 +505,10 @@ enum Tag { /** * Tag::APPLICATION_ID. When provided to generateKey or importKey, this tag specifies data - * that is necessary during all uses of the key. In particular, calls to exportKey() must - * provide the same value to the clientId parameter, and calls to begin() must provide this - * tag and the same associated data as part of the inParams set. If the correct data is not - * provided, the method must return ErrorCode::INVALID_KEY_BLOB. + * that is necessary during all uses of the key. In particular, calls to exportKey() and + * getKeyCharacteristics() must provide the same value to the clientId parameter, and calls to + * begin() must provide this tag and the same associated data as part of the inParams set. If + * the correct data is not provided, the method must return ErrorCode::INVALID_KEY_BLOB. * * The content of this tag must be bound to the key cryptographically, meaning it must not be * possible for an adversary who has access to all of the secure world secrets but does not have @@ -573,8 +573,8 @@ enum Tag { * Tag::OS_VERSION specifies the system OS version with which the key may be used. This tag is * never sent to the IKeyMintDevice, but is added to the hardware-enforced authorization list * by the TA. Any attempt to use a key with a Tag::OS_VERSION value different from the - * currently-running OS version must cause begin() or exportKey() to return - * ErrorCode::KEY_REQUIRES_UPGRADE. See upgradeKey() for details. + * currently-running OS version must cause begin(), getKeyCharacteristics() or exportKey() to + * return ErrorCode::KEY_REQUIRES_UPGRADE. See upgradeKey() for details. * * The value of the tag is an integer of the form MMmmss, where MM is the major version number, * mm is the minor version number, and ss is the sub-minor version number. For example, for a @@ -596,8 +596,9 @@ enum Tag { * Tag::OS_PATCHLEVEL specifies the system security patch level with which the key may be used. * This tag is never sent to the keyMint TA, but is added to the hardware-enforced * authorization list by the TA. Any attempt to use a key with a Tag::OS_PATCHLEVEL value - * different from the currently-running system patchlevel must cause begin() or - * exportKey() to return ErrorCode::KEY_REQUIRES_UPGRADE. See upgradeKey() for details. + * different from the currently-running system patchlevel must cause begin(), + * getKeyCharacteristics() or exportKey() to return ErrorCode::KEY_REQUIRES_UPGRADE. See + * upgradeKey() for details. * * The value of the tag is an integer of the form YYYYMM, where YYYY is the four-digit year of * the last update and MM is the two-digit month of the last update. For example, for a key @@ -789,8 +790,9 @@ enum Tag { * Tag::VENDOR_PATCHLEVEL specifies the vendor image security patch level with which the key may * be used. This tag is never sent to the keyMint TA, but is added to the hardware-enforced * authorization list by the TA. Any attempt to use a key with a Tag::VENDOR_PATCHLEVEL value - * different from the currently-running system patchlevel must cause begin() or - * exportKey() to return ErrorCode::KEY_REQUIRES_UPGRADE. See upgradeKey() for details. + * different from the currently-running system patchlevel must cause begin(), + * getKeyCharacteristics() or exportKey() to return ErrorCode::KEY_REQUIRES_UPGRADE. See + * upgradeKey() for details. * * The value of the tag is an integer of the form YYYYMMDD, where YYYY is the four-digit year of * the last update, MM is the two-digit month and DD is the two-digit day of the last @@ -811,8 +813,8 @@ enum Tag { * key may be used. This tag is never sent to the keyMint TA, but is added to the * hardware-enforced authorization list by the TA. Any attempt to use a key with a * Tag::BOOT_PATCHLEVEL value different from the currently-running system patchlevel must - * cause begin() or exportKey() to return ErrorCode::KEY_REQUIRES_UPGRADE. See upgradeKey() for - * details. + * cause begin(), getKeyCharacteristics() or exportKey() to return + * ErrorCode::KEY_REQUIRES_UPGRADE. See upgradeKey() for details. * * The value of the tag is an integer of the form YYYYMMDD, where YYYY is the four-digit year of * the last update, MM is the two-digit month and DD is the two-digit day of the last From c9bc2f742d084b7ec4e367b78ae009c3bdae7efe Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Tue, 4 May 2021 10:47:58 +0100 Subject: [PATCH 2/2] KeyMint VTS: symmetric import test with bad keylen Test: VtsAidlKeyMintTargetTest Change-Id: I32ad8ad2ca2b18d3279ebe77ba63b34457ab888d --- .../aidl/vts/functional/KeyMintTest.cpp | 46 +++++++++++++++++-- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/security/keymint/aidl/vts/functional/KeyMintTest.cpp b/security/keymint/aidl/vts/functional/KeyMintTest.cpp index 016a09e302..cd7d603a09 100644 --- a/security/keymint/aidl/vts/functional/KeyMintTest.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintTest.cpp @@ -654,7 +654,8 @@ TEST_P(NewKeyGenerationTest, AesInvalidPadding) { } auto result = Begin(KeyPurpose::ENCRYPT, params); EXPECT_TRUE(result == ErrorCode::INCOMPATIBLE_PADDING_MODE || - result == ErrorCode::INVALID_KEY_BLOB); + result == ErrorCode::INVALID_KEY_BLOB) + << "unexpected result: " << result; } else { // The KeyMint implementation detected that the generated key // is unusable. @@ -3263,6 +3264,7 @@ TEST_P(ImportKeyTest, AesFailure) { string key = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; uint32_t bitlen = key.size() * 8; for (uint32_t key_size : {bitlen - 1, bitlen + 1, bitlen - 8, bitlen + 8}) { + // Explicit key size doesn't match that of the provided key. auto result = ImportKey(AuthorizationSetBuilder() .Authorization(TAG_NO_AUTH_REQUIRED) .AesEncryptionKey(key_size) @@ -3270,8 +3272,27 @@ TEST_P(ImportKeyTest, AesFailure) { .Padding(PaddingMode::PKCS7), KeyFormat::RAW, key); ASSERT_TRUE(result == ErrorCode::IMPORT_PARAMETER_MISMATCH || - result == ErrorCode::UNSUPPORTED_KEY_SIZE); + result == ErrorCode::UNSUPPORTED_KEY_SIZE) + << "unexpected result: " << result; } + + // Explicit key size matches that of the provided key, but it's not a valid size. + string long_key = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; + ASSERT_EQ(ErrorCode::UNSUPPORTED_KEY_SIZE, + ImportKey(AuthorizationSetBuilder() + .Authorization(TAG_NO_AUTH_REQUIRED) + .AesEncryptionKey(long_key.size() * 8) + .EcbMode() + .Padding(PaddingMode::PKCS7), + KeyFormat::RAW, long_key)); + string short_key = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; + ASSERT_EQ(ErrorCode::UNSUPPORTED_KEY_SIZE, + ImportKey(AuthorizationSetBuilder() + .Authorization(TAG_NO_AUTH_REQUIRED) + .AesEncryptionKey(short_key.size() * 8) + .EcbMode() + .Padding(PaddingMode::PKCS7), + KeyFormat::RAW, short_key)); } /* @@ -3310,6 +3331,7 @@ TEST_P(ImportKeyTest, TripleDesFailure) { string key = hex2str("a49d7564199e97cb529d2c9d97bf2f98d35edf57ba1f7358"); uint32_t bitlen = key.size() * 8; for (uint32_t key_size : {bitlen - 1, bitlen + 1, bitlen - 8, bitlen + 8}) { + // Explicit key size doesn't match that of the provided key. auto result = ImportKey(AuthorizationSetBuilder() .Authorization(TAG_NO_AUTH_REQUIRED) .TripleDesEncryptionKey(key_size) @@ -3317,8 +3339,26 @@ TEST_P(ImportKeyTest, TripleDesFailure) { .Padding(PaddingMode::PKCS7), KeyFormat::RAW, key); ASSERT_TRUE(result == ErrorCode::IMPORT_PARAMETER_MISMATCH || - result == ErrorCode::UNSUPPORTED_KEY_SIZE); + result == ErrorCode::UNSUPPORTED_KEY_SIZE) + << "unexpected result: " << result; } + // Explicit key size matches that of the provided key, but it's not a valid size. + string long_key = hex2str("a49d7564199e97cb529d2c9d97bf2f98d35edf57ba1f7358"); + ASSERT_EQ(ErrorCode::UNSUPPORTED_KEY_SIZE, + ImportKey(AuthorizationSetBuilder() + .Authorization(TAG_NO_AUTH_REQUIRED) + .TripleDesEncryptionKey(long_key.size() * 8) + .EcbMode() + .Padding(PaddingMode::PKCS7), + KeyFormat::RAW, long_key)); + string short_key = hex2str("a49d7564199e97cb529d2c9d97bf2f98d35edf57ba1f7358"); + ASSERT_EQ(ErrorCode::UNSUPPORTED_KEY_SIZE, + ImportKey(AuthorizationSetBuilder() + .Authorization(TAG_NO_AUTH_REQUIRED) + .TripleDesEncryptionKey(short_key.size() * 8) + .EcbMode() + .Padding(PaddingMode::PKCS7), + KeyFormat::RAW, short_key)); } /*