From db0dcf5034cedf82d4f9cd9b452c9d0290b4d4f5 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Tue, 18 May 2021 11:43:31 +0100 Subject: [PATCH] KeyMint: improve HAL spec and tests - clarify & test BIGNUM spec - allow alternative return codes when requesting device unique attestation - use specific error for early boot import failure - test more early boot key scenarios (in post-early-boot mode) Test: VtsAidlKeyMintTargetTest Change-Id: I70a342084a29144aef1ed0ff80fec02cc06ffbc0 --- .../hardware/security/keymint/Tag.aidl | 2 +- .../hardware/security/keymint/TagType.aidl | 16 +++++++- .../aidl/vts/functional/AttestKeyTest.cpp | 6 ++- .../DeviceUniqueAttestationTest.cpp | 4 +- .../vts/functional/KeyMintAidlTestBase.cpp | 8 ++++ .../aidl/vts/functional/KeyMintTest.cpp | 41 +++++++++++++++++++ 6 files changed, 71 insertions(+), 6 deletions(-) diff --git a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl index 66f79ce041..4ff4574b58 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl @@ -234,7 +234,7 @@ enum Tag { * IKeyMintDevice::earlyBootEnded() is called. Early boot keys may be created after * early boot. Early boot keys may not be imported at all, if Tag::EARLY_BOOT_ONLY is * provided to IKeyMintDevice::importKey, the import must fail with - * ErrorCode::INVALID_ARGUMENT. + * ErrorCode::EARLY_BOOT_ENDED. */ EARLY_BOOT_ONLY = (7 << 28) /* TagType:BOOL */ | 305, diff --git a/security/keymint/aidl/android/hardware/security/keymint/TagType.aidl b/security/keymint/aidl/android/hardware/security/keymint/TagType.aidl index 1ba6ededf2..d46e504178 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/TagType.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/TagType.aidl @@ -39,7 +39,21 @@ enum TagType { DATE = 6 << 28, /** Boolean. If a tag with this type is present, the value is "true". If absent, "false". */ BOOL = 7 << 28, - /** Byte string containing an arbitrary-length integer, big-endian ordering. */ + /** + * Byte string containing an arbitrary-length integer, in a two's-complement big-endian + * ordering. The byte array contains the minimum number of bytes needed to represent the + * integer, including at least one sign bit (so zero encodes as the single byte 0x00. This + * matches the encoding of both java.math.BigInteger.toByteArray() and contents octets for an + * ASN.1 INTEGER value (X.690 section 8.3). Examples: + * - value 65536 encodes as 0x01 0x00 0x00 + * - value 65535 encodes as 0x00 0xFF 0xFF + * - value 255 encodes as 0x00 0xFF + * - value 1 encodes as 0x01 + * - value 0 encodes as 0x00 + * - value -1 encodes as 0xFF + * - value -255 encodes as 0xFF 0x01 + * - value -256 encodes as 0xFF 0x00 + */ BIGNUM = 8 << 28, /** Byte string */ BYTES = 9 << 28, diff --git a/security/keymint/aidl/vts/functional/AttestKeyTest.cpp b/security/keymint/aidl/vts/functional/AttestKeyTest.cpp index e4a877c0cb..b8699e9d50 100644 --- a/security/keymint/aidl/vts/functional/AttestKeyTest.cpp +++ b/security/keymint/aidl/vts/functional/AttestKeyTest.cpp @@ -180,7 +180,9 @@ TEST_P(AttestKeyTest, RsaAttestedAttestKeys) { auto subject = "cert subj 2"; vector subject_der(make_name_from_str(subject)); - uint64_t serial_int = 66; + // An X.509 certificate serial number SHOULD be >0, but this is not policed. Check + // that a zero value doesn't cause problems. + uint64_t serial_int = 0; vector serial_blob(build_serial_blob(serial_int)); /* @@ -223,7 +225,7 @@ TEST_P(AttestKeyTest, RsaAttestedAttestKeys) { auto subject2 = "cert subject"; vector subject_der2(make_name_from_str(subject2)); - uint64_t serial_int2 = 987; + uint64_t serial_int2 = 255; vector serial_blob2(build_serial_blob(serial_int2)); EXPECT_EQ(ErrorCode::OK, diff --git a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp index 6f0ee4e74d..b0f056a68c 100644 --- a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp +++ b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp @@ -75,7 +75,7 @@ TEST_P(DeviceUniqueAttestationTest, RsaNonStrongBoxUnimplemented) { .Authorization(TAG_DEVICE_UNIQUE_ATTESTATION), &key_blob, &key_characteristics); - ASSERT_EQ(result, ErrorCode::INVALID_ARGUMENT); + ASSERT_TRUE(result == ErrorCode::INVALID_ARGUMENT || result == ErrorCode::UNSUPPORTED_TAG); } /* @@ -101,7 +101,7 @@ TEST_P(DeviceUniqueAttestationTest, EcdsaNonStrongBoxUnimplemented) { .Authorization(TAG_DEVICE_UNIQUE_ATTESTATION), &key_blob, &key_characteristics); - ASSERT_EQ(result, ErrorCode::INVALID_ARGUMENT); + ASSERT_TRUE(result == ErrorCode::INVALID_ARGUMENT || result == ErrorCode::UNSUPPORTED_TAG); } /* diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp index 3a2717b9d9..0eac033cd1 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp @@ -1185,6 +1185,14 @@ vector build_serial_blob(const uint64_t serial_int) { return {}; } + if (serial_blob.empty() || serial_blob[0] & 0x80) { + // An empty blob is OpenSSL's encoding of the zero value; we need single zero byte. + // Top bit being set indicates a negative number in two's complement, but our input + // was positive. + // In either case, prepend a zero byte. + serial_blob.insert(serial_blob.begin(), 0x00); + } + return serial_blob; } diff --git a/security/keymint/aidl/vts/functional/KeyMintTest.cpp b/security/keymint/aidl/vts/functional/KeyMintTest.cpp index d2b1f4f34e..8b1eb30959 100644 --- a/security/keymint/aidl/vts/functional/KeyMintTest.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintTest.cpp @@ -6320,7 +6320,13 @@ INSTANTIATE_KEYMINT_AIDL_TEST(DestroyAttestationIdsTest); using EarlyBootKeyTest = KeyMintAidlTestBase; +/* + * EarlyBootKeyTest.CreateEarlyBootKeys + * + * Verifies that creating early boot keys succeeds, even at a later stage (after boot). + */ TEST_P(EarlyBootKeyTest, CreateEarlyBootKeys) { + // Early boot keys can be created after early boot. auto [aesKeyData, hmacKeyData, rsaKeyData, ecdsaKeyData] = CreateTestKeys(TAG_EARLY_BOOT_ONLY, ErrorCode::OK); @@ -6330,6 +6336,41 @@ TEST_P(EarlyBootKeyTest, CreateEarlyBootKeys) { CheckedDeleteKey(&ecdsaKeyData.blob); } +/* + * EarlyBootKeyTest.UsetEarlyBootKeyFailure + * + * Verifies that using early boot keys at a later stage fails. + */ +TEST_P(EarlyBootKeyTest, UseEarlyBootKeyFailure) { + ASSERT_EQ(ErrorCode::OK, GenerateKey(AuthorizationSetBuilder() + .Authorization(TAG_NO_AUTH_REQUIRED) + .Authorization(TAG_EARLY_BOOT_ONLY) + .HmacKey(128) + .Digest(Digest::SHA_2_256) + .Authorization(TAG_MIN_MAC_LENGTH, 256))); + AuthorizationSet output_params; + EXPECT_EQ(ErrorCode::EARLY_BOOT_ENDED, Begin(KeyPurpose::SIGN, key_blob_, + AuthorizationSetBuilder() + .Digest(Digest::SHA_2_256) + .Authorization(TAG_MAC_LENGTH, 256), + &output_params)); +} + +/* + * EarlyBootKeyTest.ImportEarlyBootKeyFailure + * + * Verifies that importing early boot keys fails. + */ +TEST_P(EarlyBootKeyTest, ImportEarlyBootKeyFailure) { + ASSERT_EQ(ErrorCode::EARLY_BOOT_ENDED, ImportKey(AuthorizationSetBuilder() + .Authorization(TAG_NO_AUTH_REQUIRED) + .Authorization(TAG_EARLY_BOOT_ONLY) + .EcdsaSigningKey(256) + .Digest(Digest::SHA_2_256) + .SetDefaultValidity(), + KeyFormat::PKCS8, ec_256_key)); +} + // This is a more comprehensive test, but it can only be run on a machine which is still in early // boot stage, which no proper Android device is by the time we can run VTS. To use this, // un-disable it and modify vold to remove the call to earlyBootEnded(). Running the test will end