From 4dc010739d287542518ba65701e5f2ac7a400a2e Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Thu, 1 Apr 2021 12:17:35 +0100 Subject: [PATCH] Check that KeyMint provides IRemotelyProvisionedComponent Move helper utilities across into KeyMintAidlTestBase to allow re-use. Test: VtsHalRemotelyProvisionedComponentTargetTest, VtsAidlKeyMintTargetTest Change-Id: Ib9e55a7d72fd197016ae1a1f073dadedafa09c25 --- .../keymint/aidl/vts/functional/Android.bp | 5 + .../vts/functional/KeyMintAidlTestBase.cpp | 121 ++++++++++++++++++ .../aidl/vts/functional/KeyMintAidlTestBase.h | 5 + .../aidl/vts/functional/KeyMintTest.cpp | 100 +++++++++++++++ .../VtsRemotelyProvisionedComponentTests.cpp | 111 ---------------- 5 files changed, 231 insertions(+), 111 deletions(-) diff --git a/security/keymint/aidl/vts/functional/Android.bp b/security/keymint/aidl/vts/functional/Android.bp index c1affa6f07..6c7309eb7a 100644 --- a/security/keymint/aidl/vts/functional/Android.bp +++ b/security/keymint/aidl/vts/functional/Android.bp @@ -43,6 +43,8 @@ cc_test { "android.hardware.security.keymint-V1-ndk_platform", "android.hardware.security.secureclock-V1-ndk_platform", "libcppbor_external", + "libcppcose", + "libkeymint_remote_prov_support", "libkeymint_vts_test_utils", ], test_suites: [ @@ -73,6 +75,9 @@ cc_test_library { "android.hardware.security.keymint-V1-ndk_platform", "android.hardware.security.secureclock-V1-ndk_platform", "libcppbor_external", + "libcppcose", + "libgmock_ndk", + "libkeymint_remote_prov_support", ], } diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp index ce6f67a84a..ec2194454f 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp @@ -22,8 +22,12 @@ #include #include +#include +#include #include +#include #include +#include #include #include @@ -32,6 +36,7 @@ namespace aidl::android::hardware::security::keymint { +using namespace cppcose; using namespace std::literals::chrono_literals; using std::endl; using std::optional; @@ -39,6 +44,7 @@ using std::unique_ptr; using ::testing::AssertionFailure; using ::testing::AssertionResult; using ::testing::AssertionSuccess; +using ::testing::MatchesRegex; ::std::ostream& operator<<(::std::ostream& os, const AuthorizationSet& set) { if (set.size() == 0) @@ -1118,6 +1124,121 @@ vector make_name_from_str(const string& name) { return retval; } +namespace { + +void check_cose_key(const vector& data, bool testMode) { + auto [parsedPayload, __, payloadParseErr] = cppbor::parse(data); + ASSERT_TRUE(parsedPayload) << "Key parse failed: " << payloadParseErr; + + // The following check assumes that canonical CBOR encoding is used for the COSE_Key. + if (testMode) { + EXPECT_THAT(cppbor::prettyPrint(parsedPayload.get()), + MatchesRegex("{\n" + " 1 : 2,\n" // kty: EC2 + " 3 : -7,\n" // alg: ES256 + " -1 : 1,\n" // EC id: P256 + // The regex {(0x[0-9a-f]{2}, ){31}0x[0-9a-f]{2}} matches a + // sequence of 32 hexadecimal bytes, enclosed in braces and + // separated by commas. In this case, some Ed25519 public key. + " -2 : {(0x[0-9a-f]{2}, ){31}0x[0-9a-f]{2}},\n" // pub_x: data + " -3 : {(0x[0-9a-f]{2}, ){31}0x[0-9a-f]{2}},\n" // pub_y: data + " -70000 : null,\n" // test marker + "}")); + } else { + EXPECT_THAT(cppbor::prettyPrint(parsedPayload.get()), + MatchesRegex("{\n" + " 1 : 2,\n" // kty: EC2 + " 3 : -7,\n" // alg: ES256 + " -1 : 1,\n" // EC id: P256 + // The regex {(0x[0-9a-f]{2}, ){31}0x[0-9a-f]{2}} matches a + // sequence of 32 hexadecimal bytes, enclosed in braces and + // separated by commas. In this case, some Ed25519 public key. + " -2 : {(0x[0-9a-f]{2}, ){31}0x[0-9a-f]{2}},\n" // pub_x: data + " -3 : {(0x[0-9a-f]{2}, ){31}0x[0-9a-f]{2}},\n" // pub_y: data + "}")); + } +} + +} // namespace + +void check_maced_pubkey(const MacedPublicKey& macedPubKey, bool testMode, + vector* payload_value) { + auto [coseMac0, _, mac0ParseErr] = cppbor::parse(macedPubKey.macedKey); + ASSERT_TRUE(coseMac0) << "COSE Mac0 parse failed " << mac0ParseErr; + + ASSERT_NE(coseMac0->asArray(), nullptr); + ASSERT_EQ(coseMac0->asArray()->size(), kCoseMac0EntryCount); + + auto protParms = coseMac0->asArray()->get(kCoseMac0ProtectedParams)->asBstr(); + ASSERT_NE(protParms, nullptr); + + // Header label:value of 'alg': HMAC-256 + ASSERT_EQ(cppbor::prettyPrint(protParms->value()), "{\n 1 : 5,\n}"); + + auto unprotParms = coseMac0->asArray()->get(kCoseMac0UnprotectedParams)->asMap(); + ASSERT_NE(unprotParms, nullptr); + ASSERT_EQ(unprotParms->size(), 0); + + // The payload is a bstr holding an encoded COSE_Key + auto payload = coseMac0->asArray()->get(kCoseMac0Payload)->asBstr(); + ASSERT_NE(payload, nullptr); + check_cose_key(payload->value(), testMode); + + auto coseMac0Tag = coseMac0->asArray()->get(kCoseMac0Tag)->asBstr(); + ASSERT_TRUE(coseMac0Tag); + auto extractedTag = coseMac0Tag->value(); + EXPECT_EQ(extractedTag.size(), 32U); + + // Compare with tag generated with kTestMacKey. Should only match in test mode + auto testTag = cppcose::generateCoseMac0Mac(remote_prov::kTestMacKey, {} /* external_aad */, + payload->value()); + ASSERT_TRUE(testTag) << "Tag calculation failed: " << testTag.message(); + + if (testMode) { + EXPECT_EQ(*testTag, extractedTag); + } else { + EXPECT_NE(*testTag, extractedTag); + } + if (payload_value != nullptr) { + *payload_value = payload->value(); + } +} + +void p256_pub_key(const vector& coseKeyData, EVP_PKEY_Ptr* signingKey) { + // Extract x and y affine coordinates from the encoded Cose_Key. + auto [parsedPayload, __, payloadParseErr] = cppbor::parse(coseKeyData); + ASSERT_TRUE(parsedPayload) << "Key parse failed: " << payloadParseErr; + auto coseKey = parsedPayload->asMap(); + const std::unique_ptr& xItem = coseKey->get(cppcose::CoseKey::PUBKEY_X); + ASSERT_NE(xItem->asBstr(), nullptr); + vector x = xItem->asBstr()->value(); + const std::unique_ptr& yItem = coseKey->get(cppcose::CoseKey::PUBKEY_Y); + ASSERT_NE(yItem->asBstr(), nullptr); + vector y = yItem->asBstr()->value(); + + // Concatenate: 0x04 (uncompressed form marker) | x | y + vector pubKeyData{0x04}; + pubKeyData.insert(pubKeyData.end(), x.begin(), x.end()); + pubKeyData.insert(pubKeyData.end(), y.begin(), y.end()); + + EC_KEY_Ptr ecKey = EC_KEY_Ptr(EC_KEY_new()); + ASSERT_NE(ecKey, nullptr); + EC_GROUP_Ptr group = EC_GROUP_Ptr(EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1)); + ASSERT_NE(group, nullptr); + ASSERT_EQ(EC_KEY_set_group(ecKey.get(), group.get()), 1); + EC_POINT_Ptr point = EC_POINT_Ptr(EC_POINT_new(group.get())); + ASSERT_NE(point, nullptr); + ASSERT_EQ(EC_POINT_oct2point(group.get(), point.get(), pubKeyData.data(), pubKeyData.size(), + nullptr), + 1); + ASSERT_EQ(EC_KEY_set_public_key(ecKey.get(), point.get()), 1); + + EVP_PKEY_Ptr pubKey = EVP_PKEY_Ptr(EVP_PKEY_new()); + ASSERT_NE(pubKey, nullptr); + EVP_PKEY_assign_EC_KEY(pubKey.get(), ecKey.release()); + *signingKey = std::move(pubKey); +} + } // namespace test } // namespace aidl::android::hardware::security::keymint diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h index 4d97ea9de3..da174ce7b1 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h @@ -25,6 +25,7 @@ #include #include +#include #include #include @@ -277,6 +278,10 @@ bool verify_attestation_record(const string& challenge, // string bin2hex(const vector& data); X509_Ptr parse_cert_blob(const vector& blob); vector make_name_from_str(const string& name); +void check_maced_pubkey(const MacedPublicKey& macedPubKey, bool testMode, + vector* payload_value); +void p256_pub_key(const vector& coseKeyData, EVP_PKEY_Ptr* signingKey); + AuthorizationSet HwEnforcedAuthorizations(const vector& key_characteristics); AuthorizationSet SwEnforcedAuthorizations(const vector& key_characteristics); ::testing::AssertionResult ChainSignaturesAreValid(const vector& chain); diff --git a/security/keymint/aidl/vts/functional/KeyMintTest.cpp b/security/keymint/aidl/vts/functional/KeyMintTest.cpp index 7ecfa3723f..d5308dc9a4 100644 --- a/security/keymint/aidl/vts/functional/KeyMintTest.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintTest.cpp @@ -27,6 +27,9 @@ #include +#include + +#include #include #include @@ -209,6 +212,32 @@ class AidlBuf : public vector { string to_string() const { return string(reinterpret_cast(data()), size()); } }; +string device_suffix(const string& name) { + size_t pos = name.find('/'); + if (pos == string::npos) { + return name; + } + return name.substr(pos + 1); +} + +bool matching_rp_instance(const string& km_name, + std::shared_ptr* rp) { + string km_suffix = device_suffix(km_name); + + vector rp_names = + ::android::getAidlHalInstanceNames(IRemotelyProvisionedComponent::descriptor); + for (const string& rp_name : rp_names) { + // If the suffix of the RemotelyProvisionedComponent instance equals the suffix of the + // KeyMint instance, assume they match. + if (device_suffix(rp_name) == km_suffix && AServiceManager_isDeclared(rp_name.c_str())) { + ::ndk::SpAIBinder binder(AServiceManager_waitForService(rp_name.c_str())); + *rp = IRemotelyProvisionedComponent::fromBinder(binder); + return true; + } + } + return false; +} + } // namespace class NewKeyGenerationTest : public KeyMintAidlTestBase { @@ -321,6 +350,77 @@ TEST_P(NewKeyGenerationTest, RsaWithAttestation) { } } +/* + * NewKeyGenerationTest.RsaWithRpkAttestation + * + * Verifies that keymint can generate all required RSA key sizes, using an attestation key + * that has been generated using an associate IRemotelyProvisionedComponent. + */ +TEST_P(NewKeyGenerationTest, RsaWithRpkAttestation) { + // There should be an IRemotelyProvisionedComponent instance associated with the KeyMint + // instance. + std::shared_ptr rp; + ASSERT_TRUE(matching_rp_instance(GetParam(), &rp)) + << "No IRemotelyProvisionedComponent found that matches KeyMint device " << GetParam(); + + // Generate a P-256 keypair to use as an attestation key. + MacedPublicKey macedPubKey; + std::vector privateKeyBlob; + auto status = + rp->generateEcdsaP256KeyPair(/* testMode= */ false, &macedPubKey, &privateKeyBlob); + ASSERT_TRUE(status.isOk()); + vector coseKeyData; + check_maced_pubkey(macedPubKey, /* testMode= */ false, &coseKeyData); + + AttestationKey attestation_key; + attestation_key.keyBlob = std::move(privateKeyBlob); + attestation_key.issuerSubjectName = make_name_from_str("Android Keystore Key"); + + for (auto key_size : ValidKeySizes(Algorithm::RSA)) { + auto challenge = "hello"; + auto app_id = "foo"; + + vector key_blob; + vector key_characteristics; + ASSERT_EQ(ErrorCode::OK, + GenerateKey(AuthorizationSetBuilder() + .RsaSigningKey(key_size, 65537) + .Digest(Digest::NONE) + .Padding(PaddingMode::NONE) + .AttestationChallenge(challenge) + .AttestationApplicationId(app_id) + .Authorization(TAG_NO_AUTH_REQUIRED) + .SetDefaultValidity(), + attestation_key, &key_blob, &key_characteristics, &cert_chain_)); + + ASSERT_GT(key_blob.size(), 0U); + CheckBaseParams(key_characteristics); + + AuthorizationSet crypto_params = SecLevelAuthorizations(key_characteristics); + + EXPECT_TRUE(crypto_params.Contains(TAG_ALGORITHM, Algorithm::RSA)); + EXPECT_TRUE(crypto_params.Contains(TAG_KEY_SIZE, key_size)) + << "Key size " << key_size << "missing"; + EXPECT_TRUE(crypto_params.Contains(TAG_RSA_PUBLIC_EXPONENT, 65537U)); + + // Attestation by itself is not valid (last entry is not self-signed). + EXPECT_FALSE(ChainSignaturesAreValid(cert_chain_)); + + // The signature over the attested key should correspond to the P256 public key. + X509_Ptr key_cert(parse_cert_blob(cert_chain_[0].encodedCertificate)); + ASSERT_TRUE(key_cert.get()); + EVP_PKEY_Ptr signing_pubkey; + p256_pub_key(coseKeyData, &signing_pubkey); + ASSERT_TRUE(signing_pubkey.get()); + + ASSERT_TRUE(X509_verify(key_cert.get(), signing_pubkey.get())) + << "Verification of attested certificate failed " + << "OpenSSL error string: " << ERR_error_string(ERR_get_error(), NULL); + + CheckedDeleteKey(&key_blob); + } +} + /* * NewKeyGenerationTest.LimitedUsageRsa * diff --git a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp index 57bc27a3f6..9e52b20675 100644 --- a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp +++ b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp @@ -55,117 +55,6 @@ bytevec string_to_bytevec(const char* s) { return bytevec(p, p + strlen(s)); } -void p256_pub_key(const vector& coseKeyData, EVP_PKEY_Ptr* signingKey) { - // Extract x and y affine coordinates from the encoded Cose_Key. - auto [parsedPayload, __, payloadParseErr] = cppbor::parse(coseKeyData); - ASSERT_TRUE(parsedPayload) << "Key parse failed: " << payloadParseErr; - auto coseKey = parsedPayload->asMap(); - const std::unique_ptr& xItem = coseKey->get(cppcose::CoseKey::PUBKEY_X); - ASSERT_NE(xItem->asBstr(), nullptr); - vector x = xItem->asBstr()->value(); - const std::unique_ptr& yItem = coseKey->get(cppcose::CoseKey::PUBKEY_Y); - ASSERT_NE(yItem->asBstr(), nullptr); - vector y = yItem->asBstr()->value(); - - // Concatenate: 0x04 (uncompressed form marker) | x | y - vector pubKeyData{0x04}; - pubKeyData.insert(pubKeyData.end(), x.begin(), x.end()); - pubKeyData.insert(pubKeyData.end(), y.begin(), y.end()); - - EC_KEY_Ptr ecKey = EC_KEY_Ptr(EC_KEY_new()); - ASSERT_NE(ecKey, nullptr); - EC_GROUP_Ptr group = EC_GROUP_Ptr(EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1)); - ASSERT_NE(group, nullptr); - ASSERT_EQ(EC_KEY_set_group(ecKey.get(), group.get()), 1); - EC_POINT_Ptr point = EC_POINT_Ptr(EC_POINT_new(group.get())); - ASSERT_NE(point, nullptr); - ASSERT_EQ(EC_POINT_oct2point(group.get(), point.get(), pubKeyData.data(), pubKeyData.size(), - nullptr), - 1); - ASSERT_EQ(EC_KEY_set_public_key(ecKey.get(), point.get()), 1); - - EVP_PKEY_Ptr pubKey = EVP_PKEY_Ptr(EVP_PKEY_new()); - ASSERT_NE(pubKey, nullptr); - EVP_PKEY_assign_EC_KEY(pubKey.get(), ecKey.release()); - *signingKey = std::move(pubKey); -} - -void check_cose_key(const vector& data, bool testMode) { - auto [parsedPayload, __, payloadParseErr] = cppbor::parse(data); - ASSERT_TRUE(parsedPayload) << "Key parse failed: " << payloadParseErr; - - // The following check assumes that canonical CBOR encoding is used for the COSE_Key. - if (testMode) { - EXPECT_THAT(cppbor::prettyPrint(parsedPayload.get()), - MatchesRegex("{\n" - " 1 : 2,\n" // kty: EC2 - " 3 : -7,\n" // alg: ES256 - " -1 : 1,\n" // EC id: P256 - // The regex {(0x[0-9a-f]{2}, ){31}0x[0-9a-f]{2}} matches a - // sequence of 32 hexadecimal bytes, enclosed in braces and - // separated by commas. In this case, some Ed25519 public key. - " -2 : {(0x[0-9a-f]{2}, ){31}0x[0-9a-f]{2}},\n" // pub_x: data - " -3 : {(0x[0-9a-f]{2}, ){31}0x[0-9a-f]{2}},\n" // pub_y: data - " -70000 : null,\n" // test marker - "}")); - } else { - EXPECT_THAT(cppbor::prettyPrint(parsedPayload.get()), - MatchesRegex("{\n" - " 1 : 2,\n" // kty: EC2 - " 3 : -7,\n" // alg: ES256 - " -1 : 1,\n" // EC id: P256 - // The regex {(0x[0-9a-f]{2}, ){31}0x[0-9a-f]{2}} matches a - // sequence of 32 hexadecimal bytes, enclosed in braces and - // separated by commas. In this case, some Ed25519 public key. - " -2 : {(0x[0-9a-f]{2}, ){31}0x[0-9a-f]{2}},\n" // pub_x: data - " -3 : {(0x[0-9a-f]{2}, ){31}0x[0-9a-f]{2}},\n" // pub_y: data - "}")); - } -} - -void check_maced_pubkey(const MacedPublicKey& macedPubKey, bool testMode, - vector* payload_value) { - auto [coseMac0, _, mac0ParseErr] = cppbor::parse(macedPubKey.macedKey); - ASSERT_TRUE(coseMac0) << "COSE Mac0 parse failed " << mac0ParseErr; - - ASSERT_NE(coseMac0->asArray(), nullptr); - ASSERT_EQ(coseMac0->asArray()->size(), kCoseMac0EntryCount); - - auto protParms = coseMac0->asArray()->get(kCoseMac0ProtectedParams)->asBstr(); - ASSERT_NE(protParms, nullptr); - - // Header label:value of 'alg': HMAC-256 - ASSERT_EQ(cppbor::prettyPrint(protParms->value()), "{\n 1 : 5,\n}"); - - auto unprotParms = coseMac0->asArray()->get(kCoseMac0UnprotectedParams)->asMap(); - ASSERT_NE(unprotParms, nullptr); - ASSERT_EQ(unprotParms->size(), 0); - - // The payload is a bstr holding an encoded COSE_Key - auto payload = coseMac0->asArray()->get(kCoseMac0Payload)->asBstr(); - ASSERT_NE(payload, nullptr); - check_cose_key(payload->value(), testMode); - - auto coseMac0Tag = coseMac0->asArray()->get(kCoseMac0Tag)->asBstr(); - ASSERT_TRUE(coseMac0Tag); - auto extractedTag = coseMac0Tag->value(); - EXPECT_EQ(extractedTag.size(), 32U); - - // Compare with tag generated with kTestMacKey. Should only match in test mode - auto testTag = cppcose::generateCoseMac0Mac(remote_prov::kTestMacKey, {} /* external_aad */, - payload->value()); - ASSERT_TRUE(testTag) << "Tag calculation failed: " << testTag.message(); - - if (testMode) { - EXPECT_EQ(*testTag, extractedTag); - } else { - EXPECT_NE(*testTag, extractedTag); - } - if (payload_value != nullptr) { - *payload_value = payload->value(); - } -} - ErrMsgOr corrupt_maced_key(const MacedPublicKey& macedPubKey) { auto [coseMac0, _, mac0ParseErr] = cppbor::parse(macedPubKey.macedKey); if (!coseMac0 || coseMac0->asArray()->size() != kCoseMac0EntryCount) {