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

Commit 8eac8ba5 authored by David Drysdale's avatar David Drysdale Committed by Automerger Merge Worker
Browse files

Merge "Use RAII to ensure KeyMint keyblobs deleted" am: 462eccaa am: 15bc932a 

Original change: https://android-review.googlesource.com/c/platform/hardware/interfaces/+/2617612



Change-Id: I3ca6d49de74aaa2b99d3cd1c17c1a9b3952330e4
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents f07c90d9 15bc932a

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

+23 −39
Original line number Diff line number Diff line
@@ -120,6 +120,7 @@ TEST_P(AttestKeyTest, AllRsaSizes) { 
                                            .SetDefaultValidity(),

                                    {} /* attestation signing key */, &attest_key.keyBlob,

                                    &attest_key_characteristics, &attest_key_cert_chain));

        KeyBlobDeleter attest_deleter(keymint_, attest_key.keyBlob);



        ASSERT_GT(attest_key_cert_chain.size(), 0);

        EXPECT_EQ(attest_key_cert_chain.size(), 1);
@@ -141,8 +142,7 @@ TEST_P(AttestKeyTest, AllRsaSizes) { 
                                      .SetDefaultValidity(),

                              attest_key, &attested_key_blob, &attested_key_characteristics,

                              &attested_key_cert_chain));



        CheckedDeleteKey(&attested_key_blob);

        KeyBlobDeleter attested_deleter(keymint_, attested_key_blob);



        AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics);

        AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics);
@@ -174,8 +174,7 @@ TEST_P(AttestKeyTest, AllRsaSizes) { 
                                      .SetDefaultValidity(),

                              attest_key, &attested_key_blob, &attested_key_characteristics,

                              &attested_key_cert_chain));



        CheckedDeleteKey(&attested_key_blob);

        KeyBlobDeleter attested_deleter2(keymint_, attested_key_blob);



        hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics);

        sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics);
@@ -207,6 +206,7 @@ TEST_P(AttestKeyTest, AllRsaSizes) { 
                                      .SetDefaultValidity(),

                              attest_key, &attested_key_blob, &attested_key_characteristics,

                              &attested_key_cert_chain));

        KeyBlobDeleter attested_deleter3(keymint_, attested_key_blob);



        // The returned key characteristics will include CREATION_DATETIME (checked below)

        // in SecurityLevel::KEYSTORE; this will be stripped out in the CheckCharacteristics()
@@ -214,9 +214,6 @@ TEST_P(AttestKeyTest, AllRsaSizes) { 
        // any SecurityLevel::KEYSTORE characteristics).

        CheckCharacteristics(attested_key_blob, attested_key_characteristics);



        CheckedDeleteKey(&attested_key_blob);

        CheckedDeleteKey(&attest_key.keyBlob);



        hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics);

        sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics);
@@ -308,6 +305,7 @@ TEST_P(AttestKeyTest, RsaAttestedAttestKeys) { 
        if (result == ErrorCode::ATTESTATION_KEYS_NOT_PROVISIONED) return;

    }

    ASSERT_EQ(ErrorCode::OK, result);

    KeyBlobDeleter attest_deleter(keymint_, attest_key.keyBlob);



    EXPECT_GT(attest_key_cert_chain.size(), 1);

    verify_subject_and_serial(attest_key_cert_chain[0], serial_int, subject, false);
@@ -344,9 +342,7 @@ TEST_P(AttestKeyTest, RsaAttestedAttestKeys) { 
                                  .SetDefaultValidity(),

                          attest_key, &attested_key_blob, &attested_key_characteristics,

                          &attested_key_cert_chain));



    CheckedDeleteKey(&attested_key_blob);

    CheckedDeleteKey(&attest_key.keyBlob);

    KeyBlobDeleter attested_deleter(keymint_, attested_key_blob);



    AuthorizationSet hw_enforced2 = HwEnforcedAuthorizations(attested_key_characteristics);

    AuthorizationSet sw_enforced2 = SwEnforcedAuthorizations(attested_key_characteristics);
@@ -376,6 +372,7 @@ TEST_P(AttestKeyTest, RsaAttestKeyChaining) { 
    const int chain_size = 6;

    vector<vector<uint8_t>> key_blob_list(chain_size);

    vector<vector<Certificate>> cert_chain_list(chain_size);

    vector<KeyBlobDeleter> deleters;



    for (int i = 0; i < chain_size; i++) {

        string sub = "attest key chaining ";
@@ -412,6 +409,7 @@ TEST_P(AttestKeyTest, RsaAttestKeyChaining) { 
            if (result == ErrorCode::ATTESTATION_KEYS_NOT_PROVISIONED) return;

        }

        ASSERT_EQ(ErrorCode::OK, result);

        deleters.push_back(KeyBlobDeleter(keymint_, key_blob_list[i]));



        AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics);

        AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics);
@@ -437,10 +435,6 @@ TEST_P(AttestKeyTest, RsaAttestKeyChaining) { 
        EXPECT_GT(cert_chain_list[i].size(), i + 1);

        verify_subject_and_serial(cert_chain_list[i][0], serial_int, subject, false);

    }



    for (int i = 0; i < chain_size; i++) {

        CheckedDeleteKey(&key_blob_list[i]);

    }

}



/*
@@ -453,6 +447,7 @@ TEST_P(AttestKeyTest, EcAttestKeyChaining) { 
    const int chain_size = 6;

    vector<vector<uint8_t>> key_blob_list(chain_size);

    vector<vector<Certificate>> cert_chain_list(chain_size);

    vector<KeyBlobDeleter> deleters;



    for (int i = 0; i < chain_size; i++) {

        string sub = "Ec attest key chaining ";
@@ -489,6 +484,7 @@ TEST_P(AttestKeyTest, EcAttestKeyChaining) { 
            if (result == ErrorCode::ATTESTATION_KEYS_NOT_PROVISIONED) return;

        }

        ASSERT_EQ(ErrorCode::OK, result);

        deleters.push_back(KeyBlobDeleter(keymint_, key_blob_list[i]));



        AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics);

        AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics);
@@ -514,10 +510,6 @@ TEST_P(AttestKeyTest, EcAttestKeyChaining) { 
        EXPECT_GT(cert_chain_list[i].size(), i + 1);

        verify_subject_and_serial(cert_chain_list[i][0], serial_int, subject, false);

    }



    for (int i = 0; i < chain_size; i++) {

        CheckedDeleteKey(&key_blob_list[i]);

    }

}



/*
@@ -557,6 +549,7 @@ TEST_P(AttestKeyTest, AlternateAttestKeyChaining) { 
    const int chain_size = 6;

    vector<vector<uint8_t>> key_blob_list(chain_size);

    vector<vector<Certificate>> cert_chain_list(chain_size);

    vector<KeyBlobDeleter> deleters;



    for (int i = 0; i < chain_size; i++) {

        string sub = "Alt attest key chaining ";
@@ -607,6 +600,7 @@ TEST_P(AttestKeyTest, AlternateAttestKeyChaining) { 
            if (result == ErrorCode::ATTESTATION_KEYS_NOT_PROVISIONED) return;

        }

        ASSERT_EQ(ErrorCode::OK, result);

        deleters.push_back(KeyBlobDeleter(keymint_, key_blob_list[i]));



        AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics);

        AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics);
@@ -632,10 +626,6 @@ TEST_P(AttestKeyTest, AlternateAttestKeyChaining) { 
        EXPECT_GT(cert_chain_list[i].size(), i + 1);

        verify_subject_and_serial(cert_chain_list[i][0], serial_int, subject, false);

    }



    for (int i = 0; i < chain_size; i++) {

        CheckedDeleteKey(&key_blob_list[i]);

    }

}



TEST_P(AttestKeyTest, MissingChallenge) {
@@ -653,6 +643,7 @@ TEST_P(AttestKeyTest, MissingChallenge) { 
                                            .SetDefaultValidity(),

                                    {} /* attestation signing key */, &attest_key.keyBlob,

                                    &attest_key_characteristics, &attest_key_cert_chain));

        KeyBlobDeleter attest_deleter(keymint_, attest_key.keyBlob);



        EXPECT_EQ(attest_key_cert_chain.size(), 1);

        EXPECT_TRUE(IsSelfSigned(attest_key_cert_chain)) << "Failed on size " << size;
@@ -681,8 +672,6 @@ TEST_P(AttestKeyTest, MissingChallenge) { 
                                      .SetDefaultValidity(),

                              attest_key, &attested_key_blob, &attested_key_characteristics,

                              &attested_key_cert_chain));



        CheckedDeleteKey(&attest_key.keyBlob);

    }

}
@@ -700,6 +689,7 @@ TEST_P(AttestKeyTest, AllEcCurves) { 
                        AuthorizationSetBuilder().EcdsaKey(curve).AttestKey().SetDefaultValidity(),

                        {} /* attestation signing key */, &attest_key.keyBlob,

                        &attest_key_characteristics, &attest_key_cert_chain));

        KeyBlobDeleter attest_deleter(keymint_, attest_key.keyBlob);



        ASSERT_GT(attest_key_cert_chain.size(), 0);

        EXPECT_EQ(attest_key_cert_chain.size(), 1);
@@ -721,9 +711,9 @@ TEST_P(AttestKeyTest, AllEcCurves) { 
                                      .SetDefaultValidity(),

                              attest_key, &attested_key_blob, &attested_key_characteristics,

                              &attested_key_cert_chain));

        KeyBlobDeleter attested_deleter(keymint_, attested_key_blob);



        ASSERT_GT(attested_key_cert_chain.size(), 0);

        CheckedDeleteKey(&attested_key_blob);



        AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics);

        AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics);
@@ -752,10 +742,9 @@ TEST_P(AttestKeyTest, AllEcCurves) { 
                                      .SetDefaultValidity(),

                              attest_key, &attested_key_blob, &attested_key_characteristics,

                              &attested_key_cert_chain));

        KeyBlobDeleter attested_deleter2(keymint_, attested_key_blob);



        ASSERT_GT(attested_key_cert_chain.size(), 0);

        CheckedDeleteKey(&attested_key_blob);

        CheckedDeleteKey(&attest_key.keyBlob);



        hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics);

        sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics);
@@ -825,6 +814,7 @@ TEST_P(AttestKeyTest, EcdsaAttestationID) { 
                                        .SetDefaultValidity(),

                                {} /* attestation signing key */, &attest_key.keyBlob,

                                &attest_key_characteristics, &attest_key_cert_chain));

    KeyBlobDeleter attest_deleter(keymint_, attest_key.keyBlob);

    attest_key.issuerSubjectName = make_name_from_str("Android Keystore Key");

    ASSERT_GT(attest_key_cert_chain.size(), 0);

    EXPECT_EQ(attest_key_cert_chain.size(), 1);
@@ -891,8 +881,7 @@ TEST_P(AttestKeyTest, EcdsaAttestationID) { 
        }



        ASSERT_EQ(result, ErrorCode::OK);



        CheckedDeleteKey(&attested_key_blob);

        KeyBlobDeleter attested_deleter(keymint_, attested_key_blob);



        AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics);

        AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics);
@@ -906,7 +895,6 @@ TEST_P(AttestKeyTest, EcdsaAttestationID) { 
                                              hw_enforced, SecLevel(),

                                              attested_key_cert_chain[0].encodedCertificate));

    }

    CheckedDeleteKey(&attest_key.keyBlob);

}



TEST_P(AttestKeyTest, EcdsaAttestationMismatchID) {
@@ -921,6 +909,7 @@ TEST_P(AttestKeyTest, EcdsaAttestationMismatchID) { 
                                        .SetDefaultValidity(),

                                {} /* attestation signing key */, &attest_key.keyBlob,

                                &attest_key_characteristics, &attest_key_cert_chain));

    KeyBlobDeleter attest_deleter(keymint_, attest_key.keyBlob);

    attest_key.issuerSubjectName = make_name_from_str("Android Keystore Key");

    ASSERT_GT(attest_key_cert_chain.size(), 0);

    EXPECT_EQ(attest_key_cert_chain.size(), 1);
@@ -966,7 +955,6 @@ TEST_P(AttestKeyTest, EcdsaAttestationMismatchID) { 
                << "result = " << result;

        device_id_attestation_vsr_check(result);

    }

    CheckedDeleteKey(&attest_key.keyBlob);

}



TEST_P(AttestKeyTest, SecondIMEIAttestationIDSuccess) {
@@ -997,6 +985,7 @@ TEST_P(AttestKeyTest, SecondIMEIAttestationIDSuccess) { 
                                        .SetDefaultValidity(),

                                {} /* attestation signing key */, &attest_key.keyBlob,

                                &attest_key_characteristics, &attest_key_cert_chain));

    KeyBlobDeleter attest_deleter(keymint_, attest_key.keyBlob);

    attest_key.issuerSubjectName = make_name_from_str("Android Keystore Key");

    EXPECT_EQ(attest_key_cert_chain.size(), 1);

    EXPECT_TRUE(IsSelfSigned(attest_key_cert_chain));
