veehaitch / devicecheck-appattest

Server-side library to validate the authenticity of Apple App Attest artifacts, written in Kotlin.
Apache License 2.0
66 stars 8 forks source link

Possible bug or java compatibility issue in AAGUID comparison #29

Closed jeremydavidson closed 1 year ago

jeremydavidson commented 2 years ago

I really appreciate the creation and maintenance of this library! I am using it in a Java implementation but ran into an issue in AttestationValidator.kt verifyAuthenticatorData: the "AAGUID does match neither" exception is being thrown. I find that the values being compared are similar to: expected=12300456-4242-1234-1464-6571236c6f70 and actual=12300456-4242-1234-1400-000000000000 (not the real values).

Observe that the first 16-bytes of the two values are equivalent, as expected. But, the comparison is on the whole 36-bytes on this line: authenticatorData.attestedCredentialData.aaguid != appleAppAttestEnvironment.aaguid.

The comment on Extensions.kt ByteArray.toUUID() function agrees with the Apple documentation "Create a UUID from a [ByteArray] of a length no more than 16 bytes, padded with zeros, if necessary." So, the intention is clear. But, my implementation shows that the actual UUID is padded with zeroes but the expected AAGUID is not padded with zeroes, so the comparison fails.

I patched the library to compare only the first 16-bytes and that makes my attestations work! So, I am pleased about that.

But, do you think the root cause is because I am using the library from Java? What is the proper way to fix it?

veehaitch commented 1 year ago

Sorry for not coming back to you earlier. I am, however, unable to reproduce your issue. To make sure the code produces the correct AAGUIDs, I added an additional test in #30.

Feel free to add additional information and I'll try to figure out what's going on.

veehaitch commented 1 year ago

Feel free to reopen if necessary.

seik0ixtem commented 1 year ago

One of my prod envs (but not all) gives same. But i'm failing to reproduce locally too yet.

seik0ixtem commented 1 year ago

oh, sorry, abort mission - attestation object was for PRODUCTION, but server env expected DEVELOPMENT, not the same issue.