[webauthn] Ambiguous instructions in the Android Key Attestation Statement Format verification procedure (#1980)

vanbukin has just created a new issue for https://github.com/w3c/webauthn:

== Ambiguous instructions in the Android Key Attestation Statement Format verification procedure ==
According to: https://w3c.github.io/webauthn/#android-key-attestation


* Verify the following using the appropriate authorization list from the attestation certificate [extension data](https://w3c.github.io/webauthn/#android-key-attestation-certificate-extension-data):
    * The AuthorizationList.allApplications field is not present on either authorization list (softwareEnforced nor teeEnforced), since PublicKeyCredential MUST be [scoped](https://w3c.github.io/webauthn/#scope) to the [RP ID](https://w3c.github.io/webauthn/#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](https://w3c.github.io/webauthn/#attestation-type) [Basic](https://w3c.github.io/webauthn/#basic) and [attestation trust path](https://w3c.github.io/webauthn/#attestation-trust-path) x5c.

According to the [schema](https://source.android.com/docs/security/features/keystore/attestation#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 whole 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 whole 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.



Please view or discuss this issue at https://github.com/w3c/webauthn/issues/1980 using your GitHub account


-- 
Sent via github-notify-ml as configured in https://github.com/w3c/github-notify-ml-config

Received on Sunday, 1 October 2023 00:32:58 UTC