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

Commit d39b9fb6 authored by Selene Huang's avatar Selene Huang Committed by David Zeuthen
Browse files

Fix IC vts bugs and add tests for IC IWritableIdentityCredential.aidl interface. 

Fixed following bugs in WritableIdentityCredential.cpp
  - Do not allow startPersonalization to be called more than once per
  aidl.
  - Do not preceed with beginAddEntry if addAccessControlProfile and
  startPersonalization profile count mismatch.
  - Verify access control profile ids are unique.
  - Do not let empty name space to mess up beginAddEntry.
  - Do not allow beginAddEntry to add entries interleaving namespace
    groupings. Enforce all entries must be added in namespace "groups"
    per aidl.
  - Fix counting error that allowed one entries to be added per name
    space than startPersonalization limit.
  - Do not approve finishAddingEntries if there are more profiles or
    entries to be added than startPersonalization set accounting.
  - Add testing utilities library for identity credential.
  - Refactored end to end tests.

Bug: 154909726
Test: atest VtsHalIdentityTargetTest
Test: atest android.security.identity.cts
Merged-In: I51902681776c6230e49589fc75a8145e79d7d1a6

Change-Id: Ib7c108f67c61125edba6177dcac61cfbf58da671
parent d78626d9

identity/aidl/default/WritableIdentityCredential.cpp

+38 −1
Original line number Diff line number Diff line
@@ -44,6 +44,8 @@ bool WritableIdentityCredential::initialize() { 
        return false;

    }

    storageKey_ = random.value();

    startPersonalizationCalled_ = false;

    firstEntry_ = true;



    return true;

}
@@ -105,6 +107,12 @@ ndk::ScopedAStatus WritableIdentityCredential::getAttestationCertificate( 


ndk::ScopedAStatus WritableIdentityCredential::startPersonalization(

        int32_t accessControlProfileCount, const vector<int32_t>& entryCounts) {

    if (startPersonalizationCalled_) {

        return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage(

                IIdentityCredentialStore::STATUS_FAILED, "startPersonalization called already"));

    }



    startPersonalizationCalled_ = true;

    numAccessControlProfileRemaining_ = accessControlProfileCount;

    remainingEntryCounts_ = entryCounts;

    entryNameSpace_ = "";
@@ -128,6 +136,13 @@ ndk::ScopedAStatus WritableIdentityCredential::addAccessControlProfile( 
                "numAccessControlProfileRemaining_ is 0 and expected non-zero"));

    }



    if (accessControlProfileIds_.find(id) != accessControlProfileIds_.end()) {

        return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage(

                IIdentityCredentialStore::STATUS_INVALID_DATA,

                "Access Control Profile id must be unique"));

    }

    accessControlProfileIds_.insert(id);



    // Spec requires if |userAuthenticationRequired| is false, then |timeoutMillis| must also

    // be zero.

    if (!userAuthenticationRequired && timeoutMillis != 0) {
@@ -184,12 +199,20 @@ ndk::ScopedAStatus WritableIdentityCredential::beginAddEntry( 
    }



    // Handle initial beginEntry() call.

    if (entryNameSpace_ == "") {

    if (firstEntry_) {

        firstEntry_ = false;

        entryNameSpace_ = nameSpace;

        allNameSpaces_.insert(nameSpace);

    }



    // If the namespace changed...

    if (nameSpace != entryNameSpace_) {

        if (allNameSpaces_.find(nameSpace) != allNameSpaces_.end()) {

            return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage(

                    IIdentityCredentialStore::STATUS_INVALID_DATA,

                    "Name space cannot be added in interleaving fashion"));

        }



        // Then check that all entries in the previous namespace have been added..

        if (remainingEntryCounts_[0] != 0) {

            return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage(
@@ -197,6 +220,8 @@ ndk::ScopedAStatus WritableIdentityCredential::beginAddEntry( 
                    "New namespace but a non-zero number of entries remain to be added"));

        }

        remainingEntryCounts_.erase(remainingEntryCounts_.begin());

        remainingEntryCounts_[0] -= 1;

        allNameSpaces_.insert(nameSpace);



        if (signedDataCurrentNamespace_.size() > 0) {

            signedDataNamespaces_.add(entryNameSpace_, std::move(signedDataCurrentNamespace_));
@@ -330,6 +355,18 @@ bool generateCredentialData(const vector<uint8_t>& hardwareBoundKey, const strin 


ndk::ScopedAStatus WritableIdentityCredential::finishAddingEntries(

        vector<int8_t>* outCredentialData, vector<int8_t>* outProofOfProvisioningSignature) {

    if (numAccessControlProfileRemaining_ != 0) {

        return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage(

                IIdentityCredentialStore::STATUS_INVALID_DATA,

                "numAccessControlProfileRemaining_ is not 0 and expected zero"));

    }



    if (remainingEntryCounts_.size() > 1 || remainingEntryCounts_[0] != 0) {

        return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage(

                IIdentityCredentialStore::STATUS_INVALID_DATA,

                "More entry spaces remain than startPersonalization configured"));

    }



    if (signedDataCurrentNamespace_.size() > 0) {

        signedDataNamespaces_.add(entryNameSpace_, std::move(signedDataCurrentNamespace_));

    }

identity/aidl/default/WritableIdentityCredential.h

+8 −0
Original line number Diff line number Diff line
@@ -21,9 +21,11 @@ 
#include <android/hardware/identity/support/IdentityCredentialSupport.h>



#include <cppbor.h>

#include <set>



namespace aidl::android::hardware::identity {



using ::std::set;

using ::std::string;

using ::std::vector;
@@ -66,6 +68,8 @@ class WritableIdentityCredential : public BnWritableIdentityCredential { 


    // This is set in initialize().

    vector<uint8_t> storageKey_;

    bool startPersonalizationCalled_;

    bool firstEntry_;



    // These are set in getAttestationCertificate().

    vector<uint8_t> credentialPrivKey_;
@@ -79,6 +83,9 @@ class WritableIdentityCredential : public BnWritableIdentityCredential { 
    cppbor::Map signedDataNamespaces_;

    cppbor::Array signedDataCurrentNamespace_;



    // This field is initialized in addAccessControlProfile

    set<int32_t> accessControlProfileIds_;



    // These fields are initialized during beginAddEntry()

    size_t entryRemainingBytes_;

    vector<uint8_t> entryAdditionalData_;
@@ -86,6 +93,7 @@ class WritableIdentityCredential : public BnWritableIdentityCredential { 
    string entryName_;

    vector<int32_t> entryAccessControlProfileIds_;

    vector<uint8_t> entryBytes_;

    set<string> allNameSpaces_;

};



}  // namespace aidl::android::hardware::identity

identity/aidl/vts/Android.bp

+5 −1
Original line number Diff line number Diff line
@@ -4,7 +4,11 @@ cc_test { 
        "VtsHalTargetTestDefaults",

        "use_libaidlvintf_gtest_helper_static",

    ],

    srcs: ["VtsHalIdentityTargetTest.cpp"],

    srcs: [

        "VtsHalIdentityEndToEndTest.cpp",

        "VtsIWritableIdentityCredentialTests.cpp",

        "VtsIdentityTestUtils.cpp",

    ],

    shared_libs: [

        "libbinder",

        "libcrypto",

identity/aidl/vts/VtsHalIdentityTargetTest.cpp→identity/aidl/vts/VtsHalIdentityEndToEndTest.cpp

+33 −120
Original line number Diff line number Diff line
@@ -28,8 +28,11 @@ 
#include <future>

#include <map>



#include "VtsIdentityTestUtils.h"



namespace android::hardware::identity {



using std::endl;

using std::map;

using std::optional;

using std::string;
@@ -41,51 +44,6 @@ using ::android::binder::Status; 


using ::android::hardware::keymaster::HardwareAuthToken;



// ---------------------------------------------------------------------------

// Test Data.

// ---------------------------------------------------------------------------



struct TestEntryData {

    TestEntryData(string nameSpace, string name, vector<int32_t> profileIds)

        : nameSpace(nameSpace), name(name), profileIds(profileIds) {}



    TestEntryData(string nameSpace, string name, const string& value, vector<int32_t> profileIds)

        : TestEntryData(nameSpace, name, profileIds) {

        valueCbor = cppbor::Tstr(((const char*)value.data())).encode();

    }

    TestEntryData(string nameSpace, string name, const vector<uint8_t>& value,

                  vector<int32_t> profileIds)

        : TestEntryData(nameSpace, name, profileIds) {

        valueCbor = cppbor::Bstr(value).encode();

    }

    TestEntryData(string nameSpace, string name, bool value, vector<int32_t> profileIds)

        : TestEntryData(nameSpace, name, profileIds) {

        valueCbor = cppbor::Bool(value).encode();

    }

    TestEntryData(string nameSpace, string name, int64_t value, vector<int32_t> profileIds)

        : TestEntryData(nameSpace, name, profileIds) {

        if (value >= 0) {

            valueCbor = cppbor::Uint(value).encode();

        } else {

            valueCbor = cppbor::Nint(-value).encode();

        }

    }



    string nameSpace;

    string name;

    vector<uint8_t> valueCbor;

    vector<int32_t> profileIds;

};



struct TestProfile {

    uint16_t id;

    vector<uint8_t> readerCertificate;

    bool userAuthenticationRequired;

    uint64_t timeoutMillis;

};



// ----------------------------------------------------------------



class IdentityAidl : public testing::TestWithParam<std::string> {

  public:

    virtual void SetUp() override {
@@ -108,31 +66,18 @@ TEST_P(IdentityAidl, hardwareInformation) { 
TEST_P(IdentityAidl, createAndRetrieveCredential) {

    // First, generate a key-pair for the reader since its public key will be

    // part of the request data.

    optional<vector<uint8_t>> readerKeyPKCS8 = support::createEcKeyPair();

    ASSERT_TRUE(readerKeyPKCS8);

    optional<vector<uint8_t>> readerPublicKey =

            support::ecKeyPairGetPublicKey(readerKeyPKCS8.value());

    optional<vector<uint8_t>> readerKey = support::ecKeyPairGetPrivateKey(readerKeyPKCS8.value());

    string serialDecimal = "1234";

    string issuer = "Android Open Source Project";

    string subject = "Android IdentityCredential VTS Test";

    time_t validityNotBefore = time(nullptr);

    time_t validityNotAfter = validityNotBefore + 365 * 24 * 3600;

    optional<vector<uint8_t>> readerCertificate = support::ecPublicKeyGenerateCertificate(

            readerPublicKey.value(), readerKey.value(), serialDecimal, issuer, subject,

            validityNotBefore, validityNotAfter);

    vector<uint8_t> readerKey;

    optional<vector<uint8_t>> readerCertificate =

            test_utils::GenerateReaderCertificate("1234", readerKey);

    ASSERT_TRUE(readerCertificate);



    // Make the portrait image really big (just shy of 256 KiB) to ensure that

    // the chunking code gets exercised.

    vector<uint8_t> portraitImage;

    portraitImage.resize(256 * 1024 - 10);

    for (size_t n = 0; n < portraitImage.size(); n++) {

        portraitImage[n] = (uint8_t)n;

    }

    test_utils::SetImageData(portraitImage);



    // Access control profiles:

    const vector<TestProfile> testProfiles = {// Profile 0 (reader authentication)

    const vector<test_utils::TestProfile> testProfiles = {// Profile 0 (reader authentication)

                                                          {0, readerCertificate.value(), false, 0},

                                                          // Profile 1 (no authentication)

                                                          {1, {}, false, 0}};
@@ -140,7 +85,7 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { 
    HardwareAuthToken authToken;



    // Here's the actual test data:

    const vector<TestEntryData> testEntries = {

    const vector<test_utils::TestEntryData> testEntries = {

            {"PersonalData", "Last name", string("Turing"), vector<int32_t>{0, 1}},

            {"PersonalData", "Birth date", string("19120623"), vector<int32_t>{0, 1}},

            {"PersonalData", "First name", string("Alan"), vector<int32_t>{0, 1}},
@@ -155,67 +100,33 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { 


    string cborPretty;

    sp<IWritableIdentityCredential> writableCredential;

    string docType = "org.iso.18013-5.2019.mdl";

    bool testCredential = true;

    ASSERT_TRUE(credentialStore_->createCredential(docType, testCredential, &writableCredential)

                        .isOk());

    ASSERT_NE(writableCredential, nullptr);

    ASSERT_TRUE(test_utils::SetupWritableCredential(writableCredential, credentialStore_));



    string challenge = "attestationChallenge";

    test_utils::AttestationData attData(writableCredential, challenge, {});

    ASSERT_TRUE(attData.result.isOk())

            << attData.result.exceptionCode() << "; " << attData.result.exceptionMessage() << endl;

    ASSERT_EQ(binder::Status::EX_NONE, attData.result.exceptionCode());

    ASSERT_EQ(IIdentityCredentialStore::STATUS_OK, attData.result.serviceSpecificErrorCode());



    // TODO: set it to something random and check it's in the cert chain

    vector<uint8_t> attestationApplicationId = {};

    vector<uint8_t> attestationChallenge(challenge.begin(), challenge.end());

    vector<Certificate> attestationCertificates;

    ASSERT_TRUE(writableCredential

                        ->getAttestationCertificate(attestationApplicationId, attestationChallenge,

                                                    &attestationCertificates)

                        .isOk());

    ASSERT_GE(attestationCertificates.size(), 2);

    ASSERT_GE(attData.attestationCertificate.size(), 2);



    ASSERT_TRUE(

            writableCredential->startPersonalization(testProfiles.size(), testEntriesEntryCounts)

                    .isOk());



    vector<SecureAccessControlProfile> returnedSecureProfiles;

    for (const auto& testProfile : testProfiles) {

        SecureAccessControlProfile profile;

        Certificate cert;

        cert.encodedCertificate = testProfile.readerCertificate;

        ASSERT_TRUE(writableCredential

                            ->addAccessControlProfile(testProfile.id, cert,

                                                      testProfile.userAuthenticationRequired,

                                                      testProfile.timeoutMillis,

                                                      0,  // secureUserId

                                                      &profile)

                            .isOk());

        ASSERT_EQ(testProfile.id, profile.id);

        ASSERT_EQ(testProfile.readerCertificate, profile.readerCertificate.encodedCertificate);

        ASSERT_EQ(testProfile.userAuthenticationRequired, profile.userAuthenticationRequired);

        ASSERT_EQ(testProfile.timeoutMillis, profile.timeoutMillis);

        ASSERT_EQ(support::kAesGcmTagSize + support::kAesGcmIvSize, profile.mac.size());

        returnedSecureProfiles.push_back(profile);

    }

    optional<vector<SecureAccessControlProfile>> secureProfiles =

            test_utils::AddAccessControlProfiles(writableCredential, testProfiles);

    ASSERT_TRUE(secureProfiles);



    // Uses TestEntryData* pointer as key and values are the encrypted blobs. This

    // is a little hacky but it works well enough.

    map<const TestEntryData*, vector<vector<uint8_t>>> encryptedBlobs;

    map<const test_utils::TestEntryData*, vector<vector<uint8_t>>> encryptedBlobs;



    for (const auto& entry : testEntries) {

        vector<vector<uint8_t>> chunks =

                support::chunkVector(entry.valueCbor, hwInfo.dataChunkSize);



        ASSERT_TRUE(writableCredential

                            ->beginAddEntry(entry.profileIds, entry.nameSpace, entry.name,

                                            entry.valueCbor.size())

                            .isOk());



        vector<vector<uint8_t>> encryptedChunks;

        for (const auto& chunk : chunks) {

            vector<uint8_t> encryptedChunk;

            ASSERT_TRUE(writableCredential->addEntryValue(chunk, &encryptedChunk).isOk());

            encryptedChunks.push_back(encryptedChunk);

        }

        encryptedBlobs[&entry] = encryptedChunks;

        ASSERT_TRUE(test_utils::AddEntry(writableCredential, entry, hwInfo.dataChunkSize,

                                         encryptedBlobs, true));

    }



    vector<uint8_t> credentialData;
@@ -276,8 +187,8 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { 
            "]",

            cborPretty);



    optional<vector<uint8_t>> credentialPubKey =

            support::certificateChainGetTopMostKey(attestationCertificates[0].encodedCertificate);

    optional<vector<uint8_t>> credentialPubKey = support::certificateChainGetTopMostKey(

            attData.attestationCertificate[0].encodedCertificate);

    ASSERT_TRUE(credentialPubKey);

    EXPECT_TRUE(support::coseCheckEcDsaSignature(proofOfProvisioningSignature,

                                                 {},  // Additional data
@@ -347,7 +258,7 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { 
                                         .add(cppbor::Semantic(24, itemsRequestBytes))

                                         .encode();

    optional<vector<uint8_t>> readerSignature =

            support::coseSignEcDsa(readerKey.value(), {},  // content

            support::coseSignEcDsa(readerKey, {},  // content

                                   dataToSign,     // detached content

                                   readerCertificate.value());

    ASSERT_TRUE(readerSignature);
@@ -358,7 +269,7 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { 
    ASSERT_TRUE(credential->generateSigningKeyPair(&signingKeyBlob, &signingKeyCertificate).isOk());



    ASSERT_TRUE(credential

                        ->startRetrieval(returnedSecureProfiles, authToken, itemsRequestBytes,

                        ->startRetrieval(secureProfiles.value(), authToken, itemsRequestBytes,

                                         signingKeyBlob, sessionTranscriptBytes,

                                         readerSignature.value(), testEntriesEntryCounts)

                        .isOk());
@@ -405,6 +316,8 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { 
    cppbor::Array deviceAuthentication;

    deviceAuthentication.add("DeviceAuthentication");

    deviceAuthentication.add(sessionTranscript.clone());



    string docType = "org.iso.18013-5.2019.mdl";

    deviceAuthentication.add(docType);

    deviceAuthentication.add(cppbor::Semantic(24, deviceNameSpacesBytes));

    vector<uint8_t> encodedDeviceAuthentication = deviceAuthentication.encode();

identity/aidl/vts/VtsIWritableIdentityCredentialTests.cpp

0 → 100644
+649 −0

File added.

Preview size limit exceeded, changes collapsed.