Add AttestationApplicationId checking.

README.md

@@ -10,17 +10,11 @@ A Kotlin library for verifying Android key attestation certificate chains.
 val verifier = Verifier(
   GoogleTrustAnchors,                  // Trust anchors source
   ::getGoogleRevocationStatusFromWeb,  // Revoked serials source
-  { Instant.now() },                   // Time source
-  ConstraintConfig(attestationApplicationId =
-      AttestationApplicationIdConstraint.ALLOW_UNKNOWN_PACKAGE(expectedAppId)),
-
+  { Instant.now() }                    // Time source
 )
 
 // Verify an attestation certificate chain
-val result = verifier.verify(
-  certificateChain,
-  challengeChecker,
-)
+val result = verifier.verify(certificateChain)
 
 // Handle the verification result
 when (result) {
@@ -35,47 +29,21 @@ when (result) {
   is VerificationResult.PathValidationFailure -> // Handle validation failure
   is VerificationResult.ChainParsingFailure -> // Handle parsing failure
   is VerificationResult.ExtensionParsingFailure -> // Handle extension parsing issues
-  is VerificationResult.ConstraintViolation -> // Handle constraint violations
+  is VerificationResult.ExtensionConstraintViolation -> // Handle constraint violations
 }
 ```
 
-### Choose a strong attestation challenge {#use-the-challenge}
-
-Correctly generating your attestation challenges can:
-
-  * Prevent replay attacks (where attackers use an attestation more than once).
-  * Set a time-bound on a replay/relay (where attackers use an attestation from
-    one device on another device) attacks.
-
-**Important:** A challenge alone, unless it is bound to the protocol in some
-other way, cannot entirely prevent relay attacks.
-
-#### Include information unique to the request in the challenge
-
-See the
-[PIA](https://developer.android.com/google/play/integrity/standard#protect-requests)
-documentation for guidance on how to choose a strong challenge by including
-information from the request in the challenge. Note that the equivalent of the
-attestation challenge is `requestHash` in the PIA context.
-
-#### Set time bounds for attestation validity
-
-It’s important that the attestation not be valid for eternity. The longer an
-attestation lives, the more likely it is to be used for a replay or relay
-attack.
-
-The easiest way to do this is to include a timestamp signed by your server-side
-code in the challenge. When you verify the attestation's challenge, you'll check
-the challenge signature and then make sure the timestamp is sufficiently fresh.
-
-#### Example implementations
-
-For example, if you expect the challenge to be equal to "challenge123", then
-usage would look like
+If there is additional verification you'd like to perform on the challenge
+associated with the attestation certificate chain, pass in a `ChallengeChecker`
+when verifying. For example, if you expect the challenge to be equal to
+"challenge123", then usage would look like
 
 ```kotlin
 // Create a ChallengeChecker
 val challengeChecker = ChallengeMatcher(ByteString.copyFromUtf8("challenge123"))
+
+// Verify an attestation certificate chain with the checker
+val result = verifier.verify(certificateChain, challengeChecker)
 ```
 
 If there are multiple checks to perform on the challenge, use a
@@ -108,17 +76,6 @@ against the `InMemoryLruCache` if the challenge doesn't match.
 If the implementations in `challengecheckers/` don't fit your needs, simply
 extend the `ChallengeChecker` interface.
 
-### Getting the expected Attestation Application Id
-
-It is important to check the attestation application ID when verifying a key
-attestation. This assures that you don't accept attestations for keys controlled
-by other applications, and can provide some assurance against relay attacks.
-
-The package list should be the names of all applications you expect to verify
-against and their minimum accepted version numbers. You can get the signature
-digests to put in in `signatures` from the Play Console as the app certificate
-digests.
-
 ## Building
 
 ```bash

src/main/kotlin/ConstraintConfig.kt

@@ -21,7 +21,6 @@ import com.android.keyattestation.verifier.provider.ProvisioningMethod
 import com.google.common.collect.ImmutableList
 import com.google.errorprone.annotations.Immutable
 import com.google.errorprone.annotations.ThreadSafe
-import java.math.BigInteger
 
 private typealias AttributeMapper = (KeyDescription) -> Any?
 
@@ -51,23 +50,13 @@ class ConstraintConfig
 @JvmOverloads
 constructor(
   val allowSoftwareRoot: Boolean = false,
-  val attestationApplicationId: AttestationApplicationIdConstraint =
-    AttestationApplicationIdConstraint.ALLOW_UNKNOWN_PACKAGE(
-      AttestationApplicationIdConstraint.DEFAULT_MAX_OS_VERSION
-    ),
   val keyOrigin: Constraint? = null,
   val securityLevel: Constraint? = null,
   val rootOfTrust: Constraint? = null,
   val additionalConstraints: ImmutableList<Constraint> = ImmutableList.of(),
   val inputLimits: InputLimits = InputLimits(),
 ) {
-  /**
-   * Returns the list of constraints for this [ConstraintConfig] which can be checked using only the
-   * [KeyDescription] and [KeyAttestationCertPath].
-   *
-   * @return The list of generic constraints.
-   */
-  fun getGenericConstraints(): ImmutableList<Constraint> =
+  fun getConstraints() =
     ImmutableList.builder<Constraint>()
       .add(
         keyOrigin
@@ -92,10 +81,6 @@ constructor(
  */
 class ConstraintConfigBuilder() {
   var allowSoftwareRoot: Boolean = false
-  var attestationApplicationId: AttestationApplicationIdConstraint =
-    AttestationApplicationIdConstraint.ALLOW_UNKNOWN_PACKAGE(
-      AttestationApplicationIdConstraint.DEFAULT_MAX_OS_VERSION
-    )
   var keyOrigin: Constraint? = null
   var securityLevel: Constraint? = null
   var rootOfTrust: Constraint? = null
@@ -114,10 +99,6 @@ class ConstraintConfigBuilder() {
     this.rootOfTrust = constraint()
   }
 
-  fun attestationApplicationId(constraint: () -> AttestationApplicationIdConstraint) {
-    this.attestationApplicationId = constraint()
-  }
-
   fun additionalConstraint(constraint: () -> Constraint) {
     additionalConstraints.add(constraint())
   }
@@ -129,7 +110,6 @@ class ConstraintConfigBuilder() {
   fun build(): ConstraintConfig =
     ConstraintConfig(
       allowSoftwareRoot,
-      attestationApplicationId,
       keyOrigin,
       securityLevel,
       rootOfTrust,
@@ -367,88 +347,3 @@ sealed class ProvisioningMethodConstraint(val provisioningMethod: ProvisioningMe
   @Immutable
   data object REMOTE : ProvisioningMethodConstraint(ProvisioningMethod.REMOTELY_PROVISIONED)
 }
-
-/**
- * Configuration for validating the attestation application ID in an Android attestation
- * certificate.
- */
-@Immutable
-sealed class AttestationApplicationIdConstraint(
-  val isSatisfied: (AttestationApplicationId?, AttestationApplicationId?, BigInteger?) -> Boolean
-) : Constraint {
-  companion object {
-    const val LABEL = "Attestation application ID"
-    val DEFAULT_MAX_OS_VERSION = BigInteger.valueOf(35)
-
-    fun hasUnknownPackage(id: AttestationApplicationId?): Boolean =
-      id != null && id.packages.any { it.name == "UnknownPackage" && it.version == BigInteger.ONE }
-  }
-
-  override val label = LABEL
-
-  /**
-   * This should never be called.
-   *
-   * [AttestationApplicationIdConstraint] should be checked outside of the genericConstraints list,
-   * using [check(KeyDescription, AttestationApplicationId)].
-   */
-  override fun check(description: KeyDescription, certPath: KeyAttestationCertPath) =
-    Constraint.Violated(
-      "This should never be called this way, please use the check method that takes an AttestationApplicationId."
-    )
-
-  fun check(
-    expectedAttestationApplicationId: AttestationApplicationId?,
-    description: KeyDescription,
-  ): Constraint.Result =
-    if (
-      isSatisfied(
-        expectedAttestationApplicationId,
-        description.softwareEnforced.attestationApplicationId,
-        description.hardwareEnforced.osVersion,
-      )
-    ) {
-      Constraint.Satisfied
-    } else {
-      Constraint.Violated(getFailureMessage(description.softwareEnforced.attestationApplicationId))
-    }
-
-  open fun getFailureMessage(attestationApplicationId: AttestationApplicationId?): String =
-    "$LABEL violates constraint: attestationApplicationId=$attestationApplicationId, config=$this"
-
-  /**
-   * Checks that the attestation application ID matches the expected value.
-   *
-   * This is the strictest form of attestation application ID check. There is no leniency for the OS
-   * version so it may fail on older devices.
-   *
-   * @param expectedId The expected value of the attestation application ID.
-   */
-  @Immutable
-  data object STRICT :
-    AttestationApplicationIdConstraint({ expectedId, actualId, _ ->
-      expectedId?.isSatisfiedBy(actualId) ?: true
-    })
-
-  /**
-   * Checks that the attestation application ID matches the expected value or that the OS version is
-   * at or below the maximum OS version.
-   *
-   * This is a lenient form of attestation application ID check. It allows for older devices to pass
-   * the check if they have an unknown package name.
-   *
-   * @param expectedId The expected value of the attestation application ID.
-   * @param maxOsVersion The maximum OS version that the device can be on to pass the check.
-   */
-  @Immutable
-  data class ALLOW_UNKNOWN_PACKAGE(val maxOsVersion: BigInteger) :
-    AttestationApplicationIdConstraint({ expectedId, actualId, osVersion ->
-      expectedId?.isSatisfiedBy(actualId) ?: true ||
-        (osVersion != null && osVersion <= maxOsVersion && hasUnknownPackage(actualId))
-    })
-
-  /**
-   * Does not check the attestation application ID. This should only be used for testing purposes.
-   */
-  @Immutable data object NONE : AttestationApplicationIdConstraint({ _, _, _ -> true })
-}

src/main/kotlin/Extension.kt

@@ -657,26 +657,6 @@ data class AttestationApplicationId(
       }
       .let { DERSequence(it.toTypedArray()) }
 
-  /**
-   * Checks if the AttestationApplicationId is satisfied by the [candidate]
-   * AttestationApplicationId.
-   *
-   * @param candidate The actual AttestationApplicationId.
-   * @return True if the candidate satisfies the AttestationApplicationId, false otherwise.
-   */
-  fun isSatisfiedBy(candidate: AttestationApplicationId?): Boolean {
-    if (candidate == null) return false
-
-    if (packages.isNotEmpty()) {
-      if (packages.none { it.isSatisfiedBy(candidate.packages) }) return false
-    }
-
-    if (signatures.isNotEmpty()) {
-      if (signatures.none { it in candidate.signatures }) return false
-    }
-    return true
-  }
-
   companion object {
     internal fun from(
       seq: ASN1Sequence,
@@ -721,10 +701,6 @@ data class AttestationPackageInfo(val name: String, val version: BigInteger) {
       }
       .let { DERSequence(it.toTypedArray()) }
 
-  internal fun isSatisfiedBy(candidate: Set<AttestationPackageInfo>) = candidate.any {
-    it.name == name && it.version >= version
-  }
-
   internal companion object {
     fun from(attestationPackageInfo: ASN1Sequence): AttestationPackageInfo {
       require(attestationPackageInfo.size() == 2) {

src/main/kotlin/Verifier.kt

@@ -151,8 +151,6 @@ constructor(
   private val trustAnchorsSource: () -> Set<TrustAnchor>,
   private val revokedSerialsSource: () -> Set<String>,
   private val instantSource: InstantSource,
-  // TODO(google-internal bug): Make required, though still allow null value.
-  private val expectedAttestationApplicationId: AttestationApplicationId? = null,
   private val constraintConfig: ConstraintConfig = ConstraintConfig(),
 ) {
   init {
@@ -183,16 +181,13 @@ constructor(
   fun verify(
     chain: List<X509Certificate>,
     challengeChecker: ChallengeChecker? = null,
-    additionalAttestationApplicationId: AttestationApplicationId? = null,
     log: LogHook? = null,
   ): VerificationResult {
     val requestLog = log?.createRequestLog()
     val result =
       try {
         val certPath = KeyAttestationCertPath(chain)
-        runBlocking {
-          internalVerify(certPath, challengeChecker, additionalAttestationApplicationId, requestLog)
-        }
+        runBlocking { internalVerify(certPath, challengeChecker, requestLog) }
       } catch (e: CertificateException) {
         requestLog?.logInputChain(chain.map { it.getEncoded().toByteString() })
         VerificationResult.ChainParsingFailure(e)
@@ -208,8 +203,6 @@ constructor(
    * @param chain The attestation certificate chain to verify.
    * @param coroutineScope The coroutine scope from which to run the verification.
    * @param challengeChecker The challenge checker to use for additional challenge validation.
-   * @param additionalAttestationApplicationId An additional attestation application ID to use for
-   *   constraint checking.
    * @param log The log hook to use for logging.
    * @return A [ListenableFuture] containing the [VerificationResult].
    */
@@ -218,7 +211,6 @@ constructor(
     coroutineScope: CoroutineScope,
     chain: List<X509Certificate>,
     challengeChecker: ChallengeChecker? = null,
-    additionalAttestationApplicationId: AttestationApplicationId? = null,
     log: LogHook? = null,
   ): ListenableFuture<VerificationResult> {
     val immutableChain = ImmutableList.copyOf(chain)
@@ -227,7 +219,7 @@ constructor(
       val result =
         try {
           val certPath = KeyAttestationCertPath(immutableChain)
-          internalVerify(certPath, challengeChecker, additionalAttestationApplicationId, requestLog)
+          internalVerify(certPath, challengeChecker, requestLog)
         } catch (e: CertificateException) {
           requestLog?.logInputChain(immutableChain.map { it.getEncoded().toByteString() })
           VerificationResult.ChainParsingFailure(e)
@@ -241,7 +233,6 @@ constructor(
   private suspend fun internalVerify(
     certPath: KeyAttestationCertPath,
     challengeChecker: ChallengeChecker? = null,
-    additionalAttestationApplicationId: AttestationApplicationId? = null,
     log: VerifyRequestLog? = null,
   ): VerificationResult {
     log?.logInputChain(certPath.certificatesWithAnchor.map { it.getEncoded().toByteString() })
@@ -314,7 +305,7 @@ constructor(
       }
     }
 
-    for (constraint in constraintConfig.getGenericConstraints()) {
+    for (constraint in constraintConfig.getConstraints()) {
       val result = constraint.check(keyDescription, certPath)
       when (result) {
         is Constraint.Satisfied -> {}
@@ -324,21 +315,6 @@ constructor(
       }
     }
 
-    val attestationApplicationIdCheckResult =
-      constraintConfig.attestationApplicationId.check(
-        combine(expectedAttestationApplicationId, additionalAttestationApplicationId),
-        keyDescription,
-      )
-    when (attestationApplicationIdCheckResult) {
-      is Constraint.Satisfied -> {}
-      is Constraint.Violated -> {
-        return VerificationResult.ConstraintViolation(
-          constraintConfig.attestationApplicationId.label,
-          attestationApplicationIdCheckResult.failureMessage,
-        )
-      }
-    }
-
     val securityLevel =
       minOf(keyDescription.attestationSecurityLevel, keyDescription.keyMintSecurityLevel)
 
@@ -357,15 +333,3 @@ constructor(
     )
   }
 }
-
-private fun combine(
-  stored: AttestationApplicationId?,
-  additional: AttestationApplicationId?,
-): AttestationApplicationId? {
-  if (stored == null) return additional
-  if (additional == null) return stored
-  return AttestationApplicationId(
-    stored.packages + additional.packages,
-    stored.signatures + additional.signatures,
-  )
-}