From 8f810b130317fa5e6526a307236ad93f156ba297 Mon Sep 17 00:00:00 2001 From: Seth Moore Date: Mon, 12 Dec 2022 16:51:01 -0800 Subject: [PATCH 1/3] Move remotely provisioned component tests to rkp directory Now that the RKP HAL AIDL has been moved to it's own directory, we should keep the tests with the AIDL. Test: atest VtsAidlKeyMintTargetTest Test: atest VtsHalRemotelyProvisionedComponentTargetTest Change-Id: Ia87d3ea0a1b9e6704f0dea8f98b0bbaa049472fe --- .../keymint/aidl/vts/functional/Android.bp | 21 --------- security/rkp/TEST_MAPPING | 7 +++ security/rkp/aidl/vts/functional/Android.bp | 44 +++++++++++++++++++ .../aidl/vts/functional/AndroidTest.xml} | 0 .../VtsRemotelyProvisionedComponentTests.cpp | 2 +- .../VtsRemotelyProvisionedComponentTests.xml | 34 ++++++++++++++ 6 files changed, 86 insertions(+), 22 deletions(-) create mode 100644 security/rkp/TEST_MAPPING create mode 100644 security/rkp/aidl/vts/functional/Android.bp rename security/{keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.xml => rkp/aidl/vts/functional/AndroidTest.xml} (100%) rename security/{keymint => rkp}/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp (99%) create mode 100644 security/rkp/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.xml diff --git a/security/keymint/aidl/vts/functional/Android.bp b/security/keymint/aidl/vts/functional/Android.bp index 13143bf620..e7f5a0fbd8 100644 --- a/security/keymint/aidl/vts/functional/Android.bp +++ b/security/keymint/aidl/vts/functional/Android.bp @@ -90,24 +90,3 @@ cc_test_library { "libgmock_ndk", ], } - -cc_test { - name: "VtsHalRemotelyProvisionedComponentTargetTest", - defaults: [ - "keymint_vts_defaults", - ], - srcs: [ - "VtsRemotelyProvisionedComponentTests.cpp", - ], - static_libs: [ - "libgmock_ndk", - "libkeymaster_portable", - "libkeymint_vts_test_utils", - "libpuresoftkeymasterdevice", - ], - test_config: "VtsRemotelyProvisionedComponentTests.xml", - test_suites: [ - "general-tests", - "vts", - ], -} diff --git a/security/rkp/TEST_MAPPING b/security/rkp/TEST_MAPPING new file mode 100644 index 0000000000..9ce5e9bced --- /dev/null +++ b/security/rkp/TEST_MAPPING @@ -0,0 +1,7 @@ +{ + "presubmit": [ + { + "name": "VtsHalRemotelyProvisionedComponentTargetTest" + } + ] +} diff --git a/security/rkp/aidl/vts/functional/Android.bp b/security/rkp/aidl/vts/functional/Android.bp new file mode 100644 index 0000000000..9c2b6e17e9 --- /dev/null +++ b/security/rkp/aidl/vts/functional/Android.bp @@ -0,0 +1,44 @@ +// +// Copyright (C) 2020 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package { + // See: http://go/android-license-faq + default_applicable_licenses: ["hardware_interfaces_license"], +} + +cc_test { + name: "VtsHalRemotelyProvisionedComponentTargetTest", + defaults: [ + "keymint_vts_defaults", + ], + srcs: [ + "VtsRemotelyProvisionedComponentTests.cpp", + ], + shared_libs: [ + "libbinder_ndk", + "libcrypto", + ], + static_libs: [ + "libcppbor_external", + "libgmock_ndk", + "libkeymint_vts_test_utils", + ], + test_config: "VtsRemotelyProvisionedComponentTests.xml", + test_suites: [ + "general-tests", + "vts", + ], +} diff --git a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.xml b/security/rkp/aidl/vts/functional/AndroidTest.xml similarity index 100% rename from security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.xml rename to security/rkp/aidl/vts/functional/AndroidTest.xml diff --git a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp b/security/rkp/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp similarity index 99% rename from security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp rename to security/rkp/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp index 6d9c8c9aee..cb1c692df1 100644 --- a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp +++ b/security/rkp/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp @@ -18,7 +18,7 @@ #include #define LOG_TAG "VtsRemotelyProvisionableComponentTests" -#include +#include #include #include #include diff --git a/security/rkp/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.xml b/security/rkp/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.xml new file mode 100644 index 0000000000..2375bde0ff --- /dev/null +++ b/security/rkp/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.xml @@ -0,0 +1,34 @@ + + + + From 7dc1fda7a402923c3560fa5736e0fd92ed128d95 Mon Sep 17 00:00:00 2001 From: Seth Moore Date: Mon, 12 Dec 2022 16:56:20 -0800 Subject: [PATCH 2/3] Enable RKP+KeyMint integration test This integration was technically a requirement on keymint v2, but we weren't enforcing it with a test. So realistically we are only able to start enforcing the test with keymint v3. Test: atest VtsAidlKeyMintTargetTest Change-Id: Ia4feb8ce4b7fd1e47a5c6c9b06ddb12276a9c5ee --- .../aidl/vts/functional/KeyMintTest.cpp | 91 +++++++++++++++++-- 1 file changed, 83 insertions(+), 8 deletions(-) diff --git a/security/keymint/aidl/vts/functional/KeyMintTest.cpp b/security/keymint/aidl/vts/functional/KeyMintTest.cpp index b8d0c200fc..5a86283b1f 100644 --- a/security/keymint/aidl/vts/functional/KeyMintTest.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintTest.cpp @@ -1027,7 +1027,7 @@ TEST_P(NewKeyGenerationTest, Rsa) { * without providing NOT_BEFORE and NOT_AFTER parameters. */ TEST_P(NewKeyGenerationTest, RsaWithMissingValidity) { - if (AidlVersion() < 2) { + if (AidlVersion() < 3) { /* * The KeyMint V1 spec required that CERTIFICATE_NOT_{BEFORE,AFTER} be * specified for asymmetric key generation. However, this was not @@ -1130,16 +1130,16 @@ TEST_P(NewKeyGenerationTest, RsaWithAttestation) { } /* - * NewKeyGenerationTest.RsaWithRpkAttestation + * NewKeyGenerationTest.RsaWithRkpAttestation * - * Verifies that keymint can generate all required RSA key sizes, using an attestation key + * Verifies that keymint can generate all required RSA key sizes using an attestation key * that has been generated using an associate IRemotelyProvisionedComponent. - * - * This test is disabled because the KeyMint specification does not require that implementations - * of the first version of KeyMint have to also implement IRemotelyProvisionedComponent. - * However, the test is kept in the code because KeyMint v2 will impose this requirement. */ -TEST_P(NewKeyGenerationTest, DISABLED_RsaWithRpkAttestation) { +TEST_P(NewKeyGenerationTest, RsaWithRkpAttestation) { + if (AidlVersion() < 2) { + GTEST_SKIP() << "Only required starting with KeyMint v2"; + } + // There should be an IRemotelyProvisionedComponent instance associated with the KeyMint // instance. std::shared_ptr rp; @@ -1207,6 +1207,81 @@ TEST_P(NewKeyGenerationTest, DISABLED_RsaWithRpkAttestation) { } } +/* + * NewKeyGenerationTest.EcdsaWithRkpAttestation + * + * Verifies that keymint can generate all required ECDSA key sizes using an attestation key + * that has been generated using an associate IRemotelyProvisionedComponent. + */ +TEST_P(NewKeyGenerationTest, EcdsaWithRkpAttestation) { + if (AidlVersion() < 2) { + GTEST_SKIP() << "Only required starting with KeyMint v2"; + } + + // 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 curve : ValidCurves()) { + SCOPED_TRACE(testing::Message() << "Curve::" << curve); + auto challenge = "hello"; + auto app_id = "foo"; + + vector key_blob; + vector key_characteristics; + ASSERT_EQ(ErrorCode::OK, + GenerateKey(AuthorizationSetBuilder() + .EcdsaSigningKey(curve) + .Digest(Digest::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); + CheckCharacteristics(key_blob, key_characteristics); + + AuthorizationSet crypto_params = SecLevelAuthorizations(key_characteristics); + + EXPECT_TRUE(crypto_params.Contains(TAG_ALGORITHM, Algorithm::EC)); + EXPECT_TRUE(crypto_params.Contains(TAG_EC_CURVE, curve)) << "Curve " << curve << "missing"; + + // 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. + ASSERT_GT(cert_chain_.size(), 0); + 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.RsaEncryptionWithAttestation * From 34d856e8f70c6cec5e353ac38453a3367b5cfd3f Mon Sep 17 00:00:00 2001 From: Seth Moore Date: Mon, 12 Dec 2022 17:05:59 -0800 Subject: [PATCH 3/3] Add RKP-specific owners file Test: N/A Change-Id: I844f3e88b2b89337d843f63e63c9dcbcbb921b77 --- security/rkp/OWNERS | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 security/rkp/OWNERS diff --git a/security/rkp/OWNERS b/security/rkp/OWNERS new file mode 100644 index 0000000000..d25977f8e0 --- /dev/null +++ b/security/rkp/OWNERS @@ -0,0 +1,5 @@ +# Bug component: 1084908 + +jbires@google.com +sethmo@google.com +trong@google.com