Skip to content

Commit 6dae35c

Browse files
carmenyhcopybara-github
carmenyh
authored and
copybara-github
committed
Update LogHook for updated thread safety annotations.
PiperOrigin-RevId: 830853528
1 parent 41cc2b5 commit 6dae35c

5 files changed

Lines changed: 98 additions & 41 deletions

File tree

‎src/main/kotlin/Verifier.kt‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,17 @@ sealed interface VerificationResult {
6464
VerificationResult
6565
}
6666

67-
/** Interface for logging info level key attestation events and information. */
67+
/**
68+
* Interface for logging info level key attestation events and information.
69+
*
70+
* Uses of the log hook must be thread safe within a single verify call.
71+
*/
6872
@ThreadSafe
6973
interface LogHook {
74+
fun createRequestLog(): VerifyRequestLog
75+
}
7076

77+
interface VerifyRequestLog {
7178
/**
7279
* Logs the certificate chain which is being verified. Called for each call to [verify].
7380
*
@@ -114,6 +121,9 @@ interface LogHook {
114121
* @param infoMessage The info level message to log.
115122
*/
116123
fun logInfoMessage(infoMessage: String)
124+
125+
/* Flushes any buffered logs. Called just before [verify] returns. */
126+
fun flush()
117127
}
118128

119129
/**
@@ -151,15 +161,17 @@ open class Verifier(
151161
challengeChecker: ChallengeChecker? = null,
152162
log: LogHook? = null,
153163
): VerificationResult {
164+
val requestLog = log?.createRequestLog()
154165
val result =
155166
try {
156167
val certPath = KeyAttestationCertPath(chain)
157-
runBlocking { internalVerify(certPath, challengeChecker, log) }
168+
runBlocking { internalVerify(certPath, challengeChecker, requestLog) }
158169
} catch (e: CertificateException) {
159-
log?.logInputChain(chain.map { it.getEncoded().toByteString() })
170+
requestLog?.logInputChain(chain.map { it.getEncoded().toByteString() })
160171
VerificationResult.ChainParsingFailure(e)
161172
}
162-
log?.logResult(result)
173+
requestLog?.logResult(result)
174+
requestLog?.flush()
163175
return result
164176
}
165177

@@ -182,15 +194,17 @@ open class Verifier(
182194
): ListenableFuture<VerificationResult> {
183195
val immutableChain = ImmutableList.copyOf(chain)
184196
return coroutineScope.future {
197+
val requestLog = log?.createRequestLog()
185198
val result =
186199
try {
187200
val certPath = KeyAttestationCertPath(immutableChain)
188-
internalVerify(certPath, challengeChecker, log)
201+
internalVerify(certPath, challengeChecker, requestLog)
189202
} catch (e: CertificateException) {
190-
log?.logInputChain(immutableChain.map { it.getEncoded().toByteString() })
203+
requestLog?.logInputChain(immutableChain.map { it.getEncoded().toByteString() })
191204
VerificationResult.ChainParsingFailure(e)
192205
}
193-
log?.logResult(result)
206+
requestLog?.logResult(result)
207+
requestLog?.flush()
194208
result
195209
}
196210
}
@@ -199,7 +213,7 @@ open class Verifier(
199213
private suspend fun internalVerify(
200214
certPath: KeyAttestationCertPath,
201215
challengeChecker: ChallengeChecker? = null,
202-
log: LogHook? = null,
216+
log: VerifyRequestLog? = null,
203217
): VerificationResult {
204218
log?.logInputChain(certPath.certificatesWithAnchor.map { it.getEncoded().toByteString() })
205219
log?.logCertSerialNumbers(

‎src/main/kotlin/testing/Certs.kt‎

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import com.android.keyattestation.verifier.AuthorizationList
2020
import com.android.keyattestation.verifier.KeyDescription
2121
import com.android.keyattestation.verifier.KeyMintTag
2222
import com.android.keyattestation.verifier.Origin
23+
import com.android.keyattestation.verifier.ProvisioningInfoMap
2324
import com.android.keyattestation.verifier.RootOfTrust
2425
import com.android.keyattestation.verifier.SecurityLevel
2526
import com.android.keyattestation.verifier.VerifiedBootState
@@ -52,7 +53,6 @@ object Certs {
5253
val rootKey = certFactory.rootKey
5354
val factoryIntermediate = certFactory.factoryIntermediate
5455
val remoteIntermediate = certFactory.remoteIntermediate
55-
val strongBoxIntermediate = certFactory.strongBoxIntermediate
5656
val factoryAttestation = certFactory.factoryAttestation
5757
}
5858

@@ -100,8 +100,8 @@ object CertLists {
100100
val validStrongboxFactoryProvisioned by lazy {
101101
listOf(
102102
certFactory.generateLeafCert(extension = certFactory.STRONG_BOX_KEY_DESCRIPTION_EXT),
103-
certFactory.generateAttestationCert(issuer = Certs.strongBoxIntermediate.subject),
104-
Certs.strongBoxIntermediate,
103+
certFactory.generateAttestationCert(issuer = certFactory.strongBoxIntermediate.subject),
104+
certFactory.strongBoxIntermediate,
105105
Certs.root,
106106
)
107107
}
@@ -340,6 +340,43 @@ object CertLists {
340340
)
341341
}
342342

343+
val malformedProvisioningInfo by lazy {
344+
listOf(
345+
certFactory.generateLeafCert(),
346+
certFactory.generateAttestationCert(
347+
issuer = certFactory.rkpIntermediate.subject,
348+
signingKey = certFactory.rkpKey.private,
349+
extraExtension =
350+
Extension(
351+
ProvisioningInfoMap.OID,
352+
/* critical= */ false,
353+
ASN1Integer(BigInteger.valueOf(1234567)).encoded,
354+
),
355+
),
356+
certFactory.rkpIntermediate,
357+
Certs.remoteIntermediate,
358+
certFactory.root,
359+
)
360+
}
361+
362+
/* Different revoked serial numbers for testing. */
363+
@JvmField val REVOKED_SERIAL_NUMBER = 42.toBigInteger()
364+
@JvmField val REVOKED_SERIAL_NUMBER_BIG = 8000000000000.toBigInteger()
365+
@JvmField
366+
val REVOKED_SERIAL_NUMBER_LONG_STRING = "c35747a084470c3135aeefe2b8d40cd6".toBigInteger(16)
367+
@JvmField val REVOKED_SERIAL_NUMBER_ODD_LENGTH = 1228286566665971148.toBigInteger()
368+
369+
/* A chain where the attesstation certificate has {@link REVOKED_SERIAL_NUMBER}. */
370+
@JvmStatic
371+
val revoked by lazy {
372+
listOf(
373+
certFactory.generateLeafCert(),
374+
certFactory.generateAttestationCert(serialNumber = REVOKED_SERIAL_NUMBER),
375+
Certs.factoryIntermediate,
376+
certFactory.root,
377+
)
378+
}
379+
343380
private fun generateValidLeafCertWithAppendedTag(appendedTag: Int, appendedValue: ASN1Encodable) =
344381
certFactory.generateLeafCert(
345382
extension =
@@ -471,24 +508,6 @@ object Chains {
471508
)
472509
}
473510

474-
/* Different revoked serial numbers for testing. */
475-
@JvmField val REVOKED_SERIAL_NUMBER = 42.toBigInteger()
476-
@JvmField val REVOKED_SERIAL_NUMBER_BIG = 8000000000000.toBigInteger()
477-
@JvmField
478-
val REVOKED_SERIAL_NUMBER_LONG_STRING = "c35747a084470c3135aeefe2b8d40cd6".toBigInteger(16)
479-
@JvmField val REVOKED_SERIAL_NUMBER_ODD_LENGTH = 1228286566665971148.toBigInteger()
480-
481-
/* A chain where the attesstation certificate has {@link REVOKED_SERIAL_NUMBER}. */
482-
@JvmStatic
483-
val revoked by lazy {
484-
KeyAttestationCertPath(
485-
certFactory.generateLeafCert(),
486-
certFactory.generateAttestationCert(serialNumber = REVOKED_SERIAL_NUMBER),
487-
Certs.factoryIntermediate,
488-
certFactory.root,
489-
)
490-
}
491-
492511
/* A factory chain with an additional intermediate certificate. */
493512
val forgedKeybox by lazy {
494513
val compromisedAttestationKey = certFactory.generateEcKeyPair()

‎src/main/kotlin/testing/FakeLogHook.kt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import com.android.keyattestation.verifier.KeyDescription
2020
import com.android.keyattestation.verifier.LogHook
2121
import com.android.keyattestation.verifier.ProvisioningInfoMap
2222
import com.android.keyattestation.verifier.VerificationResult
23+
import com.android.keyattestation.verifier.VerifyRequestLog
2324
import com.google.errorprone.annotations.ThreadSafe
2425
import com.google.protobuf.ByteString
2526

@@ -30,6 +31,20 @@ import com.google.protobuf.ByteString
3031
* test.
3132
*/
3233
class FakeLogHook : LogHook {
34+
var fakeVerifyRequestLog = FakeVerifyRequestLog()
35+
36+
override fun createRequestLog(): VerifyRequestLog {
37+
return fakeVerifyRequestLog
38+
}
39+
}
40+
41+
/**
42+
* Fake implementation of [VerifyRequestLog] for testing.
43+
*
44+
* Stores the last values passed to each logging method. A new instance should be created for each
45+
* test.
46+
*/
47+
class FakeVerifyRequestLog : VerifyRequestLog {
3348
var inputChain = mutableListOf<ByteString>()
3449
var result: VerificationResult? = null
3550
var keyDescription: KeyDescription? = null
@@ -60,4 +75,6 @@ class FakeLogHook : LogHook {
6075
override fun logInfoMessage(infoMessage: String) {
6176
this.infoMessages.add(infoMessage)
6277
}
78+
79+
override fun flush() {}
6380
}

‎src/test/kotlin/VerifierTest.kt‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ class VerifierTest {
201201
)
202202
.await()
203203
)
204-
assertThat(logHook.inputChain)
204+
assertThat(logHook.fakeVerifyRequestLog.inputChain)
205205
.isEqualTo(CertLists.wrongTrustAnchor.map { it.encoded.toByteString() })
206206
}
207207

