webauthn icon indicating copy to clipboard operation
webauthn copied to clipboard

Ambiguous instructions in the Android Key Attestation Statement Format verification procedure

Open vanbukin opened this issue 9 months ago • 7 comments

According to: https://w3c.github.io/webauthn/#android-key-attestation

  • Verify the following using the appropriate authorization list from the attestation certificate extension data:
    • The AuthorizationList.allApplications field is not present on either authorization list (softwareEnforced nor teeEnforced), since PublicKeyCredential MUST be scoped to the RP ID.
    • For the following, use only the teeEnforced authorization list if the RP wants to accept only keys from a trusted execution environment, otherwise use the union of teeEnforced and softwareEnforced.
      • The value in the AuthorizationList.origin field is equal to KM_ORIGIN_GENERATED.
      • The value in the AuthorizationList.purpose field is equal to KM_PURPOSE_SIGN.
  • If successful, return implementation-specific values representing attestation type Basic and attestation trust path x5c.

According to the schema, KeyDescription can contain 2 AuthorizationList elements: softwareEnforced and teeEnforced.

And the problem lies in how the specification describes otherwise use the union of teeEnforced and softwareEnforced.

KeyDescription can (and probably should) be interpreted as a singular (indivisible) object. From this, it follows that the formulation should be somewhat different: otherwise, the following conditions must be met for either teeEnforced or softwareEnforced.

The following combinations of parameters should be considered valid:

  • softwareEnforced:
    • purpose - ✅valid
    • origin - ✅valid
  • teeEnforced:
    • purpose - 🚫invalid
    • origin - 🚫invalid

  • softwareEnforced:
    • purpose - 🚫invalid
    • origin - 🚫invalid
  • teeEnforced:
    • purpose - ✅valid
    • origin - ✅valid

  • softwareEnforced:
    • purpose - ✅valid
    • origin - ✅valid
  • teeEnforced:
    • purpose - ✅valid
    • origin - ✅valid

Because currently, the wording implies that you can combine origin from softwareEnforced and teeEnforced, and check that at least one of them contains a valid value, and then do the same for purpose.

And moreover, this is how it is done in the real world. https://github.com/webauthn4j/webauthn4j/blob/master/webauthn4j-core/src/main/java/com/webauthn4j/validator/attestation/statement/androidkey/KeyDescriptionValidator.java#L122-L134

This leads to a scenario where such a combination of values

  • softwareEnforced:
    • purpose - ✅valid
    • origin - 🚫invalid
  • teeEnforced:
    • purpose - 🚫invalid
    • origin - ✅valid

would pass the validation.

If KeyDescription can be interpreted as not a singular (indivisible) object, then the wording can also be changed to something more comprehensible: otherwise, verify that each of the unions of teeEnforced and softwareEnforced parameter values meet the requirements. In such case, the scheme outlined above would be considered legitimate.

vanbukin avatar Oct 01 '23 00:10 vanbukin

After reading Android key attestation related docs, I'm thinking that the key description may have both teeEnforced and softwareEnforced at the same time.

See the following links:

  • https://source.android.com/docs/security/features/keystore/implementer-ref#get_key_characteristics

So, the current implementation you've provided is valid per the spec. Our implementation has similar validation logics.

Kieun avatar Oct 03 '23 02:10 Kieun

@Kieun I apologize if my description of the problem was not precise enough. As you correctly pointed out - KeyDescription can simultaneously have both teeEnforced and softwareEnforced. The question lies in how the origin and purpose, which are nested within teeEnforced and softwareEnforced, should be validated. Because currently, the specification states otherwise use the union of teeEnforced and softwareEnforced, and then lists the validation rules for origin and purpose:

- The value in the AuthorizationList.origin field is equal to KM_ORIGIN_GENERATED.
- The value in the AuthorizationList.purpose field is equal to KM_PURPOSE_SIGN.

As an implementer, it's unclear to me how validation should look.

bool isValid = IsOneOfOriginsValid(new [] {keyDescription.teeEnforced.origin, keyDescription.softwareEnforced.origin})
&&
IsOneOfPurposesValid(new [] {keyDescription.teeEnforced.purpose, keyDescription.softwareEnforced.purpose})

or

bool isValid = IsPurposeAndOriginValid(keyDescription.teeEnforced) || IsPurposeAndOriginValid(keyDescription.softwareEnforced)

vanbukin avatar Oct 04 '23 10:10 vanbukin

First of all, the KeyDescription may have teeEnforced and softwareEnforced at the same time. But the thing is that, if the both are presented in the KeyDescription, the same filed must not be present. For example, teeEnforced.origin field and softwareEnforced.origin field should not be presented together. This is the same for purpose field as well. If the same fields are present both in teeEnforced and softwareEnforced, that would be the invalid structure.

So, the union means that, you just get authorization list from teeEnforced and softwareEnforced and let it as single authorization list saying temp. Then, you need to verify (not validate) temp.origin and temp.purpose.

I'm not a native english speaker, but my interpretation was something like this. It's better to get response from any Googler, @agl .

Kieun avatar Oct 04 '23 10:10 Kieun

@Kieun You have provided good clarifications. It has become clear to me. It would be helpful to add a small Note to the specification, for the benefit of others as well.

vanbukin avatar Oct 05 '23 09:10 vanbukin

Let me verify this with Android experts, but my interpretation is that each security property can be either enforced in software or in TEE, and they can combine independently. E.g. KM_ORIGIN_GENERATED in the teeEnforced would mean the key was generated in the TEE but enforcing that the key is only used for sign ops (KM_PURPOSE_SIGN) could still be enforced by software, and therefore that property would appear in the softwareEnforced field.

I don't actually know if that's a valid combination, it at least seems odd. But assuming it is valid then I think the spec is correct. In essence:

  1. If an RP cares about TEE enforcement, check only the teeEnforced list.
  2. If an RP is ok with software enforcement, check each property appears on /either/ softwareEnforced or teeEnforced, individually.

I.e. there's no reason for an RP that accepts software enforcement to reject a credential where one of these is actually TEE enforced.

I'll double check if this is actually a valid scenario.

arnar avatar Jan 24 '24 21:01 arnar

We should poke Arnar in two weeks time about this.

agl avatar Jan 31 '24 19:01 agl

@arnar please review

nadalin avatar Mar 20 '24 19:03 nadalin