feat: Add PK11_CheckUserPassword() and PK11_Logout() to Slot

[Not-Nik]

Fixes #42

[github-actions]

Clean, minimal addition that follows the existing Slot patterns well. One inline comment on error variant consistency.

Design note: Issue #42 shows the downstream caller (lockstore) distinguishes wrong-PIN (SECWouldBlock) from other failures via a dedicated AuthResult enum. The current secstatus_to_res API supports this — callers can match on Error::Nss { code, .. } — but it's worth confirming that's ergonomic enough for the intended use site, since the original code had an explicit three-way return type.

Testing: Neither method has test coverage. logout is a trivial wrapper so the risk is low, but check_user_password has a CString conversion path that could use a NUL-byte rejection test at minimum. Integration testing against a password-protected slot is harder but would be valuable if the test harness supports it.

[github-actions]

Clean, minimal addition — both methods follow the established Slot patterns exactly.

The previous review's error-variant concern (using Error::String for NulError) was addressed in c686cbb by switching to ?, which routes through the existing From<NulError> → Error::Internal impl. This is consistent with CString::new(...)? usage elsewhere in the crate (e.g., lib.rs:119, lib.rs:151). Note that generate_token_key at src/p11.rs:240 is the one outlier that uses .map_err(|_| Error::InvalidInput) instead — not a problem for this PR, but worth harmonizing if the From impl is ever revisited.

One inline comment suggesting a doc note on the side-effect semantics of PK11_CheckUserPassword.

[github-actions]

Clean, minimal PR that correctly wraps PK11_CheckUserPassword and PK11_Logout following established Slot patterns. FFI boundaries are sound — CString lifetime spans the unsafe call, pointer types align, and secstatus_to_res maps errors correctly.

On existing review comments:

• NulError mapping (comment on earlier commit): Confirmed fixed in c686cbb. The ? operator now routes through the existing From<NulError> → Error::Internal impl, consistent with other CString::new(...)? usage in the crate (e.g., lib.rs).

• Side-effect documentation (open suggestion on check_user_password): Agreed — PK11_CheckUserPassword internally calls logout before re-authenticating, so a failed password check leaves the slot logged out. The suggested doc comment should be applied; callers expecting a read-only "verify" operation would otherwise be surprised by the auth state mutation.

Testing: Neither method has test coverage. logout is a trivial one-liner so the risk is low. For check_user_password, a unit test confirming that a password containing \0 is rejected (returns Error::Internal) would exercise the CString conversion path with no slot setup needed. Integration tests against a password-protected slot would be more valuable but depend on test harness capabilities.

[github-actions]

Clean, minimal PR. Both methods follow the established Slot FFI patterns correctly — CString lifetimes span the unsafe calls, pointer types align, and secstatus_to_res handles error mapping.

The NulError mapping issue from the first review round is resolved (636db70 switched to ?, routing through From<NulError> → Error::Internal).

One outstanding item: check_user_password still lacks a doc comment noting that PK11_CheckUserPassword mutates slot auth state (logs out before re-authenticating). See inline comment.

Testing: Neither method has test coverage. logout is trivial, but a unit test for check_user_password rejecting a NUL-containing password (CString::new("a\0b") → Error::Internal) would exercise the conversion path with no slot setup needed.

[github-actions]

Clean, minimal PR. Both wrappers follow the established Slot FFI patterns — CString lifetime spans the unsafe call, pointer types align, and secstatus_to_res handles error mapping.

Previously raised issues — all resolved:

• NulError mapping (first review round): fixed — ? now routes through From<NulError> → Error::Internal, consistent with other CString::new(...)? usage in the crate.
• Side-effect documentation for check_user_password: addressed in 5610bf3. The doc comment correctly notes the logout-before-reauth behavior and the logged-out state on failure.

Testing: Neither method has test coverage. A unit test confirming check_user_password rejects a NUL-containing password (CString::new("a\0b") → Error::Internal) would exercise the conversion path with no slot setup. Integration tests against a password-protected slot would be more valuable but depend on test harness capabilities.

[github-actions]

Clean, minimal PR. Both wrappers are sound:

• FFI correctness: CString lifetime spans each unsafe call, as_ptr() yields the correct *const c_char, and secstatus_to_res properly maps SECStatus → Res<()>. No aliasing or lifetime issues.
• Error handling: CString::new(password)? routes through the existing From<NulError> → Error::Internal impl, consistent with other CString::new(...)? call sites (lib.rs, agent.rs, ech.rs).
• Side-effect docs on check_user_password: Accurately describes the logout-before-reauth behavior of the underlying PK11_CheckUserPassword. Good.

Outstanding from prior reviews:

• logout is missing a doc comment (existing suggestion) — still unaddressed.

Testing: Neither method has test coverage. A unit test confirming check_user_password rejects a NUL-containing password ("a\0b" → Error::Internal) would exercise the CString conversion path cheaply — no slot setup needed. For logout, the risk is low given it's a one-line FFI delegation.