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

Commit e4f58f89 authored by David Zeuthen's avatar David Zeuthen
Browse files

identity: Fix attestation and documentation problems. 

- The docs said that IdentityCredential.createEphemeralKey() returned
  data encoded PKCS#8 which is wrong. It's supposed to be in DER format
  which is also what the VTS tests and credstore expects.

- Clarify that createEphemeralKeyPair(), setReaderEphemeralPublicKey(),
  and createAuthChallenge() are all optional.

- Avoid passing an invalid profile ID in the IdentityCredentialTests.
  verifyOneProfileAndEntryPass test.

- Update requirements for which tags must be present in the attestation
  for CredentialKey as well as the requirements on expiration date and
  the issuer name.  Update default implementation to satisfy these
  requirements. Update VTS tests to carefully verify these requrements
  are met.

- Clarify requirements for X.509 cert for AuthenticationKey. Add VTS
  test to verify.

- Mandate that TAG_IDENTITY_CREDENTIAL_KEY must not be set for test
  credentials. Add VTS test to verify this.

- Make default implementation pretend to be implemented in a trusted
  environment and streamline VTS tests to not special-case for the
  default implementation.

- Switch to using the attestation extension parser from the KM 4.1
  support library instead of the one from system/keymaster. The latter
  one did not support the latest attestation extension and thus would
  fail for pretty much anything that wasn't the default HAL impl.

- Fix a couple of bugs in keymaster::V4_1::parse_attestation_record():
  - Report root_of_trust.security_level
  - Add support for Tag::IDENTITY_CREDENTIAL_KEY

- Fix how EMacKey is calculated.

- Add test vectors to verify how EMacKey and DeviceMac is calculated.

Test: atest VtsHalIdentityTargetTest
Test: atest android.security.identity.cts
Bug: 171745570
Change-Id: Ic906fa24baa3a475585e543dc03aaf1d0f8d19a0
Merged-In: I2f8bd772de078556733f769cec2021918d1d7de6
parent c9e720b3

identity/aidl/android/hardware/identity/IIdentityCredential.aidl

