Merge "[AVF] Make patch level diff flexible by a requirement" into main
diff --git a/core/java/android/security/attestationverification/AttestationVerificationManager.java b/core/java/android/security/attestationverification/AttestationVerificationManager.java
index 2e61db1..acf3382 100644
--- a/core/java/android/security/attestationverification/AttestationVerificationManager.java
+++ b/core/java/android/security/attestationverification/AttestationVerificationManager.java
@@ -322,6 +322,10 @@
/** Requirements bundle parameter for a challenge. */
public static final String PARAM_CHALLENGE = "localbinding.challenge";
+ /** Requirements bundle parameter for max patch level diff (int) for a peer device. **/
+ public static final String PARAM_MAX_PATCH_LEVEL_DIFF_MONTHS =
+ "param_max_patch_level_diff_months";
+
/** @hide */
public static String localBindingTypeToString(@LocalBindingType int localBindingType) {
final String text;
diff --git a/core/java/android/security/attestationverification/OWNERS b/core/java/android/security/attestationverification/OWNERS
index 80a1f44..15b9ce4 100644
--- a/core/java/android/security/attestationverification/OWNERS
+++ b/core/java/android/security/attestationverification/OWNERS
@@ -2,3 +2,6 @@
dlm@google.com
dkrahn@google.com
+guojing@google.com
+raphk@google.com
+yukl@google.com
\ No newline at end of file
diff --git a/services/companion/java/com/android/server/companion/securechannel/AttestationVerifier.java b/services/companion/java/com/android/server/companion/securechannel/AttestationVerifier.java
index 1559a3f..df3071e 100644
--- a/services/companion/java/com/android/server/companion/securechannel/AttestationVerifier.java
+++ b/services/companion/java/com/android/server/companion/securechannel/AttestationVerifier.java
@@ -53,9 +53,8 @@
*
* @param remoteAttestation the full certificate chain containing attestation extension.
* @param attestationChallenge attestation challenge for authentication.
- * @return true if attestation is successfully verified; false otherwise.
+ * @return 1 if attestation is successfully verified; 0 otherwise.
*/
- @NonNull
public int verifyAttestation(
@NonNull byte[] remoteAttestation,
@NonNull byte[] attestationChallenge
diff --git a/services/core/java/com/android/server/security/AttestationVerificationPeerDeviceVerifier.java b/services/core/java/com/android/server/security/AttestationVerificationPeerDeviceVerifier.java
index 945a340..41e3d00 100644
--- a/services/core/java/com/android/server/security/AttestationVerificationPeerDeviceVerifier.java
+++ b/services/core/java/com/android/server/security/AttestationVerificationPeerDeviceVerifier.java
@@ -17,6 +17,7 @@
package com.android.server.security;
import static android.security.attestationverification.AttestationVerificationManager.PARAM_CHALLENGE;
+import static android.security.attestationverification.AttestationVerificationManager.PARAM_MAX_PATCH_LEVEL_DIFF_MONTHS;
import static android.security.attestationverification.AttestationVerificationManager.PARAM_PUBLIC_KEY;
import static android.security.attestationverification.AttestationVerificationManager.RESULT_FAILURE;
import static android.security.attestationverification.AttestationVerificationManager.RESULT_SUCCESS;
@@ -174,8 +175,8 @@
MyDumpData dumpData = new MyDumpData();
- int result =
- verifyAttestationInternal(localBindingType, requirements, attestation, dumpData);
+ int result = verifyAttestationInternal(localBindingType, requirements, attestation,
+ dumpData);
dumpData.mResult = result;
mDumpLogger.logAttempt(dumpData);
return result;
@@ -222,7 +223,8 @@
final var attestationExtension = fromCertificate(leafCertificate);
// Second: verify if the attestation satisfies the "peer device" profile.
- if (!checkAttestationForPeerDeviceProfile(attestationExtension, dumpData)) {
+ if (!checkAttestationForPeerDeviceProfile(requirements, attestationExtension,
+ dumpData)) {
failed = true;
}
@@ -400,6 +402,7 @@
}
private boolean checkAttestationForPeerDeviceProfile(
+ @NonNull Bundle requirements,
@NonNull AndroidKeystoreAttestationVerificationAttributes attestationAttributes,
MyDumpData dumpData) {
boolean result = true;
@@ -461,30 +464,37 @@
result = false;
}
- // Patch level integer YYYYMM is expected to be within 1 year of today.
- if (!isValidPatchLevel(attestationAttributes.getKeyOsPatchLevel())) {
+ int maxPatchLevelDiffMonths = requirements.getInt(PARAM_MAX_PATCH_LEVEL_DIFF_MONTHS,
+ MAX_PATCH_AGE_MONTHS);
+
+ // Patch level integer YYYYMM is expected to be within maxPatchLevelDiffMonths of today.
+ if (!isValidPatchLevel(attestationAttributes.getKeyOsPatchLevel(),
+ maxPatchLevelDiffMonths)) {
debugVerboseLog("OS patch level is not within valid range.");
result = false;
} else {
dumpData.mOsPatchLevelInRange = true;
}
- // Patch level integer YYYYMMDD is expected to be within 1 year of today.
- if (!isValidPatchLevel(attestationAttributes.getKeyBootPatchLevel())) {
+ // Patch level integer YYYYMMDD is expected to be within maxPatchLevelDiffMonths of today.
+ if (!isValidPatchLevel(attestationAttributes.getKeyBootPatchLevel(),
+ maxPatchLevelDiffMonths)) {
debugVerboseLog("Boot patch level is not within valid range.");
result = false;
} else {
dumpData.mKeyBootPatchLevelInRange = true;
}
- if (!isValidPatchLevel(attestationAttributes.getKeyVendorPatchLevel())) {
+ if (!isValidPatchLevel(attestationAttributes.getKeyVendorPatchLevel(),
+ maxPatchLevelDiffMonths)) {
debugVerboseLog("Vendor patch level is not within valid range.");
result = false;
} else {
dumpData.mKeyVendorPatchLevelInRange = true;
}
- if (!isValidPatchLevel(attestationAttributes.getKeyBootPatchLevel())) {
+ if (!isValidPatchLevel(attestationAttributes.getKeyBootPatchLevel(),
+ maxPatchLevelDiffMonths)) {
debugVerboseLog("Boot patch level is not within valid range.");
result = false;
} else {
@@ -525,7 +535,7 @@
* is not enough. Therefore, we also confirm the patch level for the remote and local device are
* similar.
*/
- private boolean isValidPatchLevel(int patchLevel) {
+ private boolean isValidPatchLevel(int patchLevel, int maxPatchLevelDiffMonths) {
LocalDate currentDate = mTestSystemDate != null
? mTestSystemDate : LocalDate.now(ZoneId.systemDefault());
@@ -543,7 +553,9 @@
return false;
}
- // Check local patch date is not in last year of system clock.
+ // Check local patch date is not in last year of system clock. If the local patch already
+ // has a year's worth of bugs and vulnerabilities, it has no security meanings to check the
+ // remote patch level.
if (ChronoUnit.MONTHS.between(localPatchDate, currentDate) > MAX_PATCH_AGE_MONTHS) {
return true;
}
@@ -559,19 +571,9 @@
int patchMonth = Integer.parseInt(remoteDeviceDateStr.substring(4, 6));
LocalDate remotePatchDate = LocalDate.of(patchYear, patchMonth, 1);
- // Check patch dates are within 1 year of each other
- boolean IsRemotePatchWithinOneYearOfLocalPatch;
- if (remotePatchDate.compareTo(localPatchDate) > 0) {
- IsRemotePatchWithinOneYearOfLocalPatch = ChronoUnit.MONTHS.between(
- localPatchDate, remotePatchDate) <= MAX_PATCH_AGE_MONTHS;
- } else if (remotePatchDate.compareTo(localPatchDate) < 0) {
- IsRemotePatchWithinOneYearOfLocalPatch = ChronoUnit.MONTHS.between(
- remotePatchDate, localPatchDate) <= MAX_PATCH_AGE_MONTHS;
- } else {
- IsRemotePatchWithinOneYearOfLocalPatch = true;
- }
-
- return IsRemotePatchWithinOneYearOfLocalPatch;
+ // Check patch dates are within the max patch level diff of each other
+ return Math.abs(ChronoUnit.MONTHS.between(localPatchDate, remotePatchDate))
+ <= maxPatchLevelDiffMonths;
}
/**
diff --git a/tests/AttestationVerificationTest/src/com/android/server/security/AttestationVerificationPeerDeviceVerifierTest.kt b/tests/AttestationVerificationTest/src/com/android/server/security/AttestationVerificationPeerDeviceVerifierTest.kt
index afb3593..4712d6b 100644
--- a/tests/AttestationVerificationTest/src/com/android/server/security/AttestationVerificationPeerDeviceVerifierTest.kt
+++ b/tests/AttestationVerificationTest/src/com/android/server/security/AttestationVerificationPeerDeviceVerifierTest.kt
@@ -4,6 +4,7 @@
import android.content.Context
import android.os.Bundle
import android.security.attestationverification.AttestationVerificationManager.PARAM_CHALLENGE
+import android.security.attestationverification.AttestationVerificationManager.PARAM_MAX_PATCH_LEVEL_DIFF_MONTHS
import android.security.attestationverification.AttestationVerificationManager.PARAM_PUBLIC_KEY
import android.security.attestationverification.AttestationVerificationManager.RESULT_FAILURE
import android.security.attestationverification.AttestationVerificationManager.RESULT_SUCCESS
@@ -162,6 +163,41 @@
}
@Test
+ fun verifyAttestation_returnsSuccessPatchDataWithinMaxPatchDiff() {
+ val verifier = AttestationVerificationPeerDeviceVerifier(
+ context, dumpLogger, trustAnchors, false, LocalDate.of(2023, 3, 1),
+ LocalDate.of(2023, 2, 1)
+ )
+ val challengeRequirements = Bundle()
+ challengeRequirements.putByteArray(PARAM_CHALLENGE, "player456".encodeToByteArray())
+ challengeRequirements.putInt(PARAM_MAX_PATCH_LEVEL_DIFF_MONTHS, 24)
+
+ val result = verifier.verifyAttestation(
+ TYPE_CHALLENGE, challengeRequirements,
+ TEST_ATTESTATION_WITH_ROOT_CERT_FILENAME.fromPEMFileToByteArray()
+ )
+ assertThat(result).isEqualTo(RESULT_SUCCESS)
+ }
+
+ @Test
+ fun verifyAttestation_returnsFailurePatchDataNotWithinMaxPatchDiff() {
+ val verifier = AttestationVerificationPeerDeviceVerifier(
+ context, dumpLogger, trustAnchors, false, LocalDate.of(2024, 10, 1),
+ LocalDate.of(2024, 9, 1)
+ )
+ val challengeRequirements = Bundle()
+ challengeRequirements.putByteArray(PARAM_CHALLENGE, "player456".encodeToByteArray())
+ challengeRequirements.putInt(PARAM_MAX_PATCH_LEVEL_DIFF_MONTHS, 24)
+
+ val result = verifier.verifyAttestation(
+ TYPE_CHALLENGE, challengeRequirements,
+ // The patch date of this file is early 2022
+ TEST_ATTESTATION_WITH_ROOT_CERT_FILENAME.fromPEMFileToByteArray()
+ )
+ assertThat(result).isEqualTo(RESULT_FAILURE)
+ }
+
+ @Test
fun verifyAttestation_returnsFailureTrustedAnchorEmpty() {
val verifier = AttestationVerificationPeerDeviceVerifier(
context, dumpLogger, HashSet(), false, LocalDate.of(2022, 1, 1),