[lovasoa]

Hi @kaczmarczyck ! The code and description below are fully written by codex, but we went over multiple iterations of it, and I quite like the final result.

I have a draft pr in fidophir that switches to this new api, and it's a nice simplification: lovasoa/fidophir#15

---

CTAP currently reads the configured PIN hash from Persist and passes it to KeyStore. Let software and hardware backends own PIN storage and verification through the public KeyStore interface:

fn has_pin(&mut self) -> CtapResult<bool>;
fn verify_pin_hash(&mut self, candidate: &[u8; 16]) -> CtapResult<bool>;
fn store_pin(&mut self, pin_hash: &[u8; 16], code_point_length: u8) -> CtapResult<()>;

With config_command, backends implement is_pin_longer_than and get a default has_pin helper. Other builds only require has_pin. The length comparison is strict, counts Unicode code points, and returns false when no PIN is configured. It is needed only when setting the minimum PIN length. Backends may use a guaranteed lower bound or conservatively return false for unknown positive thresholds, at the cost of unnecessary PIN changes on policy updates; exact length storage is optional.

CTAP owns PIN policy and retry accounting. It compares the proposed replacement against the just-verified candidate, invalidates old tokens before replacement, and clears forced-change state after successful storage. A backend that could not check a candidate because of temporary hardware lockout returns CTAP2_ERR_PIN_AUTH_BLOCKED; CTAP refunds that attempt. An attempted but incorrect PIN still returns false. Backends need no knowledge of CTAP retry or policy storage.

The software backend retains its existing 17-byte PIN record. Tests reuse the existing CTAP cases, extend the small storage/verification test, and add a short table test for hardware-lockout retry accounting.

[kaczmarczyck]

I think this change is still bigger than necessary. I thought all we do is move the API functions around.

So for the remaining changes, I suspect they are useful for Fidophir somehow?

If you explain what you need, I can think of alternatives. Otherwise I'd prefer the API only version of this PR.

[kaczmarczyck]

Why do we need this? Can't CTAP just call persist().pin_code_point_length() like before?

What do we gain from adding this function, if the length is available? It doesn't increase security, and I don't think we really need to.

[kaczmarczyck]

There is already a

storage::decr_pin_retries(env)?;

in line 225, what do we need this function for?

[kaczmarczyck]

Why do we move storing out of this function? We have env, so we shouldn't need to change this?

[kaczmarczyck]

This comment seems to talk about your implementation of this trait, not the trait itself. I don't think it really makes sense here?

[kaczmarczyck]

CTAP clears the forced-change flag.

I think correct API documentation shouldn't talk about what happens outside of the module. Keep documentation self-contained.