@@ -1025,11 +1014,10 @@ TEST_P(AttestKeyTest, SecondIMEIAttestationIDSuccess) { 
    }



    ASSERT_EQ(result, ErrorCode::OK);

    KeyBlobDeleter attested_deleter(keymint_, attested_key_blob);



    device_id_attestation_vsr_check(result);



    CheckedDeleteKey(&attested_key_blob);



    AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics);

    AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics);
@@ -1043,8 +1031,6 @@ TEST_P(AttestKeyTest, SecondIMEIAttestationIDSuccess) { 
    EXPECT_TRUE(verify_attestation_record(AidlVersion(), "challenge", "foo", sw_enforced,

                                          hw_enforced, SecLevel(),

                                          attested_key_cert_chain[0].encodedCertificate));



    CheckedDeleteKey(&attest_key.keyBlob);

}



TEST_P(AttestKeyTest, MultipleIMEIAttestationIDSuccess) {
@@ -1081,6 +1067,7 @@ TEST_P(AttestKeyTest, MultipleIMEIAttestationIDSuccess) { 
                                        .SetDefaultValidity(),

                                {} /* attestation signing key */, &attest_key.keyBlob,

                                &attest_key_characteristics, &attest_key_cert_chain));

    KeyBlobDeleter attest_deleter(keymint_, attest_key.keyBlob);

    attest_key.issuerSubjectName = make_name_from_str("Android Keystore Key");

    EXPECT_EQ(attest_key_cert_chain.size(), 1);

    EXPECT_TRUE(IsSelfSigned(attest_key_cert_chain));
@@ -1106,11 +1093,10 @@ TEST_P(AttestKeyTest, MultipleIMEIAttestationIDSuccess) { 
    }



    ASSERT_EQ(result, ErrorCode::OK);

    KeyBlobDeleter attested_deleter(keymint_, attested_key_blob);



    device_id_attestation_vsr_check(result);



    CheckedDeleteKey(&attested_key_blob);



    AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics);

    AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics);
@@ -1127,8 +1113,6 @@ TEST_P(AttestKeyTest, MultipleIMEIAttestationIDSuccess) { 
    EXPECT_TRUE(verify_attestation_record(AidlVersion(), "challenge", "foo", sw_enforced,

                                          hw_enforced, SecLevel(),

                                          attested_key_cert_chain[0].encodedCertificate));



    CheckedDeleteKey(&attest_key.keyBlob);

}



INSTANTIATE_KEYMINT_AIDL_TEST(AttestKeyTest);

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

+1 −1
Original line number Diff line number Diff line
@@ -560,7 +560,7 @@ TEST_P(KeyBlobUpgradeTest, UseKeyBlobsBeforeOrAfter) { 
                                              .SetDefaultValidity(),

                                      attest_key, &attested_key_blob, &attested_key_characteristics,

                                      &attested_key_cert_chain));

                CheckedDeleteKey(&attested_key_blob);

                KeyBlobDeleter(keymint_, attested_key_blob);

            } else {

                FAIL() << "Unexpected name: " << name;

            }

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

+23 −16
Original line number Diff line number Diff line
@@ -176,6 +176,17 @@ bool KeyMintAidlTestBase::dump_Attestations = false; 
std::string KeyMintAidlTestBase::keyblob_dir;

std::optional<bool> KeyMintAidlTestBase::expect_upgrade = std::nullopt;



KeyBlobDeleter::~KeyBlobDeleter() {

    if (key_blob_.empty()) {

        return;

    }

    Status result = keymint_->deleteKey(key_blob_);

    key_blob_.clear();

    EXPECT_TRUE(result.isOk()) << result.getServiceSpecificError() << "\n";

    ErrorCode rc = GetReturnErrorCode(result);

    EXPECT_TRUE(rc == ErrorCode::OK || rc == ErrorCode::UNIMPLEMENTED) << result << "\n";

}



uint32_t KeyMintAidlTestBase::boot_patch_level(

        const vector<KeyCharacteristics>& key_characteristics) {

    // The boot patchlevel is not available as a property, but should be present
@@ -229,16 +240,6 @@ bool KeyMintAidlTestBase::Curve25519Supported() { 
    return version >= 2;

}



ErrorCode KeyMintAidlTestBase::GetReturnErrorCode(const Status& result) {

    if (result.isOk()) return ErrorCode::OK;



    if (result.getExceptionCode() == EX_SERVICE_SPECIFIC) {

        return static_cast<ErrorCode>(result.getServiceSpecificError());

    }



    return ErrorCode::UNKNOWN_ERROR;

}



void KeyMintAidlTestBase::InitializeKeyMint(std::shared_ptr<IKeyMintDevice> keyMint) {

    ASSERT_NE(keyMint, nullptr);

    keymint_ = std::move(keyMint);
@@ -513,13 +514,9 @@ ErrorCode KeyMintAidlTestBase::DestroyAttestationIds() { 
    return GetReturnErrorCode(result);

}



void KeyMintAidlTestBase::CheckedDeleteKey(vector<uint8_t>* key_blob, bool keep_key_blob) {

    ErrorCode result = DeleteKey(key_blob, keep_key_blob);

    EXPECT_TRUE(result == ErrorCode::OK || result == ErrorCode::UNIMPLEMENTED) << result << endl;

}



void KeyMintAidlTestBase::CheckedDeleteKey() {

    CheckedDeleteKey(&key_blob_);

    ErrorCode result = DeleteKey(&key_blob_, /* keep_key_blob = */ false);

    EXPECT_TRUE(result == ErrorCode::OK || result == ErrorCode::UNIMPLEMENTED) << result << endl;

}



ErrorCode KeyMintAidlTestBase::Begin(KeyPurpose purpose, const vector<uint8_t>& key_blob,
@@ -1986,6 +1983,16 @@ AssertionResult ChainSignaturesAreValid(const vector<Certificate>& chain, 
    return AssertionSuccess();

}



ErrorCode GetReturnErrorCode(const Status& result) {

    if (result.isOk()) return ErrorCode::OK;



    if (result.getExceptionCode() == EX_SERVICE_SPECIFIC) {

        return static_cast<ErrorCode>(result.getServiceSpecificError());

    }



    return ErrorCode::UNKNOWN_ERROR;

}



X509_Ptr parse_cert_blob(const vector<uint8_t>& blob) {

    const uint8_t* p = blob.data();

    return X509_Ptr(d2i_X509(nullptr /* allocate new */, &p, blob.size()));

security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h

+14 −3
Original line number Diff line number Diff line
@@ -57,6 +57,18 @@ constexpr uint64_t kOpHandleSentinel = 0xFFFFFFFFFFFFFFFF; 
const string FEATURE_KEYSTORE_APP_ATTEST_KEY = "android.hardware.keystore.app_attest_key";

const string FEATURE_STRONGBOX_KEYSTORE = "android.hardware.strongbox_keystore";



// RAII class to ensure that a keyblob is deleted regardless of how a test exits.

class KeyBlobDeleter {

  public:

    KeyBlobDeleter(const shared_ptr<IKeyMintDevice>& keymint, const vector<uint8_t>& key_blob)

        : keymint_(keymint), key_blob_(key_blob) {}

    ~KeyBlobDeleter();



  private:

    shared_ptr<IKeyMintDevice> keymint_;

    vector<uint8_t> key_blob_;

};



class KeyMintAidlTestBase : public ::testing::TestWithParam<string> {

  public:

    struct KeyData {
@@ -94,8 +106,6 @@ class KeyMintAidlTestBase : public ::testing::TestWithParam<string> { 


    bool Curve25519Supported();



    ErrorCode GetReturnErrorCode(const Status& result);



    ErrorCode GenerateKey(const AuthorizationSet& key_desc, vector<uint8_t>* key_blob,

                          vector<KeyCharacteristics>* key_characteristics) {

        return GenerateKey(key_desc, std::nullopt /* attest_key */, key_blob, key_characteristics,
@@ -159,7 +169,6 @@ class KeyMintAidlTestBase : public ::testing::TestWithParam<string> { 


    ErrorCode DestroyAttestationIds();



    void CheckedDeleteKey(vector<uint8_t>* key_blob, bool keep_key_blob = false);

    void CheckedDeleteKey();



    ErrorCode Begin(KeyPurpose purpose, const vector<uint8_t>& key_blob,
@@ -431,6 +440,8 @@ AuthorizationSet SwEnforcedAuthorizations(const vector<KeyCharacteristics>& key_ 
::testing::AssertionResult ChainSignaturesAreValid(const vector<Certificate>& chain,

                                                   bool strict_issuer_check = true);



ErrorCode GetReturnErrorCode(const Status& result);



#define INSTANTIATE_KEYMINT_AIDL_TEST(name)                                          \

    INSTANTIATE_TEST_SUITE_P(PerInstance, name,                                      \

                             testing::ValuesIn(KeyMintAidlTestBase::build_params()), \

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

+53 −86

File changed.

Preview size limit exceeded, changes collapsed.