[Bug 3938] New: FIDO2 verify-required keys fail to sign on non-biometric tokens ("option uv is unknown")

[bugzilla-daemon at mindrot.org]

https://bugzilla.mindrot.org/show_bug.cgi?id=3938

            Bug ID: 3938
           Summary: FIDO2 verify-required keys fail to sign on
                    non-biometric tokens ("option uv is unknown")
           Product: Portable OpenSSH
           Version: 10.2p1
          Hardware: ARM64
                OS: Mac OS X
            Status: NEW
          Severity: minor
          Priority: P5
         Component: ssh
          Assignee: unassigned-bugs at mindrot.org
          Reporter: hello at niklaas.eu

## Summary

SSH signing with ED25519-SK keys created with `-O verify-required`
fails on
non-biometric FIDO2 tokens (e.g. YubiKey 5 series). The failure occurs
because
`sk_sign()` in `sk-usbhid.c` checks for the FIDO2 `"uv"` option to
determine
whether the device can perform user verification, but non-biometric
tokens
don't advertise `"uv"`. They use `clientPin` for user verification
instead.
The code returns `SSH_SK_ERR_PIN_REQUIRED`, which should trigger a PIN
prompt,
but a second issue in `sshconnect2.c` prevents the fallback from being
reached.

See disclaimer below regarding the analysis above.

## Environment

- OpenSSH 10.2p1 (also affects 9.1p1 through current HEAD)
- libfido2 1.16.0
- YubiKey 5 series, firmware 5.7.1
- macOS (arm64), but the bug is platform-independent

## Steps to reproduce

1. Set a FIDO2 PIN on a non-biometric YubiKey.
2. Generate a key with verify-required:
   ```
   ssh-keygen -t ed25519-sk -O verify-required -C "test"
   ```
3. Attempt to use the key:
   ```
   ssh -v -i ~/.ssh/id_ed25519_sk -o IdentitiesOnly=yes git at github.com
   ```

## Expected behavior

SSH prompts for the YubiKey PIN, then requests a touch, and the
signature
succeeds.

## Actual behavior

Signing fails immediately without a PIN prompt:

```
debug1: check_sk_options: option uv is unknown
debug1: ssh_sk_sign: check_sk_options uv
debug1: sshsk_sign: sk_sign failed with code -3
debug1: ssh-sk-helper: Signing failed: incorrect passphrase supplied to
decrypt private key
sign_and_send_pubkey: signing failed for ED25519-SK "test": incorrect
passphrase supplied to decrypt private key
```

When the key is loaded in ssh-agent, the error is:

```
sign_and_send_pubkey: signing failed for ED25519-SK "test" from agent:
agent refused operation
```

---

Disclaimer: Everything below this line was analyzed and written by my
clanker. I don't know C and I didn't verify what follows. I'm posting
this just in case there might be something truthful and worth following
up to in it. I got curious and wanted to take a glimpse at
understanding what might cause the bug. Again, please be aware that
parts (if not all) could be hallucinated.

---

## Root cause

There are two interacting issues:

### 1. `sk_sign()` treats absent `"uv"` option as "device cannot do
user verification"

In `sk-usbhid.c`, `sk_sign()` (around line 1216 in 10.2p1) does:

```c
if (pin == NULL && (flags & SSH_SK_USER_VERIFICATION_REQD)) {
    if (check_sk_options(sk->dev, "uv", &internal_uv) < 0 ||
        internal_uv != 1) {
        skdebug(__func__, "check_sk_options uv");
        ret = SSH_SK_ERR_PIN_REQUIRED;
        goto out;
    }
    ...
}
```

`check_sk_options()` queries the device's CBOR option list. The `"uv"`
option
indicates built-in biometric verification (e.g. a fingerprint reader).
A
YubiKey 5 reports these options:

```
options: rk, up, noplat, noalwaysUv, credMgmt, authnrCfg, clientPin,
         largeBlobs, pinUvAuthToken, setMinPINLength, makeCredUvNotRqd,
         credentialMgmtPreview
```

There is no `"uv"` because the device has no biometric sensor. It
supports
user verification through `clientPin` and `pinUvAuthToken` instead.
Since
`"uv"` is absent, `check_sk_options()` returns 0 with `internal_uv =
-1`
(unknown). The condition `internal_uv != 1` is true, and the function
returns
`SSH_SK_ERR_PIN_REQUIRED` without attempting to prompt for a PIN.

The intent of this code (introduced in f3c34df8, 2021-11-02) was to
detect
whether a biometric token can handle UV internally so the PIN prompt
can be
skipped. But the fallback for the `"uv"` absent case should be to
request a
PIN, not to fail.

### 2. `identity_sign()` in `sshconnect2.c` never retries with a PIN

The `SSH_SK_ERR_PIN_REQUIRED` error propagates as
`SSH_ERR_KEY_WRONG_PASSPHRASE`.
In `identity_sign()`, there is retry logic to prompt for a PIN on this
error:

```c
if (!retried && pin == NULL && !is_agent &&
    sshkey_is_sk(sign_key) &&
    r == SSH_ERR_KEY_WRONG_PASSPHRASE) {
    ...
    pin = read_passphrase(prompt, 0);
    retried = 1;
    goto retry_pin;
}
```

However, the `!is_agent` guard (added in f9648090, 2022-08-19) prevents
this
path from being taken. For SK keys loaded from files, `is_agent` is set
to 1
because `id->key->flags & SSHKEY_FLAG_EXT` is true:

```c
if (id->key != NULL &&
    (id->isprivate || (id->key->flags & SSHKEY_FLAG_EXT))) {
    sign_key = id->key;
    is_agent = 1;  // <-- set for all external/hardware keys
}
```

So the retry path is unreachable for SK keys, whether loaded from a
file or
through ssh-agent. The signing attempt fails on the first try and the
error
is reported to the user.

## Affected versions

- `sk_sign()` issue: introduced in f3c34df8 (2021-11-02), first release
V_8_9_P1
- `identity_sign()` issue: introduced in f9648090 (2022-08-19), first
release V_9_1_P1
- Both issues are present on current HEAD

## Suggested fix

In `sk_sign()` (`sk-usbhid.c`): when `"uv"` is not reported by the
device but
`clientPin` is available, the function should return
`SSH_SK_ERR_PIN_REQUIRED`
and let the caller prompt for a PIN and retry. Currently it does return
this
error code, but the caller doesn't act on it (issue 2).

In `identity_sign()` (`sshconnect2.c`): the `!is_agent` condition in
the PIN
retry block should not apply to SK keys loaded from files. The
`is_agent`
variable conflates "key is in ssh-agent" with "key is hardware-backed",
but
these are different things. Hardware-backed keys loaded from handle
files
should still be eligible for PIN retry.

A minimal fix for the `sshconnect2.c` side would be to change the
condition to
also allow retry when the key is an SK key loaded from a file (i.e.
`SSHKEY_FLAG_EXT` is set but `agent_fd == -1`).

-- 
You are receiving this mail because:
You are watching the assignee of the bug.