From 4683a5383ffd8ac0e76b09d096b9a3d85dd0a379 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Fri, 2 Sep 2022 10:05:05 +0100 Subject: [PATCH 1/2] KeyMint HAL: clarify obsolete tags The tag enum names can't be removed due to AIDL back-compatibility requirements, and also it's useful to have the values present to avoid inadvertent reuse. Update the tag comment text to indicate that these tags are obsolete. Bug: 191738660 Test: TreeHugger, comment change only Change-Id: Icbd4c9cd0313f93bc491b49eb9077766d0f44e34 --- .../hardware/security/keymint/Tag.aidl | 37 +++++++------------ 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl index 871a1ac5b5..47361d5536 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl @@ -274,25 +274,10 @@ enum Tag { USAGE_EXPIRE_DATETIME = TagType.DATE | 402, /** - * Tag::MIN_SECONDS_BETWEEN_OPS specifies the minimum amount of time that elapses between - * allowed operations using a key. This can be used to rate-limit uses of keys in contexts - * where unlimited use may enable brute force attacks. + * OBSOLETE: Do not use. * - * The value is a 32-bit integer representing seconds between allowed operations. - * - * When a key with this tag is used in an operation, the IKeyMintDevice must start a timer - * during the finish() or abort() call. Any call to begin() that is received before the timer - * indicates that the interval specified by Tag::MIN_SECONDS_BETWEEN_OPS has elapsed must fail - * with ErrorCode::KEY_RATE_LIMIT_EXCEEDED. This implies that the IKeyMintDevice must keep a - * table of use counters for keys with this tag. Because memory is often limited, this table - * may have a fixed maximum size and KeyMint may fail operations that attempt to use keys with - * this tag when the table is full. The table must accommodate at least 8 in-use keys and - * aggressively reuse table slots when key minimum-usage intervals expire. If an operation - * fails because the table is full, KeyMint returns ErrorCode::TOO_MANY_OPERATIONS. - * - * Must be hardware-enforced. - * - * TODO(b/191738660): Remove in KeyMint V2. Currently only used for FDE. + * This tag value is included for historical reason, as it was present in Keymaster. + * KeyMint implementations do not need to support this tag. */ MIN_SECONDS_BETWEEN_OPS = TagType.UINT | 403, @@ -898,8 +883,12 @@ enum Tag { STORAGE_KEY = TagType.BOOL | 722, /** - * OBSOLETE: Do not use. See IKeyMintOperation.updateAad instead. - * TODO(b/191738660): Remove in KeyMint v2. + * OBSOLETE: Do not use. + * + * This tag value is included for historical reasons -- in Keymaster it was used to hold + * associated data for AEAD encryption, as an additional parameter to + * IKeymasterDevice::finish(). In KeyMint the IKeyMintOperation::updateAad() method is used for + * this. */ ASSOCIATED_DATA = TagType.BYTES | 1000, @@ -938,10 +927,12 @@ enum Tag { RESET_SINCE_ID_ROTATION = TagType.BOOL | 1004, /** - * OBSOLETE: Do not use. See the authToken parameter for IKeyMintDevice::begin and for - * IKeyMintOperation methods instead. + * OBSOLETE: Do not use. * - * TODO(b/191738660): Delete when keystore1 is deleted. + * This tag value is included for historical reasons -- in Keymaster it was used to hold + * a confirmation token as an additional parameter to + * IKeymasterDevice::finish(). In KeyMint the IKeyMintOperation::finish() method includes + * a confirmationToken argument for this. */ CONFIRMATION_TOKEN = TagType.BYTES | 1005, From 7ea97a310a4b95cbd92d001a330a65fc69c96ff6 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Fri, 2 Sep 2022 10:08:40 +0100 Subject: [PATCH 2/2] KeyMint HAL: reinstate tags in extension schema Commit 93c72cef924e ("KeyMint: sync all attestation tags", http://aosp/1719302) removed various tags from the attestation that are only applicable to symmetric keys, on the assumption that these are irrelevant for the attestation extension that is generated for the certificate holding asymmetric public keys. However, that change did not take into account the fact that the AuthorizationList ASN.1 schema is re-used elsewhere in the KeyMint API, specifically as a way of describing the characteristics associated with a key that is being securely imported via IKeyMintDevice::importWrappedKey. That import process may be used for symmetrics keys, and so the tags that are specific to symmetric keys still need to be included in AuthorizationList. Similarly, USER_SECURE_ID values are never included in attestation extensions because they have no meaning off-device, but they may be needed as part of the import of a wrapped key. Test: TreeHugger, comment change only Bug: 244693617 Change-Id: Iaa941e120e3641a6e6c369b7c6a51f10b44df78a --- .../hardware/security/keymint/KeyCreationResult.aidl | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/security/keymint/aidl/android/hardware/security/keymint/KeyCreationResult.aidl b/security/keymint/aidl/android/hardware/security/keymint/KeyCreationResult.aidl index ae755791f6..4c2be89195 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/KeyCreationResult.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/KeyCreationResult.aidl @@ -158,12 +158,23 @@ parcelable KeyCreationResult { * Failed (3), * } * + * -- Note that the AuthorizationList SEQUENCE is also used in IKeyMintDevice::importWrappedKey + * -- as a way of describing the authorizations associated with a key that is being securely + * -- imported. As such, it includes the ability to describe tags that are only relevant for + * -- symmetric keys, and which will never appear in the attestation extension of an X.509 + * -- certificate that holds the public key part of an asymmetric keypair. Importing a wrapped + * -- key also allows the use of Tag::USER_SECURE_ID, which is never included in an attestation + * -- extension because it has no meaning off-device. + * * AuthorizationList ::= SEQUENCE { * purpose [1] EXPLICIT SET OF INTEGER OPTIONAL, * algorithm [2] EXPLICIT INTEGER OPTIONAL, * keySize [3] EXPLICIT INTEGER OPTIONAL, + * blockMode [4] EXPLICIT SET OF INTEGER OPTIONAL, -- symmetric only * digest [5] EXPLICIT SET OF INTEGER OPTIONAL, * padding [6] EXPLICIT SET OF INTEGER OPTIONAL, + * callerNonce [7] EXPLICIT NULL OPTIONAL, -- symmetric only + * minMacLength [8] EXPLICIT INTEGER OPTIONAL, -- symmetric only * ecCurve [10] EXPLICIT INTEGER OPTIONAL, * rsaPublicExponent [200] EXPLICIT INTEGER OPTIONAL, * mgfDigest [203] EXPLICIT SET OF INTEGER OPTIONAL, @@ -173,6 +184,7 @@ parcelable KeyCreationResult { * originationExpireDateTime [401] EXPLICIT INTEGER OPTIONAL, * usageExpireDateTime [402] EXPLICIT INTEGER OPTIONAL, * usageCountLimit [405] EXPLICIT INTEGER OPTIONAL, + * userSecureId [502] EXPLICIT INTEGER OPTIONAL, -- only used on import * noAuthRequired [503] EXPLICIT NULL OPTIONAL, * userAuthType [504] EXPLICIT INTEGER OPTIONAL, * authTimeout [505] EXPLICIT INTEGER OPTIONAL,