Add mechanism for adding debug logging.

PiperOrigin-RevId: 809000598

Author: carmenyh

src/main/kotlin/Extension.kt

@@ -97,14 +97,14 @@ data class ProvisioningInfoMap(
 }
 
 data class DeviceIdentity(
-  val brand: String?,
-  val device: String?,
-  val product: String?,
-  val serialNumber: String?,
-  val imeis: Set<String>,
-  val meid: String?,
-  val manufacturer: String?,
-  val model: String?,
+  val brand: String? = null,
+  val device: String? = null,
+  val product: String? = null,
+  val serialNumber: String? = null,
+  val imeis: Set<String> = emptySet(),
+  val meid: String? = null,
+  val manufacturer: String? = null,
+  val model: String? = null,
 ) {
   companion object {
     @JvmStatic
@@ -160,23 +160,25 @@ data class KeyDescription(
     @JvmField val OID = ASN1ObjectIdentifier("1.3.6.1.4.1.11129.2.1.17")
 
     @JvmStatic
-    fun parseFrom(cert: X509Certificate) =
+    @JvmOverloads
+    fun parseFrom(cert: X509Certificate, logFn: (String) -> Unit = {}) =
       cert
         .getExtensionValue(OID.id)
         .let { ASN1OctetString.getInstance(it).octets }
-        .let { parseFrom(it) }
+        .let { parseFrom(it, logFn) }
 
     @JvmStatic
-    fun parseFrom(bytes: ByteArray) =
+    @JvmOverloads
+    fun parseFrom(bytes: ByteArray, logFn: (String) -> Unit = {}) =
       try {
-        from(ASN1Sequence.getInstance(bytes))
+        from(ASN1Sequence.getInstance(bytes), logFn)
       } catch (e: NullPointerException) {
         // Workaround for a NPE in BouncyCastle.
         // https://github.com/bcgit/bc-java/blob/228211ecb973fe87fdd0fc4ab16ba0446ec1a29c/core/src/main/java/org/bouncycastle/asn1/ASN1UniversalType.java#L24
         throw IllegalArgumentException(e)
       }
 
-    private fun from(seq: ASN1Sequence): KeyDescription {
+    private fun from(seq: ASN1Sequence, logFn: (String) -> Unit = {}): KeyDescription {
       require(seq.size() == 8)
       return KeyDescription(
         attestationVersion = seq.getObjectAt(0).toInt(),
@@ -185,8 +187,8 @@ data class KeyDescription(
         keyMintSecurityLevel = seq.getObjectAt(3).toSecurityLevel(),
         attestationChallenge = seq.getObjectAt(4).toByteString(),
         uniqueId = seq.getObjectAt(5).toByteString(),
-        softwareEnforced = seq.getObjectAt(6).toAuthorizationList(),
-        hardwareEnforced = seq.getObjectAt(7).toAuthorizationList(),
+        softwareEnforced = seq.getObjectAt(6).toAuthorizationList(logFn),
+        hardwareEnforced = seq.getObjectAt(7).toAuthorizationList(logFn),
       )
     }
   }
@@ -218,7 +220,7 @@ enum class Origin(val value: Long) {
   RESERVED(3),
   SECURELY_IMPORTED(4);
 
-  internal fun toAsn1() = ASN1Integer(value)
+  fun toAsn1() = ASN1Integer(value)
 }
 
 /**
@@ -397,7 +399,7 @@ data class AuthorizationList(
       .let { DERSequence(it.toTypedArray()) }
 
   internal companion object {
-    fun from(seq: ASN1Sequence, validateTagOrder: Boolean = false): AuthorizationList {
+    fun from(seq: ASN1Sequence, logFn: (String) -> Unit = { _ -> }): AuthorizationList {
       val objects =
         seq.associate {
           require(it is ASN1TaggedObject) {
@@ -417,9 +419,8 @@ data class AuthorizationList(
        * 2. within each class of tags, the elements or alternatives shall appear in ascending order
        *    of their tag numbers.
        */
-      // TODO: b/356172932 - Add test data once an example certificate is found in the wild.
-      if (validateTagOrder && !objects.keys.zipWithNext().all { (lhs, rhs) -> rhs > lhs }) {
-        throw IllegalArgumentException("AuthorizationList tags must appear in ascending order")
+      if (!objects.keys.zipWithNext().all { (lhs, rhs) -> rhs > lhs }) {
+        logFn("AuthorizationList tags should appear in ascending order")
       }
 
       return AuthorizationList(
@@ -449,7 +450,7 @@ data class AuthorizationList(
         rollbackResistant = if (objects.containsKey(KeyMintTag.ROLLBACK_RESISTANT)) true else null,
         rootOfTrust = objects[KeyMintTag.ROOT_OF_TRUST]?.toRootOfTrust(),
         osVersion = objects[KeyMintTag.OS_VERSION]?.toInt(),
-        osPatchLevel = objects[KeyMintTag.OS_PATCH_LEVEL]?.toPatchLevel(),
+        osPatchLevel = objects[KeyMintTag.OS_PATCH_LEVEL]?.toPatchLevel("OS", logFn),
         attestationApplicationId =
           objects[KeyMintTag.ATTESTATION_APPLICATION_ID]?.toAttestationApplicationId(),
         attestationIdBrand = objects[KeyMintTag.ATTESTATION_ID_BRAND]?.toStr(),
@@ -460,8 +461,8 @@ data class AuthorizationList(
         attestationIdMeid = objects[KeyMintTag.ATTESTATION_ID_MEID]?.toStr(),
         attestationIdManufacturer = objects[KeyMintTag.ATTESTATION_ID_MANUFACTURER]?.toStr(),
         attestationIdModel = objects[KeyMintTag.ATTESTATION_ID_MODEL]?.toStr(),
-        vendorPatchLevel = objects[KeyMintTag.VENDOR_PATCH_LEVEL]?.toPatchLevel(),
-        bootPatchLevel = objects[KeyMintTag.BOOT_PATCH_LEVEL]?.toPatchLevel(),
+        vendorPatchLevel = objects[KeyMintTag.VENDOR_PATCH_LEVEL]?.toPatchLevel("vendor", logFn),
+        bootPatchLevel = objects[KeyMintTag.BOOT_PATCH_LEVEL]?.toPatchLevel("boot", logFn),
         attestationIdSecondImei = objects[KeyMintTag.ATTESTATION_ID_SECOND_IMEI]?.toStr(),
         moduleHash = objects[KeyMintTag.MODULE_HASH]?.toByteString(),
       )
@@ -480,14 +481,24 @@ data class PatchLevel(val yearMonth: YearMonth, val version: Int? = null) {
   }
 
   companion object {
-    fun from(patchLevel: ASN1Encodable): PatchLevel? {
+    fun from(
+      patchLevel: ASN1Encodable,
+      partitionName: String = "",
+      logFn: (String) -> Unit = { _ -> },
+    ): PatchLevel? {
       check(patchLevel is ASN1Integer) { "Must be an ASN1Integer, was ${this::class.simpleName}" }
-      return from(patchLevel.value.toString())
+      return from(patchLevel.value.toString(), partitionName, logFn)
     }
 
     @JvmStatic
-    fun from(patchLevel: String): PatchLevel? {
+    @JvmOverloads
+    fun from(
+      patchLevel: String,
+      partitionName: String = "",
+      logFn: (String) -> Unit = { _ -> },
+    ): PatchLevel? {
       if (patchLevel.length != 6 && patchLevel.length != 8) {
+        logFn("Invalid $partitionName patch level: $patchLevel")
         return null
       }
       try {
@@ -496,6 +507,7 @@ data class PatchLevel(val yearMonth: YearMonth, val version: Int? = null) {
         val version = if (patchLevel.length == 8) patchLevel.substring(6).toInt() else null
         return PatchLevel(yearMonth, version)
       } catch (e: DateTimeParseException) {
+        logFn("Invalid $partitionName patch level: $patchLevel")
         return null
       }
     }
@@ -625,12 +637,9 @@ private fun ASN1Encodable.toAttestationApplicationId(): AttestationApplicationId
   return AttestationApplicationId.from(ASN1Sequence.getInstance(this.octets))
 }
 
-// TODO: b/356172932 - `validateTagOrder` should default to true after making it user configurable.
-private fun ASN1Encodable.toAuthorizationList(
-  validateTagOrder: Boolean = false
-): AuthorizationList {
+private fun ASN1Encodable.toAuthorizationList(logFn: (String) -> Unit): AuthorizationList {
   check(this is ASN1Sequence) { "Object must be an ASN1Sequence, was ${this::class.simpleName}" }
-  return AuthorizationList.from(this, validateTagOrder)
+  return AuthorizationList.from(this, logFn)
 }
 
 private fun ASN1Encodable.toBoolean(): Boolean {
@@ -657,7 +666,10 @@ private fun ASN1Encodable.toInt(): BigInteger {
   return this.value
 }
 
-private fun ASN1Encodable.toPatchLevel(): PatchLevel? = PatchLevel.from(this)
+private fun ASN1Encodable.toPatchLevel(
+  partitionName: String = "",
+  logFn: (String) -> Unit = { _ -> },
+): PatchLevel? = PatchLevel.from(this, partitionName, logFn)
 
 private fun ASN1Encodable.toRootOfTrust(): RootOfTrust {
   check(this is ASN1Sequence) { "Object must be an ASN1Sequence, was ${this::class.simpleName}" }

src/main/kotlin/Verifier.kt

@@ -22,6 +22,7 @@ import com.android.keyattestation.verifier.provider.ProvisioningMethod
 import com.android.keyattestation.verifier.provider.RevocationChecker
 import com.google.errorprone.annotations.ThreadSafe
 import com.google.protobuf.ByteString
+import com.google.protobuf.kotlin.toByteString
 import java.security.PublicKey
 import java.security.Security
 import java.security.cert.CertPathValidator
@@ -47,21 +48,71 @@ sealed interface VerificationResult {
 
   data class PathValidationFailure(val cause: CertPathValidatorException) : VerificationResult
 
-  data object ChainParsingFailure : VerificationResult
+  data class ChainParsingFailure(val cause: Exception) : VerificationResult
 
   data class ExtensionParsingFailure(val cause: Exception) : VerificationResult
 
   data class ExtensionConstraintViolation(val cause: String) : VerificationResult
 }
 
+/** Interface for logging info level key attestation events and information. */
+interface LogHook {
+
+  /**
+   * Logs the certificate chain which is being verified. Called for each call to [verify].
+   *
+   * @param inputChain The certificate chain which is being verified.
+   */
+  fun logInputChain(inputChain: List<ByteString>)
+
+  /**
+   * Logs the result of the verification. Called for each call to [verify].
+   *
+   * @param result The result of the verification.
+   */
+  fun logResult(result: VerificationResult)
+
+  /**
+   * Logs the key description of the leaf certificate. Called if [verify] reaches the point where
+   * the key description is parsed.
+   *
+   * @param keyDescription The key description of the leaf certificate.
+   */
+  fun logKeyDescription(keyDescription: KeyDescription)
+
+  /**
+   * Logs the provisioning info map extension of the attestation certificate. Called if [verify]
+   * reaches the point where the provisioning info map is parsed, if present in the attestation
+   * certificate.
+   *
+   * @param provisioningInfoMap The provisioning info map extension of the leaf certificate.
+   */
+  fun logProvisioningInfoMap(provisioningInfoMap: ProvisioningInfoMap)
+
+  /**
+   * Logs the serial numbers of the intermediate certificates in the certificate chain. Called if
+   * [verify] reaches the point where the certificate chain is parsed.
+   *
+   * @param certSerialNumbers The serial numbers of the intermediate certificates in the certificate
+   *   chain.
+   */
+  fun logCertSerialNumbers(certSerialNumbers: List<String>)
+
+  /**
+   * Logs an info level message. May be called throughout the verification process.
+   *
+   * @param infoMessage The info level message to log.
+   */
+  fun logInfoMessage(infoMessage: String)
+}
+
 /**
  * Verifier for Android Key Attestation certificate chain.
  *
  * https://developer.android.com/privacy-and-security/security-key-attestation
  *
  * @param anchor a [TrustAnchor] to use for certificate path verification.
  */
-// TODO: b/356234568 - Verify intermediate certificate revocation status.
 @ThreadSafe
 open class Verifier(
   private val trustAnchorsSource: () -> Set<TrustAnchor>,
@@ -83,14 +134,20 @@ open class Verifier(
   fun verify(
     chain: List<X509Certificate>,
     challengeChecker: ChallengeChecker? = null,
+    log: LogHook? = null,
   ): VerificationResult {
     val certPath =
       try {
         KeyAttestationCertPath(chain)
       } catch (e: Exception) {
-        return VerificationResult.ChainParsingFailure
+        val result = VerificationResult.ChainParsingFailure(e)
+        log?.logInputChain(chain.map { it.getEncoded().toByteString() })
+        log?.logResult(result)
+        return result
       }
-    return verify(certPath, challengeChecker)
+    val result = internalVerify(certPath, challengeChecker, log)
+    log?.logResult(result)
+    return result
   }
 
   /**
@@ -99,21 +156,44 @@ open class Verifier(
    * @param chain The attestation certificate chain to verify.
    * @param challengeChecker The challenge checker to use for additional validation of the challenge
    *   in the attestation chain.
-   * @return [VerificationResult]
-   *
-   * TODO: b/366058500 - Make the challenge required after Apparat's changes are rollback safe.
+   * @return [VerificationEvent]
    */
   @JvmOverloads
   fun verify(
     certPath: KeyAttestationCertPath,
     challengeChecker: ChallengeChecker? = null,
+    log: LogHook? = null,
+  ): VerificationResult {
+    val result = internalVerify(certPath, challengeChecker, log)
+    log?.logResult(result)
+    return result
+  }
+
+  internal fun internalVerify(
+    certPath: KeyAttestationCertPath,
+    challengeChecker: ChallengeChecker? = null,
+    log: LogHook? = null,
   ): VerificationResult {
+    log?.logInputChain(certPath.certificatesWithAnchor.map { it.getEncoded().toByteString() })
+    log?.logCertSerialNumbers(
+      certPath.certificatesWithAnchor.subList(1, certPath.certificatesWithAnchor.size).map {
+        it.serialNumber.toString(16)
+      }
+    )
     val certPathValidator = CertPathValidator.getInstance("KeyAttestation")
     val certPathParameters =
       PKIXParameters(trustAnchorsSource()).apply {
         date = Date.from(instantSource.instant())
         addCertPathChecker(RevocationChecker(revokedSerialsSource()))
       }
+
+    val deviceInformation =
+      if (certPath.provisioningMethod() == ProvisioningMethod.REMOTELY_PROVISIONED) {
+        certPath.attestationCert().provisioningInfo()
+      } else {
+        null
+      }
+    deviceInformation?.let { log?.logProvisioningInfoMap(it) }
     val pathValidationResult =
       try {
         certPathValidator.validate(certPath, certPathParameters) as PKIXCertPathValidatorResult
@@ -123,11 +203,15 @@ open class Verifier(
 
     val keyDescription =
       try {
-        checkNotNull(certPath.leafCert().keyDescription()) { "Key attestation extension not found" }
+        checkNotNull(
+          KeyDescription.parseFrom(certPath.leafCert(), { msg -> log?.logInfoMessage(msg) })
+        ) {
+          "Key attestation extension not found"
+        }
       } catch (e: Exception) {
         return VerificationResult.ExtensionParsingFailure(e)
       }
-
+    log?.logKeyDescription(keyDescription)
     if (
       challengeChecker != null &&
         !challengeChecker.checkChallenge(keyDescription.attestationChallenge)
@@ -157,12 +241,6 @@ open class Verifier(
         ?: return VerificationResult.ExtensionConstraintViolation(
           "hardwareEnforced.rootOfTrust is null"
         )
-    val deviceInformation =
-      if (certPath.provisioningMethod() == ProvisioningMethod.REMOTELY_PROVISIONED) {
-        certPath.attestationCert().provisioningInfo()
-      } else {
-        null
-      }
     return VerificationResult.Success(
       pathValidationResult.publicKey,
       keyDescription.attestationChallenge,

src/main/kotlin/provider/KeyAttestationCertPath.kt

@@ -49,7 +49,10 @@ class KeyAttestationCertPath(certs: List<X509Certificate>) : CertPath("X.509") {
     when (certs.indexOfLast { it.hasAttestationExtension() }) {
       0 -> {} // expected value
       -1 -> throw CertificateException("Attestation extension not found")
-      else -> throw CertificateException("Attestation extension on unexpected certificate")
+      else ->
+        if (certs[0].hasAttestationExtension())
+          throw CertificateException("Additional attestation extension found")
+        else throw CertificateException("Certificate after target certificate")
     }
     if (!certs.last().isSelfIssued()) throw CertificateException("Root certificate not found")
     this.certificatesWithAnchor = certs

src/main/kotlin/provider/KeyAttestationCertPathValidator.kt

@@ -253,11 +253,9 @@ private class BasicChecker(
 
   private fun verifyValidity(cert: X509Certificate) {
     /*
-     * KAVS does not check the validity of the final certificate in the path. For the purposes of
-     * migration this path validator is intended to be bug compatible with KAVS, so we do not check
-     * the validity of the final certificate either.
-     *
-     * TODO: b/355190989 - explore if is viable to check the validity of the final certificate.
+     * Do not check the validity of the final certificate in the path. The
+     * validity period of the final cert is set on the device so could both be
+     * subject to tampering and could be impacted by clock skew.
      */
     if (remainingCerts == 0) return
     try {

src/main/kotlin/provider/RevocationChecker.kt

@@ -45,7 +45,7 @@ class RevocationChecker(private val revokedSerials: Set<String>) : PKIXRevocatio
     if (revokedSerials.contains(cert.serialNumber.toString(16))) {
       // TODO: b/356234568 - Surface the revocation reason.
       throw CertPathValidatorException(
-        "Certificate has been revoked",
+        "Certificate has been revoked: ${cert.serialNumber}",
         null,
         null,
         -1,