@@ -210,7 +210,8 @@ class VerifierTest {
210210
val logHook = FakeLogHook()
211211
val chain = readCertList("blueline/sdk28/TEE_EC_NONE.pem")
212212
assertIs<VerificationResult.Success>(verifier.verifyAsync(this, chain, log = logHook).await())
213-
assertThat(logHook.keyDescription).isEqualTo(chain[0].keyDescription())
213+
assertThat(logHook.fakeVerifyRequestLog.keyDescription)
214+
.isEqualTo(chain.first().keyDescription())
214215
}
215216

216217
@Test
@@ -225,7 +226,7 @@ class VerifierTest {
225226
assertIs<VerificationResult.Success>(
226227
verifierWithTestRoot.verifyAsync(this, CertLists.invalidBootPatchLevel, log = logHook).await()
227228
)
228-
assertThat(logHook.infoMessages).isNotEmpty()
229+
assertThat(logHook.fakeVerifyRequestLog.infoMessages).isNotEmpty()
229230
}
230231

231232
@Test

‎src/test/kotlin/provider/RevocationCheckerTest.kt‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616

1717
package com.android.keyattestation.verifier.provider
1818

19+
import com.android.keyattestation.verifier.testing.CertLists
1920
import com.android.keyattestation.verifier.testing.Certs
20-
import com.android.keyattestation.verifier.testing.Chains
2121
import com.android.keyattestation.verifier.testing.FakeCalendar
2222
import java.security.Security
2323
import java.security.cert.CertPathValidator
@@ -39,28 +39,34 @@ class RevocationCheckerTest {
3939
private val revocationChecker =
4040
RevocationChecker(
4141
setOf(
42-
Chains.REVOKED_SERIAL_NUMBER.toString(16),
43-
Chains.REVOKED_SERIAL_NUMBER_BIG.toString(16),
44-
Chains.REVOKED_SERIAL_NUMBER_LONG_STRING.toString(16),
45-
Chains.REVOKED_SERIAL_NUMBER_ODD_LENGTH.toString(16),
42+
CertLists.REVOKED_SERIAL_NUMBER.toString(16),
43+
CertLists.REVOKED_SERIAL_NUMBER_BIG.toString(16),
44+
CertLists.REVOKED_SERIAL_NUMBER_LONG_STRING.toString(16),
45+
CertLists.REVOKED_SERIAL_NUMBER_ODD_LENGTH.toString(16),
4646
)
4747
)
4848
private val validator = CertPathValidator.getInstance("KeyAttestation")
4949
private val pkixValidator = CertPathValidator.getInstance("PKIX")
5050

5151
@Test
5252
fun withoutRevocationChecker_validationSucceeds() {
53-
validator.validate(Chains.revoked, params)
54-
pkixValidator.validate(Chains.revoked, params)
53+
validator.validate(KeyAttestationCertPath(CertLists.revoked), params)
54+
pkixValidator.validate(KeyAttestationCertPath(CertLists.revoked), params)
5555
}
5656

5757
@Test
5858
fun withRevocationChecker_throwsCertPathValidatorException() {
5959
assertFailsWith<CertPathValidatorException> {
60-
validator.validate(Chains.revoked, params.apply { addCertPathChecker(revocationChecker) })
60+
validator.validate(
61+
KeyAttestationCertPath(CertLists.revoked),
62+
params.apply { addCertPathChecker(revocationChecker) },
63+
)
6164
}
6265
assertFailsWith<CertPathValidatorException> {
63-
pkixValidator.validate(Chains.revoked, params.apply { addCertPathChecker(revocationChecker) })
66+
pkixValidator.validate(
67+
KeyAttestationCertPath(CertLists.revoked),
68+
params.apply { addCertPathChecker(revocationChecker) },
69+
)
6470
}
6571
}
6672

0 commit comments

Comments
 (0)