From 19acbe9f66d64a7b949c07bfb8279893d413baae Mon Sep 17 00:00:00 2001 From: Seth Moore Date: Wed, 23 Jun 2021 15:15:52 -0700 Subject: [PATCH] Update KeyMint VTS tests with prod GEEK Now that we have the production Google Endpoint Encryption Key, we can update the tests to use the correct GEEK cert chain where applicable. Test: VtsHalRemotelyProvisionedComponentTargetTest Test: VtsAidlKeyMintTargetTest Bug: 191301285 Change-Id: I84b557c6bad34741ffe6671fc941d9e266b73241 Merged-In: I84b557c6bad34741ffe6671fc941d9e266b73241 --- .../VtsRemotelyProvisionedComponentTests.cpp | 149 ++++++++---------- 1 file changed, 62 insertions(+), 87 deletions(-) diff --git a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp index 32765ad460..af951e8861 100644 --- a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp +++ b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp @@ -103,8 +103,8 @@ ErrMsgOr corrupt_sig(const cppbor::Array* coseSign1) { return std::move(corruptSig); } -ErrMsgOr corrupt_sig_chain(const EekChain& eek, int which) { - auto [chain, _, parseErr] = cppbor::parse(eek.chain); +ErrMsgOr corrupt_sig_chain(const bytevec& encodedEekChain, int which) { + auto [chain, _, parseErr] = cppbor::parse(encodedEekChain); if (!chain || !chain->asArray()) { return "EekChain parse failed"; } @@ -126,7 +126,7 @@ ErrMsgOr corrupt_sig_chain(const EekChain& eek, int which) { corruptChain.add(eekChain->get(ii)->clone()); } } - return EekChain{corruptChain.encode(), eek.last_pubkey, eek.last_privkey}; + return corruptChain.encode(); } string device_suffix(const string& name) { @@ -272,14 +272,14 @@ TEST_P(GenerateKeyTests, generateEcdsaP256Key_testMode) { class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests { protected: CertificateRequestTest() : eekId_(string_to_bytevec("eekid")), challenge_(randomBytes(32)) { - generateEek(3); + generateTestEekChain(3); } - void generateEek(size_t eekLength) { + void generateTestEekChain(size_t eekLength) { auto chain = generateEekChain(eekLength, eekId_); EXPECT_TRUE(chain) << chain.message(); - if (chain) eekChain_ = chain.moveValue(); - eekLength_ = eekLength; + if (chain) testEekChain_ = chain.moveValue(); + testEekLength_ = eekLength; } void generateKeys(bool testMode, size_t numKeys) { @@ -309,8 +309,9 @@ class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests { ASSERT_TRUE(senderPubkey) << senderPubkey.message(); EXPECT_EQ(senderPubkey->second, eekId_); - auto sessionKey = x25519_HKDF_DeriveKey(eekChain_.last_pubkey, eekChain_.last_privkey, - senderPubkey->first, false /* senderIsA */); + auto sessionKey = + x25519_HKDF_DeriveKey(testEekChain_.last_pubkey, testEekChain_.last_privkey, + senderPubkey->first, false /* senderIsA */); ASSERT_TRUE(sessionKey) << sessionKey.message(); auto protectedDataPayload = @@ -363,8 +364,8 @@ class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests { } bytevec eekId_; - size_t eekLength_; - EekChain eekChain_; + size_t testEekLength_; + EekChain testEekChain_; bytevec challenge_; std::vector keysToSign_; cppbor::Array cborKeysToSign_; @@ -378,13 +379,13 @@ TEST_P(CertificateRequestTest, EmptyRequest_testMode) { bool testMode = true; for (size_t eekLength : {2, 3, 7}) { SCOPED_TRACE(testing::Message() << "EEK of length " << eekLength); - generateEek(eekLength); + generateTestEekChain(eekLength); bytevec keysToSignMac; DeviceInfo deviceInfo; ProtectedData protectedData; auto status = provisionable_->generateCertificateRequest( - testMode, {} /* keysToSign */, eekChain_.chain, challenge_, &deviceInfo, + testMode, {} /* keysToSign */, testEekChain_.chain, challenge_, &deviceInfo, &protectedData, &keysToSignMac); ASSERT_TRUE(status.isOk()) << status.getMessage(); @@ -400,25 +401,22 @@ TEST_P(CertificateRequestTest, EmptyRequest_testMode) { */ TEST_P(CertificateRequestTest, NewKeyPerCallInTestMode) { constexpr bool testMode = true; - constexpr size_t eekLength = 2; - - generateEek(eekLength); bytevec keysToSignMac; DeviceInfo deviceInfo; ProtectedData protectedData; auto status = provisionable_->generateCertificateRequest( - testMode, {} /* keysToSign */, eekChain_.chain, challenge_, &deviceInfo, &protectedData, - &keysToSignMac); + testMode, {} /* keysToSign */, testEekChain_.chain, challenge_, &deviceInfo, + &protectedData, &keysToSignMac); ASSERT_TRUE(status.isOk()) << status.getMessage(); std::vector firstBcc; checkProtectedData(deviceInfo, /*keysToSign=*/cppbor::Array(), keysToSignMac, protectedData, &firstBcc); - status = provisionable_->generateCertificateRequest(testMode, {} /* keysToSign */, - eekChain_.chain, challenge_, &deviceInfo, - &protectedData, &keysToSignMac); + status = provisionable_->generateCertificateRequest( + testMode, {} /* keysToSign */, testEekChain_.chain, challenge_, &deviceInfo, + &protectedData, &keysToSignMac); ASSERT_TRUE(status.isOk()) << status.getMessage(); std::vector secondBcc; @@ -435,28 +433,20 @@ TEST_P(CertificateRequestTest, NewKeyPerCallInTestMode) { } /** - * Generate an empty certificate request in prod mode. Generation will fail because we don't have a - * valid GEEK. - * - * TODO(swillden): Get a valid GEEK and use it so the generation can succeed, though we won't be - * able to decrypt. + * Generate an empty certificate request in prod mode. This test must be run explicitly, and + * is not run by default. Not all devices are GMS devices, and therefore they do not all + * trust the Google EEK root. */ -TEST_P(CertificateRequestTest, EmptyRequest_prodMode) { +TEST_P(CertificateRequestTest, DISABLED_EmptyRequest_prodMode) { bool testMode = false; - for (size_t eekLength : {2, 3, 7}) { - SCOPED_TRACE(testing::Message() << "EEK of length " << eekLength); - generateEek(eekLength); - bytevec keysToSignMac; - DeviceInfo deviceInfo; - ProtectedData protectedData; - auto status = provisionable_->generateCertificateRequest( - testMode, {} /* keysToSign */, eekChain_.chain, challenge_, &deviceInfo, - &protectedData, &keysToSignMac); - EXPECT_FALSE(status.isOk()); - EXPECT_EQ(status.getServiceSpecificError(), - BnRemotelyProvisionedComponent::STATUS_INVALID_EEK); - } + bytevec keysToSignMac; + DeviceInfo deviceInfo; + ProtectedData protectedData; + auto status = provisionable_->generateCertificateRequest( + testMode, {} /* keysToSign */, getProdEekChain(), challenge_, &deviceInfo, + &protectedData, &keysToSignMac); + EXPECT_TRUE(status.isOk()); } /** @@ -468,13 +458,13 @@ TEST_P(CertificateRequestTest, NonEmptyRequest_testMode) { for (size_t eekLength : {2, 3, 7}) { SCOPED_TRACE(testing::Message() << "EEK of length " << eekLength); - generateEek(eekLength); + generateTestEekChain(eekLength); bytevec keysToSignMac; DeviceInfo deviceInfo; ProtectedData protectedData; auto status = provisionable_->generateCertificateRequest( - testMode, keysToSign_, eekChain_.chain, challenge_, &deviceInfo, &protectedData, + testMode, keysToSign_, testEekChain_.chain, challenge_, &deviceInfo, &protectedData, &keysToSignMac); ASSERT_TRUE(status.isOk()) << status.getMessage(); @@ -483,30 +473,21 @@ TEST_P(CertificateRequestTest, NonEmptyRequest_testMode) { } /** - * Generate a non-empty certificate request in prod mode. Must fail because we don't have a valid - * GEEK. - * - * TODO(swillden): Get a valid GEEK and use it so the generation can succeed, though we won't be - * able to decrypt. + * Generate a non-empty certificate request in prod mode. This test must be run explicitly, and + * is not run by default. Not all devices are GMS devices, and therefore they do not all + * trust the Google EEK root. */ -TEST_P(CertificateRequestTest, NonEmptyRequest_prodMode) { +TEST_P(CertificateRequestTest, DISABLED_NonEmptyRequest_prodMode) { bool testMode = false; generateKeys(testMode, 4 /* numKeys */); - for (size_t eekLength : {2, 3, 7}) { - SCOPED_TRACE(testing::Message() << "EEK of length " << eekLength); - generateEek(eekLength); - - bytevec keysToSignMac; - DeviceInfo deviceInfo; - ProtectedData protectedData; - auto status = provisionable_->generateCertificateRequest( - testMode, keysToSign_, eekChain_.chain, challenge_, &deviceInfo, &protectedData, - &keysToSignMac); - EXPECT_FALSE(status.isOk()); - EXPECT_EQ(status.getServiceSpecificError(), - BnRemotelyProvisionedComponent::STATUS_INVALID_EEK); - } + bytevec keysToSignMac; + DeviceInfo deviceInfo; + ProtectedData protectedData; + auto status = provisionable_->generateCertificateRequest( + testMode, keysToSign_, getProdEekChain(), challenge_, &deviceInfo, &protectedData, + &keysToSignMac); + EXPECT_TRUE(status.isOk()); } /** @@ -521,8 +502,8 @@ TEST_P(CertificateRequestTest, NonEmptyRequestCorruptMac_testMode) { DeviceInfo deviceInfo; ProtectedData protectedData; auto status = provisionable_->generateCertificateRequest( - testMode, {keyWithCorruptMac}, eekChain_.chain, challenge_, &deviceInfo, &protectedData, - &keysToSignMac); + testMode, {keyWithCorruptMac}, testEekChain_.chain, challenge_, &deviceInfo, + &protectedData, &keysToSignMac); ASSERT_FALSE(status.isOk()) << status.getMessage(); EXPECT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_INVALID_MAC); } @@ -531,7 +512,7 @@ TEST_P(CertificateRequestTest, NonEmptyRequestCorruptMac_testMode) { * Generate a non-empty certificate request in prod mode, but with the MAC corrupted on the keypair. */ TEST_P(CertificateRequestTest, NonEmptyRequestCorruptMac_prodMode) { - bool testMode = true; + bool testMode = false; generateKeys(testMode, 1 /* numKeys */); MacedPublicKey keyWithCorruptMac = corrupt_maced_key(keysToSign_[0]).moveValue(); @@ -539,38 +520,35 @@ TEST_P(CertificateRequestTest, NonEmptyRequestCorruptMac_prodMode) { DeviceInfo deviceInfo; ProtectedData protectedData; auto status = provisionable_->generateCertificateRequest( - testMode, {keyWithCorruptMac}, eekChain_.chain, challenge_, &deviceInfo, &protectedData, - &keysToSignMac); + testMode, {keyWithCorruptMac}, getProdEekChain(), challenge_, &deviceInfo, + &protectedData, &keysToSignMac); ASSERT_FALSE(status.isOk()) << status.getMessage(); - auto rc = status.getServiceSpecificError(); - - // TODO(drysdale): drop the INVALID_EEK potential error code when a real GEEK is available. - EXPECT_TRUE(rc == BnRemotelyProvisionedComponent::STATUS_INVALID_EEK || - rc == BnRemotelyProvisionedComponent::STATUS_INVALID_MAC); + EXPECT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_INVALID_MAC); } /** * Generate a non-empty certificate request in prod mode that has a corrupt EEK chain. * Confirm that the request is rejected. - * - * TODO(drysdale): Update to use a valid GEEK, so that the test actually confirms that the - * implementation is checking signatures. */ TEST_P(CertificateRequestTest, NonEmptyCorruptEekRequest_prodMode) { bool testMode = false; generateKeys(testMode, 4 /* numKeys */); - for (size_t ii = 0; ii < eekLength_; ii++) { - auto chain = corrupt_sig_chain(eekChain_, ii); + auto prodEekChain = getProdEekChain(); + auto [parsedChain, _, parseErr] = cppbor::parse(prodEekChain); + ASSERT_NE(parsedChain, nullptr) << parseErr; + ASSERT_NE(parsedChain->asArray(), nullptr); + + for (int ii = 0; ii < parsedChain->asArray()->size(); ++ii) { + auto chain = corrupt_sig_chain(prodEekChain, ii); ASSERT_TRUE(chain) << chain.message(); - EekChain corruptEek = chain.moveValue(); bytevec keysToSignMac; DeviceInfo deviceInfo; ProtectedData protectedData; - auto status = provisionable_->generateCertificateRequest( - testMode, keysToSign_, corruptEek.chain, challenge_, &deviceInfo, &protectedData, - &keysToSignMac); + auto status = provisionable_->generateCertificateRequest(testMode, keysToSign_, *chain, + challenge_, &deviceInfo, + &protectedData, &keysToSignMac); ASSERT_FALSE(status.isOk()); ASSERT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_INVALID_EEK); @@ -580,9 +558,6 @@ TEST_P(CertificateRequestTest, NonEmptyCorruptEekRequest_prodMode) { /** * Generate a non-empty certificate request in prod mode that has an incomplete EEK chain. * Confirm that the request is rejected. - * - * TODO(drysdale): Update to use a valid GEEK, so that the test actually confirms that the - * implementation is checking signatures. */ TEST_P(CertificateRequestTest, NonEmptyIncompleteEekRequest_prodMode) { bool testMode = false; @@ -590,7 +565,7 @@ TEST_P(CertificateRequestTest, NonEmptyIncompleteEekRequest_prodMode) { // Build an EEK chain that omits the first self-signed cert. auto truncatedChain = cppbor::Array(); - auto [chain, _, parseErr] = cppbor::parse(eekChain_.chain); + auto [chain, _, parseErr] = cppbor::parse(getProdEekChain()); ASSERT_TRUE(chain); auto eekChain = chain->asArray(); ASSERT_NE(eekChain, nullptr); @@ -619,7 +594,7 @@ TEST_P(CertificateRequestTest, NonEmptyRequest_prodKeyInTestCert) { DeviceInfo deviceInfo; ProtectedData protectedData; auto status = provisionable_->generateCertificateRequest( - true /* testMode */, keysToSign_, eekChain_.chain, challenge_, &deviceInfo, + true /* testMode */, keysToSign_, testEekChain_.chain, challenge_, &deviceInfo, &protectedData, &keysToSignMac); ASSERT_FALSE(status.isOk()); ASSERT_EQ(status.getServiceSpecificError(), @@ -637,7 +612,7 @@ TEST_P(CertificateRequestTest, NonEmptyRequest_testKeyInProdCert) { DeviceInfo deviceInfo; ProtectedData protectedData; auto status = provisionable_->generateCertificateRequest( - false /* testMode */, keysToSign_, eekChain_.chain, challenge_, &deviceInfo, + false /* testMode */, keysToSign_, testEekChain_.chain, challenge_, &deviceInfo, &protectedData, &keysToSignMac); ASSERT_FALSE(status.isOk()); ASSERT_EQ(status.getServiceSpecificError(),