Skip to content

fix: update client key uri without restarting tedge-p11-server#3772

Merged
Bravo555 merged 3 commits intothin-edge:mainfrom
reubenmiller:fix-pkcs11-change-key-at-runtime
Sep 10, 2025
Merged

fix: update client key uri without restarting tedge-p11-server#3772
Bravo555 merged 3 commits intothin-edge:mainfrom
reubenmiller:fix-pkcs11-change-key-at-runtime

Conversation

@reubenmiller
Copy link
Copy Markdown
Contributor

@reubenmiller reubenmiller commented Aug 27, 2025

Proposed changes

Support changing the pkcs11 client key uri without requiring a restart of the tedge-p11-server.
Call C_Initialize again so C_GetSlotList returns updated slot list.

  • Test
  • Fix

PR should be rebased after #3771 is merged

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

#3766

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s. You can activate automatic signing by running just prepare-dev once)
  • I ran just format as mentioned in CODING_GUIDELINES
  • I used just check as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

According to PKCS #11 Version 2.40 Specification, Section 5.5 C_GetSlotList:

[...] If an application calls C_GetSlotList with a non-NULL pSlotList, and then the user adds or removes a hardware device, the changed slot list will only be visible and effective if C_GetSlotList is called again with NULL. Even if C_ GetSlotList is successfully called this way, it may or may not be the case that the changed slot list will be successfully recognized depending on the library implementation. On some platforms, or earlier PKCS11 compliant libraries, it may be necessary to successfully call C_Initialize or to restart the entire system.

In our case, SoftHSM2 is an example of such a library:

  1. C_GetSlotList calls slotManager.getSlotList()
  2. getSlotList reads from slots
  3. slots is initialized in the constructor
  4. SlotManager is created when C_Initialize is called

The disadvantage of that approach is we have to call dlopen again, per request, and in some environments this can be very slow apparently, but unless we can empirically confirm that this poses a problem, we'll proceed with this. Other libraries may be more reasonable and refresh the list on C_GetSlotList length request, as the spec suggests, but as we primarily develop and test against SoftHSM2, we'll proceed with this for now.

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 27, 2025

Codecov Report

❌ Patch coverage is 0% with 33 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ates/extensions/tedge-p11-server/src/pkcs11/mod.rs 0.00% 33 Missing ⚠️

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 27, 2025

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
685 0 3 685 100 2h9m1.540279s

@reubenmiller reubenmiller force-pushed the fix-pkcs11-change-key-at-runtime branch from 5787e87 to d49fe21 Compare August 27, 2025 13:27
@reubenmiller reubenmiller force-pushed the fix-pkcs11-change-key-at-runtime branch from d49fe21 to 478b634 Compare August 27, 2025 17:51
@reubenmiller reubenmiller added the theme:hsm Hardware Security Module related topics label Aug 28, 2025
@reubenmiller reubenmiller marked this pull request as draft August 28, 2025 07:28
@Bravo555 Bravo555 temporarily deployed to Test Pull Request September 3, 2025 08:46 — with GitHub Actions Inactive
@Bravo555 Bravo555 force-pushed the fix-pkcs11-change-key-at-runtime branch from 3aca834 to ec5d52b Compare September 3, 2025 09:11
@Bravo555 Bravo555 temporarily deployed to Test Pull Request September 3, 2025 09:11 — with GitHub Actions Inactive
@Bravo555 Bravo555 changed the title fix: update client key uri without restarting tedge-p11-server fix(cryptoki): Fix slot list not changing when slots are added or removed Sep 3, 2025
reubenmiller and others added 2 commits September 3, 2025 11:10
Signed-off-by: reubenmiller <reuben.d.miller@gmail.com>
Signed-off-by: Marcel Guzik <marcel.guzik@cumulocity.com>
@Bravo555 Bravo555 force-pushed the fix-pkcs11-change-key-at-runtime branch from ec5d52b to 07e0ce6 Compare September 3, 2025 11:10
@Bravo555 Bravo555 marked this pull request as ready for review September 3, 2025 11:10
This fixes an issue where C_GetSlotList isn't properly updated when a
new slot is created

Signed-off-by: Marcel Guzik <marcel.guzik@cumulocity.com>
@Bravo555 Bravo555 force-pushed the fix-pkcs11-change-key-at-runtime branch from 07e0ce6 to 0812ea3 Compare September 3, 2025 11:58
@Bravo555 Bravo555 temporarily deployed to Test Pull Request September 3, 2025 11:58 — with GitHub Actions Inactive
@Bravo555 Bravo555 changed the title fix(cryptoki): Fix slot list not changing when slots are added or removed fix: update client key uri without restarting tedge-p11-server Sep 3, 2025
Copy link
Copy Markdown
Contributor

@didier-wenzek didier-wenzek left a comment

Choose a reason for hiding this comment

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

Approved

Copy link
Copy Markdown
Contributor Author

@reubenmiller reubenmiller left a comment

Choose a reason for hiding this comment

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

Approved (though since I created the original PR my approval doesn't count)

@reubenmiller reubenmiller removed their assignment Sep 10, 2025
@Bravo555 Bravo555 added this pull request to the merge queue Sep 10, 2025
Merged via the queue into thin-edge:main with commit 8770cbb Sep 10, 2025
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

theme:hsm Hardware Security Module related topics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants