Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit cceca9f5 authored by David Drysdale's avatar David Drysdale
Browse files

Add more EEK variant tests and related fixes 

 - Test with deliberately-invalid EEK in request:
    - corrupt signature
    - missing initial self-signed cert
 - Test with different sizes of EEK chain.

These tests will only really take effect when we have a valid GEEK to
test with.

Other changes:
 - Fix encoding of KeyUsage bitset.
 - Add a made-up allowed-root pubkey for prod mode. This needs to be
   replaced with the real GEEK when available.
 - Fix generateEek() so that the first private key isn't used for
   all signing operations.

Test: VtsHalRemotelyProvisionedComponentTargetTest
Change-Id: I833894d33cd1757b7a0cfcf18f79b61e4e56a556
parent c8400772

security/keymint/aidl/default/RemotelyProvisionedComponent.cpp

+17 −2
Original line number Diff line number Diff line
@@ -46,6 +46,14 @@ using namespace keymaster; 


namespace {



// Hard-coded set of acceptable public keys that can act as roots of EEK chains.

inline const vector<bytevec> kAuthorizedEekRoots = {

        // TODO(drysdale): replace this random value with real root pubkey(s).

        {0x5c, 0xea, 0x4b, 0xd2, 0x31, 0x27, 0x15, 0x5e, 0x62, 0x94, 0x70,

         0x53, 0x94, 0x43, 0x0f, 0x9a, 0x89, 0xd5, 0xc5, 0x0f, 0x82, 0x9b,

         0xcd, 0x10, 0xe0, 0x79, 0xef, 0xf3, 0xfa, 0x40, 0xeb, 0x0a},

};



constexpr auto STATUS_FAILED = RemotelyProvisionedComponent::STATUS_FAILED;

constexpr auto STATUS_INVALID_EEK = RemotelyProvisionedComponent::STATUS_INVALID_EEK;

constexpr auto STATUS_INVALID_MAC = RemotelyProvisionedComponent::STATUS_INVALID_MAC;
@@ -135,6 +143,13 @@ StatusOr<std::pair<bytevec /* EEK pub */, bytevec /* EEK ID */>> validateAndExtr 
                          "Failed to validate EEK chain: " + cosePubKey.moveMessage());

        }

        lastPubKey = *std::move(cosePubKey);



        // In prod mode the first pubkey should match a well-known Google public key.

        if (!testMode && i == 0 &&

            std::find(kAuthorizedEekRoots.begin(), kAuthorizedEekRoots.end(), lastPubKey) ==

                    kAuthorizedEekRoots.end()) {

            return Status(STATUS_INVALID_EEK, "Unrecognized root of EEK chain");

        }

    }



    auto eek = CoseKey::parseX25519(lastPubKey, true /* requireKid */);
@@ -417,8 +432,8 @@ RemotelyProvisionedComponent::generateBcc() { 
                                .add(1 /* Issuer */, "Issuer")

                                .add(2 /* Subject */, "Subject")

                                .add(-4670552 /* Subject Pub Key */, coseKey)

                                .add(-4670553 /* Key Usage */,

                                     std::vector<uint8_t>(0x05) /* Big endian order */)

                                .add(-4670553 /* Key Usage (little-endian order) */,

                                     std::vector<uint8_t>{0x20} /* keyCertSign = 1<<5 */)

                                .canonicalize()

                                .encode();

    auto coseSign1 = constructCoseSign1(privKey,       /* signing key */

security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp

+165 −29
Original line number Diff line number Diff line
@@ -128,6 +128,55 @@ void check_maced_pubkey(const MacedPublicKey& macedPubKey, bool testMode, 
    }

}



ErrMsgOr<cppbor::Array> corrupt_sig(const cppbor::Array* coseSign1) {

    if (coseSign1->size() != kCoseSign1EntryCount) {

        return "Invalid COSE_Sign1, wrong entry count";

    }

    const cppbor::Bstr* protectedParams = coseSign1->get(kCoseSign1ProtectedParams)->asBstr();

    const cppbor::Map* unprotectedParams = coseSign1->get(kCoseSign1UnprotectedParams)->asMap();

    const cppbor::Bstr* payload = coseSign1->get(kCoseSign1Payload)->asBstr();

    const cppbor::Bstr* signature = coseSign1->get(kCoseSign1Signature)->asBstr();

    if (!protectedParams || !unprotectedParams || !payload || !signature) {

        return "Invalid COSE_Sign1: missing content";

    }



    auto corruptSig = cppbor::Array();

    corruptSig.add(protectedParams->clone());

    corruptSig.add(unprotectedParams->clone());

    corruptSig.add(payload->clone());

    vector<uint8_t> sigData = signature->value();

    sigData[0] ^= 0x08;

    corruptSig.add(cppbor::Bstr(sigData));



    return std::move(corruptSig);

}



ErrMsgOr<EekChain> corrupt_sig_chain(const EekChain& eek, int which) {

    auto [chain, _, parseErr] = cppbor::parse(eek.chain);

    if (!chain || !chain->asArray()) {

        return "EekChain parse failed";

    }



    cppbor::Array* eekChain = chain->asArray();

    if (which >= eekChain->size()) {

        return "selected sig out of range";

    }

    auto corruptChain = cppbor::Array();



    for (int ii = 0; ii < eekChain->size(); ++ii) {

        if (ii == which) {

            auto sig = corrupt_sig(eekChain->get(which)->asArray());

            if (!sig) {

                return "Failed to build corrupted signature" + sig.moveMessage();

            }

            corruptChain.add(sig.moveValue());

        } else {

            corruptChain.add(eekChain->get(ii)->clone());

        }

    }

    return EekChain{corruptChain.encode(), eek.last_pubkey, eek.last_privkey};

}



}  // namespace



class VtsRemotelyProvisionedComponentTests : public testing::TestWithParam<std::string> {
@@ -182,9 +231,14 @@ TEST_P(GenerateKeyTests, generateEcdsaP256Key_testMode) { 
class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests {

  protected:

    CertificateRequestTest() : eekId_(string_to_bytevec("eekid")), challenge_(randomBytes(32)) {

        auto chain = generateEekChain(3, eekId_);

        generateEek(3);

    }



    void generateEek(size_t eekLength) {

        auto chain = generateEekChain(eekLength, eekId_);

        EXPECT_TRUE(chain) << chain.message();

        if (chain) eekChain_ = chain.moveValue();

        eekLength_ = eekLength;

    }



    void generateKeys(bool testMode, size_t numKeys) {
@@ -258,6 +312,7 @@ class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests { 
    }



    bytevec eekId_;

    size_t eekLength_;

    EekChain eekChain_;

    bytevec challenge_;

    std::vector<MacedPublicKey> keysToSign_;
@@ -270,16 +325,21 @@ class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests { 
 */

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);



        bytevec keysToSignMac;

        DeviceInfo deviceInfo;

        ProtectedData protectedData;

        auto status = provisionable_->generateCertificateRequest(

            testMode, {} /* keysToSign */, eekChain_.chain, challenge_, &deviceInfo, &protectedData,

            &keysToSignMac);

                testMode, {} /* keysToSign */, eekChain_.chain, challenge_, &deviceInfo,

                &protectedData, &keysToSignMac);

        ASSERT_TRUE(status.isOk()) << status.getMessage();



        checkProtectedData(testMode, cppbor::Array(), keysToSignMac, protectedData);

    }

}



/**

 * Generate an empty certificate request in prod mode.  Generation will fail because we don't have a
@@ -290,14 +350,20 @@ TEST_P(CertificateRequestTest, EmptyRequest_testMode) { 
 */

TEST_P(CertificateRequestTest, 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);

    ASSERT_FALSE(status.isOk());

    ASSERT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_INVALID_EEK);

                testMode, {} /* keysToSign */, eekChain_.chain, challenge_, &deviceInfo,

                &protectedData, &keysToSignMac);

        EXPECT_FALSE(status.isOk());

        EXPECT_EQ(status.getServiceSpecificError(),

                  BnRemotelyProvisionedComponent::STATUS_INVALID_EEK);

    }

}



/**
@@ -307,16 +373,21 @@ TEST_P(CertificateRequestTest, NonEmptyRequest_testMode) { 
    bool testMode = true;

    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);

        auto status = provisionable_->generateCertificateRequest(

                testMode, keysToSign_, eekChain_.chain, challenge_, &deviceInfo, &protectedData,

                &keysToSignMac);

        ASSERT_TRUE(status.isOk()) << status.getMessage();



        checkProtectedData(testMode, cborKeysToSign_, keysToSignMac, protectedData);

    }

}



/**

 * Generate a non-empty certificate request in prod mode.  Must fail because we don't have a valid
@@ -329,12 +400,77 @@ TEST_P(CertificateRequestTest, 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);

        auto status = provisionable_->generateCertificateRequest(

                testMode, keysToSign_, eekChain_.chain, challenge_, &deviceInfo, &protectedData,

                &keysToSignMac);

        EXPECT_FALSE(status.isOk());

        EXPECT_EQ(status.getServiceSpecificError(),

                  BnRemotelyProvisionedComponent::STATUS_INVALID_EEK);

    }

}



/**

 * 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);

        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);

        ASSERT_FALSE(status.isOk());

        ASSERT_EQ(status.getServiceSpecificError(),

                  BnRemotelyProvisionedComponent::STATUS_INVALID_EEK);

    }

}



/**

 * 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;

    generateKeys(testMode, 4 /* numKeys */);



    // Build an EEK chain that omits the first self-signed cert.

    auto truncatedChain = cppbor::Array();

    auto [chain, _, parseErr] = cppbor::parse(eekChain_.chain);

    ASSERT_TRUE(chain);

    auto eekChain = chain->asArray();

    ASSERT_NE(eekChain, nullptr);

    for (size_t ii = 1; ii < eekChain->size(); ii++) {

        truncatedChain.add(eekChain->get(ii)->clone());

    }



    bytevec keysToSignMac;

    DeviceInfo deviceInfo;

    ProtectedData protectedData;

    auto status = provisionable_->generateCertificateRequest(

            testMode, keysToSign_, truncatedChain.encode(), challenge_, &deviceInfo, &protectedData,

            &keysToSignMac);

    ASSERT_FALSE(status.isOk());

    ASSERT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_INVALID_EEK);

}

security/keymint/support/remote_prov_utils.cpp

+2 −0
Original line number Diff line number Diff line
@@ -54,6 +54,8 @@ ErrMsgOr<EekChain> generateEekChain(size_t length, const bytevec& eekId) { 
                                            {} /* AAD */);

        if (!coseSign1) return coseSign1.moveMessage();

        eekChain.add(coseSign1.moveValue());



        prev_priv_key = priv_key;

    }



    bytevec pub_key(X25519_PUBLIC_VALUE_LEN);