From a9ce01cfed8367e01ea8a2ec5dd142229b5cf4d4 Mon Sep 17 00:00:00 2001
From: Eran Messeri <eranm@google.com>
Date: Thu, 27 May 2021 15:08:03 +0100
Subject: [PATCH] Improve unique attestation docs & tests

Improve the documentation and tests related to device-unique
attestation on StrongBox KeyMint devices:
* Test that the chain produced is exactly of length 2.
* Document how the chain needs to be structured.
* Explain the trust properties of the key used for the
  self-signed root.

Test: atest VtsAidlKeyMintTargetTest
Bug: 187803288
Ignore-AOSP-First: Already merged in AOSP
Merged-In: I09bb16d6938b567c114485d2df00bde9d3e1ccf9
Change-Id: Ib7efdd428ce5a2e14c281077e3a77048c9721702
---
 .../hardware/security/keymint/Tag.aidl        | 20 ++++++++++++++-----
 .../DeviceUniqueAttestationTest.cpp           |  5 ++++-
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl
index 4ff4574b58..58e02b35b2 100644
--- a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl
+++ b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl
@@ -831,14 +831,24 @@ enum Tag {
     /**
      * DEVICE_UNIQUE_ATTESTATION is an argument to IKeyMintDevice::attested key generation/import
      * operations.  It indicates that attestation using a device-unique key is requested, rather
-     * than a batch key.  When a device-unique key is used, only the attestation certificate is
-     * returned; no additional chained certificates are provided.  It's up to the caller to
-     * recognize the device-unique signing key.  Only SecurityLevel::STRONGBOX IKeyMintDevices may
-     * support device-unique attestations.  SecurityLevel::TRUSTED_ENVIRONMENT IKeyMintDevices must
-     * return ErrorCode::INVALID_ARGUMENT if they receive DEVICE_UNIQUE_ATTESTATION.
+     * than a batch key.  When a device-unique key is used, the returned chain should contain two
+     * certificates:
+     *    * The attestation certificate, containing the attestation extension, as described in
+            KeyCreationResult.aidl.
+     *    * A self-signed root certificate, signed by the device-unique key.
+     * No additional chained certificates are provided. Only SecurityLevel::STRONGBOX
+     * IKeyMintDevices may support device-unique attestations.  SecurityLevel::TRUSTED_ENVIRONMENT
+     * IKeyMintDevices must return ErrorCode::INVALID_ARGUMENT if they receive
+     * DEVICE_UNIQUE_ATTESTATION.
      * SecurityLevel::STRONGBOX IKeyMintDevices need not support DEVICE_UNIQUE_ATTESTATION, and
      * return ErrorCode::CANNOT_ATTEST_IDS if they do not support it.
      *
+     * The caller needs to obtain the device-unique keys out-of-band and compare them against the
+     * key used to sign the self-signed root certificate.
+     * To ease this process, the IKeyMintDevice implementation should include, both in the subject
+     * and issuer fields of the self-signed root, the unique identifier of the device. Using the
+     * unique identifier will make it straightforward for the caller to link a device to its key.
+     *
      * IKeyMintDevice implementations that support device-unique attestation MUST add the
      * DEVICE_UNIQUE_ATTESTATION tag to device-unique attestations.
      */
diff --git a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp
index b0f056a68c..732d9ebadd 100644
--- a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp
+++ b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp
@@ -42,8 +42,11 @@ class DeviceUniqueAttestationTest : public KeyMintAidlTestBase {
 
         EXPECT_TRUE(crypto_params.Contains(TAG_KEY_SIZE, key_size)) << "Key size missing";
 
+        // The device-unique attestation chain should contain exactly two certificates:
+        // * The leaf with the attestation extension.
+        // * A self-signed root, signed using the device-unique key.
+        ASSERT_EQ(cert_chain_.size(), 2);
         EXPECT_TRUE(ChainSignaturesAreValid(cert_chain_));
-        ASSERT_GT(cert_chain_.size(), 0);
 
         AuthorizationSet sw_enforced = SwEnforcedAuthorizations(key_characteristics);
         EXPECT_TRUE(verify_attestation_record("challenge", "foo", sw_enforced, hw_enforced,