From 87cb07bd0868874208cd8f48d19ca98ae51610ce Mon Sep 17 00:00:00 2001 From: David Zeuthen Date: Thu, 30 Jan 2020 16:15:50 -0500 Subject: [PATCH] Identity Credential: Require passing applicationId when generating attestation. Since the attestation format includes the applicationId, we need this to be passed from credstore. Also clarify other requirements about what needs to be in the attestation data. Bug: 111446262 Test: atest android.security.identity.cts Test: VtsHalIdentityCredentialTargetTest Test: android.hardware.identity-support-lib-test Change-Id: I623849bd61e55752a573002dc7a97c6658d94c91 --- current.txt | 2 +- identity/1.0/IWritableIdentityCredential.hal | 52 ++++++++++++++++--- .../default/WritableIdentityCredential.cpp | 16 +++++- .../1.0/default/WritableIdentityCredential.h | 3 +- .../VtsHalIdentityCredentialTargetTest.cpp | 11 ++-- 5 files changed, 70 insertions(+), 14 deletions(-) diff --git a/current.txt b/current.txt index 89a0cbc390..dbca2fc6eb 100644 --- a/current.txt +++ b/current.txt @@ -618,7 +618,7 @@ db47f4ceceb1f06c656f39caa70c557b0f8471ef59fd58611bea667ffca20101 android.hardwar 0589e410f519e36514e7ece18f283f022df0f70efd2c12821d822f67f74aba98 android.hardware.identity@1.0::types bbeee9604128ede83ee755b67e73b5ad29e6e1dbac9ec41fea6ffe2745b0c50a android.hardware.identity@1.0::IIdentityCredential 96ce8aad80f4c476f25261f790d357c117e79e18474c7dadd850dac704bbe65e android.hardware.identity@1.0::IIdentityCredentialStore -6e1e28a96c90ba78d47257faea3f3bb4e6360affbbfa5822f0dc31211f9266ff android.hardware.identity@1.0::IWritableIdentityCredential +8da9c938e58f7d636ddd2f92c646f99d9a9e79612e6441b6380ab12744251873 android.hardware.identity@1.0::IWritableIdentityCredential 27ae3724053940462114228872b3ffaf0b8e6177d5ba97f5a76339d12b8a99dd android.hardware.keymaster@4.1::IKeymasterDevice adb0efdf1462e9b2e742c0dcadd598666aac551f178be06e755bfcdf5797abd0 android.hardware.keymaster@4.1::IOperation ac429fca0da4ce91218768ec31b64ded88251f8a26d8c4f27c06abdc5b1926d9 android.hardware.keymaster@4.1::types diff --git a/identity/1.0/IWritableIdentityCredential.hal b/identity/1.0/IWritableIdentityCredential.hal index b1ce00de4f..f26f76349f 100644 --- a/identity/1.0/IWritableIdentityCredential.hal +++ b/identity/1.0/IWritableIdentityCredential.hal @@ -26,20 +26,56 @@ interface IWritableIdentityCredential { * characteristics to an issuing authority. Must not be called more than once. * * The certificate chain must be generated using Keymaster Attestation - * (see https://source.android.com/security/keystore/attestation) and must also - * have the Tag::IDENTITY_CREDENTIAL_KEY tag from KeyMaster 4.1 set. This tag indicates - * that this 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). + * (see https://source.android.com/security/keystore/attestation) with the + * following additional requirements: + * + * - The attestationVersion field in the attestation extension must be at least 3. + * + * - 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). + * + * - 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 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 attestationChallenge field must be set to the passed-in challenge. + * + * - The uniqueId field must be empty. + * + * - The softwareEnforced field in the attestation extension must include + * Tag::ATTESTATION_APPLICATION_ID which must be set to the bytes of the passed-in + * attestationApplicationId. + * + * - The teeEnforced field in the attestation extension must include + * Tag::IDENTITY_CREDENTIAL_KEY. This tag 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). + * + * Additional authorizations may be needed in the softwareEnforced and teeEnforced + * fields - the above is not an exhaustive list. + * + * @param attestationApplicationId is the DER encoded value to be stored + * in Tag::ATTESTATION_APPLICATION_ID. This schema is described in + * https://developer.android.com/training/articles/security-key-attestation#certificate_schema_attestationid * * @param attestationChallenge a challenge set by the issuer to ensure freshness. * * @return result is OK on success, FAILED if an error occurred. * - * @return certificate is the X.509 certificate chain for the credentialKey + * @return certificateChain is the X.509 certificate chain for the credentialKey */ - getAttestationCertificate(vec attestationChallenge) - generates(Result result, vec certificate); + getAttestationCertificate(vec attestationApplicationId, + vec attestationChallenge) + generates(Result result, vec> certificateChain); /** * Start the personalization process. diff --git a/identity/1.0/default/WritableIdentityCredential.cpp b/identity/1.0/default/WritableIdentityCredential.cpp index 548b4c071a..4c39f85eb3 100644 --- a/identity/1.0/default/WritableIdentityCredential.cpp +++ b/identity/1.0/default/WritableIdentityCredential.cpp @@ -108,7 +108,12 @@ bool WritableIdentityCredential::initialize() { return true; } +// TODO: use |attestationApplicationId| and |attestationChallenge| and also +// ensure the returned certificate chain satisfy the requirements listed in +// the docs for IWritableIdentityCredential::getAttestationCertificate() +// Return WritableIdentityCredential::getAttestationCertificate( + const hidl_vec& /* attestationApplicationId */, const hidl_vec& /* attestationChallenge */, getAttestationCertificate_cb _hidl_cb) { // For now, we dynamically generate an attestion key on each and every @@ -181,7 +186,16 @@ Return WritableIdentityCredential::getAttestationCertificate( certificateChain.insert(certificateChain.end(), attestationKeyCertificate.value().begin(), attestationKeyCertificate.value().end()); - _hidl_cb(support::resultOK(), certificateChain); + optional>> splitCertChain = + support::certificateChainSplit(certificateChain); + if (!splitCertChain) { + _hidl_cb(support::result(ResultCode::FAILED, "Error splitting certificate chain"), {}); + return Void(); + } + hidl_vec> ret; + ret.resize(splitCertChain.value().size()); + std::copy(splitCertChain.value().begin(), splitCertChain.value().end(), ret.begin()); + _hidl_cb(support::resultOK(), ret); return Void(); } diff --git a/identity/1.0/default/WritableIdentityCredential.h b/identity/1.0/default/WritableIdentityCredential.h index 9f4e303ac4..b1deb16750 100644 --- a/identity/1.0/default/WritableIdentityCredential.h +++ b/identity/1.0/default/WritableIdentityCredential.h @@ -51,7 +51,8 @@ class WritableIdentityCredential : public IWritableIdentityCredential { // Methods from ::android::hardware::identity::IWritableIdentityCredential // follow. - Return getAttestationCertificate(const hidl_vec& attestationChallenge, + Return getAttestationCertificate(const hidl_vec& attestationApplicationId, + const hidl_vec& attestationChallenge, getAttestationCertificate_cb _hidl_cb) override; Return startPersonalization(uint16_t accessControlProfileCount, diff --git a/identity/1.0/vts/functional/VtsHalIdentityCredentialTargetTest.cpp b/identity/1.0/vts/functional/VtsHalIdentityCredentialTargetTest.cpp index 903e912b83..88b06df046 100644 --- a/identity/1.0/vts/functional/VtsHalIdentityCredentialTargetTest.cpp +++ b/identity/1.0/vts/functional/VtsHalIdentityCredentialTargetTest.cpp @@ -201,13 +201,18 @@ TEST_P(IdentityCredentialStoreHidlTest, createAndRetrieveCredential) { ASSERT_NE(writableCredential, nullptr); string challenge = "attestationChallenge"; + // TODO: set it to something random and check it's in the cert chain + vector attestationApplicationId = {}; vector attestationChallenge(challenge.begin(), challenge.end()); vector attestationCertificate; writableCredential->getAttestationCertificate( - attestationChallenge, - [&](const Result& _result, const hidl_vec& _attestationCertificate) { + attestationApplicationId, attestationChallenge, + [&](const Result& _result, const hidl_vec>& _splitCertChain) { result = _result; - attestationCertificate = _attestationCertificate; + vector> splitCerts; + std::copy(_splitCertChain.begin(), _splitCertChain.end(), + std::back_inserter(splitCerts)); + attestationCertificate = support::certificateChainJoin(splitCerts); }); EXPECT_EQ("", result.message); ASSERT_EQ(ResultCode::OK, result.code);