+10 −9
Original line number Diff line number Diff line
@@ -55,7 +55,7 @@ interface IIdentityCredential { 
     * This method may only be called once per instance. If called more than once, STATUS_FAILED

     * will be returned.

     *

     * @return the unencrypted key-pair in PKCS#8 format.

     * @return the private key, in DER format as specified in RFC 5915.

     */

    byte[] createEphemeralKeyPair();
@@ -88,10 +88,10 @@ interface IIdentityCredential { 
     * The setRequestedNamespaces() and setVerificationToken() methods will be called before

     * this method is called.

     *

     * This method be called after createEphemeralKeyPair(), setReaderEphemeralPublicKey(),

     * createAuthChallenge() and before startRetrieveEntry(). This method call is followed by

     * multiple calls of startRetrieveEntryValue(), retrieveEntryValue(), and finally

     * finishRetrieval().

     * This method is called after createEphemeralKeyPair(), setReaderEphemeralPublicKey(),

     * createAuthChallenge() (note that those calls are optional) and before startRetrieveEntry().

     * This method call is followed by multiple calls of startRetrieveEntryValue(),

     * retrieveEntryValue(), and finally finishRetrieval().

     *

     * It is permissible to perform data retrievals multiple times using the same instance (e.g.

     * startRetrieval(), then multiple calls of startRetrieveEntryValue(), retrieveEntryValue(),
@@ -343,12 +343,13 @@ interface IIdentityCredential { 
     *

     *  - signature: must be set to ECDSA.

     *

     *  - subject: CN shall be set to "Android Identity Credential Authentication Key".

     *  - subject: CN shall be set to "Android Identity Credential Authentication Key". (fixed

     *    value: same on all certs)

     *

     *  - issuer: shall be set to "credentialStoreName (credentialStoreAuthorName)" using the

     *    values returned in HardwareInformation.

     *  - issuer: CN shall be set to "Android Identity Credential Key". (fixed value:

     *    same on all certs)

     *

     *  - validity: should be from current time and one year in the future.

     *  - validity: should be from current time and one year in the future (365 days).

     *

     *  - subjectPublicKeyInfo: must contain attested public key.

     *

identity/aidl/android/hardware/identity/IWritableIdentityCredential.aidl

+19 −20
Original line number Diff line number Diff line
@@ -37,12 +37,12 @@ interface IWritableIdentityCredential { 
     *

     *  - signature: must be set to ECDSA.

     *

     *  - subject: CN shall be set to "Android Identity Credential Key".

     *  - subject: CN shall be set to "Android Identity Credential Key". (fixed value:

     *    same on all certs)

     *

     *  - issuer: shall be set to "credentialStoreName (credentialStoreAuthorName)" using the

     *    values returned in HardwareInformation.

     *  - issuer: Same as the subject field of the batch attestation key.

     *

     *  - validity: should be from current time and expire at the same time as the

     *  - validity: Should be set to current time and expire at the same time as the

     *    attestation batch certificate used.

     *

     *  - subjectPublicKeyInfo: must contain attested public key.
@@ -55,19 +55,14 @@ interface IWritableIdentityCredential { 
     *

     *  - The attestationSecurityLevel field must be set to either Software (0),

     *    TrustedEnvironment (1), or StrongBox (2) depending on how attestation is

     *    implemented. Only the default AOSP implementation of this HAL may use

     *    value 0 (additionally, this implementation must not be used on production

     *    devices).

     *    implemented.

     *

     *  - The keymasterVersion field in the attestation extension must be set to (10*major + minor)

     *    where major and minor are the Identity Credential interface major and minor versions.

     *    Specifically for this version of the interface (1.0) this value is 10.

     *  - The keymasterVersion field in the attestation extension must be set to the.

     *    same value as used for Android Keystore keys.

     *

     *  - The keymasterSecurityLevel field in the attestation extension must be set to

     *    either Software (0), TrustedEnvironment (1), or StrongBox (2) depending on how

     *    the Trusted Application backing the HAL implementation is implemented. Only

     *    the default AOSP implementation of this HAL may use value 0 (additionally, this

     *    implementation must not be used on production devices)

     *    the Trusted Application backing the HAL implementation is implemented.

     *

     *  - The attestationChallenge field must be set to the passed-in challenge.

     *
@@ -81,7 +76,8 @@ interface IWritableIdentityCredential { 
     *

     *    - Tag::IDENTITY_CREDENTIAL_KEY which indicates that the key is an Identity

     *      Credential key (which can only sign/MAC very specific messages) and not an Android

     *      Keystore key (which can be used to sign/MAC anything).

     *      Keystore key (which can be used to sign/MAC anything). This must not be set

     *      for test credentials.

     *

     *    - Tag::PURPOSE must be set to SIGN

     *
@@ -95,10 +91,13 @@ interface IWritableIdentityCredential { 
     *

     *    - Tag::EC_CURVE must be set to P_256

     *

     * Additional authorizations may be needed in the softwareEnforced and teeEnforced

     * fields - the above is not an exhaustive list. Specifically, authorizations containing

     * information about the root of trust, OS version, verified boot state, and so on should

     * be included.

     *    - Tag::ROOT_OF_TRUST must be set

     *

     *    - Tag::OS_VERSION and Tag::OS_PATCHLEVEL must be set

     *

     * Additional authorizations may be appear in the softwareEnforced and teeEnforced

     * fields. For example if the device has a boot or vendor partitions, then BOOT_PATCHLEVEL

     * and VENDOR_PATCHLEVEL should be set.

     *

     * Since the chain is required to be generated using Keymaster Attestation, the returned

     * certificate chain has the following properties:
@@ -112,8 +111,8 @@ interface IWritableIdentityCredential { 
     * As with any user of attestation, the Issuing Authority (as a relying party) wishing

     * to issue a credential to a device using these APIs, must carefully examine the

     * returned certificate chain for all of the above (and more). In particular, the Issuing

     * Authority should check the root of trust, verified boot state, patch level,

     * application id, etc.

     * Authority should check the root of trust (which include verified boot state), patch level,

     * attestation application id, etc.

     *

     * This all depends on the needs of the Issuing Authority and the kind of credential but

     * in general an Issuing Authority should never issue a credential to a device without

identity/aidl/default/IdentityCredential.cpp

+12 −34
Original line number Diff line number Diff line
@@ -276,6 +276,7 @@ ndk::ScopedAStatus IdentityCredential::startRetrieval( 
    auto itemsRequest = byteStringToUnsigned(itemsRequestS);

    auto readerSignature = byteStringToUnsigned(readerSignatureS);



    std::unique_ptr<cppbor::Item> sessionTranscriptItem;

    if (sessionTranscript.size() > 0) {

        auto [item, _, message] = cppbor::parse(sessionTranscript);

        if (item == nullptr) {
@@ -283,7 +284,7 @@ ndk::ScopedAStatus IdentityCredential::startRetrieval( 
                    IIdentityCredentialStore::STATUS_INVALID_DATA,

                    "SessionTranscript contains invalid CBOR"));

        }

        sessionTranscriptItem_ = std::move(item);

        sessionTranscriptItem = std::move(item);

    }

    if (numStartRetrievalCalls_ > 0) {

        if (sessionTranscript_ != sessionTranscript) {
@@ -323,7 +324,7 @@ ndk::ScopedAStatus IdentityCredential::startRetrieval( 
        vector<uint8_t> encodedReaderAuthentication =

                cppbor::Array()

                        .add("ReaderAuthentication")

                        .add(sessionTranscriptItem_->clone())

                        .add(std::move(sessionTranscriptItem))

                        .add(cppbor::Semantic(24, itemsRequestBytes))

                        .encode();

        vector<uint8_t> encodedReaderAuthenticationBytes =
@@ -781,13 +782,6 @@ ndk::ScopedAStatus IdentityCredential::finishRetrieval(vector<int8_t>* outMac, 
    optional<vector<uint8_t>> mac;

    if (signingKeyBlob_.size() > 0 && sessionTranscript_.size() > 0 &&

        readerPublicKey_.size() > 0) {

        cppbor::Array array;

        array.add("DeviceAuthentication");

        array.add(sessionTranscriptItem_->clone());

        array.add(docType_);

        array.add(cppbor::Semantic(24, encodedDeviceNameSpaces));

        vector<uint8_t> deviceAuthenticationBytes = cppbor::Semantic(24, array.encode()).encode();



        vector<uint8_t> docTypeAsBlob(docType_.begin(), docType_.end());

        optional<vector<uint8_t>> signingKey =

                support::decryptAes128Gcm(storageKey_, signingKeyBlob_, docTypeAsBlob);
@@ -797,31 +791,15 @@ ndk::ScopedAStatus IdentityCredential::finishRetrieval(vector<int8_t>* outMac, 
                    "Error decrypting signingKeyBlob"));

        }



        optional<vector<uint8_t>> sharedSecret =

                support::ecdh(readerPublicKey_, signingKey.value());

        if (!sharedSecret) {

            return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage(

                    IIdentityCredentialStore::STATUS_FAILED, "Error doing ECDH"));

        }



        // Mix-in SessionTranscriptBytes

        vector<uint8_t> sessionTranscriptBytes = cppbor::Semantic(24, sessionTranscript_).encode();

        vector<uint8_t> sharedSecretWithSessionTranscriptBytes = sharedSecret.value();

        std::copy(sessionTranscriptBytes.begin(), sessionTranscriptBytes.end(),

                  std::back_inserter(sharedSecretWithSessionTranscriptBytes));



        vector<uint8_t> salt = {0x00};

        vector<uint8_t> info = {};

        optional<vector<uint8_t>> derivedKey =

                support::hkdf(sharedSecretWithSessionTranscriptBytes, salt, info, 32);

        if (!derivedKey) {

        optional<vector<uint8_t>> eMacKey =

                support::calcEMacKey(signingKey.value(), readerPublicKey_, sessionTranscriptBytes);

        if (!eMacKey) {

            return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage(

                    IIdentityCredentialStore::STATUS_FAILED,

                    "Error deriving key from shared secret"));

                    IIdentityCredentialStore::STATUS_FAILED, "Error calculating EMacKey"));

        }



        mac = support::coseMac0(derivedKey.value(), {},      // payload

                                deviceAuthenticationBytes);  // detached content

        mac = support::calcMac(sessionTranscript_, docType_, encodedDeviceNameSpaces,

                               eMacKey.value());

        if (!mac) {

            return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage(

                    IIdentityCredentialStore::STATUS_FAILED, "Error MACing data"));
@@ -835,9 +813,9 @@ ndk::ScopedAStatus IdentityCredential::finishRetrieval(vector<int8_t>* outMac, 


ndk::ScopedAStatus IdentityCredential::generateSigningKeyPair(

        vector<int8_t>* outSigningKeyBlob, Certificate* outSigningKeyCertificate) {

    string serialDecimal = "0";  // TODO: set serial to something unique

    string issuer = "Android Open Source Project";

    string subject = "Android IdentityCredential Reference Implementation";

    string serialDecimal = "1";

    string issuer = "Android Identity Credential Key";

    string subject = "Android Identity Credential Authentication Key";

    time_t validityNotBefore = time(nullptr);

    time_t validityNotAfter = validityNotBefore + 365 * 24 * 3600;

identity/aidl/default/IdentityCredential.h

+0 −1
Original line number Diff line number Diff line
@@ -103,7 +103,6 @@ class IdentityCredential : public BnIdentityCredential { 
    map<int32_t, int> profileIdToAccessCheckResult_;

    vector<uint8_t> signingKeyBlob_;

    vector<uint8_t> sessionTranscript_;

    std::unique_ptr<cppbor::Item> sessionTranscriptItem_;

    vector<uint8_t> itemsRequest_;

    vector<int32_t> requestCountsRemaining_;

    map<string, set<string>> requestedNameSpacesAndNames_;

identity/aidl/default/WritableIdentityCredential.cpp

+1 −1
Original line number Diff line number Diff line
@@ -74,7 +74,7 @@ ndk::ScopedAStatus WritableIdentityCredential::getAttestationCertificate( 
    vector<uint8_t> appId(attestationApplicationId.begin(), attestationApplicationId.end());



    optional<std::pair<vector<uint8_t>, vector<vector<uint8_t>>>> keyAttestationPair =

            support::createEcKeyPairAndAttestation(challenge, appId);

            support::createEcKeyPairAndAttestation(challenge, appId, testCredential_);

    if (!keyAttestationPair) {

        LOG(ERROR) << "Error creating credentialKey and attestation";

        return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage(