Skip to content

Conversation

@JacobBarthelmeh
Copy link
Contributor

No description provided.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds coverage and a code-path fix for loading/storing encrypted token objects when the user PIN is empty (i.e., token usable without calling C_Login).

Changes:

  • Add a new automated test that initializes a token with an empty user PIN, stores an encrypted token object, finalizes, reloads, and verifies the object can be decoded without login.
  • Update token load logic to derive the token key from the empty PIN + seed before decoding stored objects.
  • Register the new test binary in the Automake test build.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
tests/include.am Adds tests/empty_pin_store_test to the test programs and links it appropriately.
tests/empty_pin_store_test.c New test covering empty user PIN token storage/load/decode behavior.
src/internal.c Attempts to derive token->key during token load for empty-PIN scenarios prior to object decode.
Comments suppressed due to low confidence (1)

src/internal.c:5306

  • wp11_Token_Load is already inside #ifndef WOLFPKCS11_NO_STORE, so this extra nested #ifndef WOLFPKCS11_NO_STORE block is redundant and makes the control flow harder to read. Consider removing the inner guard.
#ifndef WOLFPKCS11_NO_STORE
            /* Derive token->key from empty PIN + seed before decoding */
            ret = HashPIN((char*)"", 0, token->seed, sizeof(token->seed),
                          token->key, sizeof(token->key));
            object = token->object;
            while (ret == 0 && object != NULL) {
                ret = wp11_Object_Decode(object);
                object = object->next;
            }
#endif

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/internal.c:5306

  • wp11_Token_Load() is already inside #ifndef WOLFPKCS11_NO_STORE (starting near line 5148), so the inner #ifndef WOLFPKCS11_NO_STORE/#endif around the empty-PIN decode block is redundant. Removing the inner guard will simplify control flow and reduce the risk of mismatched preprocessor blocks in future edits.
#ifndef WOLFPKCS11_NO_STORE
            /* Derive token->key from empty PIN + seed before decoding */
            ret = HashPIN((char*)"", 0, token->seed, sizeof(token->seed),
                          token->key, sizeof(token->key), slot);
            object = token->object;
            while (ret == 0 && object != NULL) {
                ret = wp11_Object_Decode(object);
                object = object->next;
            }
#endif

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant