-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Re-Evaluate iOS Key Attestation Format #12
Comments
Should definitely be option 2 if we go for it. Guessing is bad. The old interface could be a |
So I'm thinking: sealed interface Attestation { val jsonEncoded: String; val cborEncoded: ByteArray }
data class LegacyIOSAttestation(attestation: ByteArray, assertion: ByteArray): Attestation
fun verify(attestation: Attestation)
fun verify(oldAttestation: ByteArray, oldAssertion: ByteArray) = verify(LegacyIOSAttestation(oldAttestation, oldAssertion)) |
I'm going with option 2, too. But I would place the |
Since this will be needed on the client and on the back-end, I'd vote for putting it into the signum indispensable module, since this is a dependency that is present on both ends anyway |
already live in signum. "just" needs integration |
@iaik-jheher proposed an intriguing idea while implementing iOS attestation in Signum: why not encode the key to be attested directly into the attestation challenge hash? this way we have timestamping guaranteed, don't need to evaluate the assertion (hence, remove complexity), and/or signature counter, only require one operation on the client. Also: no more encoding guessing.
On a technical level, I see only upside. On an operational level, I see the risk of increasing complexity in a critical code path that is being used in production, because we will need to maintain backwards compatibility. We do have regression tests, so this should be covered just fine. Still, it makes me feel a bit queasy.
Then there's two ways to handle this compatibility issue:
Just use the Interfaces that are there and if the
attestationProof
is just a single element, it has to be the new iOS key attestation format. The interface could then never be extended, because the length of the proof takes all three possible valid values. Realistically this is not an issue, because, if either Google or Apple change something, we need to rework anyways. In the end though, not very future-proof!Add a new interface:
AttestationStatement
(as in WebAuthn. For Android, we keep to the spec, for iOS see here.AttestationService
. The Android-Path will remain the same, the generic attestation-without-assertion path for iOS can remain untouched as well and the check for a matching key in the encoded hash really straight-forward, even though it will require three lines of code compared to the single one for Android to match the key.This will finally close #1 and also make things like the mess discussed in #10 obsolete.
Discuss here.
The text was updated successfully, but these errors were encountered: