a-sit-plus / warden

Server-Side Mobile Client Attestation Library
https://a-sit-plus.github.io/warden/
Apache License 2.0
10 stars 2 forks source link

Re-Evaluate iOS Key Attestation Format #12

Open JesusMcCloud opened 2 months ago

JesusMcCloud commented 2 months ago

@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:

  1. 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!

  2. Add a new interface:

    • Introduce a new datatype in signum-indispensable: AttestationStatement (as in WebAuthn. For Android, we keep to the spec, for iOS see here.
    • pass this structure to a new interface in 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.
    • The iOS key will be in a well-defined format, just as the Android one so no more encoding guessing.
    • We can extend that to Attestations for HSMs that support it, as we have a structured key format.

This will finally close #1 and also make things like the mess discussed in #10 obsolete.
Discuss here.

iaik-jheher commented 2 months ago

Should definitely be option 2 if we go for it. Guessing is bad.

The old interface could be a LegacyIOSAttestation format , and can keep its code paths for now.

iaik-jheher commented 2 months ago

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))
nodh commented 2 months ago

I'm going with option 2, too. But I would place the AttestationStatement is this library here.

JesusMcCloud commented 2 months ago

I'm going with option 2, too. But I would place the AttestationStatement is this library here.

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

JesusMcCloud commented 1 month ago

already live in signum. "just" needs integration