Refactor authentication handling#354
Open
keldonin wants to merge 1 commit intoparallaxsecond:mainfrom
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors authentication handling around a new Credential<'a> type to support multiple PIN forms (UTF-8, raw bytes, protected authentication path) and PKCS#11 v3.0 C_LoginUser, while changing several public APIs (init_token, init_pin, set_pin, login) to use this new abstraction.
Changes:
- Introduces
Credential<'a>and helper constructors/Fromimpls to encapsulate UTF-8 pins, raw pins, protected authentication path, and optional usernames, plus corresponding unit tests. - Refactors
Session::login,Session::init_pin,Session::set_pin,Session::login_with_raw, andPkcs11::init_tokento consumeCredential(viaInto<Credential<'a>>), enabling protected authentication paths andC_LoginUserwhile rejecting usernames where not allowed. - Adds
Error::UsernameNotExpectedfor clearer error reporting when a username is provided in contexts that do not support it, and updates docs/examples accordingly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
cryptoki/src/types.rs |
Adds Credential<'a> enum and helpers, switches RawAuthPin to a new secret type (currently mis-specified), and adds unit tests for various Credential forms. |
cryptoki/src/session/slot_token_management.rs |
Refactors Session::init_pin and Session::set_pin to accept generic credentials, support protected auth path and raw pins, and reject usernames in these contexts. |
cryptoki/src/session/session_management.rs |
Refactors Session::login to accept generic Credential (including protected auth path and username variants) and to use C_LoginUser when a username is present; deprecates login_with_raw in favor of the unified API. |
cryptoki/src/error/mod.rs |
Adds Error::UsernameNotExpected and wires it into Display/Error for clearer diagnostics when usernames are passed where unsupported. |
cryptoki/src/context/slot_token_management.rs |
Changes Pkcs11::init_token to accept generic credentials (including protected auth path) and to reject credentials carrying a username, aligning SO token initialization with the new auth model. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
12e8f61 to
7f9b9b1
Compare
e1d782d to
9c5466b
Compare
- refactor API to use new Credential type - integrated with current AuthPin and RawAuthPin - support for authenticated path on all PIN-related functions - support for PKCS#11 v3.0 C_LoginUser - using SecretSlice<u8> for RawAuthPin type alias Signed-off-by: Eric Devolder <eric.devolder@gmail.com>
9c5466b to
aca47c3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR should hopefully address #201 and #353.
The main idea is to support different methods of authentication around a new type,
Credential, which is essentially an enum.Fromimplementations are provided for transparent conversion fromAuthPinandRawAuthPin, minimizing code change.Note:
init_token(),set_pin()andinit_pin()signatures have changed, making this PR a breaking change.The changes introduced are:
AuthPinandRawAuthPinC_LoginUserSecretSlice<u8>forRawAuthPintype alias (PR AuthPin forces conversions that can leak secret #353)login_with_raw()as it is now unified underlogin()